Reland "[inspector] Allow Debugger.setScriptSource to edit top-most function"

This is a reland of commit dac61556

This is a straight-up reland with no changes, because:
  1) The failure doesn't reproduce locally
  2) The failing flaky test that triggered the revert is not related
     to the code modified by this CL and should (in theory) not be
     impacted.

Original change's description:
> [inspector] Allow Debugger.setScriptSource to edit top-most function
>
> This CL adds a new boolean flag on the Debugger.setScriptSource CDP
> method that gets piped all the way through to the live-edit mechanism.
> The new flag enables live-editing of the top-most function while
> paused.
>
> The CL adds a couple of tests that cover the new core use cases for
> this flag.
>
> R=jarin@chromium.org
>
> Bug: chromium:1334484
> Change-Id: I12fec591b2b6550d89748714620e629548e1b9c1
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3695354
> Reviewed-by: Kim-Anh Tran <kimanh@chromium.org>
> Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
> Commit-Queue: Simon Zünd <szuend@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#81127}

Bug: chromium:1334484
Change-Id: I9a9bf7e03d81c86adb4819b9756dd9afcf6fa021
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3706398Reviewed-by: Kim-Anh Tran <kimanh@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81171}

include/js_protocol.pdl

@@ -441,6 +441,12 @@ domain Debugger
       Runtime.CallArgument newValue
 
   # Edits JavaScript source live.
+  #
+  # In general, functions that are currently on the stack can not be edited with
+  # a single exception: If the edited function is the top-most stack frame and
+  # that is the only activation of that function on the stack. In this case
+  # the live edit will be successful and a `Debugger.restartFrame` for the
+  # top-most function is automatically triggered.
   command setScriptSource
     parameters
       # Id of the script to edit.
@@ -450,6 +456,9 @@ domain Debugger
       #  If true the change will not actually be applied. Dry run may be used to get result
       # description without actually modifying the code.
       optional boolean dryRun
+      # If true, then `scriptSource` is allowed to change the function on top of the stack
+      # as long as the top-most stack frame is the only activation of that function.
+      experimental optional boolean allowTopFrameEditing
     returns
       # New stack trace in case editing has happened while VM was stopped.
       deprecated optional array of CallFrame callFrames

src/debug/debug-interface.cc

@@ -669,15 +669,13 @@ Location Script::GetSourceLocation(int offset) const {
 }
 
 bool Script::SetScriptSource(Local<String> newSource, bool preview,
+                             bool allow_top_frame_live_editing,
                              LiveEditResult* result) const {
   i::Handle<i::Script> script = Utils::OpenHandle(this);
   i::Isolate* isolate = script->GetIsolate();
-  // TODO(crbug.com/1334484): Pass `allow_top_frame_live_editing` through from
-  // the
-  //                          inspector.
   return isolate->debug()->SetScriptSource(
       script, Utils::OpenHandle(*newSource), preview,
-      /* allow_top_frame_live_editing */ false, result);
+      allow_top_frame_live_editing, result);
 }
 
 bool Script::SetBreakpoint(Local<String> condition, Location* location,

src/debug/debug-interface.h

@@ -223,6 +223,7 @@ class V8_EXPORT_PRIVATE Script {
       GetSourceOffsetMode mode = GetSourceOffsetMode::kStrict) const;
   v8::debug::Location GetSourceLocation(int offset) const;
   bool SetScriptSource(v8::Local<v8::String> newSource, bool preview,
+                       bool allow_top_frame_live_editing,
                        LiveEditResult* result) const;
   bool SetBreakpoint(v8::Local<v8::String> condition, debug::Location* location,
                      BreakpointId* id) const;

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

@@ -1021,6 +1021,7 @@ const char* buildStatus(v8::debug::LiveEditResult::Status status) {
 
 Response V8DebuggerAgentImpl::setScriptSource(
     const String16& scriptId, const String16& newContent, Maybe<bool> dryRun,
+    Maybe<bool> allowTopFrameEditing,
     Maybe<protocol::Array<protocol::Debugger::CallFrame>>* newCallFrames,
     Maybe<bool>* stackChanged,
     Maybe<protocol::Runtime::StackTrace>* asyncStackTrace,
@@ -1040,9 +1041,11 @@ Response V8DebuggerAgentImpl::setScriptSource(
   v8::HandleScope handleScope(m_isolate);
   v8::Local<v8::Context> context = inspected->context();
   v8::Context::Scope contextScope(context);
+  const bool allowTopFrameLiveEditing = allowTopFrameEditing.fromMaybe(false);
 
   v8::debug::LiveEditResult result;
-  it->second->setSource(newContent, dryRun.fromMaybe(false), &result);
+  it->second->setSource(newContent, dryRun.fromMaybe(false),
+                        allowTopFrameLiveEditing, &result);
   *status = buildStatus(result.status);
   if (result.status == v8::debug::LiveEditResult::COMPILE_ERROR) {
     *optOutCompileError =
@@ -1056,6 +1059,16 @@ Response V8DebuggerAgentImpl::setScriptSource(
             .build();
     return Response::Success();
   }
+
+  if (result.restart_top_frame_required) {
+    CHECK(allowTopFrameLiveEditing);
+    // Nothing could have happened to the JS stack since the live edit so
+    // restarting the top frame is guaranteed to be successful.
+    CHECK(m_debugger->restartFrame(m_session->contextGroupId(),
+                                   /* callFrameOrdinal */ 0));
+    m_session->releaseObjectGroup(kBacktraceObjectGroup);
+  }
+
   return Response::Success();
 }
 

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

@@ -83,7 +83,7 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend {
           locations) override;
   Response setScriptSource(
       const String16& inScriptId, const String16& inScriptSource,
-      Maybe<bool> dryRun,
+      Maybe<bool> dryRun, Maybe<bool> allowTopFrameEditing,
       Maybe<protocol::Array<protocol::Debugger::CallFrame>>* optOutCallFrames,
       Maybe<bool>* optOutStackChanged,
       Maybe<protocol::Runtime::StackTrace>* optOutAsyncStackTrace,

src/inspector/v8-debugger-script.cc

@@ -124,10 +124,12 @@ class ActualScript : public V8DebuggerScript {
   }
 
   void setSource(const String16& newSource, bool preview,
+                 bool allowTopFrameLiveEditing,
                  v8::debug::LiveEditResult* result) override {
     v8::EscapableHandleScope scope(m_isolate);
     v8::Local<v8::String> v8Source = toV8String(m_isolate, newSource);
-    if (!m_script.Get(m_isolate)->SetScriptSource(v8Source, preview, result)) {
+    if (!m_script.Get(m_isolate)->SetScriptSource(
+            v8Source, preview, allowTopFrameLiveEditing, result)) {
       result->message = scope.Escape(result->message);
       return;
     }

src/inspector/v8-debugger-script.h

@@ -82,6 +82,7 @@ class V8DebuggerScript {
   void setSourceURL(const String16&);
   virtual void setSourceMappingURL(const String16&) = 0;
   virtual void setSource(const String16& source, bool preview,
+                         bool allowTopFrameLiveEditing,
                          v8::debug::LiveEditResult* result) = 0;
 
   virtual bool getPossibleBreakpoints(

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

+Checks that setScriptSource fails for editing functions that are below the top-most frame on the stack
+Paused at (before live edit):
+function foo(b) {
+  #debugger;
+  return b + 25;
+
+Debugger.setScriptSource result:
+{
+    status : BlockedByActiveFunction
+}
+Evaluation result:
+{
+    description : 33
+    type : number
+    value : 33
+}

test/inspector/debugger/set-script-source-active-function.js

+// Copyright 2022 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.
+
+const {session, contextGroup, Protocol} = InspectorTest.start(
+  'Checks that setScriptSource fails for editing functions that are below the top-most frame on the stack');
+
+const originalScript = `
+function foo(b) {
+  debugger;
+  return b + 25;
+}
+function testExpression(a, b) {
+  return a + foo(b);
+}`;
+
+contextGroup.addScript(originalScript);
+
+const replacementScript = originalScript.replace('a + foo(b)', 'a * foo(b)');
+
+session.setupScriptMap();
+
+(async () => {
+  Protocol.Debugger.enable();
+  const { params: { scriptId } } = await Protocol.Debugger.onceScriptParsed();
+
+  const evaluatePromise = Protocol.Runtime.evaluate({ expression: 'testExpression(3, 5)' });
+  let { params: { callFrames: pausedCallFrames } } = await Protocol.Debugger.oncePaused();
+  InspectorTest.log('Paused at (before live edit):');
+  await session.logSourceLocation(pausedCallFrames[0].location);
+
+  const response = await Protocol.Debugger.setScriptSource({ scriptId, scriptSource: replacementScript, allowTopFrameEditing: true })
+  InspectorTest.log('Debugger.setScriptSource result:');
+  InspectorTest.logMessage(response.result);
+
+  Protocol.Debugger.resume();
+
+  const { result: { result } } = await evaluatePromise;
+  InspectorTest.log('Evaluation result:');
+  InspectorTest.logMessage(result);
+
+  InspectorTest.completeTest();
+})();

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

+Checks that setScriptSource works for editing the top-most stack frame
+Paused at (before live edit):
+function testExpression(a, b) {
+  #debugger;
+  return a + b;
+
+Paused at (after live edit):
+function testExpression(a, b) {
+  return a * b;#
+}
+
+Result:
+{
+    description : 15
+    type : number
+    value : 15
+}

test/inspector/debugger/set-script-source-top-frame-with-additional-activations-expected.txt

+Checks that setScriptSource fails when editing the top-most stack frame, but that function also has an activation further down the stack
+First pause at (before live edit):
+function testExpression(a, b) {
+  #debugger;
+  if (!a && !b) {
+
+Second pause at (before live edit):
+function testExpression(a, b) {
+  #debugger;
+  if (!a && !b) {
+
+Debugger.setScriptSource result:
+{
+    status : BlockedByActiveFunction
+}
+Evaluation result:
+{
+    description : 8
+    type : number
+    value : 8
+}

test/inspector/debugger/set-script-source-top-frame-with-additional-activations.js

+// Copyright 2022 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.
+
+const {session, contextGroup, Protocol} = InspectorTest.start(
+  'Checks that setScriptSource fails when editing the top-most stack frame, but that function also has an activation further down the stack');
+
+const script = `
+function callsTestExpression(a, b) {
+  return testExpression(a, b);
+}
+
+function testExpression(a, b) {
+  debugger;
+  if (!a && !b) {
+    return callsTestExpression(5, 3);
+  }
+  return a + b;
+}`;
+const replacementScript = script.replace('a + b', 'a * b');
+
+contextGroup.addScript(script);
+
+session.setupScriptMap();
+
+(async () => {
+  Protocol.Debugger.enable();
+  const { params: { scriptId } } = await Protocol.Debugger.onceScriptParsed();
+
+  const evaluatePromise = Protocol.Runtime.evaluate({ expression: 'callsTestExpression()' });
+  let { params: { callFrames: pausedCallFrames } } = await Protocol.Debugger.oncePaused();
+  InspectorTest.log('First pause at (before live edit):');
+  await session.logSourceLocation(pausedCallFrames[0].location);
+
+  Protocol.Debugger.resume();
+  ({ params: { callFrames: pausedCallFrames } } = await Protocol.Debugger.oncePaused());
+  InspectorTest.log('Second pause at (before live edit):');
+  await session.logSourceLocation(pausedCallFrames[0].location);
+
+  const response = await Protocol.Debugger.setScriptSource({ scriptId, scriptSource: replacementScript, allowTopFrameEditing: true })
+  InspectorTest.log('Debugger.setScriptSource result:');
+  InspectorTest.logMessage(response.result);
+
+  Protocol.Debugger.resume();
+
+  const { result: { result } } = await evaluatePromise;
+  InspectorTest.log('Evaluation result:');
+  InspectorTest.logMessage(result);
+
+  InspectorTest.completeTest();
+})();

test/inspector/debugger/set-script-source-top-frame.js

+// Copyright 2022 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.
+
+const {session, contextGroup, Protocol} = InspectorTest.start(
+  'Checks that setScriptSource works for editing the top-most stack frame');
+
+contextGroup.addScript(`
+function testExpression(a, b) {
+  debugger;
+  return a + b;
+}`);
+
+const replacementScript = `
+function testExpression(a, b) {
+  return a * b;
+}`;
+
+session.setupScriptMap();
+
+(async () => {
+  Protocol.Debugger.enable();
+  const { params: { scriptId } } = await Protocol.Debugger.onceScriptParsed();
+
+  const evaluatePromise = Protocol.Runtime.evaluate({ expression: 'testExpression(3, 5)' });
+  let { params: { callFrames: pausedCallFrames } } = await Protocol.Debugger.oncePaused();
+  InspectorTest.log('Paused at (before live edit):');
+  await session.logSourceLocation(pausedCallFrames[0].location);
+
+  Protocol.Debugger.setScriptSource({ scriptId, scriptSource: replacementScript, allowTopFrameEditing: true });
+  ({ params: { callFrames: pausedCallFrames } } = await Protocol.Debugger.oncePaused());
+  InspectorTest.log('Paused at (after live edit):');
+  await session.logSourceLocation(pausedCallFrames[0].location, /* forceSourceRequest */ true);
+
+  Protocol.Debugger.resume();
+
+  const { result: { result } } = await evaluatePromise;
+  InspectorTest.log('Result:');
+  InspectorTest.logMessage(result);
+
+  InspectorTest.completeTest();
+})();