[inspector] Pass the Context into terminateExecution

This is a reland of commit 8016f5c667
Additional HandleScopes are added in 3 spots and an additional
test was added to cover the crash that caused the revert.

Adding and removing the MicrotasksCompletedCallback should be
associated with the microtask queue of the Context. We store the
context as WeakPtr and always remove the callback when it completes
regardless of the state of the debugger.

BUG=v8:13450

Change-Id: Ie4d6edcb561c6753a6d34d84cfcf4989bb6e9321
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4062397
Reviewed-by: Simon Zünd <szuend@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84584}

src/inspector/v8-debugger.cc

@@ -84,8 +84,14 @@ V8Debugger::V8Debugger(v8::Isolate* isolate, V8InspectorImpl* inspector)
 V8Debugger::~V8Debugger() {
   m_isolate->RemoveCallCompletedCallback(
       &V8Debugger::terminateExecutionCompletedCallback);
-  m_isolate->RemoveMicrotasksCompletedCallback(
-      &V8Debugger::terminateExecutionCompletedCallbackIgnoringData);
+  if (!m_terminateExecutionCallbackContext.IsEmpty()) {
+    v8::HandleScope handles(m_isolate);
+    v8::MicrotaskQueue* microtask_queue =
+        m_terminateExecutionCallbackContext.Get(m_isolate)->GetMicrotaskQueue();
+    microtask_queue->RemoveMicrotasksCompletedCallback(
+        &V8Debugger::terminateExecutionCompletedCallbackIgnoringData,
+        microtask_queue);
+  }
 }
 
 void V8Debugger::enable() {
@@ -283,6 +289,7 @@ void V8Debugger::stepOutOfFunction(int targetContextGroupId) {
 }
 
 void V8Debugger::terminateExecution(
+    v8::Local<v8::Context> context,
     std::unique_ptr<TerminateExecutionCallback> callback) {
   if (m_terminateExecutionCallback) {
     if (callback) {
@@ -291,23 +298,38 @@ void V8Debugger::terminateExecution(
     }
     return;
   }
+  v8::HandleScope handles(m_isolate);
   m_terminateExecutionCallback = std::move(callback);
+  m_terminateExecutionCallbackContext.Reset(m_isolate, context);
+  m_terminateExecutionCallbackContext.SetWeak();
   m_isolate->AddCallCompletedCallback(
       &V8Debugger::terminateExecutionCompletedCallback);
-  m_isolate->AddMicrotasksCompletedCallback(
-      &V8Debugger::terminateExecutionCompletedCallbackIgnoringData);
+  v8::MicrotaskQueue* microtask_queue = context->GetMicrotaskQueue();
+  microtask_queue->AddMicrotasksCompletedCallback(
+      &V8Debugger::terminateExecutionCompletedCallbackIgnoringData,
+      microtask_queue);
   m_isolate->TerminateExecution();
 }
 
 void V8Debugger::reportTermination() {
-  if (!m_terminateExecutionCallback) return;
+  if (!m_terminateExecutionCallback) {
+    DCHECK(m_terminateExecutionCallbackContext.IsEmpty());
+    return;
+  }
+  v8::HandleScope handles(m_isolate);
   m_isolate->RemoveCallCompletedCallback(
       &V8Debugger::terminateExecutionCompletedCallback);
-  m_isolate->RemoveMicrotasksCompletedCallback(
-      &V8Debugger::terminateExecutionCompletedCallbackIgnoringData);
+  if (!m_terminateExecutionCallbackContext.IsEmpty()) {
+    v8::MicrotaskQueue* microtask_queue =
+        m_terminateExecutionCallbackContext.Get(m_isolate)->GetMicrotaskQueue();
+    microtask_queue->RemoveMicrotasksCompletedCallback(
+        &V8Debugger::terminateExecutionCompletedCallbackIgnoringData,
+        microtask_queue);
+  }
   m_isolate->CancelTerminateExecution();
   m_terminateExecutionCallback->sendSuccess();
   m_terminateExecutionCallback.reset();
+  m_terminateExecutionCallbackContext.Reset();
 }
 
 void V8Debugger::terminateExecutionCompletedCallback(v8::Isolate* isolate) {
@@ -318,7 +340,12 @@ void V8Debugger::terminateExecutionCompletedCallback(v8::Isolate* isolate) {
 }
 
 void V8Debugger::terminateExecutionCompletedCallbackIgnoringData(
-    v8::Isolate* isolate, void*) {
+    v8::Isolate* isolate, void* data) {
+  DCHECK(data);
+  // Ensure that after every microtask completed callback we remove the
+  // callback regardless of how `terminateExecutionCompletedCallback` behaves.
+  static_cast<v8::MicrotaskQueue*>(data)->RemoveMicrotasksCompletedCallback(
+      &V8Debugger::terminateExecutionCompletedCallbackIgnoringData, data);
   terminateExecutionCompletedCallback(isolate);
 }
 

src/inspector/v8-debugger.h

@@ -71,7 +71,8 @@ class V8Debugger : public v8::debug::DebugDelegate,
   void stepOverStatement(int targetContextGroupId);
   void stepOutOfFunction(int targetContextGroupId);
 
-  void terminateExecution(std::unique_ptr<TerminateExecutionCallback> callback);
+  void terminateExecution(v8::Local<v8::Context> context,
+                          std::unique_ptr<TerminateExecutionCallback> callback);
 
   Response continueToLocation(int targetContextGroupId,
                               V8DebuggerScript* script,
@@ -297,6 +298,7 @@ class V8Debugger : public v8::debug::DebugDelegate,
   std::unordered_map<int, internal::V8DebuggerId> m_contextGroupIdToDebuggerId;
 
   std::unique_ptr<TerminateExecutionCallback> m_terminateExecutionCallback;
+  v8::Global<v8::Context> m_terminateExecutionCallbackContext;
 };
 
 }  // namespace v8_inspector

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

@@ -709,7 +709,13 @@ Response V8RuntimeAgentImpl::getHeapUsage(double* out_usedSize,
 
 void V8RuntimeAgentImpl::terminateExecution(
     std::unique_ptr<TerminateExecutionCallback> callback) {
-  m_inspector->debugger()->terminateExecution(std::move(callback));
+  v8::HandleScope handles(m_inspector->isolate());
+  v8::Local<v8::Context> defaultContext =
+      m_inspector->client()->ensureDefaultContextInGroup(
+          m_session->contextGroupId());
+
+  m_inspector->debugger()->terminateExecution(defaultContext,
+                                              std::move(callback));
 }
 
 namespace {

test/inspector/runtime/terminate-execution-expected.txt

@@ -70,4 +70,4 @@ Terminate execution with pending microtasks
     result : {
     }
 }
-
+Terminate execution does not crash on destroy

test/inspector/runtime/terminate-execution.js

@@ -66,7 +66,19 @@ let {session, contextGroup, Protocol} =
   await paused2;
   Protocol.Runtime.terminateExecution().then(InspectorTest.logMessage);
   await Protocol.Debugger.resume();
-
   await Protocol.Runtime.disable();
+
+  InspectorTest.log('Terminate execution does not crash on destroy');
+  Protocol.Debugger.enable();
+  Protocol.Runtime.evaluate({
+    expression: `
+    while(true) {
+      let p = new Promise(resolve => setTimeout(resolve, 0));
+      await p;
+    }`
+  });
+  Protocol.Runtime.terminateExecution();
+  await Protocol.Debugger.disable();
+
   InspectorTest.completeTest();
 })();