[inspector] don't make v8::debug::Call for breakProgram.

We emulate break by callling breakProgramCallback function in debugger context, we can just use HandleDebugBreak.
It allows us to move all stepping logic to debug.cc later and remove one usage of debugger context.
+ two minor issues fixed, see tests.

BUG=v8:5510
R=yangguo@chromium.org

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

src/api.cc

@@ -9105,6 +9105,12 @@ void debug::ClearStepping(Isolate* v8_isolate) {
   isolate->debug()->ClearStepping();
 }
 
+void debug::BreakRightNow(Isolate* v8_isolate) {
+  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
+  ENTER_V8(isolate);
+  isolate->debug()->HandleDebugBreak(i::kIgnoreIfAllFramesBlackboxed);
+}
+
 bool debug::AllFramesOnStackAreBlackboxed(Isolate* v8_isolate) {
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
   ENTER_V8(isolate);

src/debug/debug-interface.h

@@ -103,6 +103,7 @@ enum StepAction {
 
 void PrepareStep(Isolate* isolate, StepAction action);
 void ClearStepping(Isolate* isolate);
+void BreakRightNow(Isolate* isolate);
 
 bool AllFramesOnStackAreBlackboxed(Isolate* isolate);
 

src/debug/debug.cc

@@ -2132,8 +2132,7 @@ MaybeHandle<Object> Debug::Call(Handle<Object> fun, Handle<Object> data) {
       argv);
 }
 
-
-void Debug::HandleDebugBreak() {
+void Debug::HandleDebugBreak(IgnoreBreakMode ignore_break_mode) {
   // Initialize LiveEdit.
   LiveEdit::InitializeThreadLocal(this);
   // Ignore debug break during bootstrapping.
@@ -2154,7 +2153,10 @@ void Debug::HandleDebugBreak() {
       // Don't stop in builtin and blackboxed functions.
       Handle<SharedFunctionInfo> shared(JSFunction::cast(fun)->shared(),
                                         isolate_);
-      if (IsBlackboxed(shared)) {
+      bool ignore_break = ignore_break_mode == kIgnoreIfTopFrameBlackboxed
+                              ? IsBlackboxed(shared)
+                              : AllFramesOnStackAreBlackboxed();
+      if (ignore_break) {
         // Inspector uses pause on next statement for asynchronous breakpoints.
         // When breakpoint is fired we try to break on first not blackboxed
         // statement. To achieve this goal we need to deoptimize current

src/debug/debug.h

@@ -65,6 +65,11 @@ enum DebugBreakType {
   DEBUG_BREAK_SLOT_AT_TAIL_CALL,
 };
 
+enum IgnoreBreakMode {
+  kIgnoreIfAllFramesBlackboxed,
+  kIgnoreIfTopFrameBlackboxed
+};
+
 class BreakLocation {
  public:
   static BreakLocation FromFrame(Handle<DebugInfo> debug_info,
@@ -276,7 +281,7 @@ class Debug {
   MUST_USE_RESULT MaybeHandle<Object> Call(Handle<Object> fun,
                                            Handle<Object> data);
   Handle<Context> GetDebugContext();
-  void HandleDebugBreak();
+  void HandleDebugBreak(IgnoreBreakMode ignore_break_mode);
 
   // Internal logic
   bool Load();

src/execution.cc

@@ -471,7 +471,7 @@ Object* StackGuard::HandleInterrupts() {
   }
 
   if (CheckDebugBreak()) {
-    isolate_->debug()->HandleDebugBreak();
+    isolate_->debug()->HandleDebugBreak(kIgnoreIfTopFrameBlackboxed);
   }
 
   if (CheckAndClearInterrupt(TERMINATE_EXECUTION)) {

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

@@ -1309,6 +1309,9 @@ void V8DebuggerAgentImpl::breakProgram(
   m_debugger->breakProgram();
   popBreakDetails();
   m_breakReason.swap(currentScheduledReason);
+  if (!m_breakReason.empty()) {
+    m_debugger->setPauseOnNextStatement(true);
+  }
 }
 
 void V8DebuggerAgentImpl::breakProgramOnException(

src/inspector/v8-debugger.cc

@@ -345,16 +345,7 @@ void V8Debugger::breakProgram() {
   // Don't allow nested breaks.
   if (isPaused()) return;
   if (!canBreakProgram()) return;
-
-  v8::HandleScope scope(m_isolate);
-  v8::Local<v8::Function> breakFunction;
-  if (!v8::Function::New(m_isolate->GetCurrentContext(),
-                         &V8Debugger::breakProgramCallback,
-                         v8::External::New(m_isolate, this), 0,
-                         v8::ConstructorBehavior::kThrow)
-           .ToLocal(&breakFunction))
-    return;
-  v8::debug::Call(debuggerContext(), breakFunction).ToLocalChecked();
+  v8::debug::BreakRightNow(m_isolate);
 }
 
 void V8Debugger::continueProgram() {
@@ -505,25 +496,6 @@ JavaScriptCallFrames V8Debugger::currentCallFrames(int limit) {
   return callFrames;
 }
 
-static V8Debugger* toV8Debugger(v8::Local<v8::Value> data) {
-  void* p = v8::Local<v8::External>::Cast(data)->Value();
-  return static_cast<V8Debugger*>(p);
-}
-
-void V8Debugger::breakProgramCallback(
-    const v8::FunctionCallbackInfo<v8::Value>& info) {
-  DCHECK_EQ(info.Length(), 2);
-  V8Debugger* thisPtr = toV8Debugger(info.Data());
-  if (!thisPtr->enabled()) return;
-  v8::Local<v8::Context> pausedContext =
-      thisPtr->m_isolate->GetCurrentContext();
-  v8::Local<v8::Value> exception;
-  v8::Local<v8::Array> hitBreakpoints;
-  thisPtr->handleProgramBreak(pausedContext,
-                              v8::Local<v8::Object>::Cast(info[0]), exception,
-                              hitBreakpoints);
-}
-
 void V8Debugger::handleProgramBreak(v8::Local<v8::Context> pausedContext,
                                     v8::Local<v8::Object> executionState,
                                     v8::Local<v8::Value> exception,

src/inspector/v8-debugger.h

@@ -105,7 +105,6 @@ class V8Debugger : public v8::debug::DebugDelegate {
 
   static void v8OOMCallback(void* data);
 
-  static void breakProgramCallback(const v8::FunctionCallbackInfo<v8::Value>&);
   void handleProgramBreak(v8::Local<v8::Context> pausedContext,
                           v8::Local<v8::Object> executionState,
                           v8::Local<v8::Value> exception,

src/runtime/runtime-debug.cc

@@ -68,7 +68,7 @@ RUNTIME_FUNCTION(Runtime_HandleDebuggerStatement) {
   SealHandleScope shs(isolate);
   DCHECK_EQ(0, args.length());
   if (isolate->debug()->break_points_active()) {
-    isolate->debug()->HandleDebugBreak();
+    isolate->debug()->HandleDebugBreak(kIgnoreIfTopFrameBlackboxed);
   }
   return isolate->heap()->undefined_value();
 }

test/inspector/debugger/stepping-and-break-program-api-expected.txt

@@ -0,0 +1,19 @@
+Checks that stepping is cleared after breakProgram.
+paused at:
+function callBreakProgram() {
+  #debugger;
+  breakProgram('reason', '');
+
+paused at:
+  debugger;
+  #breakProgram('reason', '');
+}
+
+paused at:
+  debugger;
+  #breakProgram('reason', '');
+}
+
+paused at:
+#debugger;
+

test/inspector/debugger/stepping-and-break-program-api.js

@@ -0,0 +1,34 @@
+// 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 that stepping is cleared after breakProgram.');
+
+InspectorTest.addScript(`
+function callBreakProgram() {
+  debugger;
+  breakProgram('reason', '');
+}`);
+
+InspectorTest.setupScriptMap();
+(async function test() {
+  Protocol.Debugger.enable();
+  Protocol.Runtime.evaluate({expression: 'callBreakProgram();'});
+  // Should break at this debugger statement, not at end of callBreakProgram.
+  Protocol.Runtime.evaluate({expression: 'setTimeout(\'debugger;\', 0);'});
+  await waitPauseAndDumpLocation();
+  Protocol.Debugger.stepOver();
+  await waitPauseAndDumpLocation();
+  Protocol.Debugger.stepOver();
+  await waitPauseAndDumpLocation();
+  Protocol.Debugger.resume();
+  await waitPauseAndDumpLocation();
+  InspectorTest.completeTest();
+})();
+
+async function waitPauseAndDumpLocation() {
+  var message = await Protocol.Debugger.oncePaused();
+  InspectorTest.log('paused at:');
+  InspectorTest.logSourceLocation(message.params.callFrames[0].location);
+  return message;
+}