[debugger] implement legacy debug event listeners via debug delegate.

R=jgruber@chromium.org
BUG=v8:5530

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

src/api.cc

@@ -8912,11 +8912,15 @@ bool Debug::SetDebugEventListener(Isolate* isolate, EventCallback that,
   i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
   ENTER_V8(i_isolate);
   i::HandleScope scope(i_isolate);
-  i::Handle<i::Object> foreign = i_isolate->factory()->undefined_value();
-  if (that != NULL) {
-    foreign = i_isolate->factory()->NewForeign(FUNCTION_ADDR(that));
+  if (that == nullptr) {
+    i_isolate->debug()->SetDebugDelegate(nullptr, false);
+  } else {
+    i::Handle<i::Object> i_data = i_isolate->factory()->undefined_value();
+    if (!data.IsEmpty()) i_data = Utils::OpenHandle(*data);
+    i::NativeDebugDelegate* delegate =
+        new i::NativeDebugDelegate(i_isolate, that, i_data);
+    i_isolate->debug()->SetDebugDelegate(delegate, true);
   }
-  i_isolate->debug()->SetEventListener(foreign, Utils::OpenHandle(*data, true));
   return true;
 }
 
@@ -9369,7 +9373,7 @@ void debug::SetDebugDelegate(Isolate* v8_isolate,
                              debug::DebugDelegate* delegate) {
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
   ENTER_V8(isolate);
-  isolate->debug()->SetDebugDelegate(delegate);
+  isolate->debug()->SetDebugDelegate(delegate, false);
 }
 
 void debug::ResetBlackboxedStateCache(Isolate* v8_isolate,

src/debug/debug-interface.h

@@ -163,7 +163,8 @@ class DebugDelegate {
   virtual void ExceptionThrown(v8::Local<v8::Context> paused_context,
                                v8::Local<v8::Object> exec_state,
                                v8::Local<v8::Value> exception,
-                               bool is_promise_rejection, bool is_uncaught) {}
+                               v8::Local<v8::Value> promise, bool is_uncaught) {
+  }
   virtual bool IsFunctionBlackboxed(v8::Local<debug::Script> script,
                                     const debug::Location& start,
                                     const debug::Location& end) {

src/debug/debug.cc

@@ -38,15 +38,12 @@ namespace internal {
 
 Debug::Debug(Isolate* isolate)
     : debug_context_(Handle<Context>()),
-      event_listener_(Handle<Object>()),
-      event_listener_data_(Handle<Object>()),
       is_active_(false),
       hook_on_function_call_(false),
       is_suppressed_(false),
       live_edit_enabled_(true),  // TODO(yangguo): set to false by default.
       break_disabled_(false),
       break_points_active_(true),
-      in_debug_event_listener_(false),
       break_on_exception_(false),
       break_on_uncaught_exception_(false),
       side_effect_check_failed_(false),
@@ -473,6 +470,7 @@ bool Debug::Load() {
 void Debug::Unload() {
   ClearAllBreakPoints();
   ClearStepping();
+  RemoveDebugDelegate();
 
   // Return debugger is not loaded.
   if (!is_loaded()) return;
@@ -497,6 +495,7 @@ void Debug::Break(JavaScriptFrame* frame) {
 
   // Postpone interrupt during breakpoint processing.
   PostponeInterruptsScope postpone(isolate_);
+  DisableBreak no_recursive_break(this);
 
   // Return if we fail to retrieve debug info.
   Handle<JSFunction> function(frame->function());
@@ -1667,11 +1666,11 @@ MaybeHandle<Object> Debug::MakeCompileEvent(Handle<Script> script,
   return CallFunction("MakeCompileEvent", arraysize(argv), argv);
 }
 
-MaybeHandle<Object> Debug::MakeAsyncTaskEvent(Handle<Smi> type,
-                                              Handle<Smi> id) {
-  DCHECK(id->IsNumber());
+MaybeHandle<Object> Debug::MakeAsyncTaskEvent(
+    v8::debug::PromiseDebugActionType type, int id) {
   // Create the async task event object.
-  Handle<Object> argv[] = {type, id};
+  Handle<Object> argv[] = {Handle<Smi>(Smi::FromInt(type), isolate_),
+                           Handle<Smi>(Smi::FromInt(id), isolate_)};
   return CallFunction("MakeAsyncTaskEvent", arraysize(argv), argv);
 }
 
@@ -1764,6 +1763,9 @@ void Debug::OnException(Handle<Object> exception, Handle<Object> promise) {
     // Check whether the promise reject is considered an uncaught exception.
     uncaught = !isolate_->PromiseHasUserDefinedRejectHandler(jspromise);
   }
+
+  if (!debug_delegate_) return;
+
   // Bail out if exception breaks are not active
   if (uncaught) {
     // Uncaught exceptions are reported by either flags.
@@ -1773,7 +1775,6 @@ void Debug::OnException(Handle<Object> exception, Handle<Object> promise) {
     if (!break_on_exception_) return;
   }
 
-  bool empty_js_stack = false;
   {
     JavaScriptFrameIterator it(isolate_);
     // Check whether the top frame is blackboxed or the break location is muted.
@@ -1781,14 +1782,14 @@ void Debug::OnException(Handle<Object> exception, Handle<Object> promise) {
                        IsExceptionBlackboxed(uncaught))) {
       return;
     }
-    empty_js_stack = it.done();
+    if (it.done()) return;  // Do not trigger an event with an empty stack.
   }
 
   DebugScope debug_scope(this);
   if (debug_scope.failed()) return;
-
-  if (debug_delegate_ && !empty_js_stack) {
   HandleScope scope(isolate_);
+  PostponeInterruptsScope postpone(isolate_);
+  DisableBreak no_recursive_break(this);
 
   // Create the execution state.
   Handle<Object> exec_state;
@@ -1798,21 +1799,7 @@ void Debug::OnException(Handle<Object> exception, Handle<Object> promise) {
   debug_delegate_->ExceptionThrown(
       GetDebugEventContext(isolate_),
       v8::Utils::ToLocal(Handle<JSObject>::cast(exec_state)),
-        v8::Utils::ToLocal(exception), promise->IsJSObject(), uncaught);
-  }
-  if (debug_delegate_ && !non_inspector_listener_exists()) return;
-
-  // Create the event data object.
-  Handle<Object> event_data;
-  // Bail out and don't call debugger if exception.
-  if (!MakeExceptionEvent(
-          exception, uncaught, promise).ToHandle(&event_data)) {
-    return;
-  }
-
-  // Process debug event.
-  ProcessDebugEvent(v8::Exception, Handle<JSObject>::cast(event_data));
-  // Return to continue execution from where the exception was thrown.
+      v8::Utils::ToLocal(exception), v8::Utils::ToLocal(promise), uncaught);
 }
 
 void Debug::OnDebugBreak(Handle<Object> break_points_hit) {
@@ -1825,32 +1812,20 @@ void Debug::OnDebugBreak(Handle<Object> break_points_hit) {
   PrintBreakLocation();
 #endif  // DEBUG
 
-  if (debug_delegate_) {
+  if (!debug_delegate_) return;
   HandleScope scope(isolate_);
+  PostponeInterruptsScope no_interrupts(isolate_);
+  DisableBreak no_recursive_break(this);
 
   // Create the execution state.
   Handle<Object> exec_state;
   // Bail out and don't call debugger if exception.
   if (!MakeExecutionState().ToHandle(&exec_state)) return;
 
-    bool previous = in_debug_event_listener_;
-    in_debug_event_listener_ = true;
   debug_delegate_->BreakProgramRequested(
       GetDebugEventContext(isolate_),
       v8::Utils::ToLocal(Handle<JSObject>::cast(exec_state)),
       v8::Utils::ToLocal(break_points_hit));
-    in_debug_event_listener_ = previous;
-    if (!non_inspector_listener_exists()) return;
-  }
-
-  HandleScope scope(isolate_);
-  // Create the event data object.
-  Handle<Object> event_data;
-  // Bail out and don't call debugger if exception.
-  if (!MakeBreakEvent(break_points_hit).ToHandle(&event_data)) return;
-
-  // Process debug event.
-  ProcessDebugEvent(v8::Break, Handle<JSObject>::cast(event_data));
 }
 
 
@@ -1881,7 +1856,7 @@ void SendAsyncTaskEventCancel(const v8::WeakCallbackInfo<void>& info) {
       reinterpret_cast<CollectedCallbackData*>(info.GetParameter()));
   if (!data->debug->is_active()) return;
   HandleScope scope(data->isolate);
-  data->debug->OnAsyncTaskEvent(debug::kDebugPromiseCollected, data->id);
+  data->debug->OnAsyncTaskEvent(debug::kDebugPromiseCollected, data->id, 0);
 }
 
 void ResetPromiseHandle(const v8::WeakCallbackInfo<void>& info) {
@@ -1929,9 +1904,9 @@ void Debug::RunPromiseHook(PromiseHookType type, Handle<JSPromise> promise,
   int id = GetReferenceAsyncTaskId(isolate_, promise);
   switch (type) {
     case PromiseHookType::kInit:
-      debug_delegate_->PromiseEventOccurred(
-          debug::kDebugPromiseCreated, id,
-          parent->IsJSPromise() ? GetReferenceAsyncTaskId(
+      OnAsyncTaskEvent(debug::kDebugPromiseCreated, id,
+                       parent->IsJSPromise()
+                           ? GetReferenceAsyncTaskId(
                                  isolate_, Handle<JSPromise>::cast(parent))
                            : 0);
       return;
@@ -1940,10 +1915,10 @@ void Debug::RunPromiseHook(PromiseHookType type, Handle<JSPromise> promise,
       // get resolved status.
       return;
     case PromiseHookType::kBefore:
-      OnAsyncTaskEvent(debug::kDebugWillHandle, id);
+      OnAsyncTaskEvent(debug::kDebugWillHandle, id, 0);
       return;
     case PromiseHookType::kAfter:
-      OnAsyncTaskEvent(debug::kDebugDidHandle, id);
+      OnAsyncTaskEvent(debug::kDebugDidHandle, id, 0);
       return;
   }
 }
@@ -1988,7 +1963,10 @@ bool Debug::IsBlackboxed(Handle<SharedFunctionInfo> shared) {
   if (!shared->computed_debug_is_blackboxed()) {
     bool is_blackboxed = false;
     if (shared->script()->IsScript()) {
+      SuppressDebug while_processing(this);
       HandleScope handle_scope(isolate_);
+      PostponeInterruptsScope no_interrupts(isolate_);
+      DisableBreak no_recursive_break(this);
       Handle<Script> script(Script::cast(shared->script()));
       if (script->type() == i::Script::TYPE_NORMAL) {
         debug::Location start =
@@ -2004,70 +1982,17 @@ bool Debug::IsBlackboxed(Handle<SharedFunctionInfo> shared) {
   return shared->debug_is_blackboxed();
 }
 
-void Debug::OnAsyncTaskEvent(debug::PromiseDebugActionType type, int id) {
+void Debug::OnAsyncTaskEvent(debug::PromiseDebugActionType type, int id,
+                             int parent_id) {
   if (in_debug_scope() || ignore_events()) return;
-
-  if (debug_delegate_) {
-    debug_delegate_->PromiseEventOccurred(type, id, 0);
-    if (!non_inspector_listener_exists()) return;
-  }
-
-  HandleScope scope(isolate_);
-  DebugScope debug_scope(this);
+  if (!debug_delegate_) return;
+  SuppressDebug while_processing(this);
+  DebugScope debug_scope(isolate_->debug());
   if (debug_scope.failed()) return;
-
-  // Create the script collected state object.
-  Handle<Object> event_data;
-  // Bail out and don't call debugger if exception.
-  if (!MakeAsyncTaskEvent(handle(Smi::FromInt(type), isolate_),
-                          handle(Smi::FromInt(id), isolate_))
-           .ToHandle(&event_data))
-    return;
-
-  // Process debug event.
-  ProcessDebugEvent(v8::AsyncTaskEvent, Handle<JSObject>::cast(event_data));
-}
-
-void Debug::ProcessDebugEvent(v8::DebugEvent event,
-                              Handle<JSObject> event_data) {
-  // Notify registered debug event listener. This can be either a C or
-  // a JavaScript function.
-  if (event_listener_.is_null()) return;
   HandleScope scope(isolate_);
-
-  // Create the execution state.
-  Handle<Object> exec_state;
-  // Bail out and don't call debugger if exception.
-  if (!MakeExecutionState().ToHandle(&exec_state)) return;
-
-  // Prevent other interrupts from triggering, for example API callbacks,
-  // while dispatching event listners.
-  PostponeInterruptsScope postpone(isolate_);
-  bool previous = in_debug_event_listener_;
-  in_debug_event_listener_ = true;
-  if (event_listener_->IsForeign()) {
-    // Invoke the C debug event listener.
-    v8::Debug::EventCallback callback = FUNCTION_CAST<v8::Debug::EventCallback>(
-        Handle<Foreign>::cast(event_listener_)->foreign_address());
-    EventDetailsImpl event_details(event, Handle<JSObject>::cast(exec_state),
-                                   Handle<JSObject>::cast(event_data),
-                                   event_listener_data_);
-    callback(event_details);
-    CHECK(!isolate_->has_scheduled_exception());
-  } else {
-    // Invoke the JavaScript debug event listener.
-    DCHECK(event_listener_->IsJSFunction());
-    Handle<Object> argv[] = { Handle<Object>(Smi::FromInt(event), isolate_),
-                              exec_state,
-                              event_data,
-                              event_listener_data_ };
-    Handle<JSReceiver> global = isolate_->global_proxy();
-    MaybeHandle<Object> result =
-        Execution::Call(isolate_, Handle<JSFunction>::cast(event_listener_),
-                        global, arraysize(argv), argv);
-    CHECK(!result.is_null());  // Listeners must not throw.
-  }
-  in_debug_event_listener_ = previous;
+  PostponeInterruptsScope no_interrupts(isolate_);
+  DisableBreak no_recursive_break(this);
+  debug_delegate_->PromiseEventOccurred(type, id, parent_id);
 }
 
 void Debug::ProcessCompileEvent(v8::DebugEvent event, Handle<Script> script) {
@@ -2076,24 +2001,15 @@ void Debug::ProcessCompileEvent(v8::DebugEvent event, Handle<Script> script) {
       script->type() != i::Script::TYPE_WASM) {
     return;
   }
+  if (!debug_delegate_) return;
   SuppressDebug while_processing(this);
   DebugScope debug_scope(this);
   if (debug_scope.failed()) return;
-
-  if (debug_delegate_) {
+  HandleScope scope(isolate_);
+  PostponeInterruptsScope postpone(isolate_);
+  DisableBreak no_recursive_break(this);
   debug_delegate_->ScriptCompiled(ToApiHandle<debug::Script>(script),
                                   event != v8::AfterCompile);
-    if (!non_inspector_listener_exists()) return;
-  }
-
-  HandleScope scope(isolate_);
-  // 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;
-
-  // Process debug event.
-  ProcessDebugEvent(event, Handle<JSObject>::cast(event_data));
 }
 
 
@@ -2105,27 +2021,6 @@ Handle<Context> Debug::GetDebugContext() {
   return handle(*debug_context(), isolate_);
 }
 
-
-void Debug::SetEventListener(Handle<Object> callback,
-                             Handle<Object> data) {
-  GlobalHandles* global_handles = isolate_->global_handles();
-
-  // Remove existing entry.
-  GlobalHandles::Destroy(event_listener_.location());
-  event_listener_ = Handle<Object>();
-  GlobalHandles::Destroy(event_listener_data_.location());
-  event_listener_data_ = Handle<Object>();
-
-  // Set new entry.
-  if (!callback->IsNullOrUndefined(isolate_)) {
-    event_listener_ = global_handles->Create(*callback);
-    if (data.is_null()) data = isolate_->factory()->undefined_value();
-    event_listener_data_ = global_handles->Create(*data);
-  }
-
-  UpdateState();
-}
-
 int Debug::CurrentFrameCount() {
   StackTraceFrameIterator it(isolate_);
   if (break_frame_id() != StackFrame::NO_ID) {
@@ -2147,13 +2042,25 @@ int Debug::CurrentFrameCount() {
   return counter;
 }
 
-void Debug::SetDebugDelegate(debug::DebugDelegate* delegate) {
+void Debug::SetDebugDelegate(debug::DebugDelegate* delegate,
+                             bool pass_ownership) {
+  RemoveDebugDelegate();
   debug_delegate_ = delegate;
+  owns_debug_delegate_ = pass_ownership;
   UpdateState();
 }
 
+void Debug::RemoveDebugDelegate() {
+  if (debug_delegate_ == nullptr) return;
+  if (owns_debug_delegate_) {
+    owns_debug_delegate_ = false;
+    delete debug_delegate_;
+  }
+  debug_delegate_ = nullptr;
+}
+
 void Debug::UpdateState() {
-  bool is_active = !event_listener_.is_null() || debug_delegate_ != nullptr;
+  bool is_active = debug_delegate_ != nullptr;
   if (is_active || in_debug_scope()) {
     // Note that the debug context could have already been loaded to
     // bootstrap test cases.
@@ -2358,23 +2265,99 @@ bool Debug::PerformSideEffectCheckForCallback(Address function) {
   return false;
 }
 
-NoSideEffectScope::~NoSideEffectScope() {
-  if (isolate_->needs_side_effect_check() &&
-      isolate_->debug()->side_effect_check_failed_) {
-    DCHECK(isolate_->has_pending_exception());
-    DCHECK_EQ(isolate_->heap()->termination_exception(),
-              isolate_->pending_exception());
-    // Convert the termination exception into a regular exception.
-    isolate_->CancelTerminateExecution();
-    isolate_->Throw(*isolate_->factory()->NewEvalError(
-        MessageTemplate::kNoSideEffectDebugEvaluate));
+void LegacyDebugDelegate::PromiseEventOccurred(
+    v8::debug::PromiseDebugActionType type, int id, int parent_id) {
+  Handle<Object> event_data;
+  if (isolate_->debug()->MakeAsyncTaskEvent(type, id).ToHandle(&event_data)) {
+    ProcessDebugEvent(v8::AsyncTaskEvent, Handle<JSObject>::cast(event_data));
   }
-  isolate_->set_needs_side_effect_check(old_needs_side_effect_check_);
-  isolate_->debug()->UpdateHookOnFunctionCall();
-  isolate_->debug()->side_effect_check_failed_ = false;
 }
 
-EventDetailsImpl::EventDetailsImpl(DebugEvent event,
+void LegacyDebugDelegate::ScriptCompiled(v8::Local<v8::debug::Script> script,
+                                         bool is_compile_error) {
+  Handle<Object> event_data;
+  v8::DebugEvent event = is_compile_error ? v8::CompileError : v8::AfterCompile;
+  if (isolate_->debug()
+          ->MakeCompileEvent(v8::Utils::OpenHandle(*script), event)
+          .ToHandle(&event_data)) {
+    ProcessDebugEvent(event, Handle<JSObject>::cast(event_data));
+  }
+}
+
+void LegacyDebugDelegate::BreakProgramRequested(
+    v8::Local<v8::Context> paused_context, v8::Local<v8::Object> exec_state,
+    v8::Local<v8::Value> break_points_hit) {
+  Handle<Object> event_data;
+  if (isolate_->debug()
+          ->MakeBreakEvent(v8::Utils::OpenHandle(*break_points_hit))
+          .ToHandle(&event_data)) {
+    ProcessDebugEvent(
+        v8::Break, Handle<JSObject>::cast(event_data),
+        Handle<JSObject>::cast(v8::Utils::OpenHandle(*exec_state)));
+  }
+}
+
+void LegacyDebugDelegate::ExceptionThrown(v8::Local<v8::Context> paused_context,
+                                          v8::Local<v8::Object> exec_state,
+                                          v8::Local<v8::Value> exception,
+                                          v8::Local<v8::Value> promise,
+                                          bool is_uncaught) {
+  Handle<Object> event_data;
+  if (isolate_->debug()
+          ->MakeExceptionEvent(v8::Utils::OpenHandle(*exception), is_uncaught,
+                               v8::Utils::OpenHandle(*promise))
+          .ToHandle(&event_data)) {
+    ProcessDebugEvent(
+        v8::Exception, Handle<JSObject>::cast(event_data),
+        Handle<JSObject>::cast(v8::Utils::OpenHandle(*exec_state)));
+  }
+}
+
+void LegacyDebugDelegate::ProcessDebugEvent(v8::DebugEvent event,
+                                            Handle<JSObject> event_data) {
+  Handle<Object> exec_state;
+  if (isolate_->debug()->MakeExecutionState().ToHandle(&exec_state)) {
+    ProcessDebugEvent(event, event_data, Handle<JSObject>::cast(exec_state));
+  }
+}
+
+JavaScriptDebugDelegate::JavaScriptDebugDelegate(Isolate* isolate,
+                                                 Handle<JSFunction> listener,
+                                                 Handle<Object> data)
+    : LegacyDebugDelegate(isolate) {
+  GlobalHandles* global_handles = isolate->global_handles();
+  listener_ = Handle<JSFunction>::cast(global_handles->Create(*listener));
+  data_ = global_handles->Create(*data);
+}
+
+JavaScriptDebugDelegate::~JavaScriptDebugDelegate() {
+  GlobalHandles::Destroy(Handle<Object>::cast(listener_).location());
+  GlobalHandles::Destroy(data_.location());
+}
+
+void JavaScriptDebugDelegate::ProcessDebugEvent(v8::DebugEvent event,
+                                                Handle<JSObject> event_data,
+                                                Handle<JSObject> exec_state) {
+  Handle<Object> argv[] = {Handle<Object>(Smi::FromInt(event), isolate_),
+                           exec_state, event_data, data_};
+  Handle<JSReceiver> global = isolate_->global_proxy();
+  // Listener must not throw.
+  Execution::Call(isolate_, listener_, global, arraysize(argv), argv)
+      .ToHandleChecked();
+}
+
+NativeDebugDelegate::NativeDebugDelegate(Isolate* isolate,
+                                         v8::Debug::EventCallback callback,
+                                         Handle<Object> data)
+    : LegacyDebugDelegate(isolate), callback_(callback) {
+  data_ = isolate->global_handles()->Create(*data);
+}
+
+NativeDebugDelegate::~NativeDebugDelegate() {
+  GlobalHandles::Destroy(data_.location());
+}
+
+NativeDebugDelegate::EventDetails::EventDetails(DebugEvent event,
                                                 Handle<JSObject> exec_state,
                                                 Handle<JSObject> event_data,
                                                 Handle<Object> callback_data)
@@ -2383,34 +2366,57 @@ EventDetailsImpl::EventDetailsImpl(DebugEvent event,
       event_data_(event_data),
       callback_data_(callback_data) {}
 
-DebugEvent EventDetailsImpl::GetEvent() const {
+DebugEvent NativeDebugDelegate::EventDetails::GetEvent() const {
   return event_;
 }
 
-
-v8::Local<v8::Object> EventDetailsImpl::GetExecutionState() const {
+v8::Local<v8::Object> NativeDebugDelegate::EventDetails::GetExecutionState()
+    const {
   return v8::Utils::ToLocal(exec_state_);
 }
 
-
-v8::Local<v8::Object> EventDetailsImpl::GetEventData() const {
+v8::Local<v8::Object> NativeDebugDelegate::EventDetails::GetEventData() const {
   return v8::Utils::ToLocal(event_data_);
 }
 
-
-v8::Local<v8::Context> EventDetailsImpl::GetEventContext() const {
+v8::Local<v8::Context> NativeDebugDelegate::EventDetails::GetEventContext()
+    const {
   return GetDebugEventContext(exec_state_->GetIsolate());
 }
 
-
-v8::Local<v8::Value> EventDetailsImpl::GetCallbackData() const {
+v8::Local<v8::Value> NativeDebugDelegate::EventDetails::GetCallbackData()
+    const {
   return v8::Utils::ToLocal(callback_data_);
 }
 
-
-v8::Isolate* EventDetailsImpl::GetIsolate() const {
+v8::Isolate* NativeDebugDelegate::EventDetails::GetIsolate() const {
   return reinterpret_cast<v8::Isolate*>(exec_state_->GetIsolate());
 }
 
+void NativeDebugDelegate::ProcessDebugEvent(v8::DebugEvent event,
+                                            Handle<JSObject> event_data,
+                                            Handle<JSObject> exec_state) {
+  EventDetails event_details(event, exec_state, event_data, data_);
+  Isolate* isolate = isolate_;
+  callback_(event_details);
+  CHECK(!isolate->has_scheduled_exception());
+}
+
+NoSideEffectScope::~NoSideEffectScope() {
+  if (isolate_->needs_side_effect_check() &&
+      isolate_->debug()->side_effect_check_failed_) {
+    DCHECK(isolate_->has_pending_exception());
+    DCHECK_EQ(isolate_->heap()->termination_exception(),
+              isolate_->pending_exception());
+    // Convert the termination exception into a regular exception.
+    isolate_->CancelTerminateExecution();
+    isolate_->Throw(*isolate_->factory()->NewEvalError(
+        MessageTemplate::kNoSideEffectDebugEvaluate));
+  }
+  isolate_->set_needs_side_effect_check(old_needs_side_effect_check_);
+  isolate_->debug()->UpdateHookOnFunctionCall();
+  isolate_->debug()->side_effect_check_failed_ = false;
+}
+
 }  // namespace internal
 }  // namespace v8

src/debug/debug.h

@@ -230,28 +230,6 @@ class DebugInfoListNode {
   DebugInfoListNode* next_;
 };
 
-// Details of the debug event delivered to the debug event listener.
-class EventDetailsImpl : public v8::Debug::EventDetails {
- public:
-  EventDetailsImpl(DebugEvent event, Handle<JSObject> exec_state,
-                   Handle<JSObject> event_data, Handle<Object> callback_data);
-  virtual DebugEvent GetEvent() const;
-  virtual v8::Local<v8::Object> GetExecutionState() const;
-  virtual v8::Local<v8::Object> GetEventData() const;
-  virtual v8::Local<v8::Context> GetEventContext() const;
-  virtual v8::Local<v8::Value> GetCallbackData() const;
-  virtual v8::Debug::ClientData* GetClientData() const { return nullptr; }
-  virtual v8::Isolate* GetIsolate() const;
-
- private:
-  DebugEvent event_;  // Debug event causing the break.
-  Handle<JSObject> exec_state_;         // Current execution state.
-  Handle<JSObject> event_data_;         // Data associated with the event.
-  Handle<Object> callback_data_;        // User data passed with the callback
-                                        // when it was registered.
-};
-
-
 class DebugFeatureTracker {
  public:
   enum Feature {
@@ -290,10 +268,9 @@ class Debug {
   void OnPromiseReject(Handle<Object> promise, Handle<Object> value);
   void OnCompileError(Handle<Script> script);
   void OnAfterCompile(Handle<Script> script);
-  void OnAsyncTaskEvent(debug::PromiseDebugActionType type, int id);
+  void OnAsyncTaskEvent(debug::PromiseDebugActionType type, int id,
+                        int parent_id);
 
-  // API facing.
-  void SetEventListener(Handle<Object> callback, Handle<Object> data);
   MUST_USE_RESULT MaybeHandle<Object> Call(Handle<Object> fun,
                                            Handle<Object> data);
   Handle<Context> GetDebugContext();
@@ -346,7 +323,7 @@ class Debug {
 
   bool IsBlackboxed(Handle<SharedFunctionInfo> shared);
 
-  void SetDebugDelegate(debug::DebugDelegate* delegate);
+  void SetDebugDelegate(debug::DebugDelegate* delegate, bool pass_ownership);
 
   // Returns whether the operation succeeded.
   bool EnsureDebugInfo(Handle<SharedFunctionInfo> shared);
@@ -446,9 +423,11 @@ class Debug {
 
  private:
   explicit Debug(Isolate* isolate);
+  ~Debug() { DCHECK_NULL(debug_delegate_); }
 
   void UpdateState();
   void UpdateHookOnFunctionCall();
+  void RemoveDebugDelegate();
   void Unload();
   void SetNextBreakId() {
     thread_local_.break_id_ = ++thread_local_.break_count_;
@@ -460,9 +439,7 @@ class Debug {
   inline bool ignore_events() const {
     return is_suppressed_ || !is_active_ || isolate_->needs_side_effect_check();
   }
-  inline bool break_disabled() const {
-    return break_disabled_ || in_debug_event_listener_;
-  }
+  inline bool break_disabled() const { return break_disabled_; }
 
   void clear_suspended_generator() {
     thread_local_.suspended_generator_ = Smi::kZero;
@@ -472,17 +449,6 @@ class Debug {
     return thread_local_.suspended_generator_ != Smi::kZero;
   }
 
-  // There are three types of event listeners: C++ message_handler,
-  // JavaScript event listener and C++ event listener.
-  // Currently inspector still uses C++ event listener and installs
-  // more specific event listeners for part of events. Calling of
-  // C++ event listener is redundant when more specific event listener
-  // is presented. Other clients can install JavaScript event listener
-  // (e.g. some of NodeJS module).
-  bool non_inspector_listener_exists() const {
-    return !event_listener_.is_null() && !event_listener_->IsForeign();
-  }
-
   bool IsExceptionBlackboxed(bool uncaught);
 
   void OnException(Handle<Object> exception, Handle<Object> promise);
@@ -497,8 +463,8 @@ class Debug {
       Handle<Object> promise);
   MUST_USE_RESULT MaybeHandle<Object> MakeCompileEvent(
       Handle<Script> script, v8::DebugEvent type);
-  MUST_USE_RESULT MaybeHandle<Object> MakeAsyncTaskEvent(Handle<Smi> type,
-                                                         Handle<Smi> id);
+  MUST_USE_RESULT MaybeHandle<Object> MakeAsyncTaskEvent(
+      v8::debug::PromiseDebugActionType type, int id);
 
   void ProcessCompileEvent(v8::DebugEvent event, Handle<Script> script);
   void ProcessDebugEvent(v8::DebugEvent event, Handle<JSObject> event_data);
@@ -538,10 +504,9 @@ class Debug {
 
   // Global handles.
   Handle<Context> debug_context_;
-  Handle<Object> event_listener_;
-  Handle<Object> event_listener_data_;
 
   debug::DebugDelegate* debug_delegate_ = nullptr;
+  bool owns_debug_delegate_ = false;
 
   // Debugger is active, i.e. there is a debug event listener attached.
   bool is_active_;
@@ -556,8 +521,6 @@ class Debug {
   bool break_disabled_;
   // Do not break on break points.
   bool break_points_active_;
-  // Nested inside a debug event listener.
-  bool in_debug_event_listener_;
   // Trigger debug break events for all exceptions.
   bool break_on_exception_;
   // Trigger debug break events for uncaught exceptions.
@@ -621,6 +584,7 @@ class Debug {
   friend class LiveEdit;
   friend class SuppressDebug;
   friend class NoSideEffectScope;
+  friend class LegacyDebugDelegate;
 
   friend Handle<FixedArray> GetDebuggedFunctions();  // In test-debug.cc
   friend void CheckDebuggerUnloaded(bool check_functions);  // In test-debug.cc
@@ -628,6 +592,84 @@ class Debug {
   DISALLOW_COPY_AND_ASSIGN(Debug);
 };
 
+class LegacyDebugDelegate : public v8::debug::DebugDelegate {
+ public:
+  explicit LegacyDebugDelegate(Isolate* isolate) : isolate_(isolate) {}
+  void PromiseEventOccurred(v8::debug::PromiseDebugActionType type, int id,
+                            int parent_id) override;
+  void ScriptCompiled(v8::Local<v8::debug::Script> script,
+                      bool has_compile_error) override;
+  void BreakProgramRequested(v8::Local<v8::Context> paused_context,
+                             v8::Local<v8::Object> exec_state,
+                             v8::Local<v8::Value> break_points_hit) override;
+  void ExceptionThrown(v8::Local<v8::Context> paused_context,
+                       v8::Local<v8::Object> exec_state,
+                       v8::Local<v8::Value> exception,
+                       v8::Local<v8::Value> promise, bool is_uncaught) override;
+  bool IsFunctionBlackboxed(v8::Local<v8::debug::Script> script,
+                            const v8::debug::Location& start,
+                            const v8::debug::Location& end) override {
+    return false;
+  }
+
+ protected:
+  Isolate* isolate_;
+
+ private:
+  void ProcessDebugEvent(v8::DebugEvent event, Handle<JSObject> event_data);
+  virtual void ProcessDebugEvent(v8::DebugEvent event,
+                                 Handle<JSObject> event_data,
+                                 Handle<JSObject> exec_state) = 0;
+};
+
+class JavaScriptDebugDelegate : public LegacyDebugDelegate {
+ public:
+  JavaScriptDebugDelegate(Isolate* isolate, Handle<JSFunction> listener,
+                          Handle<Object> data);
+  virtual ~JavaScriptDebugDelegate();
+
+ private:
+  void ProcessDebugEvent(v8::DebugEvent event, Handle<JSObject> event_data,
+                         Handle<JSObject> exec_state) override;
+
+  Handle<JSFunction> listener_;
+  Handle<Object> data_;
+};
+
+class NativeDebugDelegate : public LegacyDebugDelegate {
+ public:
+  NativeDebugDelegate(Isolate* isolate, v8::Debug::EventCallback callback,
+                      Handle<Object> data);
+  virtual ~NativeDebugDelegate();
+
+ private:
+  // Details of the debug event delivered to the debug event listener.
+  class EventDetails : public v8::Debug::EventDetails {
+   public:
+    EventDetails(DebugEvent event, Handle<JSObject> exec_state,
+                 Handle<JSObject> event_data, Handle<Object> callback_data);
+    virtual DebugEvent GetEvent() const;
+    virtual v8::Local<v8::Object> GetExecutionState() const;
+    virtual v8::Local<v8::Object> GetEventData() const;
+    virtual v8::Local<v8::Context> GetEventContext() const;
+    virtual v8::Local<v8::Value> GetCallbackData() const;
+    virtual v8::Debug::ClientData* GetClientData() const { return nullptr; }
+    virtual v8::Isolate* GetIsolate() const;
+
+   private:
+    DebugEvent event_;              // Debug event causing the break.
+    Handle<JSObject> exec_state_;   // Current execution state.
+    Handle<JSObject> event_data_;   // Data associated with the event.
+    Handle<Object> callback_data_;  // User data passed with the callback
+                                    // when it was registered.
+  };
+
+  void ProcessDebugEvent(v8::DebugEvent event, Handle<JSObject> event_data,
+                         Handle<JSObject> exec_state) override;
+
+  v8::Debug::EventCallback callback_;
+  Handle<Object> data_;
+};
 
 // This scope is used to load and enter the debug context and create a new
 // break state.  Leaving the scope will restore the previous state.
@@ -661,21 +703,16 @@ class DebugScope BASE_EMBEDDED {
 class DisableBreak BASE_EMBEDDED {
  public:
   explicit DisableBreak(Debug* debug)
-      : debug_(debug),
-        previous_break_disabled_(debug->break_disabled_),
-        previous_in_debug_event_listener_(debug->in_debug_event_listener_) {
+      : debug_(debug), previous_break_disabled_(debug->break_disabled_) {
     debug_->break_disabled_ = true;
-    debug_->in_debug_event_listener_ = true;
   }
   ~DisableBreak() {
     debug_->break_disabled_ = previous_break_disabled_;
-    debug_->in_debug_event_listener_ = previous_in_debug_event_listener_;
   }
 
  private:
   Debug* debug_;
   bool previous_break_disabled_;
-  bool previous_in_debug_event_listener_;
   DISALLOW_COPY_AND_ASSIGN(DisableBreak);
 };
 

src/inspector/v8-debugger.cc

@@ -628,7 +628,9 @@ void V8Debugger::BreakProgramRequested(v8::Local<v8::Context> pausedContext,
 void V8Debugger::ExceptionThrown(v8::Local<v8::Context> pausedContext,
                                  v8::Local<v8::Object> execState,
                                  v8::Local<v8::Value> exception,
-                                 bool isPromiseRejection, bool isUncaught) {
+                                 v8::Local<v8::Value> promise,
+                                 bool isUncaught) {
+  bool isPromiseRejection = promise->IsPromise();
   handleProgramBreak(pausedContext, execState, exception,
                      v8::Local<v8::Array>(), isPromiseRejection, isUncaught);
 }

src/inspector/v8-debugger.h

@@ -140,7 +140,7 @@ class V8Debugger : public v8::debug::DebugDelegate {
   void ExceptionThrown(v8::Local<v8::Context> paused_context,
                        v8::Local<v8::Object> exec_state,
                        v8::Local<v8::Value> exception,
-                       bool is_promise_rejection, bool is_uncaught) override;
+                       v8::Local<v8::Value> promise, bool is_uncaught) override;
   bool IsFunctionBlackboxed(v8::Local<v8::debug::Script> script,
                             const v8::debug::Location& start,
                             const v8::debug::Location& end) override;

src/runtime/runtime-debug.cc

@@ -79,8 +79,13 @@ RUNTIME_FUNCTION(Runtime_SetDebugEventListener) {
   CHECK(args[0]->IsJSFunction() || args[0]->IsNullOrUndefined(isolate));
   CONVERT_ARG_HANDLE_CHECKED(Object, callback, 0);
   CONVERT_ARG_HANDLE_CHECKED(Object, data, 1);
-  isolate->debug()->SetEventListener(callback, data);
-
+  if (callback->IsJSFunction()) {
+    JavaScriptDebugDelegate* delegate = new JavaScriptDebugDelegate(
+        isolate, Handle<JSFunction>::cast(callback), data);
+    isolate->debug()->SetDebugDelegate(delegate, true);
+  } else {
+    isolate->debug()->SetDebugDelegate(nullptr, false);
+  }
   return isolate->heap()->undefined_value();
 }
 
@@ -1853,7 +1858,7 @@ RUNTIME_FUNCTION(Runtime_DebugAsyncFunctionPromiseCreated) {
   JSObject::SetProperty(promise, async_stack_id_symbol,
                         handle(Smi::FromInt(id), isolate), STRICT)
       .Assert();
-  isolate->debug()->OnAsyncTaskEvent(debug::kDebugEnqueueAsyncFunction, id);
+  isolate->debug()->OnAsyncTaskEvent(debug::kDebugEnqueueAsyncFunction, id, 0);
   return isolate->heap()->undefined_value();
 }
 
@@ -1876,7 +1881,7 @@ RUNTIME_FUNCTION(Runtime_DebugAsyncEventEnqueueRecurring) {
     isolate->debug()->OnAsyncTaskEvent(
         status == v8::Promise::kFulfilled ? debug::kDebugEnqueuePromiseResolve
                                           : debug::kDebugEnqueuePromiseReject,
-        isolate->debug()->NextAsyncTaskId(promise));
+        isolate->debug()->NextAsyncTaskId(promise), 0);
   }
   return isolate->heap()->undefined_value();
 }

src/runtime/runtime-promise.cc

@@ -45,7 +45,7 @@ RUNTIME_FUNCTION(Runtime_PromiseRejectEventFromStack) {
     rejected_promise = isolate->GetPromiseOnStackOnThrow();
     isolate->debug()->OnAsyncTaskEvent(
         debug::kDebugEnqueuePromiseReject,
-        isolate->debug()->NextAsyncTaskId(promise));
+        isolate->debug()->NextAsyncTaskId(promise), 0);
   }
   PromiseRejectEvent(isolate, promise, rejected_promise, value, true);
   return isolate->heap()->undefined_value();

test/cctest/test-debug.cc

@@ -3756,43 +3756,6 @@ TEST(TryFinallyOriginalMessage) {
 }
 
 
-TEST(EvalJSInDebugEventListenerOnNativeReThrownException) {
-  DebugLocalContext env;
-  v8::HandleScope scope(env->GetIsolate());
-  env.ExposeDebug();
-
-  // Create functions for testing break on exception.
-  v8::Local<v8::Function> noThrowJS = CompileFunction(
-      &env, "function noThrowJS(){var a=[1]; a.push(2); return a.length;}",
-      "noThrowJS");
-
-  debug_event_listener_callback = noThrowJS;
-  debug_event_listener_callback_result = 2;
-
-  env->GetIsolate()->AddMessageListener(MessageCallbackCount);
-  v8::Debug::SetDebugEventListener(env->GetIsolate(), DebugEventCounter);
-  // Break on uncaught exception
-  ChangeBreakOnException(false, true);
-  DebugEventCounterClear();
-  MessageCallbackCountClear();
-
-  // ReThrow native error
-  {
-    v8::TryCatch tryCatch(env->GetIsolate());
-    env->GetIsolate()->ThrowException(
-        v8::Exception::TypeError(v8_str(env->GetIsolate(), "Type error")));
-    CHECK(tryCatch.HasCaught());
-    tryCatch.ReThrow();
-  }
-  CHECK_EQ(1, exception_hit_count);
-  CHECK_EQ(1, uncaught_exception_hit_count);
-  CHECK_EQ(0, message_callback_count);  // FIXME: Should it be 1 ?
-  CHECK(!debug_event_listener_callback.IsEmpty());
-
-  debug_event_listener_callback.Clear();
-}
-
-
 // Test break on exception from compiler errors. When compiling using
 // v8::Script::Compile there is no JavaScript stack whereas when compiling using
 // eval there are JavaScript frames.
@@ -3822,16 +3785,18 @@ TEST(BreakOnCompileException) {
   // Throws SyntaxError: Unexpected end of input
   CHECK(
       v8::Script::Compile(context, v8_str(env->GetIsolate(), "+++")).IsEmpty());
-  CHECK_EQ(1, exception_hit_count);
-  CHECK_EQ(1, uncaught_exception_hit_count);
+  // Exceptions with no stack are skipped.
+  CHECK_EQ(0, exception_hit_count);
+  CHECK_EQ(0, uncaught_exception_hit_count);
   CHECK_EQ(1, message_callback_count);
   CHECK_EQ(0, last_js_stack_height);  // No JavaScript stack.
 
   // Throws SyntaxError: Unexpected identifier
   CHECK(
       v8::Script::Compile(context, v8_str(env->GetIsolate(), "x x")).IsEmpty());
-  CHECK_EQ(2, exception_hit_count);
-  CHECK_EQ(2, uncaught_exception_hit_count);
+  // Exceptions with no stack are skipped.
+  CHECK_EQ(0, exception_hit_count);
+  CHECK_EQ(0, uncaught_exception_hit_count);
   CHECK_EQ(2, message_callback_count);
   CHECK_EQ(0, last_js_stack_height);  // No JavaScript stack.
 
@@ -3840,8 +3805,8 @@ TEST(BreakOnCompileException) {
             .ToLocalChecked()
             ->Run(context)
             .IsEmpty());
-  CHECK_EQ(3, exception_hit_count);
-  CHECK_EQ(3, uncaught_exception_hit_count);
+  CHECK_EQ(1, exception_hit_count);
+  CHECK_EQ(1, uncaught_exception_hit_count);
   CHECK_EQ(3, message_callback_count);
   CHECK_EQ(1, last_js_stack_height);
 
@@ -3850,8 +3815,8 @@ TEST(BreakOnCompileException) {
             .ToLocalChecked()
             ->Run(context)
             .IsEmpty());
-  CHECK_EQ(4, exception_hit_count);
-  CHECK_EQ(4, uncaught_exception_hit_count);
+  CHECK_EQ(2, exception_hit_count);
+  CHECK_EQ(2, uncaught_exception_hit_count);
   CHECK_EQ(4, message_callback_count);
   CHECK_EQ(1, last_js_stack_height);
 }