Extract common debugger code for processing compile events

src/debug/debug.cc

Index: src/debug/debug.cc
diff --git a/src/debug/debug.cc b/src/debug/debug.cc
index 297156f563e0518c4f9a5e687324dc73c1378553..08a734f7817b94eb4c522954dc177a89b5ff3399 100644
--- a/src/debug/debug.cc
+++ b/src/debug/debug.cc
@@ -1813,29 +1813,6 @@ void Debug::OnException(Handle<Object> exception, Handle<Object> promise) {
 }
 
 
-void Debug::OnCompileError(Handle<Script> script) {
-  if (ignore_events()) return;
-  SuppressDebug while_processing(this);
-
-  if (in_debug_scope()) {
-    ProcessCompileEventInDebugScope(v8::CompileError, script);
-    return;
-  }
-
-  HandleScope scope(isolate_);
-  DebugScope debug_scope(this);
-  if (debug_scope.failed()) return;
-
-  // Create the compile state object.
-  Handle<Object> event_data;
-  // Bail out and don't call debugger if exception.
-  if (!MakeCompileEvent(script, v8::CompileError).ToHandle(&event_data)) return;
-
-  // Process debug event.
-  ProcessDebugEvent(v8::CompileError, Handle<JSObject>::cast(event_data), true);
-}
-
-
 void Debug::OnDebugBreak(Handle<Object> break_points_hit,
                             bool auto_continue) {
   // The caller provided for DebugScope.
@@ -1856,56 +1833,19 @@ void Debug::OnDebugBreak(Handle<Object> break_points_hit,
 }
 
 
-void Debug::OnBeforeCompile(Handle<Script> script) {
-  if (in_debug_scope() || ignore_events()) return;
-  SuppressDebug while_processing(this);
-
-  HandleScope scope(isolate_);
-  DebugScope debug_scope(this);
-  if (debug_scope.failed()) return;
+void Debug::OnCompileError(Handle<Script> script) {
+  ProcessCompileEvent(v8::CompileError, script);
+}
 
-  // Create the event data object.
-  Handle<Object> event_data;
-  // Bail out and don't call debugger if exception.
-  if (!MakeCompileEvent(script, v8::BeforeCompile).ToHandle(&event_data))
-    return;
 
-  // Process debug event.
-  ProcessDebugEvent(v8::BeforeCompile,
-                    Handle<JSObject>::cast(event_data),
-                    true);
+void Debug::OnBeforeCompile(Handle<Script> script) {
+  ProcessCompileEvent(v8::BeforeCompile, script);
 }
 
 
 // Handle debugger actions when a new script is compiled.
 void Debug::OnAfterCompile(Handle<Script> script) {
-  if (ignore_events()) return;
-  SuppressDebug while_processing(this);
-
-  if (in_debug_scope()) {
-    ProcessCompileEventInDebugScope(v8::AfterCompile, script);
-    return;
-  }
-
-  HandleScope scope(isolate_);
-  DebugScope debug_scope(this);
-  if (debug_scope.failed()) return;
-
-  // If debugging there might be script break points registered for this
-  // script. Make sure that these break points are set.
-  Handle<Object> argv[] = {Script::GetWrapper(script)};
-  if (CallFunction("UpdateScriptBreakPoints", arraysize(argv), argv)
-          .is_null()) {
-    return;
-  }
-
-  // Create the compile state object.
-  Handle<Object> event_data;
-  // Bail out and don't call debugger if exception.
-  if (!MakeCompileEvent(script, v8::AfterCompile).ToHandle(&event_data)) return;
-
-  // Process debug event.
-  ProcessDebugEvent(v8::AfterCompile, Handle<JSObject>::cast(event_data), true);
+  ProcessCompileEvent(v8::AfterCompile, script);
 }
 
 
@@ -2009,23 +1949,44 @@ void Debug::CallEventCallback(v8::DebugEvent event,
 }
 
 
-void Debug::ProcessCompileEventInDebugScope(v8::DebugEvent event,
-                                            Handle<Script> script) {
-  if (event_listener_.is_null()) return;
+void Debug::ProcessCompileEvent(v8::DebugEvent event, Handle<Script> script) {
+  if (ignore_events()) return;
+  SuppressDebug while_processing(this);
 
+  bool in_nested_debug_scope = in_debug_scope();
+  HandleScope scope(isolate_);
   DebugScope debug_scope(this);
   if (debug_scope.failed()) return;
 
+  if (event == v8::AfterCompile) {
+    // If debugging there might be script break points registered for this
+    // script. Make sure that these break points are set.
+    Handle<Object> argv[] = {Script::GetWrapper(script)};
+    if (CallFunction("UpdateScriptBreakPoints", arraysize(argv), argv)
+            .is_null()) {
+      return;
+    }
+  }
+
+  // Create the compile state object.
   Handle<Object> event_data;
   // Bail out and don't call debugger if exception.
   if (!MakeCompileEvent(script, event).ToHandle(&event_data)) return;
 
-  // Create the execution state.
-  Handle<Object> exec_state;
-  // Bail out and don't call debugger if exception.
-  if (!MakeExecutionState().ToHandle(&exec_state)) return;
+  // Don't call NotifyMessageHandler if already in debug scope to avoid running
+  // nested command loop.
+  if (in_nested_debug_scope) {

[yurys, 2015/08/31 22:35:45]

It is unfortunate that we cannot simply call ProcessDebugEvent here as it would
break order of debug command processing (see e.g.
test-debug/DebugEvaluateWithoutStack) due running command loop on after compile
event. We could pass a flag to ProccessDebugEvent that would tell it to skip
NotifyMessageHandler step but it doesn't look much better. I wish we could drop
this command processing from debug.cc altogether.

+    if (event_listener_.is_null()) return;
+    // Create the execution state.
+    Handle<Object> exec_state;
+    // Bail out and don't call debugger if exception.
+    if (!MakeExecutionState().ToHandle(&exec_state)) return;
 
-  CallEventCallback(event, exec_state, event_data, NULL);
+    CallEventCallback(event, exec_state, event_data, NULL);
+  } else {
+    // Process debug event.
+    ProcessDebugEvent(event, Handle<JSObject>::cast(event_data), true);
+  }
 }