[inspector] fixed dcheck in stack trace

Async stack trace can contain empty syncrhonous stack and external
stack.

R=dgozman@chromium.org

Bug: chromium:790567
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I2b04743f7c4f15a038eb1041cc7fc117d438b6b2
Reviewed-on: https://chromium-review.googlesource.com/822971
Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50117}

src/inspector/v8-stack-trace-impl.cc

@@ -42,6 +42,7 @@ void calculateAsyncChain(V8Debugger* debugger, int contextGroupId,
   // not happen if we have proper instrumentation, but let's double-check to be
   // safe.
   if (contextGroupId && *asyncParent &&
+      (*asyncParent)->externalParent().IsInvalid() &&
       (*asyncParent)->contextGroupId() != contextGroupId) {
     asyncParent->reset();
     *externalParent = V8StackTraceId();
@@ -338,14 +339,15 @@ std::shared_ptr<AsyncStackTrace> AsyncStackTrace::capture(
   // but doesn't synchronous we can merge them together. e.g. Promise
   // ThenableJob.
   if (asyncParent && frames.empty() &&
-      asyncParent->m_description == description) {
+      (asyncParent->m_description == description || description.isEmpty())) {
     return asyncParent;
   }
 
-  DCHECK(contextGroupId || asyncParent);
+  DCHECK(contextGroupId || asyncParent || !externalParent.IsInvalid());
   if (!contextGroupId && asyncParent) {
     contextGroupId = asyncParent->m_contextGroupId;
   }
+
   return std::shared_ptr<AsyncStackTrace>(
       new AsyncStackTrace(contextGroupId, description, std::move(frames),
                           asyncParent, externalParent));
@@ -362,7 +364,7 @@ AsyncStackTrace::AsyncStackTrace(
       m_frames(std::move(frames)),
       m_asyncParent(asyncParent),
       m_externalParent(externalParent) {
-  DCHECK(m_contextGroupId);
+  DCHECK(m_contextGroupId || (!externalParent.IsInvalid() && m_frames.empty()));
 }
 
 std::unique_ptr<protocol::Runtime::StackTrace>

src/inspector/v8-stack-trace-impl.h

@@ -120,6 +120,7 @@ class AsyncStackTrace {
   const String16& description() const;
   std::weak_ptr<AsyncStackTrace> parent() const;
   bool isEmpty() const;
+  const V8StackTraceId& externalParent() const { return m_externalParent; }
 
   const std::vector<std::shared_ptr<StackFrame>>& frames() const {
     return m_frames;

test/inspector/debugger/async-chains-expected.txt

+Tests different combinations of async stacks in chains.
+Regular
+userFunction (test.js:1:36)
+-- inner async --
+runWithRegular (utils.js:2:12)
+inner (test.js:2:28)
+runWithNone (utils.js:27:2)
+(anonymous) (test.js:3:10)
+
+Regular - Regular
+userFunction (test.js:1:36)
+-- inner async --
+runWithRegular (utils.js:2:12)
+inner (test.js:2:28)
+-- outer async --
+runWithRegular (utils.js:2:12)
+(anonymous) (test.js:3:10)
+
+Regular - EmptyName
+userFunction (test.js:1:36)
+-- inner async --
+runWithRegular (utils.js:2:12)
+inner (test.js:2:28)
+-- <empty> --
+runWithEmptyName (utils.js:6:12)
+(anonymous) (test.js:3:10)
+
+Regular - EmptyStack
+userFunction (test.js:1:36)
+-- inner async --
+runWithRegular (utils.js:2:12)
+inner (test.js:2:28)
+
+Regular - EmptyNameEmptyStack
+userFunction (test.js:1:36)
+-- inner async --
+runWithRegular (utils.js:2:12)
+inner (test.js:2:28)
+
+Regular - External
+userFunction (test.js:1:36)
+-- inner async --
+runWithRegular (utils.js:2:12)
+inner (test.js:2:28)
+runWithRegular (utils.js:21:4)
+  <external stack>
+
+EmptyName
+userFunction (test.js:1:36)
+-- <empty> --
+runWithEmptyName (utils.js:6:12)
+inner (test.js:2:28)
+runWithNone (utils.js:27:2)
+(anonymous) (test.js:3:10)
+
+EmptyName - Regular
+userFunction (test.js:1:36)
+-- <empty> --
+runWithEmptyName (utils.js:6:12)
+inner (test.js:2:28)
+-- outer async --
+runWithRegular (utils.js:2:12)
+(anonymous) (test.js:3:10)
+
+EmptyName - EmptyName
+userFunction (test.js:1:36)
+-- <empty> --
+runWithEmptyName (utils.js:6:12)
+inner (test.js:2:28)
+-- <empty> --
+runWithEmptyName (utils.js:6:12)
+(anonymous) (test.js:3:10)
+
+EmptyName - EmptyStack
+userFunction (test.js:1:36)
+-- <empty> --
+runWithEmptyName (utils.js:6:12)
+inner (test.js:2:28)
+
+EmptyName - EmptyNameEmptyStack
+userFunction (test.js:1:36)
+-- <empty> --
+runWithEmptyName (utils.js:6:12)
+inner (test.js:2:28)
+
+EmptyName - External
+userFunction (test.js:1:36)
+-- <empty> --
+runWithEmptyName (utils.js:6:12)
+inner (test.js:2:28)
+runWithRegular (utils.js:21:4)
+  <external stack>
+
+EmptyStack
+userFunction (test.js:1:36)
+
+EmptyStack - Regular
+userFunction (test.js:1:36)
+-- inner async --
+-- outer async --
+runWithRegular (utils.js:2:12)
+(anonymous) (test.js:3:10)
+
+EmptyStack - EmptyName
+userFunction (test.js:1:36)
+-- inner async --
+-- <empty> --
+runWithEmptyName (utils.js:6:12)
+(anonymous) (test.js:3:10)
+
+EmptyStack - EmptyStack
+userFunction (test.js:1:36)
+
+EmptyStack - EmptyNameEmptyStack
+userFunction (test.js:1:36)
+
+EmptyStack - External
+userFunction (test.js:1:36)
+-- inner async --
+  <external stack>
+
+EmptyNameEmptyStack
+userFunction (test.js:1:36)
+
+EmptyNameEmptyStack - Regular
+userFunction (test.js:1:36)
+-- outer async --
+runWithRegular (utils.js:2:12)
+(anonymous) (test.js:3:10)
+
+EmptyNameEmptyStack - EmptyName
+userFunction (test.js:1:36)
+-- <empty> --
+runWithEmptyName (utils.js:6:12)
+(anonymous) (test.js:3:10)
+
+EmptyNameEmptyStack - EmptyStack
+userFunction (test.js:1:36)
+
+EmptyNameEmptyStack - EmptyNameEmptyStack
+userFunction (test.js:1:36)
+
+EmptyNameEmptyStack - External
+userFunction (test.js:1:36)
+-- <empty> --
+  <external stack>
+
+External
+userFunction (test.js:1:36)
+runWithRegular (utils.js:21:4)
+  <external stack>
+

test/inspector/debugger/async-chains.js

+// Copyright 2017 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+let {session, contextGroup, Protocol} = InspectorTest.start(
+  'Tests different combinations of async stacks in chains.');
+
+contextGroup.addScript(`
+function runWithRegular(f, name) {
+  inspector.scheduleWithAsyncStack(f, name, false);
+}
+
+function runWithEmptyName(f) {
+  inspector.scheduleWithAsyncStack(f, '', false);
+}
+
+function runWithEmptyStack(f, name) {
+  inspector.scheduleWithAsyncStack(f, name, true);
+}
+
+function runWithEmptyNameEmptyStack(f) {
+  inspector.scheduleWithAsyncStack(f, '', true);
+}
+
+function runWithExternal(f) {
+  const id = inspector.storeCurrentStackTrace('external');
+  runWithRegular(() => {
+    inspector.externalAsyncTaskStarted(id);
+    f();
+    inspector.externalAsyncTaskFinished(id);
+  }, 'not-used-async');
+}
+
+function runWithNone(f) {
+  f();
+}
+//# sourceURL=utils.js`);
+
+session.setupScriptMap();
+(async function test() {
+  Protocol.Debugger.enable();
+  Protocol.Debugger.setAsyncCallStackDepth({maxDepth: 128});
+
+  const first = ['Regular', 'EmptyName', 'EmptyStack', 'EmptyNameEmptyStack', 'External']
+  const second = ['None', 'Regular', 'EmptyName', 'EmptyStack', 'EmptyNameEmptyStack', 'External']
+
+  for (const stack1 of first) {
+    for (const stack2 of second) {
+      if (stack1 === 'External' && stack2 !== 'None') continue;
+
+      InspectorTest.log(stack2 === 'None' ? stack1 : `${stack1} - ${stack2}`);
+      Protocol.Runtime.evaluate({
+        expression: `
+          var userFunction = () => {debugger};
+          var inner = () => runWith${stack1}(userFunction, 'inner async');
+          runWith${stack2}(inner, 'outer async');
+          //# sourceURL=test.js`
+      });
+      await pauseAndDumpStack();
+    }
+  }
+
+  await Protocol.Debugger.disable();
+  InspectorTest.completeTest();
+})();
+
+async function pauseAndDumpStack() {
+  const {params:{callFrames, asyncStackTrace, asyncStackTraceId}}
+      = await Protocol.Debugger.oncePaused();
+  session.logCallFrames(callFrames);
+  if (asyncStackTrace)
+    session.logAsyncStackTrace(asyncStackTrace);
+  if (asyncStackTraceId)
+    InspectorTest.log('  <external stack>');
+  InspectorTest.log('');
+  return Protocol.Debugger.resume();
+}

test/inspector/inspector-test.cc

@@ -175,7 +175,8 @@ class SendMessageToBackendTask : public TaskRunner::Task {
   v8::internal::Vector<uint16_t> message_;
 };
 
-void RunAsyncTask(TaskRunner* task_runner, const char* task_name,
+void RunAsyncTask(TaskRunner* task_runner,
+                  const v8_inspector::StringView& task_name,
                   TaskRunner::Task* task) {
   class AsyncTask : public TaskRunner::Task {
    public:
@@ -193,10 +194,7 @@ void RunAsyncTask(TaskRunner* task_runner, const char* task_name,
     DISALLOW_COPY_AND_ASSIGN(AsyncTask);
   };
 
-  task_runner->data()->AsyncTaskScheduled(
-      v8_inspector::StringView(reinterpret_cast<const uint8_t*>(task_name),
-                               strlen(task_name)),
-      task, false);
+  task_runner->data()->AsyncTaskScheduled(task_name, task, false);
   task_runner->Append(new AsyncTask(task));
 }
 
@@ -626,13 +624,16 @@ class SetTimeoutExtension : public IsolateData::SetupGlobalTask {
     v8::Local<v8::Context> context = isolate->GetCurrentContext();
     IsolateData* data = IsolateData::FromContext(context);
     int context_group_id = data->GetContextGroupId(context);
+    const char* task_name = "setTimeout";
+    v8_inspector::StringView task_name_view(
+        reinterpret_cast<const uint8_t*>(task_name), strlen(task_name));
     if (args[0]->IsFunction()) {
-      RunAsyncTask(data->task_runner(), "setTimeout",
+      RunAsyncTask(data->task_runner(), task_name_view,
                    new SetTimeoutTask(context_group_id, isolate,
                                       v8::Local<v8::Function>::Cast(args[0])));
     } else {
       RunAsyncTask(
-          data->task_runner(), "setTimeout",
+          data->task_runner(), task_name_view,
           new ExecuteStringTask(
               context_group_id, ToVector(args[0].As<v8::String>()),
               v8::String::Empty(isolate), v8::Integer::New(isolate, 0),
@@ -703,6 +704,9 @@ class InspectorExtension : public IsolateData::SetupGlobalTask {
         ToV8String(isolate, "externalAsyncTaskFinished"),
         v8::FunctionTemplate::New(
             isolate, &InspectorExtension::ExternalAsyncTaskFinished));
+    inspector->Set(ToV8String(isolate, "scheduleWithAsyncStack"),
+                   v8::FunctionTemplate::New(
+                       isolate, &InspectorExtension::ScheduleWithAsyncStack));
     global->Set(ToV8String(isolate, "inspector"), inspector);
   }
 
@@ -926,6 +930,33 @@ class InspectorExtension : public IsolateData::SetupGlobalTask {
             args[0].As<v8::ArrayBuffer>()->GetContents().Data());
     data->ExternalAsyncTaskFinished(*id);
   }
+
+  static void ScheduleWithAsyncStack(
+      const v8::FunctionCallbackInfo<v8::Value>& args) {
+    if (args.Length() != 3 || !args[0]->IsFunction() || !args[1]->IsString() ||
+        !args[2]->IsBoolean()) {
+      fprintf(stderr,
+              "Internal error: scheduleWithAsyncStack(function, "
+              "'task-name', with_empty_stack).");
+      Exit();
+    }
+    v8::Isolate* isolate = args.GetIsolate();
+    v8::Local<v8::Context> context = isolate->GetCurrentContext();
+    IsolateData* data = IsolateData::FromContext(context);
+    int context_group_id = data->GetContextGroupId(context);
+    bool with_empty_stack = args[2].As<v8::Boolean>()->Value();
+    if (with_empty_stack) context->Exit();
+
+    v8::internal::Vector<uint16_t> task_name =
+        ToVector(args[1].As<v8::String>());
+    v8_inspector::StringView task_name_view(task_name.start(),
+                                            task_name.length());
+
+    RunAsyncTask(data->task_runner(), task_name_view,
+                 new SetTimeoutTask(context_group_id, isolate,
+                                    v8::Local<v8::Function>::Cast(args[0])));
+    if (with_empty_stack) context->Enter();
+  }
 };
 
 }  //  namespace

test/inspector/protocol-test.js

@@ -301,13 +301,9 @@ InspectorTest.Session = class {
 
   logAsyncStackTrace(asyncStackTrace) {
     while (asyncStackTrace) {
-      if (asyncStackTrace.promiseCreationFrame) {
-        var frame = asyncStackTrace.promiseCreationFrame;
-        InspectorTest.log(`-- ${asyncStackTrace.description} (${frame.url}:${frame.lineNumber}:${frame.columnNumber})--`);
-      } else {
-        InspectorTest.log(`-- ${asyncStackTrace.description} --`);
-      }
+      InspectorTest.log(`-- ${asyncStackTrace.description || '<empty>'} --`);
       this.logCallFrames(asyncStackTrace.callFrames);
+      if (asyncStackTrace.parentId) InspectorTest.log('  <external stack>');
       asyncStackTrace = asyncStackTrace.parent;
     }
   }