inspector: generates additional pause event for instrumentation pauses

When 'beforeScriptExecution' is enabled, a pause event may be generated
with a reason of 'instrumentation' rather than 'other.' This patch
ensures that in the case of a schedule-break, both an 'instrumentation'
and 'other' pause event is generated.

This is important for debuggers that rely on getting 'other' breakpoints
to determine if they should actually break, or continue executation.

Change-Id: I73613f4df6fa7942e7ca2be58853e5420589ba0f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2915680Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74827}

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

@@ -1915,6 +1915,17 @@ void V8DebuggerAgentImpl::ScriptCollected(const V8DebuggerScript* script) {
   }
 }
 
+std::vector<v8::debug::BreakpointId>
+V8DebuggerAgentImpl::instrumentationBreakpointIdsMatching(
+    const std::vector<v8::debug::BreakpointId>& ids) {
+  std::vector<v8::debug::BreakpointId> instrumentationBreakpointIds;
+  for (const v8::debug::BreakpointId& id : ids) {
+    if (m_breakpointsOnScriptRun.count(id) > 0)
+      instrumentationBreakpointIds.push_back(id);
+  }
+  return instrumentationBreakpointIds;
+}
+
 Response V8DebuggerAgentImpl::processSkipList(
     protocol::Array<protocol::Debugger::LocationRange>* skipList) {
   std::unordered_map<String16, std::vector<std::pair<int, int>>> skipListInit;

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

@@ -167,6 +167,11 @@ 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);
+
  private:
   void enableImpl();
 

src/inspector/v8-debugger.cc

@@ -404,6 +404,7 @@ void V8Debugger::handleProgramBreak(
     v8::debug::PrepareStep(m_isolate, v8::debug::StepOut);
     return;
   }
+  const bool forScheduledBreak = hasScheduledBreakOnNextFunctionCall();
   m_targetContextGroupId = 0;
   m_pauseOnNextCallRequested = false;
   m_pauseOnAsyncCall = false;
@@ -432,15 +433,78 @@ void V8Debugger::handleProgramBreak(
   DCHECK(contextGroupId);
   m_pausedContextGroupId = contextGroupId;
 
+  // First pass is for any instrumentation breakpoints. This effectively does
+  // nothing if none of the breakpoints are instrumentation breakpoints.
+  // `sessionToInstrumentationBreakpoints` is only created if there is at least
+  // one session with an instrumentation breakpoint.
+  std::unique_ptr<
+      std::map<V8InspectorSessionImpl*, std::vector<v8::debug::BreakpointId>>>
+      sessionToInstrumentationBreakpoints;
+  if (forScheduledBreak) {
+    m_inspector->forEachSession(
+        contextGroupId,
+        [&pausedContext, &breakpointIds, &sessionToInstrumentationBreakpoints](
+            V8InspectorSessionImpl* session) {
+          if (!session->debuggerAgent()->acceptsPause(false)) return;
+
+          const std::vector<v8::debug::BreakpointId>
+              instrumentationBreakpointIds =
+                  session->debuggerAgent()
+                      ->instrumentationBreakpointIdsMatching(breakpointIds);
+          if (instrumentationBreakpointIds.empty()) return;
+
+          if (!sessionToInstrumentationBreakpoints) {
+            sessionToInstrumentationBreakpoints = std::make_unique<
+                std::map<V8InspectorSessionImpl*,
+                         std::vector<v8::debug::BreakpointId>>>();
+          }
+          (*sessionToInstrumentationBreakpoints)[session] =
+              instrumentationBreakpointIds;
+          session->debuggerAgent()->didPause(
+              InspectedContext::contextId(pausedContext), {},
+              instrumentationBreakpointIds,
+              v8::debug::ExceptionType::kException, false, false, false);
+        });
+    if (sessionToInstrumentationBreakpoints) {
+      v8::Context::Scope scope(pausedContext);
+      m_inspector->client()->runMessageLoopOnPause(contextGroupId);
+
+      m_inspector->forEachSession(
+          contextGroupId, [&sessionToInstrumentationBreakpoints](
+                              V8InspectorSessionImpl* session) {
+            if (session->debuggerAgent()->enabled() &&
+                sessionToInstrumentationBreakpoints->count(session)) {
+              session->debuggerAgent()->didContinue();
+            }
+          });
+    }
+  }
+
+  // Second pass is for other breakpoints (which may include instrumentation
+  // breakpoints in certain scenarios).
   m_inspector->forEachSession(
-      contextGroupId, [&pausedContext, &exception, &breakpointIds,
-                       &exceptionType, &isUncaught, &scheduledOOMBreak,
-                       &scheduledAssertBreak](V8InspectorSessionImpl* session) {
+      contextGroupId,
+      [&pausedContext, &exception, &breakpointIds, &exceptionType, &isUncaught,
+       &scheduledOOMBreak, &scheduledAssertBreak,
+       &sessionToInstrumentationBreakpoints](V8InspectorSessionImpl* session) {
         if (session->debuggerAgent()->acceptsPause(scheduledOOMBreak)) {
+          std::vector<v8::debug::BreakpointId> sessionBreakpointIds =
+              breakpointIds;
+          if (sessionToInstrumentationBreakpoints) {
+            const std::vector<v8::debug::BreakpointId>
+                instrumentationBreakpointIds =
+                    (*sessionToInstrumentationBreakpoints)[session];
+            for (const v8::debug::BreakpointId& breakpointId :
+                 instrumentationBreakpointIds) {
+              auto iter = std::find(sessionBreakpointIds.begin(),
+                                    sessionBreakpointIds.end(), breakpointId);
+              sessionBreakpointIds.erase(iter);
+            }
+          }
           session->debuggerAgent()->didPause(
               InspectedContext::contextId(pausedContext), exception,
-              breakpointIds, exceptionType, isUncaught, scheduledOOMBreak,
-              scheduledAssertBreak);
+              sessionBreakpointIds, exceptionType, isUncaught,
+              scheduledOOMBreak, scheduledAssertBreak);
         }
       });
   {