[inspector] Align async task frame reporting for `await`.

The V8 Inspector was sending an additional frame as part of async stack
traces for async functions, which pointed to the first executed `await`
in the async function. This is leaking an implementation detail of how
(and more precisely when) the inspector decides to collect this stack
trace. From the users perspective the async part of the stack trace is
supposed to capture what happened _prior to the task_ - meaning in case
of async functions: What lead to the execution of the async function.
This is reflected by the fact that the DevTools front-end (and the V8
Inspector itself) performs post-processing on these async call stacks,
removing the misleading top frame from it. But this post-processing is
not applied consistently to all async stack traces (i.e. the Console
message stack traces don't get this), and potentially also not applied
consistently across consumers of the Chromium debugger backend.

Instead the V8 Inspector now removes the top frame itself and thus
reports `await` consistently with how other async tasks are reported to
debugger front-ends.

Note: This preserves backwards compatibility with old versions of
devtools-frontend, which do post-processing (for the Call Stack) only on
async stack traces marked with "async function", while we now mark these
async stack traces with "await" instead (aligned with what the front-end
is using as user visibile string anyways in the Call Stack section, and
this matching will be updated in a separate follow up CL to look for
"await" instead of "async function").

Before: https://imgur.com/kIrWcIc.png
After: https://imgur.com/HvZGqiP
Fixed: chromium:1254259
Bug: chromium:1229662
Change-Id: I57ce051a28892177b6b96221f083ae957f967e52
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3193535
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Kim-Anh Tran <kimanh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77157}

src/inspector/v8-debugger.cc

@@ -657,7 +657,7 @@ void V8Debugger::AsyncEventOccurred(v8::debug::DebugAsyncActionType type,
       break;
     case v8::debug::kAsyncFunctionSuspended: {
       if (m_asyncTaskStacks.find(task) == m_asyncTaskStacks.end()) {
-        asyncTaskScheduledForStack("async function", task, true);
+        asyncTaskScheduledForStack("await", task, true, true);
       }
       auto stackIt = m_asyncTaskStacks.find(task);
       if (stackIt != m_asyncTaskStacks.end() && !stackIt->second.expired()) {
@@ -977,11 +977,13 @@ void V8Debugger::asyncTaskFinished(void* task) {
 }
 
 void V8Debugger::asyncTaskScheduledForStack(const String16& taskName,
-                                            void* task, bool recurring) {
+                                            void* task, bool recurring,
+                                            bool skipTopFrame) {
   if (!m_maxAsyncCallStackDepth) return;
   v8::HandleScope scope(m_isolate);
   std::shared_ptr<AsyncStackTrace> asyncStack = AsyncStackTrace::capture(
-      this, taskName, V8StackTraceImpl::maxCallStackSizeToCapture);
+      this, taskName, V8StackTraceImpl::maxCallStackSizeToCapture,
+      skipTopFrame);
   if (asyncStack) {
     m_asyncTaskStacks[task] = asyncStack;
     if (recurring) m_recurringTasks.insert(task);

src/inspector/v8-debugger.h

@@ -161,7 +161,7 @@ class V8Debugger : public v8::debug::DebugDelegate,
                                                v8::Local<v8::Value> value);
 
   void asyncTaskScheduledForStack(const String16& taskName, void* task,
-                                  bool recurring);
+                                  bool recurring, bool skipTopFrame = false);
   void asyncTaskCanceledForStack(void* task);
   void asyncTaskStartedForStack(void* task);
   void asyncTaskFinishedForStack(void* task);

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

@@ -389,7 +389,6 @@ void V8StackTraceImpl::StackFrameIterator::next() {
   while (m_currentIt == m_currentEnd && m_parent) {
     const std::vector<std::shared_ptr<StackFrame>>& frames = m_parent->frames();
     m_currentIt = frames.begin();
-    if (m_parent->description() == "async function") ++m_currentIt;
     m_currentEnd = frames.end();
     m_parent = m_parent->parent().lock().get();
   }
@@ -405,7 +404,8 @@ StackFrame* V8StackTraceImpl::StackFrameIterator::frame() {
 
 // static
 std::shared_ptr<AsyncStackTrace> AsyncStackTrace::capture(
-    V8Debugger* debugger, const String16& description, int maxStackSize) {
+    V8Debugger* debugger, const String16& description, int maxStackSize,
+    bool skipTopFrame) {
   DCHECK(debugger);
 
   TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("v8.stack_trace"),
@@ -419,6 +419,9 @@ std::shared_ptr<AsyncStackTrace> AsyncStackTrace::capture(
     v8::Local<v8::StackTrace> v8StackTrace = v8::StackTrace::CurrentStackTrace(
         isolate, maxStackSize, stackTraceOptions);
     frames = toFramesVector(debugger, v8StackTrace, maxStackSize);
+    if (skipTopFrame && !frames.empty()) {
+      frames.erase(frames.begin());
+    }
   }
 
   std::shared_ptr<AsyncStackTrace> asyncParent;

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

@@ -118,7 +118,8 @@ class AsyncStackTrace {
   AsyncStackTrace& operator=(const AsyncStackTrace&) = delete;
   static std::shared_ptr<AsyncStackTrace> capture(V8Debugger*,
                                                   const String16& description,
-                                                  int maxStackSize);
+                                                  int maxStackSize,
+                                                  bool skipTopFrame = false);
   static uintptr_t store(V8Debugger* debugger,
                          std::shared_ptr<AsyncStackTrace> stack);
 

test/inspector/debugger/async-for-await-of-promise-stack-expected.txt

@@ -3,8 +3,7 @@ Checks that async chains for for-await-of are correct.
 Running test: testBasic
 Debugger (test.js:10:2)
 Basic (test.js:48:4)
--- async function --
+-- await --
 (anonymous) (testBasic.js:0:0)
 
 
@@ -25,16 +24,14 @@ UncaughtThrow (test.js:67:21)
 Running test: testCaughtReject
 Debugger (test.js:10:2)
 CaughtReject (test.js:76:4)
--- async function --
+-- await --
 (anonymous) (testCaughtReject.js:0:0)
 
 
 Running test: testCaughtThrow
 Debugger (test.js:10:2)
 CaughtThrow (test.js:86:4)
-CaughtThrow (test.js:86:4)
+-- await --
 (anonymous) (testCaughtThrow.js:0:0)
 
 
@@ -52,7 +49,6 @@ Running test: testCaughtRejectOnBreak
 Running test: testCaughtThrowOnBreak
 Debugger (test.js:10:2)
 CaughtThrowOnBreak (test.js:124:4)
--- async function --
+-- await --
 (anonymous) (testCaughtThrowOnBreak.js:0:0)
 

test/inspector/debugger/async-function-step-out-expected.txt

@@ -3,54 +3,42 @@ stepOut async function
 Running test: testTrivial
 Check that we have proper async stack at return
 bar (testTrivial.js:28:8)
--- async function --
+-- await --
 foo (testTrivial.js:23:14)
-foo (testTrivial.js:23:14)
+-- await --
 test (testTrivial.js:18:14)
-foo (testTrivial.js:22:22)
+-- await --
 (anonymous) (:0:0)
 
 foo (testTrivial.js:24:6)
-(anonymous) (:0:0)
+-- await --
 test (testTrivial.js:18:14)
-foo (testTrivial.js:24:6)
+-- await --
 (anonymous) (:0:0)
 
 test (testTrivial.js:19:6)
--- async function --
+-- await --
 (anonymous) (:0:0)
 
 
 Running test: testStepOutPrecision
 Check that stepOut go to resumed outer generator
 bar (testStepOutPrecision.js:61:8)
-(anonymous) (:0:0)
+-- await --
 foo (testStepOutPrecision.js:55:14)
-
+-- await --
 test (testStepOutPrecision.js:48:14)
-Check that stepOut go to resumed outer generator
+-- await --
 (anonymous) (:0:0)
 
 foo (testStepOutPrecision.js:56:8)
-foo (testStepOutPrecision.js:55:14)
+-- await --
 test (testStepOutPrecision.js:48:14)
-foo (testStepOutPrecision.js:54:22)
+-- await --
 (anonymous) (:0:0)
 
 test (testStepOutPrecision.js:49:8)
-(anonymous) (:0:0)
+-- await --
 (anonymous) (:0:0)
 
 floodWithTimeouts (testStepOutPrecision.js:40:15)
@@ -66,8 +54,7 @@ test (testStepOutPrecision.js:46:8)
 (anonymous) (:0:0)
 
 test (testStepOutPrecision.js:50:8)
--- async function --
+-- await --
 (anonymous) (:0:0)
 
 floodWithTimeouts (testStepOutPrecision.js:40:15)
@@ -88,43 +75,33 @@ test (testStepOutPrecision.js:46:8)
 Running test: testStepIntoAtReturn
 Check that stepInto at return go to resumed outer generator
 bar (testStepIntoAtReturn.js:93:8)
--- async function --
+-- await --
 foo (testStepIntoAtReturn.js:88:14)
-foo (testStepIntoAtReturn.js:88:14)
+-- await --
 test (testStepIntoAtReturn.js:82:14)
-foo (testStepIntoAtReturn.js:87:22)
+-- await --
 (anonymous) (:0:0)
 
 bar (testStepIntoAtReturn.js:94:6)
-(anonymous) (:0:0)
+-- await --
 foo (testStepIntoAtReturn.js:88:14)
-bar (testStepIntoAtReturn.js:94:6)
+-- await --
 test (testStepIntoAtReturn.js:82:14)
-bar (testStepIntoAtReturn.js:92:22)
+-- await --
 (anonymous) (:0:0)
 
 foo (testStepIntoAtReturn.js:89:6)
-test (testStepIntoAtReturn.js:82:14)
+-- await --
 test (testStepIntoAtReturn.js:82:14)
-test (testStepIntoAtReturn.js:81:14)
+-- await --
 (anonymous) (:0:0)
 
 test (testStepIntoAtReturn.js:83:8)
--- async function --
+-- await --
 (anonymous) (:0:0)
 
 test (testStepIntoAtReturn.js:84:6)
-test (testStepIntoAtReturn.js:81:14)
+-- await --
 (anonymous) (:0:0)
 
 floodWithTimeouts (testStepIntoAtReturn.js:74:15)
@@ -139,43 +116,33 @@ test (testStepIntoAtReturn.js:80:8)
 Running test: testStepOverAtReturn
 Check that stepOver at return go to resumed outer generator
 bar (testStepIntoAtReturn.js:124:8)
--- async function --
+-- await --
 foo (testStepIntoAtReturn.js:119:14)
-foo (testStepIntoAtReturn.js:119:14)
+-- await --
 test (testStepIntoAtReturn.js:113:14)
-foo (testStepIntoAtReturn.js:118:22)
+-- await --
 (anonymous) (:0:0)
 
 bar (testStepIntoAtReturn.js:125:6)
-(anonymous) (:0:0)
+-- await --
 foo (testStepIntoAtReturn.js:119:14)
-bar (testStepIntoAtReturn.js:125:6)
+-- await --
 test (testStepIntoAtReturn.js:113:14)
-bar (testStepIntoAtReturn.js:123:22)
+-- await --
 (anonymous) (:0:0)
 
 foo (testStepIntoAtReturn.js:120:6)
-test (testStepIntoAtReturn.js:113:14)
+-- await --
 test (testStepIntoAtReturn.js:113:14)
-test (testStepIntoAtReturn.js:112:14)
+-- await --
 (anonymous) (:0:0)
 
 test (testStepIntoAtReturn.js:114:8)
--- async function --
+-- await --
 (anonymous) (:0:0)
 
 test (testStepIntoAtReturn.js:115:6)
-test (testStepIntoAtReturn.js:112:14)
+-- await --
 (anonymous) (:0:0)
 
 floodWithTimeouts (testStepIntoAtReturn.js:105:15)
@@ -190,19 +157,15 @@ test (testStepIntoAtReturn.js:111:8)
 Running test: testStepOutFromNotAwaitedCall
 Checks stepOut from not awaited call
 bar (testStepIntoAtReturn.js:158:8)
--- async function --
+-- await --
 foo (testStepIntoAtReturn.js:152:8)
-foo (testStepIntoAtReturn.js:152:8)
+-- await --
 test (testStepIntoAtReturn.js:144:14)
-foo (testStepIntoAtReturn.js:151:22)
+-- await --
 (anonymous) (:0:0)
 
 test (testStepIntoAtReturn.js:145:8)
-(anonymous) (:0:0)
+-- await --
 (anonymous) (:0:0)
 
 floodWithTimeouts (testStepIntoAtReturn.js:136:15)

test/inspector/debugger/async-stack-await-expected.txt

 Checks that async stacks works for async/await
 foo2 (test.js:15:2)
--- async function --
+-- await --
 test (test.js:24:8)
 (anonymous) (expr.js:0:0)
 
 foo2 (test.js:17:2)
-foo2 (test.js:17:2)
+-- await --
 test (test.js:24:8)
 (anonymous) (expr.js:0:0)
 
 foo1 (test.js:9:2)
 foo2 (test.js:18:8)
-foo1 (test.js:9:2)
+-- await --
 test (test.js:24:8)
 (anonymous) (expr.js:0:0)
 
 foo1 (test.js:9:2)
 -- Promise.then --
 foo2 (test.js:19:43)
-foo1 (test.js:9:2)
+-- await --
 test (test.js:24:8)
 (anonymous) (expr.js:0:0)
 
 foo2 (test.js:20:2)
-test (test.js:24:8)
+-- await --
 test (test.js:24:8)
 (anonymous) (expr.js:0:0)
 

test/inspector/debugger/continue-to-location-target-call-frames-expected.txt

@@ -7,8 +7,7 @@ asyncFact (test.js:9:2)
 (anonymous) (expr.js:0:0)
 
 asyncFact (test.js:11:2)
--- async function --
+-- await --
 asyncFact (test.js:3:20)
 asyncFact (test.js:3:20)
 asyncFact (test.js:3:20)
@@ -22,8 +21,7 @@ asyncFact (test.js:9:2)
 (anonymous) (expr.js:0:0)
 
 asyncFact (test.js:11:2)
--- async function --
+-- await --
 (anonymous) (expr.js:0:0)