[inspector] move changeBreakpointState from debugger-script to native

* introduced v8::DebugInterface::ChangeBreakOnException(Isolate*,ExceptionBreakState);
* migrated inspector to new API;
* added cctest for new API;
* added inspector test for setPauseOnExceptionState.

BUG=chromium:652939,v8:5510
R=dgozman@chromium.org,yangguo@chromium.org

Review-Url: https://chromiumcodereview.appspot.com/2396193002
Cr-Commit-Position: refs/heads/master@{#40413}

src/api.cc

@@ -8805,6 +8805,15 @@ MaybeLocal<Array> DebugInterface::GetInternalProperties(Isolate* isolate,
   return Debug::GetInternalProperties(isolate, value);
 }
 
+void DebugInterface::ChangeBreakOnException(Isolate* isolate,
+                                            ExceptionBreakState type) {
+  i::Isolate* internal_isolate = reinterpret_cast<i::Isolate*>(isolate);
+  internal_isolate->debug()->ChangeBreakOnException(
+      i::BreakException, type == BreakOnAnyException);
+  internal_isolate->debug()->ChangeBreakOnException(i::BreakUncaughtException,
+                                                    type != NoBreakOnException);
+}
+
 Local<String> CpuProfileNode::GetFunctionName() const {
   const i::ProfileNode* node = reinterpret_cast<const i::ProfileNode*>(this);
   i::Isolate* isolate = node->isolate();

src/debug/debug-interface.h

@@ -112,6 +112,21 @@ class DebugInterface {
    */
   static MaybeLocal<Array> GetInternalProperties(Isolate* isolate,
                                                  Local<Value> value);
+
+  enum ExceptionBreakState {
+    NoBreakOnException = 0,
+    BreakOnUncaughtException = 1,
+    BreakOnAnyException = 2
+  };
+
+  /**
+   * Defines if VM will pause on exceptions or not.
+   * If BreakOnAnyExceptions is set then VM will pause on caught and uncaught
+   * exception, if BreakOnUncaughtException is set then VM will pause only on
+   * uncaught exception, otherwise VM won't stop on any exception.
+   */
+  static void ChangeBreakOnException(Isolate* isolate,
+                                     ExceptionBreakState state);
 };
 
 }  // namespace v8

src/inspector/debugger-script.js

@@ -33,18 +33,6 @@
 
 var DebuggerScript = {};
 
-/** @enum */
-const PauseOnExceptionsState = {
-    DontPauseOnExceptions: 0,
-    PauseOnAllExceptions: 1,
-    PauseOnUncaughtExceptions: 2
-};
-DebuggerScript.PauseOnExceptionsState = PauseOnExceptionsState;
-
-DebuggerScript._pauseOnExceptionsState = DebuggerScript.PauseOnExceptionsState.DontPauseOnExceptions;
-Debug.clearBreakOnException();
-Debug.clearBreakOnUncaughtException();
-
 /**
  * @param {?CompileEvent} eventData
  */
@@ -252,32 +240,6 @@ DebuggerScript.removeBreakpoint = function(execState, info)
     Debug.findBreakPoint(info.breakpointId, true);
 }
 
-/**
- * @return {number}
- */
-DebuggerScript.pauseOnExceptionsState = function()
-{
-    return DebuggerScript._pauseOnExceptionsState;
-}
-
-/**
- * @param {number} newState
- */
-DebuggerScript.setPauseOnExceptionsState = function(newState)
-{
-    DebuggerScript._pauseOnExceptionsState = newState;
-
-    if (DebuggerScript.PauseOnExceptionsState.PauseOnAllExceptions === newState)
-        Debug.setBreakOnException();
-    else
-        Debug.clearBreakOnException();
-
-    if (DebuggerScript.PauseOnExceptionsState.PauseOnUncaughtExceptions === newState)
-        Debug.setBreakOnUncaughtException();
-    else
-        Debug.clearBreakOnUncaughtException();
-}
-
 /**
  * @param {!ExecutionState} execState
  * @param {number} limit

src/inspector/debugger_script_externs.js

@@ -61,17 +61,6 @@ var JavaScriptCallFrame;
  */
 var Debug = {};
 
-Debug.setBreakOnException = function() {}
-
-Debug.clearBreakOnException = function() {}
-
-Debug.setBreakOnUncaughtException = function() {}
-
-/**
- * @return {undefined}
- */
-Debug.clearBreakOnUncaughtException = function() {}
-
 Debug.clearStepping = function() {}
 
 Debug.clearAllBreakPoints = function() {}

src/inspector/injected-script.cc

@@ -445,7 +445,7 @@ InjectedScript::Scope::Scope(ErrorString* errorString,
       m_handleScope(inspector->isolate()),
       m_tryCatch(inspector->isolate()),
       m_ignoreExceptionsAndMuteConsole(false),
-      m_previousPauseOnExceptionsState(V8Debugger::DontPauseOnExceptions),
+      m_previousPauseOnExceptionsState(v8::DebugInterface::NoBreakOnException),
       m_userGesture(false) {}
 
 bool InjectedScript::Scope::initialize() {
@@ -479,14 +479,14 @@ void InjectedScript::Scope::ignoreExceptionsAndMuteConsole() {
   m_inspector->client()->muteMetrics(m_contextGroupId);
   m_inspector->muteExceptions(m_contextGroupId);
   m_previousPauseOnExceptionsState =
-      setPauseOnExceptionsState(V8Debugger::DontPauseOnExceptions);
+      setPauseOnExceptionsState(v8::DebugInterface::NoBreakOnException);
 }
 
-V8Debugger::PauseOnExceptionsState
+v8::DebugInterface::ExceptionBreakState
 InjectedScript::Scope::setPauseOnExceptionsState(
-    V8Debugger::PauseOnExceptionsState newState) {
+    v8::DebugInterface::ExceptionBreakState newState) {
   if (!m_inspector->debugger()->enabled()) return newState;
-  V8Debugger::PauseOnExceptionsState presentState =
+  v8::DebugInterface::ExceptionBreakState presentState =
       m_inspector->debugger()->getPauseOnExceptionsState();
   if (presentState != newState)
     m_inspector->debugger()->setPauseOnExceptionsState(newState);

src/inspector/injected-script.h

@@ -126,15 +126,15 @@ class InjectedScript final {
 
    private:
     void cleanup();
-    V8Debugger::PauseOnExceptionsState setPauseOnExceptionsState(
-        V8Debugger::PauseOnExceptionsState);
+    v8::DebugInterface::ExceptionBreakState setPauseOnExceptionsState(
+        v8::DebugInterface::ExceptionBreakState);
 
     v8::HandleScope m_handleScope;
     v8::TryCatch m_tryCatch;
     v8::Local<v8::Context> m_context;
     std::unique_ptr<V8Console::CommandLineAPIScope> m_commandLineAPIScope;
     bool m_ignoreExceptionsAndMuteConsole;
-    V8Debugger::PauseOnExceptionsState m_previousPauseOnExceptionsState;
+    v8::DebugInterface::ExceptionBreakState m_previousPauseOnExceptionsState;
     bool m_userGesture;
   };
 

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

@@ -163,7 +163,7 @@ void V8DebuggerAgentImpl::disable(ErrorString*) {
   m_state->setObject(DebuggerAgentState::javaScriptBreakpoints,
                      protocol::DictionaryValue::create());
   m_state->setInteger(DebuggerAgentState::pauseOnExceptionsState,
-                      V8Debugger::DontPauseOnExceptions);
+                      v8::DebugInterface::NoBreakOnException);
   m_state->setInteger(DebuggerAgentState::asyncCallStackDepth, 0);
 
   if (!m_pausedContext.IsEmpty()) m_debugger->continueProgram();
@@ -201,7 +201,7 @@ void V8DebuggerAgentImpl::restore() {
   enable();
   ErrorString error;
 
-  int pauseState = V8Debugger::DontPauseOnExceptions;
+  int pauseState = v8::DebugInterface::NoBreakOnException;
   m_state->getInteger(DebuggerAgentState::pauseOnExceptionsState, &pauseState);
   setPauseOnExceptionsImpl(&error, pauseState);
   DCHECK(error.isEmpty());
@@ -688,13 +688,13 @@ void V8DebuggerAgentImpl::stepOut(ErrorString* errorString) {
 void V8DebuggerAgentImpl::setPauseOnExceptions(
     ErrorString* errorString, const String16& stringPauseState) {
   if (!checkEnabled(errorString)) return;
-  V8Debugger::PauseOnExceptionsState pauseState;
+  v8::DebugInterface::ExceptionBreakState pauseState;
   if (stringPauseState == "none") {
-    pauseState = V8Debugger::DontPauseOnExceptions;
+    pauseState = v8::DebugInterface::NoBreakOnException;
   } else if (stringPauseState == "all") {
-    pauseState = V8Debugger::PauseOnAllExceptions;
+    pauseState = v8::DebugInterface::BreakOnAnyException;
   } else if (stringPauseState == "uncaught") {
-    pauseState = V8Debugger::PauseOnUncaughtExceptions;
+    pauseState = v8::DebugInterface::BreakOnUncaughtException;
   } else {
     *errorString = "Unknown pause on exceptions mode: " + stringPauseState;
     return;
@@ -705,11 +705,8 @@ void V8DebuggerAgentImpl::setPauseOnExceptions(
 void V8DebuggerAgentImpl::setPauseOnExceptionsImpl(ErrorString* errorString,
                                                    int pauseState) {
   m_debugger->setPauseOnExceptionsState(
-      static_cast<V8Debugger::PauseOnExceptionsState>(pauseState));
-  if (m_debugger->getPauseOnExceptionsState() != pauseState)
-    *errorString = "Internal error. Could not change pause on exceptions state";
-  else
-    m_state->setInteger(DebuggerAgentState::pauseOnExceptionsState, pauseState);
+      static_cast<v8::DebugInterface::ExceptionBreakState>(pauseState));
+  m_state->setInteger(DebuggerAgentState::pauseOnExceptionsState, pauseState);
 }
 
 void V8DebuggerAgentImpl::evaluateOnCallFrame(
@@ -1210,7 +1207,7 @@ void V8DebuggerAgentImpl::breakProgramOnException(
     std::unique_ptr<protocol::DictionaryValue> data) {
   if (!enabled() ||
       m_debugger->getPauseOnExceptionsState() ==
-          V8Debugger::DontPauseOnExceptions)
+          v8::DebugInterface::NoBreakOnException)
     return;
   breakProgram(breakReason, std::move(data));
 }

src/inspector/v8-debugger.cc

@@ -55,7 +55,8 @@ V8Debugger::V8Debugger(v8::Isolate* isolate, V8InspectorImpl* inspector)
       m_breakpointsActivated(true),
       m_runningNestedMessageLoop(false),
       m_ignoreScriptParsedEventsCounter(0),
-      m_maxAsyncCallStackDepth(0) {}
+      m_maxAsyncCallStackDepth(0),
+      m_pauseOnExceptionsState(v8::DebugInterface::NoBreakOnException) {}
 
 V8Debugger::~V8Debugger() {}
 
@@ -68,6 +69,9 @@ void V8Debugger::enable() {
                                             v8::External::New(m_isolate, this));
   m_debuggerContext.Reset(m_isolate,
                           v8::DebugInterface::GetDebugContext(m_isolate));
+  v8::DebugInterface::ChangeBreakOnException(
+      m_isolate, v8::DebugInterface::NoBreakOnException);
+  m_pauseOnExceptionsState = v8::DebugInterface::NoBreakOnException;
   compileDebuggerScript();
 }
 
@@ -252,28 +256,18 @@ void V8Debugger::setBreakpointsActivated(bool activated) {
   m_breakpointsActivated = activated;
 }
 
-V8Debugger::PauseOnExceptionsState V8Debugger::getPauseOnExceptionsState() {
+v8::DebugInterface::ExceptionBreakState
+V8Debugger::getPauseOnExceptionsState() {
   DCHECK(enabled());
-  v8::HandleScope scope(m_isolate);
-  v8::Local<v8::Context> context = debuggerContext();
-  v8::Context::Scope contextScope(context);
-
-  v8::Local<v8::Value> argv[] = {v8::Undefined(m_isolate)};
-  v8::Local<v8::Value> result =
-      callDebuggerMethod("pauseOnExceptionsState", 0, argv).ToLocalChecked();
-  return static_cast<V8Debugger::PauseOnExceptionsState>(
-      result->Int32Value(context).FromJust());
+  return m_pauseOnExceptionsState;
 }
 
 void V8Debugger::setPauseOnExceptionsState(
-    PauseOnExceptionsState pauseOnExceptionsState) {
+    v8::DebugInterface::ExceptionBreakState pauseOnExceptionsState) {
   DCHECK(enabled());
-  v8::HandleScope scope(m_isolate);
-  v8::Context::Scope contextScope(debuggerContext());
-
-  v8::Local<v8::Value> argv[] = {
-      v8::Int32::New(m_isolate, pauseOnExceptionsState)};
-  callDebuggerMethod("setPauseOnExceptionsState", 1, argv);
+  if (m_pauseOnExceptionsState == pauseOnExceptionsState) return;
+  v8::DebugInterface::ChangeBreakOnException(m_isolate, pauseOnExceptionsState);
+  m_pauseOnExceptionsState = pauseOnExceptionsState;
 }
 
 void V8Debugger::setPauseOnNextStatement(bool pause) {

src/inspector/v8-debugger.h

@@ -42,13 +42,8 @@ class V8Debugger {
   void setBreakpointsActivated(bool);
   bool breakpointsActivated() const { return m_breakpointsActivated; }
 
-  enum PauseOnExceptionsState {
-    DontPauseOnExceptions,
-    PauseOnAllExceptions,
-    PauseOnUncaughtExceptions
-  };
-  PauseOnExceptionsState getPauseOnExceptionsState();
-  void setPauseOnExceptionsState(PauseOnExceptionsState);
+  v8::DebugInterface::ExceptionBreakState getPauseOnExceptionsState();
+  void setPauseOnExceptionsState(v8::DebugInterface::ExceptionBreakState);
   void setPauseOnNextStatement(bool);
   bool canBreakProgram();
   void breakProgram();
@@ -152,6 +147,8 @@ class V8Debugger {
   std::vector<std::unique_ptr<V8StackTraceImpl>> m_currentStacks;
   protocol::HashMap<V8DebuggerAgentImpl*, int> m_maxAsyncCallStackDepthMap;
 
+  v8::DebugInterface::ExceptionBreakState m_pauseOnExceptionsState;
+
   DISALLOW_COPY_AND_ASSIGN(V8Debugger);
 };
 

test/cctest/test-debug.cc

@@ -33,13 +33,13 @@
 #include "src/base/platform/condition-variable.h"
 #include "src/base/platform/platform.h"
 #include "src/compilation-cache.h"
+#include "src/debug/debug-interface.h"
 #include "src/debug/debug.h"
 #include "src/deoptimizer.h"
 #include "src/frames.h"
 #include "src/utils.h"
 #include "test/cctest/cctest.h"
 
-
 using ::v8::base::Mutex;
 using ::v8::base::LockGuard;
 using ::v8::base::ConditionVariable;
@@ -319,6 +319,11 @@ static void ChangeBreakOnExceptionFromJS(v8::Isolate* isolate, bool caught,
   }
 }
 
+// Change break on exception using the native API call.
+static void ChangeBreakOnExceptionFromAPI(
+    v8::Isolate* isolate, v8::DebugInterface::ExceptionBreakState state) {
+  v8::DebugInterface::ChangeBreakOnException(isolate, state);
+}
 
 // Prepare to step to next break location.
 static void PrepareStep(StepAction step_action) {
@@ -3986,6 +3991,48 @@ TEST(BreakOnException) {
   edgeCaseFinally->Call(context, env->Global(), 0, NULL).ToLocalChecked();
   DebugEventCounterCheck(4, 3, 2);
 
+  // No break on exception using native API
+  DebugEventCounterClear();
+  MessageCallbackCountClear();
+  ChangeBreakOnExceptionFromAPI(env->GetIsolate(),
+                                v8::DebugInterface::NoBreakOnException);
+  caught->Call(context, env->Global(), 0, NULL).ToLocalChecked();
+  DebugEventCounterCheck(0, 0, 0);
+  CHECK(notCaught->Call(context, env->Global(), 0, NULL).IsEmpty());
+  DebugEventCounterCheck(0, 0, 1);
+  CHECK(notCaughtFinally->Call(context, env->Global(), 0, NULL).IsEmpty());
+  DebugEventCounterCheck(0, 0, 2);
+  edgeCaseFinally->Call(context, env->Global(), 0, NULL).ToLocalChecked();
+  DebugEventCounterCheck(0, 0, 2);
+
+  // // Break on uncaught exception using native API
+  DebugEventCounterClear();
+  MessageCallbackCountClear();
+  ChangeBreakOnExceptionFromAPI(env->GetIsolate(),
+                                v8::DebugInterface::BreakOnUncaughtException);
+  caught->Call(context, env->Global(), 0, NULL).ToLocalChecked();
+  DebugEventCounterCheck(0, 0, 0);
+  CHECK(notCaught->Call(context, env->Global(), 0, NULL).IsEmpty());
+  DebugEventCounterCheck(1, 1, 1);
+  CHECK(notCaughtFinally->Call(context, env->Global(), 0, NULL).IsEmpty());
+  DebugEventCounterCheck(2, 2, 2);
+  edgeCaseFinally->Call(context, env->Global(), 0, NULL).ToLocalChecked();
+  DebugEventCounterCheck(3, 3, 2);
+
+  // // Break on exception and uncaught exception using native API
+  DebugEventCounterClear();
+  MessageCallbackCountClear();
+  ChangeBreakOnExceptionFromAPI(env->GetIsolate(),
+                                v8::DebugInterface::BreakOnAnyException);
+  caught->Call(context, env->Global(), 0, NULL).ToLocalChecked();
+  DebugEventCounterCheck(1, 0, 0);
+  CHECK(notCaught->Call(context, env->Global(), 0, NULL).IsEmpty());
+  DebugEventCounterCheck(2, 1, 1);
+  CHECK(notCaughtFinally->Call(context, env->Global(), 0, NULL).IsEmpty());
+  DebugEventCounterCheck(3, 2, 2);
+  edgeCaseFinally->Call(context, env->Global(), 0, NULL).ToLocalChecked();
+  DebugEventCounterCheck(4, 3, 2);
+
   v8::Debug::SetDebugEventListener(env->GetIsolate(), nullptr);
   CheckDebuggerUnloaded(env->GetIsolate());
   env->GetIsolate()->RemoveMessageListeners(MessageCallbackCount);

test/inspector/debugger/break-on-exception-expected.txt

@@ -0,0 +1,12 @@
+Check that inspector correctly change break on exception state.
+
+Running test: noBreakOnExceptionAfterEnabled
+
+Running test: breakOnUncaughtException
+paused in throwUncaughtException
+
+Running test: breakOnCaughtException
+paused in throwUncaughtException
+paused in throwCaughtException
+
+Running test: noBreakInEvaluateInSilentMode

test/inspector/debugger/break-on-exception.js

@@ -0,0 +1,71 @@
+// Copyright 2016 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("Check that inspector correctly change break on exception state.");
+
+InspectorTest.addScript(`
+function scheduleUncaughtException()
+{
+  setTimeout(throwUncaughtException, 0);
+}
+function throwUncaughtException()
+{
+  throw new Error();
+}
+function throwCaughtException()
+{
+  throw new Error();
+}`);
+
+Protocol.Debugger.onPaused(message => {
+  InspectorTest.log("paused in " + message.params.callFrames[0].functionName);
+  Protocol.Debugger.resume();
+});
+
+Protocol.Runtime.enable();
+
+InspectorTest.runTestSuite([
+  function noBreakOnExceptionAfterEnabled(next)
+  {
+    Protocol.Debugger.enable();
+    Protocol.Debugger.setPauseOnExceptions({ state: "all" });
+    Protocol.Debugger.disable();
+    Protocol.Debugger.enable();
+    Protocol.Runtime.evaluate({ expression: "scheduleUncaughtException()" })
+      .then(() => Protocol.Runtime.evaluate({ expression: "throwCaughtException()" }))
+      .then(() => Protocol.Debugger.disable())
+      .then(next);
+  },
+
+  function breakOnUncaughtException(next)
+  {
+    Protocol.Debugger.enable();
+    Protocol.Debugger.setPauseOnExceptions({ state: "uncaught" });
+    Protocol.Runtime.evaluate({ expression: "scheduleUncaughtException()" })
+      .then(() => Protocol.Runtime.onceExceptionThrown())
+      .then(() => Protocol.Runtime.evaluate({ expression: "throwCaughtException()" }))
+      .then(() => Protocol.Debugger.disable())
+      .then(next);
+  },
+
+  function breakOnCaughtException(next)
+  {
+    Protocol.Debugger.enable();
+    Protocol.Debugger.setPauseOnExceptions({ state: "all" });
+    Protocol.Runtime.evaluate({ expression: "scheduleUncaughtException()" })
+      .then(() => Protocol.Runtime.onceExceptionThrown())
+      .then(() => Protocol.Runtime.evaluate({ expression: "throwCaughtException()" }))
+      .then(() => Protocol.Debugger.disable())
+      .then(next);
+  },
+
+  function noBreakInEvaluateInSilentMode(next)
+  {
+    Protocol.Debugger.enable();
+    Protocol.Debugger.setPauseOnExceptions({ state: "all" })
+      .then(() => Protocol.Runtime.evaluate({ expression: "throwCaughtException()", silent: true }))
+      .then(() => Protocol.Debugger.disable())
+      .then(next);
+  }
+]);