[inspector] V8DebuggerAgent cleanup

src/debug/debug.cc

Index: src/debug/debug.cc
diff --git a/src/debug/debug.cc b/src/debug/debug.cc
index adf3659c68fdf560f267037772bf9baa38efd747..045dd8616010b6c0ed5b4e0bf2fe0d6f4f722a55 100644
--- a/src/debug/debug.cc
+++ b/src/debug/debug.cc
@@ -1027,6 +1027,15 @@ void Debug::PrepareStep(StepAction step_action) {
   Handle<DebugInfo> debug_info(shared->GetDebugInfo());
   BreakLocation location = BreakLocation::FromFrame(debug_info, js_frame);
 
+  // It is used at least in two cases:
+  // - step-next at return position from first callback in microtask will break
+  // in next callback,
+  // - step-next at return position when caller frame is blackboxed will break
+  // in next not blackboxed function called by caller frame if any.
+  if (step_action == StepNext && location.IsReturn()) {

[kozy, 2017/02/01 07:29:43]

Yang, WDYT about this part? This code is just moved from V8DebuggerAgentImpl but
it doesn't sound right to me.

[Yang, 2017/02/01 07:46:12]

for the microtask case, we should make a check at the microtask runner, where we
would turn pending step-next into a step-in.

for the blackbox case, it's arguable whether step-next should take us into the
next call that the blackboxed caller makes. That could be accomplished by
clicking on step-in. The current behavior assumes that a step-next is actually a
step-next to the caller, and then a step-in into the next non-blackboxed call.

[kozy, 2017/02/02 06:58:56]

I think that case with blackboxed code could be considered in the same way as
microtask case. Why we'd like to support special step-next behavior in one case
and wouldn't in another?
Async step into will enable step-in target chained callback without iterating
through all other callbacks.

Maybe we could just remove this code and suggest users to use step-in when they
actually would like to go to next chained callback or not blackboxed function. I
prefer this.

[Yang, 2017/02/02 12:10:49]

You are right. Step-in at the return position to indicate that the user wants to
step into the next callback in queue or the next call from blackboxed caller
makes sense.

+    thread_local_.last_step_action_ = StepIn;
+    UpdateHookOnFunctionCall();
+  }
   // Any step at a return is a step-out.
   if (location.IsReturn()) step_action = StepOut;
   // A step-next at a tail call is a step-out.
@@ -1820,6 +1829,13 @@ void Debug::OnException(Handle<Object> exception, Handle<Object> promise) {
   if (debug_delegate_) {
     HandleScope scope(isolate_);
 
+    {
+      // Exception from JavaScript should always contains at least one frame.
+      JavaScriptFrameIterator it(isolate_);
+      DCHECK(!it.done());
+      if (it.done()) return;

[kozy, 2017/02/01 07:29:43]

Yang, do you know when we can have empty stack here?

[Yang, 2017/02/01 07:46:12]

might be possible, e.g. if we call directly into a function, that is implemented
as C++ built-in, and that throws?

lets move this check to the previous JavaScriptFrameIterator.

[kozy, 2017/02/02 06:58:55]

Done.

+    }
+
     // Create the execution state.
     Handle<Object> exec_state;
     // Bail out and don't call debugger if exception.