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

This reverts commit dac61556.

Reason for revert: https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20TSAN%20-%20no-concurrent-marking/9288/overview

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: I165269d6c1b001b516f10ae3716ffb57b675ab39
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3705378
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
Owners-Override: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81130}

include/js_protocol.pdl

@@ -441,12 +441,6 @@ 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.
@@ -456,9 +450,6 @@ 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

@@ -668,13 +668,15 @@ 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, result);
+      /* allow_top_frame_live_editing */ false, result);
 }
 
 bool Script::SetBreakpoint(Local<String> condition, Location* location,

src/debug/debug-interface.h

@@ -223,7 +223,6 @@ 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,7 +1021,6 @@ 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,
@@ -1041,11 +1040,9 @@ 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),
-                        allowTopFrameLiveEditing, &result);
+  it->second->setSource(newContent, dryRun.fromMaybe(false), &result);
   *status = buildStatus(result.status);
   if (result.status == v8::debug::LiveEditResult::COMPILE_ERROR) {
     *optOutCompileError =
@@ -1059,16 +1056,6 @@ 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> allowTopFrameEditing,
+      Maybe<bool> dryRun,
       Maybe<protocol::Array<protocol::Debugger::CallFrame>>* optOutCallFrames,
       Maybe<bool>* optOutStackChanged,
       Maybe<protocol::Runtime::StackTrace>* optOutAsyncStackTrace,

src/inspector/v8-debugger-script.cc

@@ -124,12 +124,10 @@ 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, allowTopFrameLiveEditing, result)) {
+    if (!m_script.Get(m_isolate)->SetScriptSource(v8Source, preview, result)) {
       result->message = scope.Escape(result->message);
       return;
     }

src/inspector/v8-debugger-script.h

@@ -82,7 +82,6 @@ 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();
-})();