Reviewers: yurys,

Message:
Yury, please take a look!

Description:
[V8] Report v8::AfterCompile and v8::CompileError to listener on pause

V8 didn't report compile events on pause before this patch. These events can be
important for listener. For example, DevTools allows user to execute some JS
code on pause and needs to show correct stack trace in message from it.

BUG=396013
[email protected]

Please review this at https://codereview.chromium.org/781623004/

Base URL: https://chromium.googlesource.com/v8/v8.git@master

Affected files (+45, -8 lines):
  M src/debug.h
  M src/debug.cc
  M test/mjsunit/debug-clearbreakpointgroup.js
  M test/mjsunit/debug-compile-event.js
  M test/mjsunit/debug-evaluate-with-context.js


Index: src/debug.cc
diff --git a/src/debug.cc b/src/debug.cc
index d8844cc414e3f03c64f803dac22fdfaa8c5a003f..ce77219639d5f3aca9c9b36804d1474b5c39b0ad 100644
--- a/src/debug.cc
+++ b/src/debug.cc
@@ -2481,9 +2481,8 @@ void Debug::RecordEvalCaller(Handle<Script> script) {
 MaybeHandle<Object> Debug::MakeJSObject(const char* constructor_name,
                                         int argc,
                                         Handle<Object> argv[]) {
-  AssertDebugContext();
   // Create the execution state object.
-  Handle<GlobalObject> global(isolate_->global_object());
+ Handle<GlobalObject> global = in_debug_scope() ? Handle<GlobalObject>(debug_context()->global_object()) : Handle<GlobalObject>(isolate_->global_object());
   Handle<Object> constructor = Object::GetProperty(
       isolate_, global, constructor_name).ToHandleChecked();
   DCHECK(constructor->IsJSFunction());
@@ -2630,8 +2629,12 @@ void Debug::OnException(Handle<Object> exception, bool uncaught,


 void Debug::OnCompileError(Handle<Script> script) {
-  // No more to do if not debugging.
-  if (in_debug_scope() || ignore_events()) return;
+  if (ignore_events()) return;
+
+  if (in_debug_scope()) {
+    ProcessCompileEventInDebugScope(v8::CompileError, script);
+    return;
+  }

   HandleScope scope(isolate_);
   DebugScope debug_scope(this);
@@ -2692,8 +2695,12 @@ void Debug::OnAfterCompile(Handle<Script> script) {
   // Add the newly compiled script to the script cache.
   if (script_cache_ != NULL) script_cache_->Add(script);

-  // No more to do if not debugging.
-  if (in_debug_scope() || ignore_events()) return;
+  if (ignore_events()) return;
+
+  if (in_debug_scope()) {
+    ProcessCompileEventInDebugScope(v8::AfterCompile, script);
+    return;
+  }

   HandleScope scope(isolate_);
   DebugScope debug_scope(this);
@@ -2848,6 +2855,23 @@ void Debug::CallEventCallback(v8::DebugEvent event,
 }


+void Debug::ProcessCompileEventInDebugScope(v8::DebugEvent event,
+                                            Handle<Script> script) {
+  if (event_listener_.is_null()) return;
+
+  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;
+
+  CallEventCallback(event, exec_state, event_data, NULL);
+}
+
+
 Handle<Context> Debug::GetDebugContext() {
   DebugScope debug_scope(this);
   // The global handle may be destroyed soon after.  Return it reboxed.
Index: src/debug.h
diff --git a/src/debug.h b/src/debug.h
index 26d6ea589527744fb15b28fa87017be7bc560e15..4440d659dc18198e8c07bde4ca754efcbf7ab396 100644
--- a/src/debug.h
+++ b/src/debug.h
@@ -553,6 +553,8 @@ class Debug {
                          Handle<Object> exec_state,
                          Handle<Object> event_data,
                          v8::Debug::ClientData* client_data);
+  void ProcessCompileEventInDebugScope(v8::DebugEvent event,
+                                       Handle<Script> script);
   void ProcessDebugEvent(v8::DebugEvent event,
                          Handle<JSObject> event_data,
                          bool auto_continue);
Index: test/mjsunit/debug-clearbreakpointgroup.js
diff --git a/test/mjsunit/debug-clearbreakpointgroup.js b/test/mjsunit/debug-clearbreakpointgroup.js index 137dfecbecbedd20c0db312f5abb224806061b9f..f8c99e0760fe593124b9ccbdd8332158387d9821 100644
--- a/test/mjsunit/debug-clearbreakpointgroup.js
+++ b/test/mjsunit/debug-clearbreakpointgroup.js
@@ -36,12 +36,17 @@ var exception = false;

var base_request = '"seq":0,"type":"request","command":"clearbreakpointgroup"';
 var scriptId = null;
+var muteListener = false;

 function safeEval(code) {
   try {
-    return eval('(' + code + ')');
+    muteListener = true;
+    var result = eval('(' + code + ')');
+    muteListener = false;
+    return result;
   } catch (e) {
     assertEquals(void 0, e);
+    muteListener = false;
     return undefined;
   }
 }
@@ -58,6 +63,7 @@ function testArguments(dcp, arguments, success) {
 }

 function listener(event, exec_state, event_data, data) {
+  if (muteListener) return;
   try {
     if (event == Debug.DebugEvent.Break) {
       // Get the debug command processor.
Index: test/mjsunit/debug-compile-event.js
diff --git a/test/mjsunit/debug-compile-event.js b/test/mjsunit/debug-compile-event.js index c38cd8477a9fe4d53b110f419d7b099292a111ff..5901e51c02325a32963d385093906b474d7dd002 100644
--- a/test/mjsunit/debug-compile-event.js
+++ b/test/mjsunit/debug-compile-event.js
@@ -37,7 +37,7 @@ var current_source = ''; // Current source being compiled.
 var source_count = 0;  // Total number of scources compiled.
 var host_compilations = 0;  // Number of scources compiled through the API.
 var eval_compilations = 0;  // Number of scources compiled through eval.
-
+var mute_listener = false;

 function compileSource(source) {
   current_source = source;
@@ -47,6 +47,7 @@ function compileSource(source) {


 function listener(event, exec_state, event_data, data) {
+  if (mute_listener) return;
   try {
     if (event == Debug.DebugEvent.BeforeCompile ||
         event == Debug.DebugEvent.AfterCompile ||
@@ -81,7 +82,9 @@ function listener(event, exec_state, event_data, data) {
       }
       // Check that script context is included into the event message.
       var json = event_data.toJSONProtocol();
+      mute_listener = true;
       var msg = eval('(' + json + ')');
+      mute_listener = false;
       assertTrue('context' in msg.body.script);

       // Check that we pick script name from //# sourceURL, iff present
Index: test/mjsunit/debug-evaluate-with-context.js
diff --git a/test/mjsunit/debug-evaluate-with-context.js b/test/mjsunit/debug-evaluate-with-context.js index 5e1c83cf52cf401f615fc06ec45f0feaac125079..e2e14370d3ef3a324d1aadfd42e91aae8e6a59cf 100644
--- a/test/mjsunit/debug-evaluate-with-context.js
+++ b/test/mjsunit/debug-evaluate-with-context.js
@@ -32,6 +32,8 @@ Debug = debug.Debug
 var evaluate_callback;

 function listener(event, exec_state, event_data, data) {
+  if (event == Debug.DebugEvent.AfterCompile ||
+      event == Debug.DebugEvent.CompileError) return;
   try {
     var context = { what_is_capybara: "a fish" };
var context2 = { what_is_capybara: "a fish", what_is_parrot: "a beard" };


--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to