[inspector] support for nested scheduled breaks

In current implementation we don't support nested scheduled break at all. If one break was scheduled inside another and second one doesn't produce actual break (execution was in blackboxed code or no JavaScript was executed) then second one will clear first scheduled break even if any not blackboxed JavaScript will be executed later.

Ambiguous break reason is added for the case when we have more then one scheduled reason. "auxData" in this case contains object with array of { reason: reason, auxData: auxData } objects for each reason in 'reasons' property.

BUG=chromium:632405

Review-Url: https://codereview.chromium.org/2678313002
Cr-Commit-Position: refs/heads/master@{#43021}

src/inspector/js_protocol.json

@@ -728,7 +728,7 @@
                 "name": "paused",
                 "parameters": [
                     { "name": "callFrames", "type": "array", "items": { "$ref": "CallFrame" }, "description": "Call stack the virtual machine stopped on." },
-                    { "name": "reason", "type": "string", "enum": [ "XHR", "DOM", "EventListener", "exception", "assert", "debugCommand", "promiseRejection", "OOM", "other" ], "description": "Pause reason." },
+                    { "name": "reason", "type": "string", "enum": [ "XHR", "DOM", "EventListener", "exception", "assert", "debugCommand", "promiseRejection", "OOM", "other", "ambiguous" ], "description": "Pause reason." },
                     { "name": "data", "type": "object", "optional": true, "description": "Object containing break-specific auxiliary properties." },
                     { "name": "hitBreakpoints", "type": "array", "optional": true, "items": { "type": "string" }, "description": "Hit breakpoints IDs" },
                     { "name": "asyncStackTrace", "$ref": "Runtime.StackTrace", "optional": true, "description": "Async stack trace, if any." }

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

@@ -131,11 +131,9 @@ V8DebuggerAgentImpl::V8DebuggerAgentImpl(
       m_state(state),
       m_frontend(frontendChannel),
       m_isolate(m_inspector->isolate()),
-      m_breakReason(protocol::Debugger::Paused::ReasonEnum::Other),
       m_scheduledDebuggerStep(NoStep),
       m_javaScriptPauseScheduled(false),
       m_recursionLevelForStepOut(0) {
-  clearBreakDetails();
 }
 
 V8DebuggerAgentImpl::~V8DebuggerAgentImpl() {}
@@ -589,6 +587,22 @@ Response V8DebuggerAgentImpl::getScriptSource(const String16& scriptId,
   return Response::OK();
 }
 
+void V8DebuggerAgentImpl::pushBreakDetails(
+    const String16& breakReason,
+    std::unique_ptr<protocol::DictionaryValue> breakAuxData) {
+  m_breakReason.push_back(std::make_pair(breakReason, std::move(breakAuxData)));
+}
+
+void V8DebuggerAgentImpl::popBreakDetails() {
+  if (m_breakReason.empty()) return;
+  m_breakReason.pop_back();
+}
+
+void V8DebuggerAgentImpl::clearBreakDetails() {
+  std::vector<BreakReason> emptyBreakReason;
+  m_breakReason.swap(emptyBreakReason);
+}
+
 void V8DebuggerAgentImpl::schedulePauseOnNextStatement(
     const String16& breakReason,
     std::unique_ptr<protocol::DictionaryValue> data) {
@@ -596,9 +610,8 @@ void V8DebuggerAgentImpl::schedulePauseOnNextStatement(
       m_javaScriptPauseScheduled || isPaused() ||
       !m_debugger->breakpointsActivated())
     return;
-  m_breakReason = breakReason;
-  m_breakAuxData = std::move(data);
-  m_debugger->setPauseOnNextStatement(true);
+  if (m_breakReason.empty()) m_debugger->setPauseOnNextStatement(true);
+  pushBreakDetails(breakReason, std::move(data));
 }
 
 void V8DebuggerAgentImpl::schedulePauseOnNextStatementIfSteppingInto() {
@@ -606,14 +619,13 @@ void V8DebuggerAgentImpl::schedulePauseOnNextStatementIfSteppingInto() {
   if (m_scheduledDebuggerStep != StepInto || m_javaScriptPauseScheduled ||
       isPaused())
     return;
-  clearBreakDetails();
   m_debugger->setPauseOnNextStatement(true);
 }
 
 void V8DebuggerAgentImpl::cancelPauseOnNextStatement() {
   if (m_javaScriptPauseScheduled || isPaused()) return;
-  clearBreakDetails();
-  m_debugger->setPauseOnNextStatement(false);
+  popBreakDetails();
+  if (m_breakReason.empty()) m_debugger->setPauseOnNextStatement(false);
 }
 
 Response V8DebuggerAgentImpl::pause() {
@@ -1071,32 +1083,37 @@ void V8DebuggerAgentImpl::didPause(int contextId,
   m_pausedCallFrames.swap(frames);
   v8::HandleScope handles(m_isolate);
 
+  std::vector<BreakReason> hitReasons;
+
   if (isOOMBreak) {
-    m_breakReason = protocol::Debugger::Paused::ReasonEnum::OOM;
-    m_breakAuxData = nullptr;
+    hitReasons.push_back(
+        std::make_pair(protocol::Debugger::Paused::ReasonEnum::OOM, nullptr));
   } else if (!exception.IsEmpty()) {
     InjectedScript* injectedScript = nullptr;
     m_session->findInjectedScript(contextId, injectedScript);
     if (injectedScript) {
-      m_breakReason =
+      String16 breakReason =
           isPromiseRejection
               ? protocol::Debugger::Paused::ReasonEnum::PromiseRejection
               : protocol::Debugger::Paused::ReasonEnum::Exception;
       std::unique_ptr<protocol::Runtime::RemoteObject> obj;
       injectedScript->wrapObject(exception, kBacktraceObjectGroup, false, false,
                                  &obj);
+      std::unique_ptr<protocol::DictionaryValue> breakAuxData;
       if (obj) {
-        m_breakAuxData = obj->toValue();
-        m_breakAuxData->setBoolean("uncaught", isUncaught);
+        breakAuxData = obj->toValue();
+        breakAuxData->setBoolean("uncaught", isUncaught);
       } else {
-        m_breakAuxData = nullptr;
+        breakAuxData = nullptr;
       }
-      // m_breakAuxData might be null after this.
+      hitReasons.push_back(
+          std::make_pair(breakReason, std::move(breakAuxData)));
     }
   }
 
   std::unique_ptr<Array<String16>> hitBreakpointIds = Array<String16>::create();
 
+  bool hasDebugCommandBreakpointReason = false;
   for (const auto& point : hitBreakpoints) {
     DebugServerBreakpointToBreakpointIdAndSourceMap::iterator
         breakpointIterator = m_serverBreakpoints.find(point);
@@ -1105,17 +1122,46 @@ void V8DebuggerAgentImpl::didPause(int contextId,
       hitBreakpointIds->addItem(localId);
 
       BreakpointSource source = breakpointIterator->second.second;
-      if (m_breakReason == protocol::Debugger::Paused::ReasonEnum::Other &&
-          source == DebugCommandBreakpointSource)
-        m_breakReason = protocol::Debugger::Paused::ReasonEnum::DebugCommand;
+      if (!hasDebugCommandBreakpointReason &&
+          source == DebugCommandBreakpointSource) {
+        hasDebugCommandBreakpointReason = true;
+        hitReasons.push_back(std::make_pair(
+            protocol::Debugger::Paused::ReasonEnum::DebugCommand, nullptr));
+      }
     }
   }
 
+  for (size_t i = 0; i < m_breakReason.size(); ++i) {
+    hitReasons.push_back(std::move(m_breakReason[i]));
+  }
+  clearBreakDetails();
+
+  String16 breakReason = protocol::Debugger::Paused::ReasonEnum::Other;
+  std::unique_ptr<protocol::DictionaryValue> breakAuxData;
+  if (hitReasons.size() == 1) {
+    breakReason = hitReasons[0].first;
+    breakAuxData = std::move(hitReasons[0].second);
+  } else if (hitReasons.size() > 1) {
+    breakReason = protocol::Debugger::Paused::ReasonEnum::Ambiguous;
+    std::unique_ptr<protocol::ListValue> reasons =
+        protocol::ListValue::create();
+    for (size_t i = 0; i < hitReasons.size(); ++i) {
+      std::unique_ptr<protocol::DictionaryValue> reason =
+          protocol::DictionaryValue::create();
+      reason->setString("reason", hitReasons[i].first);
+      if (hitReasons[i].second)
+        reason->setObject("auxData", std::move(hitReasons[i].second));
+      reasons->pushValue(std::move(reason));
+    }
+    breakAuxData = protocol::DictionaryValue::create();
+    breakAuxData->setArray("reasons", std::move(reasons));
+  }
+
   std::unique_ptr<Array<CallFrame>> protocolCallFrames;
   Response response = currentCallFrames(&protocolCallFrames);
   if (!response.isSuccess()) protocolCallFrames = Array<CallFrame>::create();
-  m_frontend.paused(std::move(protocolCallFrames), m_breakReason,
-                    std::move(m_breakAuxData), std::move(hitBreakpointIds),
+  m_frontend.paused(std::move(protocolCallFrames), breakReason,
+                    std::move(breakAuxData), std::move(hitBreakpointIds),
                     currentAsyncStackTrace());
   m_scheduledDebuggerStep = NoStep;
   m_javaScriptPauseScheduled = false;
@@ -1137,10 +1183,13 @@ void V8DebuggerAgentImpl::breakProgram(
     const String16& breakReason,
     std::unique_ptr<protocol::DictionaryValue> data) {
   if (!enabled() || !m_debugger->canBreakProgram() || m_skipAllPauses) return;
-  m_breakReason = breakReason;
-  m_breakAuxData = std::move(data);
+  std::vector<BreakReason> currentScheduledReason;
+  currentScheduledReason.swap(m_breakReason);
+  pushBreakDetails(breakReason, std::move(data));
   m_scheduledDebuggerStep = NoStep;
   m_debugger->breakProgram();
+  popBreakDetails();
+  m_breakReason.swap(currentScheduledReason);
 }
 
 void V8DebuggerAgentImpl::breakProgramOnException(
@@ -1152,11 +1201,6 @@ void V8DebuggerAgentImpl::breakProgramOnException(
   breakProgram(breakReason, std::move(data));
 }
 
-void V8DebuggerAgentImpl::clearBreakDetails() {
-  m_breakReason = protocol::Debugger::Paused::ReasonEnum::Other;
-  m_breakAuxData = nullptr;
-}
-
 void V8DebuggerAgentImpl::setBreakpointAt(const String16& scriptId,
                                           int lineNumber, int columnNumber,
                                           BreakpointSource source,

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

@@ -191,8 +191,16 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend {
   BreakpointIdToDebuggerBreakpointIdsMap m_breakpointIdToDebuggerBreakpointIds;
   DebugServerBreakpointToBreakpointIdAndSourceMap m_serverBreakpoints;
   String16 m_continueToLocationBreakpointId;
-  String16 m_breakReason;
-  std::unique_ptr<protocol::DictionaryValue> m_breakAuxData;
+
+  using BreakReason =
+      std::pair<String16, std::unique_ptr<protocol::DictionaryValue>>;
+  std::vector<BreakReason> m_breakReason;
+
+  void pushBreakDetails(
+      const String16& breakReason,
+      std::unique_ptr<protocol::DictionaryValue> breakAuxData);
+  void popBreakDetails();
+
   DebuggerStep m_scheduledDebuggerStep;
   bool m_javaScriptPauseScheduled;
 

test/inspector/debugger/framework-nested-scheduled-break-expected.txt

+Checks nested scheduled break in framework code.
+break reason: framework-break
+break aux data: {
+  "data": "data for framework-break"
+}
+doFrameworkBreak (framework.js:20:2)
+doFrameworkWork (framework.js:15:2)
+frameworkCall (framework.js:9:2)
+testFunction (user.js:27:2)
+(anonymous) (expr.js:0:0)
+
+break reason: ambiguous
+break aux data: {
+  "reasons": [
+    {
+      "reason": "top-scheduled-break"
+    },
+    {
+      "reason": "top-framework-scheduled-break",
+      "auxData": {
+        "data": "data for top-framework-scheduled-break"
+      }
+    }
+  ]
+}
+callback (user.js:31:17)
+doFrameworkWork (framework.js:16:2)
+frameworkCall (framework.js:9:2)
+testFunction (user.js:27:2)
+(anonymous) (expr.js:0:0)
+
+break reason: user-break
+break aux data: {
+  "data": "data for user-break"
+}
+callback (user.js:32:2)
+doFrameworkWork (framework.js:16:2)
+frameworkCall (framework.js:9:2)
+testFunction (user.js:27:2)
+(anonymous) (expr.js:0:0)
+

test/inspector/debugger/framework-nested-scheduled-break.js

+// Copyright 2017 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.
+
+print('Checks nested scheduled break in framework code.');
+
+InspectorTest.addScript(`
+function frameworkCall(callback) {
+  callWithScheduledBreak(doFrameworkWork.bind(null, callback),
+    'top-framework-scheduled-break',
+    JSON.stringify({ data: 'data for top-framework-scheduled-break' }));
+}
+
+function doFrameworkWork(callback) {
+  callWithScheduledBreak(doFrameworkBreak, 'should-not-be-a-reason', '');
+  callback();
+}
+
+function doFrameworkBreak() {
+  breakProgram('framework-break', JSON.stringify({ data: 'data for framework-break' }));
+}
+
+//# sourceURL=framework.js`, 7, 26);
+
+InspectorTest.addScript(`
+function testFunction() {
+  callWithScheduledBreak(frameworkCall.bind(null, callback),
+    'top-scheduled-break', '');
+}
+
+function callback() {
+  breakProgram('user-break', JSON.stringify({ data: 'data for user-break' }));
+  return 42;
+}
+
+//# sourceURL=user.js`, 25, 26);
+
+InspectorTest.setupScriptMap();
+Protocol.Debugger.onPaused(message => {
+  InspectorTest.log('break reason: ' + message.params.reason);
+  InspectorTest.log('break aux data: ' + JSON.stringify(message.params.data || {}, null, '  '));
+  InspectorTest.logCallFrames(message.params.callFrames);
+  InspectorTest.log('');
+  Protocol.Debugger.resume();
+});
+Protocol.Debugger.enable()
+  .then(() => Protocol.Debugger.setBlackboxPatterns({patterns: ['framework\.js']}))
+  .then(() => Protocol.Runtime.evaluate({ expression: 'testFunction()//# sourceURL=expr.js'}))
+  .then(InspectorTest.completeTest);

test/inspector/debugger/inspector-break-api.js

@@ -37,6 +37,7 @@ InspectorTest.runTestSuite([
     Protocol.Runtime.evaluate({ expression: 'foo()//# sourceURL=expr1.js'})
       .then(() => Protocol.Runtime.evaluate({
         expression: 'foo()//# sourceURL=expr2.js'}))
+      .then(() => cancelPauseOnNextStatement())
       .then(next);
   },
 

test/inspector/inspector-test.cc

@@ -335,7 +335,8 @@ class InspectorExtension : public v8::Extension {
                       "native function detachInspector();"
                       "native function setMaxAsyncTaskStacks();"
                       "native function breakProgram();"
-                      "native function createObjectWithStrictCheck();") {}
+                      "native function createObjectWithStrictCheck();"
+                      "native function callWithScheduledBreak();") {}
 
   virtual v8::Local<v8::FunctionTemplate> GetNativeFunctionTemplate(
       v8::Isolate* isolate, v8::Local<v8::String> name) {
@@ -373,6 +374,13 @@ class InspectorExtension : public v8::Extension {
                    .FromJust()) {
       return v8::FunctionTemplate::New(
           isolate, InspectorExtension::CreateObjectWithStrictCheck);
+    } else if (name->Equals(context, v8::String::NewFromUtf8(
+                                         isolate, "callWithScheduledBreak",
+                                         v8::NewStringType::kNormal)
+                                         .ToLocalChecked())
+                   .FromJust()) {
+      return v8::FunctionTemplate::New(
+          isolate, InspectorExtension::CallWithScheduledBreak);
     }
     return v8::Local<v8::FunctionTemplate>();
   }
@@ -447,6 +455,30 @@ class InspectorExtension : public v8::Extension {
         templ->NewInstance(args.GetIsolate()->GetCurrentContext())
             .ToLocalChecked());
   }
+
+  static void CallWithScheduledBreak(
+      const v8::FunctionCallbackInfo<v8::Value>& args) {
+    if (args.Length() != 3 || !args[0]->IsFunction() || !args[1]->IsString() ||
+        !args[2]->IsString()) {
+      fprintf(stderr, "Internal error: breakProgram('reason', 'details').");
+      Exit();
+    }
+    v8_inspector::V8InspectorSession* session =
+        InspectorClientImpl::SessionFromContext(
+            args.GetIsolate()->GetCurrentContext());
+    CHECK(session);
+
+    v8::internal::Vector<uint16_t> reason = ToVector(args[1].As<v8::String>());
+    v8_inspector::StringView reason_view(reason.start(), reason.length());
+    v8::internal::Vector<uint16_t> details = ToVector(args[2].As<v8::String>());
+    v8_inspector::StringView details_view(details.start(), details.length());
+    session->schedulePauseOnNextStatement(reason_view, details_view);
+    v8::Local<v8::Context> context = args.GetIsolate()->GetCurrentContext();
+    v8::MaybeLocal<v8::Value> result;
+    result = args[0].As<v8::Function>()->Call(context, context->Global(), 0,
+                                              nullptr);
+    session->cancelPauseOnNextStatement();
+  }
 };
 
 v8::Local<v8::String> ToString(v8::Isolate* isolate,