[inspector] Introduce status result for Debugger.setScriptSource

This CL introduces a new `status` enum returned by setScriptSource.
We'll use the information in the DevTools frontend to show more
meaningful error messages as well as disambiguate compilation errors
from failed live edits.

Drive-by: Deprecate the sync and async stack traces in the result.
Currently `setScriptSource` is guaranteed to stay paused so there
is no need to send along the same information from the
preceeding `Debugger.paused` event.
In the future we will restart the top-most frame once we allow
the top-most frame to be edited. In that case the inspector
fires Debugger.resumed + Debugger.paused events following the
live edit also making the info returned here superfluous.

R=jarin@chromium.org

Bug: chromium:1334484
Change-Id: I4226491caed72013a00927273c523213d797a766
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3691850
Commit-Queue: Simon Zünd <szuend@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81031}

include/js_protocol.pdl

@@ -452,14 +452,22 @@ domain Debugger
       optional boolean dryRun
     returns
       # New stack trace in case editing has happened while VM was stopped.
-      optional array of CallFrame callFrames
+      deprecated optional array of CallFrame callFrames
       # Whether current call stack  was modified after applying the changes.
-      optional boolean stackChanged
+      deprecated optional boolean stackChanged
       # Async stack trace, if any.
-      optional Runtime.StackTrace asyncStackTrace
+      deprecated optional Runtime.StackTrace asyncStackTrace
       # Async stack trace, if any.
-      experimental optional Runtime.StackTraceId asyncStackTraceId
-      # Exception details if any.
+      deprecated optional Runtime.StackTraceId asyncStackTraceId
+      # Whether the operation was successful or not. Only `Ok` denotes a
+      # successful live edit while the other enum variants denote why
+      # the live edit failed.
+      experimental enum status
+        Ok
+        CompileError
+        BlockedByActiveGenerator
+        BlockedByActiveFunction
+      # Exception details if any. Only present when `status` is `CompileError`.
       optional Runtime.ExceptionDetails exceptionDetails
 
   # Makes page not interrupt on any pauses (breakpoint, exception, dom exception etc).

src/inspector/v8-debugger-agent-impl.cc

@@ -1002,12 +1002,29 @@ Response V8DebuggerAgentImpl::searchInContent(
   return Response::Success();
 }
 
+namespace {
+const char* buildStatus(v8::debug::LiveEditResult::Status status) {
+  switch (status) {
+    case v8::debug::LiveEditResult::OK:
+      return protocol::Debugger::SetScriptSource::StatusEnum::Ok;
+    case v8::debug::LiveEditResult::COMPILE_ERROR:
+      return protocol::Debugger::SetScriptSource::StatusEnum::CompileError;
+    case v8::debug::LiveEditResult::BLOCKED_BY_ACTIVE_FUNCTION:
+      return protocol::Debugger::SetScriptSource::StatusEnum::
+          BlockedByActiveFunction;
+    case v8::debug::LiveEditResult::BLOCKED_BY_RUNNING_GENERATOR:
+      return protocol::Debugger::SetScriptSource::StatusEnum::
+          BlockedByActiveGenerator;
+  }
+}
+}  // namespace
+
 Response V8DebuggerAgentImpl::setScriptSource(
     const String16& scriptId, const String16& newContent, Maybe<bool> dryRun,
     Maybe<protocol::Array<protocol::Debugger::CallFrame>>* newCallFrames,
     Maybe<bool>* stackChanged,
     Maybe<protocol::Runtime::StackTrace>* asyncStackTrace,
-    Maybe<protocol::Runtime::StackTraceId>* asyncStackTraceId,
+    Maybe<protocol::Runtime::StackTraceId>* asyncStackTraceId, String16* status,
     Maybe<protocol::Runtime::ExceptionDetails>* optOutCompileError) {
   if (!enabled()) return Response::ServerError(kDebuggerNotEnabled);
 
@@ -1026,7 +1043,8 @@ Response V8DebuggerAgentImpl::setScriptSource(
 
   v8::debug::LiveEditResult result;
   it->second->setSource(newContent, dryRun.fromMaybe(false), &result);
-  if (result.status != v8::debug::LiveEditResult::OK) {
+  *status = buildStatus(result.status);
+  if (result.status == v8::debug::LiveEditResult::COMPILE_ERROR) {
     *optOutCompileError =
         protocol::Runtime::ExceptionDetails::create()
             .setExceptionId(m_inspector->nextExceptionId())
@@ -1037,15 +1055,7 @@ Response V8DebuggerAgentImpl::setScriptSource(
                                                         : 0)
             .build();
     return Response::Success();
-  } else {
-    *stackChanged = result.stack_changed;
   }
-  std::unique_ptr<Array<CallFrame>> callFrames;
-  Response response = currentCallFrames(&callFrames);
-  if (!response.IsSuccess()) return response;
-  *newCallFrames = std::move(callFrames);
-  *asyncStackTrace = currentAsyncStackTrace();
-  *asyncStackTraceId = currentExternalStackTrace();
   return Response::Success();
 }
 

src/inspector/v8-debugger-agent-impl.h

@@ -88,6 +88,7 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend {
       Maybe<bool>* optOutStackChanged,
       Maybe<protocol::Runtime::StackTrace>* optOutAsyncStackTrace,
       Maybe<protocol::Runtime::StackTraceId>* optOutAsyncStackTraceId,
+      String16* outStatus,
       Maybe<protocol::Runtime::ExceptionDetails>* optOutCompileError) override;
   Response restartFrame(
       const String16& callFrameId, Maybe<String16> mode,

test/inspector/debugger/es6-module-liveedit-expected.txt

@@ -6,9 +6,7 @@ console.log message from function before patching:
 }
 Debugger.setScriptSource result:
 {
-    callFrames : [
-    ]
-    stackChanged : false
+    status : Ok
 }
 console.log message from function after patching:
 {

test/inspector/debugger/set-breakpoint-after-liveedit-expected.txt

 Breakpoint in liveedited script
 Update script source
 {
-    callFrames : [
-    ]
-    stackChanged : false
+    status : Ok
 }
 Set breakpoint
 {

test/inspector/debugger/set-script-source-exception-expected.txt

@@ -19,5 +19,6 @@ Running test: testSourceWithSyntaxError
             lineNumber : 1
             text : Uncaught SyntaxError: Invalid or unexpected token
         }
+        status : CompileError
     }
 }

test/inspector/debugger/set-script-source-expected.txt

@@ -2,9 +2,7 @@ Tests Debugger.setScriptSource
 TestExpression(2,4) === 6
 Update current script source 'a + b' -> 'a * b'..
 {
-    callFrames : [
-    ]
-    stackChanged : false
+    status : Ok
 }
 TestExpression(2,4) === 8
 Update current script source 'a * b' -> 'a # b'..
@@ -15,5 +13,6 @@ Update current script source 'a * b' -> 'a # b'..
         lineNumber : 1
         text : Uncaught SyntaxError: Invalid or unexpected token
     }
+    status : CompileError
 }
 TestExpression(2,4) === 8

test/inspector/regress/regress-crbug-1195927-expected.txt

@@ -15,12 +15,7 @@ foo (foo.js:2:2)
 (anonymous) (:0:0)
 Debugger.setScriptSource result:
 {
-    exceptionDetails : {
-        columnNumber : 0
-        exceptionId : <exceptionId>
-        lineNumber : 0
-        text : 
-    }
+    status : BlockedByActiveFunction
 }
 foo(42) result:
 {

test/inspector/regress/regress-crbug-1328453-expected.txt

@@ -2,8 +2,6 @@ Don't crash when live editing an unused inner function [crbug.com/1328453]
 {
     id : <messageId>
     result : {
-        callFrames : [
-        ]
-        stackChanged : false
+        status : Ok
     }
 }