[inspector] Make pausing on console.assert work with multiple sessions

Instead of going through debugger agent, this patch implements
console.assert pause similar to debugger statement and OOM break.

New test uncovered a bug, where pause on exceptions state mix up
between different context groups. Added a TODO to fix it.

BUG=chromium:590878

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

src/inspector/v8-console.cc

@@ -275,14 +275,7 @@ void V8Console::Assert(const v8::debug::ConsoleCallArguments& info) {
     arguments.push_back(
         toV8String(m_inspector->isolate(), String16("console.assert")));
   helper.reportCall(ConsoleAPIType::kAssert, arguments);
-
-  // TODO(dgozman): only break once, not per each session.
-  helper.forEachSession([](V8InspectorSessionImpl* session) {
-    if (session->debuggerAgent()->enabled()) {
-      session->debuggerAgent()->breakProgramOnException(
-          protocol::Debugger::Paused::ReasonEnum::Assert, nullptr);
-    }
-  });
+  m_inspector->debugger()->breakProgramOnAssert(helper.groupId());
 }
 
 void V8Console::MarkTimeline(const v8::debug::ConsoleCallArguments& info) {

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

@@ -764,6 +764,8 @@ Response V8DebuggerAgentImpl::setPauseOnExceptions(
 }
 
 void V8DebuggerAgentImpl::setPauseOnExceptionsImpl(int pauseState) {
+  // TODO(dgozman): this changes the global state and forces all context groups
+  // to pause. We should make this flag be per-context-group.
   m_debugger->setPauseOnExceptionsState(
       static_cast<v8::debug::ExceptionBreakState>(pauseState));
   m_state->setInteger(DebuggerAgentState::pauseOnExceptionsState, pauseState);
@@ -1137,7 +1139,7 @@ void V8DebuggerAgentImpl::didPause(int contextId,
                                    v8::Local<v8::Value> exception,
                                    const std::vector<String16>& hitBreakpoints,
                                    bool isPromiseRejection, bool isUncaught,
-                                   bool isOOMBreak) {
+                                   bool isOOMBreak, bool isAssert) {
   JavaScriptCallFrames frames = m_debugger->currentCallFrames();
   m_pausedCallFrames.swap(frames);
   v8::HandleScope handles(m_isolate);
@@ -1147,6 +1149,9 @@ void V8DebuggerAgentImpl::didPause(int contextId,
   if (isOOMBreak) {
     hitReasons.push_back(
         std::make_pair(protocol::Debugger::Paused::ReasonEnum::OOM, nullptr));
+  } else if (isAssert) {
+    hitReasons.push_back(std::make_pair(
+        protocol::Debugger::Paused::ReasonEnum::Assert, nullptr));
   } else if (!exception.IsEmpty()) {
     InjectedScript* injectedScript = nullptr;
     m_session->findInjectedScript(contextId, injectedScript);
@@ -1254,15 +1259,6 @@ void V8DebuggerAgentImpl::breakProgram(
   }
 }
 
-void V8DebuggerAgentImpl::breakProgramOnException(
-    const String16& breakReason,
-    std::unique_ptr<protocol::DictionaryValue> data) {
-  if (!enabled() ||
-      m_debugger->getPauseOnExceptionsState() == v8::debug::NoBreakOnException)
-    return;
-  breakProgram(breakReason, std::move(data));
-}
-
 void V8DebuggerAgentImpl::setBreakpointAt(const String16& scriptId,
                                           int lineNumber, int columnNumber,
                                           BreakpointSource source,

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

@@ -122,15 +122,14 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend {
   void cancelPauseOnNextStatement();
   void breakProgram(const String16& breakReason,
                     std::unique_ptr<protocol::DictionaryValue> data);
-  void breakProgramOnException(const String16& breakReason,
-                               std::unique_ptr<protocol::DictionaryValue> data);
 
   void reset();
 
   // Interface for V8InspectorImpl
   void didPause(int contextId, v8::Local<v8::Value> exception,
                 const std::vector<String16>& hitBreakpoints,
-                bool isPromiseRejection, bool isUncaught, bool isOOMBreak);
+                bool isPromiseRejection, bool isUncaught, bool isOOMBreak,
+                bool isAssert);
   void didContinue();
   void didParseSource(std::unique_ptr<V8DebuggerScript>, bool success);
 

src/inspector/v8-debugger.cc

@@ -363,6 +363,18 @@ void V8Debugger::continueProgram(int targetContextGroupId) {
   m_executionState.Clear();
 }
 
+void V8Debugger::breakProgramOnAssert(int targetContextGroupId) {
+  if (!enabled()) return;
+  if (m_pauseOnExceptionsState == v8::debug::NoBreakOnException) return;
+  // Don't allow nested breaks.
+  if (isPaused()) return;
+  if (!canBreakProgram()) return;
+  DCHECK(targetContextGroupId);
+  m_targetContextGroupId = targetContextGroupId;
+  m_scheduledAssertBreak = true;
+  v8::debug::BreakRightNow(m_isolate);
+}
+
 void V8Debugger::stepIntoStatement(int targetContextGroupId) {
   DCHECK(isPaused());
   DCHECK(!m_executionState.IsEmpty());
@@ -599,6 +611,7 @@ void V8Debugger::handleProgramBreak(v8::Local<v8::Context> pausedContext,
   m_breakRequested = false;
 
   bool scheduledOOMBreak = m_scheduledOOMBreak;
+  bool scheduledAssertBreak = m_scheduledAssertBreak;
   auto agentCheck = [&scheduledOOMBreak](V8DebuggerAgentImpl* agent) {
     return agent->enabled() && (scheduledOOMBreak || !agent->skipAllPauses());
   };
@@ -636,12 +649,13 @@ void V8Debugger::handleProgramBreak(v8::Local<v8::Context> pausedContext,
 
   m_inspector->forEachSession(
       contextGroupId, [&agentCheck, &pausedContext, &exception, &breakpointIds,
-                       &isPromiseRejection, &isUncaught,
-                       &scheduledOOMBreak](V8InspectorSessionImpl* session) {
+                       &isPromiseRejection, &isUncaught, &scheduledOOMBreak,
+                       &scheduledAssertBreak](V8InspectorSessionImpl* session) {
         if (agentCheck(session->debuggerAgent())) {
           session->debuggerAgent()->didPause(
               InspectedContext::contextId(pausedContext), exception,
-              breakpointIds, isPromiseRejection, isUncaught, scheduledOOMBreak);
+              breakpointIds, isPromiseRejection, isUncaught, scheduledOOMBreak,
+              scheduledAssertBreak);
         }
       });
   {
@@ -660,6 +674,7 @@ void V8Debugger::handleProgramBreak(v8::Local<v8::Context> pausedContext,
 
   if (m_scheduledOOMBreak) m_isolate->RestoreOriginalHeapLimit();
   m_scheduledOOMBreak = false;
+  m_scheduledAssertBreak = false;
   m_pausedContext.Clear();
   m_executionState.Clear();
 }

src/inspector/v8-debugger.h

@@ -52,6 +52,7 @@ class V8Debugger : public v8::debug::DebugDelegate {
   bool canBreakProgram();
   void breakProgram(int targetContextGroupId);
   void continueProgram(int targetContextGroupId);
+  void breakProgramOnAssert(int targetContextGroupId);
 
   void setPauseOnNextStatement(bool, int targetContextGroupId);
   void stepIntoStatement(int targetContextGroupId);
@@ -188,6 +189,7 @@ class V8Debugger : public v8::debug::DebugDelegate {
   v8::Local<v8::Context> m_pausedContext;
   int m_ignoreScriptParsedEventsCounter;
   bool m_scheduledOOMBreak = false;
+  bool m_scheduledAssertBreak = false;
   int m_targetContextGroupId = 0;
   int m_pausedContextGroupId = 0;
   String16 m_continueToLocationBreakpointId;

test/inspector/sessions/pause-on-console-assert-expected.txt

+Tests that multiple sessions pause once on console.assert.
+Pausing on exceptions in 1
+Asserting in 1
+Paused in 2 with reason assert
+Paused in 1 with reason assert
+Asserting in 2
+Paused in 2 with reason assert
+Paused in 1 with reason assert
+Pausing on exceptions in both
+Asserting in 1
+Paused in 2 with reason assert
+Paused in 1 with reason assert
+Asserting in 2
+Paused in 2 with reason assert
+Paused in 1 with reason assert
+Not pausing on exceptions
+Asserting in 1
+Asserting in 2
+Pausing on exceptions in 3 (different context group)
+Asserting in 3
+Paused in 3 with reason assert
+Asserting in 1
+Paused in 2 with reason assert
+Paused in 1 with reason assert

test/inspector/sessions/pause-on-console-assert.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.
+
+InspectorTest.log('Tests that multiple sessions pause once on console.assert.');
+
+(async function test() {
+  var contextGroup1 = new InspectorTest.ContextGroup();
+  var session1 = await connect(contextGroup1, 1);
+  var session2 = await connect(contextGroup1, 2);
+  var contextGroup2 = new InspectorTest.ContextGroup();
+  var session3 = await connect(contextGroup2, 3);
+
+  InspectorTest.log('Pausing on exceptions in 1');
+  await session1.Protocol.Debugger.setPauseOnExceptions({state: 'all'});
+  InspectorTest.log('Asserting in 1');
+  await session1.Protocol.Runtime.evaluate({expression: 'console.assert(false)'});
+  InspectorTest.log('Asserting in 2');
+  await session2.Protocol.Runtime.evaluate({expression: 'console.assert(false)'});
+
+  InspectorTest.log('Pausing on exceptions in both');
+  await session2.Protocol.Debugger.setPauseOnExceptions({state: 'all'});
+  InspectorTest.log('Asserting in 1');
+  await session1.Protocol.Runtime.evaluate({expression: 'console.assert(false)'});
+  InspectorTest.log('Asserting in 2');
+  await session2.Protocol.Runtime.evaluate({expression: 'console.assert(false)'});
+
+  InspectorTest.log('Not pausing on exceptions');
+  await session1.Protocol.Debugger.setPauseOnExceptions({state: 'none'});
+  await session2.Protocol.Debugger.setPauseOnExceptions({state: 'none'});
+  InspectorTest.log('Asserting in 1');
+  await session1.Protocol.Runtime.evaluate({expression: 'console.assert(false)'});
+  InspectorTest.log('Asserting in 2');
+  await session2.Protocol.Runtime.evaluate({expression: 'console.assert(false)'});
+
+  InspectorTest.log('Pausing on exceptions in 3 (different context group)');
+  await session3.Protocol.Debugger.setPauseOnExceptions({state: 'all'});
+  InspectorTest.log('Asserting in 3');
+  await session3.Protocol.Runtime.evaluate({expression: 'console.assert(false)'});
+  InspectorTest.log('Asserting in 1');
+  await session1.Protocol.Runtime.evaluate({expression: 'console.assert(false)'});
+
+  InspectorTest.completeTest();
+})();
+
+async function connect(contextGroup, num) {
+  var session = contextGroup.connect();
+  await session.Protocol.Debugger.enable();
+  session.Protocol.Debugger.onPaused(message => {
+    InspectorTest.log(`Paused in ${num} with reason ${message.params.reason}`);
+    session.Protocol.Debugger.resume();
+  });
+  return session;
+}