[promisehook] Implement PromiseHook

src/isolate.cc

 // Copyright 2012 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.
 
 #include "src/isolate.h"
 
 #include <stdlib.h>
 
 #include <fstream>  // NOLINT(readability/streams)
 #include <sstream>
@@ skipping 2133 matching lines @@
       regexp_stack_(NULL),
       date_cache_(NULL),
       call_descriptor_data_(NULL),
       // TODO(bmeurer) Initialized lazily because it depends on flags; can
       // be fixed once the default isolate cleanup is done.
       random_number_generator_(NULL),
       rail_mode_(PERFORMANCE_ANIMATION),
       serializer_enabled_(enable_serializer),
       has_fatal_error_(false),
       initialized_from_snapshot_(false),
-      is_promisehook_enabled_(false),
+      is_promise_hook_enabled_(false),
       is_tail_call_elimination_enabled_(true),
       is_isolate_in_background_(false),
       cpu_profiler_(NULL),
       heap_profiler_(NULL),
       code_event_dispatcher_(new CodeEventDispatcher()),
       function_entry_hook_(NULL),
       deferred_handles_head_(NULL),
       optimizing_compile_dispatcher_(NULL),
       stress_deopt_count_(0),
       next_optimization_id_(0),
@@ skipping 996 matching lines @@
 
   if (call_completed_callbacks_.is_empty()) return;
   // Fire callbacks.  Increase call depth to prevent recursive callbacks.
   v8::Isolate* isolate = reinterpret_cast<v8::Isolate*>(this);
   v8::Isolate::SuppressMicrotaskExecutionScope suppress(isolate);
   for (int i = 0; i < call_completed_callbacks_.length(); i++) {
     call_completed_callbacks_.at(i)(isolate);
   }
 }
 
-void Isolate::EnablePromiseHook() { is_promisehook_enabled_ = true; }
+void Isolate::SetPromiseHook(PromiseHook hook) {
+  is_promise_hook_enabled_ = hook != nullptr;

[adamk, 2016/12/15 08:46:09]

Is there a reason we still need to track this separately? Or could the CSA just
load hook itself and compare it to 0?

[gsathya, 2016/12/15 15:35:33]

I thought the boolean was cleaner. Changed.

[adamk, 2016/12/15 15:43:58]

I prefer this way to avoid having to do extra book-keeping.

+  set_promise_hook(hook);
+}
 
-void Isolate::DisablePromiseHook() { is_promisehook_enabled_ = false; }
+void Isolate::RunPromiseHook(PromiseHookType type, Handle<JSObject> promise,
+                             Handle<Object> parent) {
+  if (!is_promise_hook_enabled_) return;
+  PromiseHook hook = promise_hook();
+  HandleScope scope(this);

[adamk, 2016/12/15 08:46:09]

Where is this called from where there isn't already a HandleScope? Given that it
takes in handles that shouldn't be possible.

[gsathya, 2016/12/15 15:35:33]

I don't create handlescopes in the runtime functions that call this

[adamk, 2016/12/15 15:43:58]

Ah, you should create HandleScopes there :)

+  hook(type, v8::Utils::PromiseToLocal(promise), v8::Utils::ToLocal(parent));
+}
 
 void Isolate::SetPromiseRejectCallback(PromiseRejectCallback callback) {
   promise_reject_callback_ = callback;
 }
 
 
 void Isolate::ReportPromiseReject(Handle<JSObject> promise,
                                   Handle<Object> value,
                                   v8::PromiseRejectEvent event) {
   if (promise_reject_callback_ == NULL) return;
@@ skipping 383 matching lines @@
   // Then check whether this scope intercepts.
   if ((flag & intercept_mask_)) {
     intercepted_flags_ |= flag;
     return true;
   }
   return false;
 }
 
 }  // namespace internal
 }  // namespace v8