[debugger] Do not clear the breakpoint reasons on instrumentation break

Calling didContinue() after having paused on an instrumentation break
clears the breakpoint reasons that were stored in the debugger agent.

This removes clearBreakDetails() from didContinue() and specifically
calls it if we need it.

Drive-by: removing left-over dead code

Bug: chromium:1229541
Change-Id: I49f598d0e97801661e003c3911967c64ea63373e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3477099Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Kim-Anh Tran <kimanh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79203}

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

@@ -1740,11 +1740,6 @@ void V8DebuggerAgentImpl::setScriptInstrumentationBreakpointIfNeeded(
   }
   v8::debug::BreakpointId debuggerBreakpointId;
   if (!scriptRef->setInstrumentationBreakpoint(&debuggerBreakpointId)) return;
-  std::unique_ptr<protocol::DictionaryValue> data =
-      protocol::DictionaryValue::create();
-  data->setString("url", scriptRef->sourceURL());
-  data->setString("scriptId", scriptRef->scriptId());
-  if (!sourceMapURL.isEmpty()) data->setString("sourceMapURL", sourceMapURL);
 
   m_debuggerBreakpointIdToBreakpointId[debuggerBreakpointId] = breakpointId;
   m_breakpointIdToDebuggerBreakpointIds[breakpointId].push_back(
@@ -1897,7 +1892,6 @@ void V8DebuggerAgentImpl::didPause(
 }
 
 void V8DebuggerAgentImpl::didContinue() {
-  clearBreakDetails();
   m_frontend.resumed();
   m_frontend.flush();
 }

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

@@ -169,10 +169,7 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend {
 
   v8::Isolate* isolate() { return m_isolate; }
 
-  // Returns the intersection of `ids` and the current instrumentation
-  // breakpoint ids.
-  std::vector<v8::debug::BreakpointId> instrumentationBreakpointIdsMatching(
-      const std::vector<v8::debug::BreakpointId>& ids);
+  void clearBreakDetails();
 
  private:
   void enableImpl();
@@ -192,7 +189,6 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend {
                          v8::Local<v8::String> condition);
   void removeBreakpointImpl(const String16& breakpointId,
                             const std::vector<V8DebuggerScript*>& scripts);
-  void clearBreakDetails();
 
   void internalSetAsyncCallStackDepth(int);
   void increaseCachedSkipStackGeneration();

src/inspector/v8-debugger.cc

@@ -433,8 +433,10 @@ void V8Debugger::handleProgramBreak(
   }
   m_inspector->forEachSession(contextGroupId,
                               [](V8InspectorSessionImpl* session) {
-                                if (session->debuggerAgent()->enabled())
+                                if (session->debuggerAgent()->enabled()) {
+                                  session->debuggerAgent()->clearBreakDetails();
                                   session->debuggerAgent()->didContinue();
+                                }
                               });
 
   if (m_scheduledOOMBreak) m_isolate->RestoreOriginalHeapLimit();

test/inspector/debugger/set-instrumentation-breakpoint-expected.txt

@@ -112,3 +112,15 @@ paused with reason: instrumentation
 Remove instrumentation breakpoint..
 evaluate another script..
 no breakpoint was hit
+
+Running test: testInstrumentationCoincideWithScheduledPauseOnNextStatement
+set breakpoint..
+set instrumentation
+paused with reason: instrumentation
+{
+    scriptId : <scriptId>
+    url : 
+}
+paused with reason: instrumentation:scriptFirstStatement
+{
+}

test/inspector/debugger/set-instrumentation-breakpoint.js

@@ -193,6 +193,33 @@ InspectorTest.runAsyncTestSuite([
     await Protocol.Runtime.evaluate({expression: 'console.log(3) //# sourceURL=foo.js'});
     InspectorTest.log('no breakpoint was hit');
 
+    await Protocol.Debugger.disable();
+    await Protocol.Runtime.disable();
+  },
+
+  async function testInstrumentationCoincideWithScheduledPauseOnNextStatement() {
+    await Protocol.Runtime.enable();
+    await Protocol.Debugger.enable();
+    InspectorTest.log('set breakpoint..');
+    InspectorTest.log('set instrumentation');
+    await Protocol.Debugger.setInstrumentationBreakpoint({
+      instrumentation: 'beforeScriptExecution'
+    });
+    contextGroup.schedulePauseOnNextStatement('instrumentation:scriptFirstStatement', '{}');
+    const runPromise = Protocol.Runtime.evaluate({expression: 'console.log(3)'});
+    {
+      const { params: { reason, data } } = await Protocol.Debugger.oncePaused();
+      InspectorTest.log(`paused with reason: ${reason}`);
+      InspectorTest.logMessage(data);
+      Protocol.Debugger.resume();
+    }
+    {
+      const { params: { reason, data } } = await Protocol.Debugger.oncePaused();
+      InspectorTest.log(`paused with reason: ${reason}`);
+      InspectorTest.logMessage(data);
+      Protocol.Debugger.resume();
+    }
+    await runPromise;
     await Protocol.Debugger.disable();
     await Protocol.Runtime.disable();
   }