[inspector] use same schedulePauseOnNextStatement strategy for Debugger.pause

+ added a test.

BUG=none
R=dgozman@chromium.org

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

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

@@ -198,8 +198,7 @@ V8DebuggerAgentImpl::V8DebuggerAgentImpl(
       m_enabled(false),
       m_state(state),
       m_frontend(frontendChannel),
-      m_isolate(m_inspector->isolate()),
-      m_javaScriptPauseScheduled(false) {}
+      m_isolate(m_inspector->isolate()) {}
 
 V8DebuggerAgentImpl::~V8DebuggerAgentImpl() {}
 
@@ -251,7 +250,6 @@ Response V8DebuggerAgentImpl::disable() {
   m_debugger->setAsyncCallStackDepth(this, 0);
   m_continueToLocationBreakpointId = String16();
   clearBreakDetails();
-  m_javaScriptPauseScheduled = false;
   m_skipAllPauses = false;
   m_state->setBoolean(DebuggerAgentState::skipAllPauses, false);
   m_state->remove(DebuggerAgentState::blackboxPattern);
@@ -291,6 +289,10 @@ void V8DebuggerAgentImpl::restore() {
 Response V8DebuggerAgentImpl::setBreakpointsActive(bool active) {
   if (!enabled()) return Response::Error(kDebuggerNotEnabled);
   m_debugger->setBreakpointsActivated(active);
+  if (!active && !m_breakReason.empty()) {
+    clearBreakDetails();
+    m_debugger->setPauseOnNextStatement(false, m_session->contextGroupId());
+  }
   return Response::OK();
 }
 
@@ -685,9 +687,7 @@ void V8DebuggerAgentImpl::clearBreakDetails() {
 void V8DebuggerAgentImpl::schedulePauseOnNextStatement(
     const String16& breakReason,
     std::unique_ptr<protocol::DictionaryValue> data) {
-  if (!enabled() || m_javaScriptPauseScheduled || isPaused() ||
-      !m_debugger->breakpointsActivated())
-    return;
+  if (!enabled() || isPaused() || !m_debugger->breakpointsActivated()) return;
   if (m_breakReason.empty()) {
     m_debugger->setPauseOnNextStatement(true, m_session->contextGroupId());
   }
@@ -695,18 +695,20 @@ void V8DebuggerAgentImpl::schedulePauseOnNextStatement(
 }
 
 void V8DebuggerAgentImpl::cancelPauseOnNextStatement() {
-  if (m_javaScriptPauseScheduled || isPaused()) return;
-  popBreakDetails();
-  if (m_breakReason.empty())
+  if (!enabled() || isPaused() || !m_debugger->breakpointsActivated()) return;
+  if (m_breakReason.size() == 1) {
     m_debugger->setPauseOnNextStatement(false, m_session->contextGroupId());
+  }
+  popBreakDetails();
 }
 
 Response V8DebuggerAgentImpl::pause() {
   if (!enabled()) return Response::Error(kDebuggerNotEnabled);
-  if (m_javaScriptPauseScheduled || isPaused()) return Response::OK();
-  clearBreakDetails();
-  m_javaScriptPauseScheduled = true;
+  if (isPaused()) return Response::OK();
+  if (m_breakReason.empty()) {
     m_debugger->setPauseOnNextStatement(true, m_session->contextGroupId());
+  }
+  pushBreakDetails(protocol::Debugger::Paused::ReasonEnum::Other, nullptr);
   return Response::OK();
 }
 
@@ -1240,7 +1242,6 @@ void V8DebuggerAgentImpl::didPause(int contextId,
   m_frontend.paused(std::move(protocolCallFrames), breakReason,
                     std::move(breakAuxData), std::move(hitBreakpointIds),
                     currentAsyncStackTrace());
-  m_javaScriptPauseScheduled = false;
 
   if (!m_continueToLocationBreakpointId.isEmpty()) {
     m_debugger->removeBreakpoint(m_continueToLocationBreakpointId);

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

@@ -198,7 +198,6 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend {
       std::unique_ptr<protocol::DictionaryValue> breakAuxData);
   void popBreakDetails();
 
-  bool m_javaScriptPauseScheduled;
   bool m_skipAllPauses = false;
 
   std::unique_ptr<V8Regex> m_blackboxPattern;

test/inspector/debugger/pause-expected.txt

+Checks Debugger.pause
+
+Running test: testPause
+paused at:
+#var a = 42;
+
+
+Running test: testSkipFrameworks
+paused at:
+#var a = 239;
+
+
+Running test: testSkipOtherContext1
+paused at:
+var a = #1;
+
+
+Running test: testSkipOtherContext2
+paused at:
+#var a = 239;
+
+
+Running test: testWithNativeBreakpoint
+paused at:
+#var a = 42;
+
+paused at:
+#var a = 42;
+
+paused at:
+#var a = 42;
+
+
+Running test: testDisableBreaksShouldCancelPause
+paused at:
+#debugger
+

test/inspector/debugger/pause.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('Checks Debugger.pause');
+
+InspectorTest.setupScriptMap();
+Protocol.Debugger.enable();
+InspectorTest.runAsyncTestSuite([
+  async function testPause() {
+    Protocol.Debugger.pause();
+    Protocol.Runtime.evaluate({expression: 'var a = 42;'});
+    await waitPauseAndDumpLocation();
+    await Protocol.Debugger.resume();
+  },
+
+  async function testSkipFrameworks() {
+    Protocol.Debugger.setBlackboxPatterns({patterns: ['framework\.js']});
+    Protocol.Debugger.pause();
+    Protocol.Runtime.evaluate({expression: 'var a = 42; //# sourceURL=framework.js'});
+    Protocol.Runtime.evaluate({expression: 'var a = 239;'});
+    await waitPauseAndDumpLocation();
+    await Protocol.Debugger.resume();
+  },
+
+  async function testSkipOtherContext1() {
+    let contextGroupId = utils.createContextGroup();
+    Protocol.Debugger.enable({}, contextGroupId);
+    Protocol.Debugger.pause();
+    Protocol.Runtime.evaluate({expression: 'var a = 42; //# sourceURL=framework.js'});
+    Protocol.Runtime.evaluate({expression: 'var a = 239;'}, contextGroupId);
+    Protocol.Runtime.evaluate({expression: 'var a = 1;'});
+    await waitPauseAndDumpLocation();
+    await Protocol.Debugger.resume();
+    await Protocol.Debugger.disable({}, contextGroupId);
+  },
+
+  async function testSkipOtherContext2() {
+    let contextGroupId = utils.createContextGroup();
+    Protocol.Debugger.enable({}, contextGroupId);
+    Protocol.Debugger.pause({}, contextGroupId);
+    Protocol.Runtime.evaluate({expression: 'var a = 42; //# sourceURL=framework.js'});
+    Protocol.Runtime.evaluate({expression: 'var a = 239;'}, contextGroupId);
+    Protocol.Runtime.evaluate({expression: 'var a = 1;'});
+    await waitPauseAndDumpLocation();
+    await Protocol.Debugger.resume();
+    await Protocol.Debugger.disable({}, contextGroupId);
+  },
+
+  async function testWithNativeBreakpoint() {
+    utils.schedulePauseOnNextStatement('', '');
+    await Protocol.Debugger.pause();
+    utils.cancelPauseOnNextStatement();
+    Protocol.Runtime.evaluate({expression: 'var a = 42;'});
+    await waitPauseAndDumpLocation();
+    await Protocol.Debugger.resume();
+
+    await Protocol.Debugger.pause();
+    utils.schedulePauseOnNextStatement('', '');
+    utils.cancelPauseOnNextStatement();
+    Protocol.Runtime.evaluate({expression: 'var a = 42;'});
+    await waitPauseAndDumpLocation();
+    await Protocol.Debugger.resume();
+
+    utils.schedulePauseOnNextStatement('', '');
+    utils.cancelPauseOnNextStatement();
+    await Protocol.Debugger.pause();
+    Protocol.Runtime.evaluate({expression: 'var a = 42;'});
+    await waitPauseAndDumpLocation();
+    await Protocol.Debugger.resume();
+  },
+
+  async function testDisableBreaksShouldCancelPause() {
+    await Protocol.Debugger.pause();
+    await Protocol.Debugger.setBreakpointsActive({active: false});
+    Protocol.Runtime.evaluate({expression: 'var a = 42;'})
+      .then(() => Protocol.Debugger.setBreakpointsActive({active: true}))
+      .then(() => Protocol.Runtime.evaluate({expression: 'debugger'}));
+    await waitPauseAndDumpLocation();
+    await Protocol.Debugger.resume();
+  }
+]);
+
+async function waitPauseAndDumpLocation() {
+  var message = await Protocol.Debugger.oncePaused();
+  InspectorTest.log('paused at:');
+  await InspectorTest.logSourceLocation(message.params.callFrames[0].location);
+  return message;
+}

test/inspector/protocol-test.js

@@ -152,7 +152,8 @@ InspectorTest.logSourceLocation = function(location)
   }
   var script = InspectorTest._scriptMap.get(scriptId);
   if (!script.scriptSource) {
-    return Protocol.Debugger.getScriptSource({ scriptId })
+    // TODO(kozyatinskiy): doesn't assume that contextId == contextGroupId.
+    return Protocol.Debugger.getScriptSource({ scriptId }, script.executionContextId)
       .then(message => script.scriptSource = message.result.scriptSource)
       .then(dumpSourceWithLocation);
   }