Revert "Fix Context PromiseHook behaviour with debugger enabled"

This reverts commit 872b7faa.

Reason for revert: Somewhat speculative revert because of https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux%20-%20gc%20stress/39673/overview (reverting locally resolved the issue for me)

Original change's description:
> Fix Context PromiseHook behaviour with debugger enabled
>
> This is a solution for https://github.com/nodejs/node/issues/43148.
>
> Due to differences in behaviour between code with and without the debugger enabled, some promise lifecycle events were being missed and some extra ones were being added. This change resolves this and verifies the event sequence is consistent between code with and without the debugger.
>
> Change-Id: I3dabf1dceb14233226b1752083d659f1c2f97966
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3779922
> Reviewed-by: Victor Gomes <victorgomes@chromium.org>
> Commit-Queue: Camillo Bruni <cbruni@chromium.org>
> Reviewed-by: Camillo Bruni <cbruni@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#82132}

Change-Id: I3e05adead5d8033906055e0741854da68aade2ac
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3804859
Commit-Queue: Nico Hartmann <nicohartmann@chromium.org>
Owners-Override: Nico Hartmann <nicohartmann@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Nico Hartmann <nicohartmann@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82136}

src/builtins/builtins-microtask-queue-gen.cc

@@ -479,38 +479,30 @@ void MicrotaskQueueBuiltinsAssembler::RewindEnteredContext(
 void MicrotaskQueueBuiltinsAssembler::RunAllPromiseHooks(
     PromiseHookType type, TNode<Context> context,
     TNode<HeapObject> promise_or_capability) {
-  TNode<Uint32T> promiseHookFlags = PromiseHookFlags();
-#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
   Label hook(this, Label::kDeferred), done_hook(this);
+  TNode<Uint32T> promiseHookFlags = PromiseHookFlags();
   Branch(NeedsAnyPromiseHooks(promiseHookFlags), &hook, &done_hook);
   BIND(&hook);
   {
-#endif
     switch (type) {
       case PromiseHookType::kBefore:
-#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
         RunContextPromiseHookBefore(context, promise_or_capability,
                                     promiseHookFlags);
-#endif
         RunPromiseHook(Runtime::kPromiseHookBefore, context,
                        promise_or_capability, promiseHookFlags);
         break;
       case PromiseHookType::kAfter:
-#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
         RunContextPromiseHookAfter(context, promise_or_capability,
                                    promiseHookFlags);
-#endif
         RunPromiseHook(Runtime::kPromiseHookAfter, context,
                        promise_or_capability, promiseHookFlags);
         break;
       default:
         UNREACHABLE();
     }
-#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
     Goto(&done_hook);
   }
   BIND(&done_hook);
-#endif
 }
 
 void MicrotaskQueueBuiltinsAssembler::RunPromiseHook(

src/d8/d8.cc

@@ -2292,16 +2292,6 @@ void Shell::AsyncHooksTriggerAsyncId(
       PerIsolateData::Get(isolate)->GetAsyncHooks()->GetTriggerAsyncId()));
 }
 
-static v8::debug::DebugDelegate dummy_delegate;
-
-void Shell::EnableDebugger(const v8::FunctionCallbackInfo<v8::Value>& args) {
-  v8::debug::SetDebugDelegate(args.GetIsolate(), &dummy_delegate);
-}
-
-void Shell::DisableDebugger(const v8::FunctionCallbackInfo<v8::Value>& args) {
-  v8::debug::SetDebugDelegate(args.GetIsolate(), nullptr);
-}
-
 void Shell::SetPromiseHooks(const v8::FunctionCallbackInfo<v8::Value>& args) {
   Isolate* isolate = args.GetIsolate();
   if (i::FLAG_correctness_fuzzer_suppressions) {
@@ -3384,18 +3374,6 @@ Local<ObjectTemplate> Shell::CreateD8Template(Isolate* isolate) {
                               Local<Signature>(), 4));
     d8_template->Set(isolate, "promise", promise_template);
   }
-  {
-    Local<ObjectTemplate> debugger_template = ObjectTemplate::New(isolate);
-    debugger_template->Set(
-        isolate, "enable",
-        FunctionTemplate::New(isolate, EnableDebugger, Local<Value>(),
-                              Local<Signature>(), 0));
-    debugger_template->Set(
-        isolate, "disable",
-        FunctionTemplate::New(isolate, DisableDebugger, Local<Value>(),
-                              Local<Signature>(), 0));
-    d8_template->Set(isolate, "debugger", debugger_template);
-  }
   {
     Local<ObjectTemplate> serializer_template = ObjectTemplate::New(isolate);
     serializer_template->Set(

src/d8/d8.h

@@ -583,9 +583,6 @@ class Shell : public i::AllStatic {
 
   static void SetPromiseHooks(const v8::FunctionCallbackInfo<v8::Value>& args);
 
-  static void EnableDebugger(const v8::FunctionCallbackInfo<v8::Value>& args);
-  static void DisableDebugger(const v8::FunctionCallbackInfo<v8::Value>& args);
-
   static void SerializerSerialize(
       const v8::FunctionCallbackInfo<v8::Value>& args);
   static void SerializerDeserialize(

src/execution/isolate.cc

@@ -5111,11 +5111,9 @@ void Isolate::SetPromiseHook(PromiseHook hook) {
 void Isolate::RunAllPromiseHooks(PromiseHookType type,
                                  Handle<JSPromise> promise,
                                  Handle<Object> parent) {
-#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
   if (HasContextPromiseHooks()) {
     native_context()->RunPromiseHook(type, promise, parent);
   }
-#endif
   if (HasIsolatePromiseHooks() || HasAsyncEventDelegate()) {
     RunPromiseHook(type, promise, parent);
   }
@@ -5132,7 +5130,7 @@ void Isolate::RunPromiseHook(PromiseHookType type, Handle<JSPromise> promise,
 void Isolate::OnAsyncFunctionSuspended(Handle<JSPromise> promise,
                                        Handle<JSPromise> parent) {
   DCHECK_EQ(0, promise->async_task_id());
-  RunAllPromiseHooks(PromiseHookType::kInit, promise, parent);
+  RunPromiseHook(PromiseHookType::kInit, promise, parent);
   if (HasAsyncEventDelegate()) {
     DCHECK_NE(nullptr, async_event_delegate_);
     promise->set_async_task_id(++async_task_count_);

src/objects/contexts.cc

@@ -559,7 +559,6 @@ static_assert(NativeContext::kSize ==
               (Context::SizeFor(NativeContext::NATIVE_CONTEXT_SLOTS) +
                kSystemPointerSize));
 
-#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
 void NativeContext::RunPromiseHook(PromiseHookType type,
                                    Handle<JSPromise> promise,
                                    Handle<Object> parent) {
@@ -615,7 +614,6 @@ void NativeContext::RunPromiseHook(PromiseHookType type,
     isolate->clear_pending_exception();
   }
 }
-#endif
 
 }  // namespace internal
 }  // namespace v8

src/objects/contexts.h

@@ -786,10 +786,8 @@ class NativeContext : public Context {
   void IncrementErrorsThrown();
   int GetErrorsThrown();
 
-#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
   void RunPromiseHook(PromiseHookType type, Handle<JSPromise> promise,
                       Handle<Object> parent);
-#endif
 
  private:
   static_assert(OffsetOfElementAt(EMBEDDER_DATA_INDEX) ==

src/objects/objects.cc

@@ -5451,14 +5451,6 @@ Handle<Object> JSPromise::Fulfill(Handle<JSPromise> promise,
                                   Handle<Object> value) {
   Isolate* const isolate = promise->GetIsolate();
 
-#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
-  if (isolate->HasContextPromiseHooks()) {
-    isolate->raw_native_context().RunPromiseHook(
-        PromiseHookType::kResolve, promise,
-        isolate->factory()->undefined_value());
-  }
-#endif
-
   // 1. Assert: The value of promise.[[PromiseState]] is "pending".
   CHECK_EQ(Promise::kPending, promise->status());
 
@@ -5538,8 +5530,8 @@ MaybeHandle<Object> JSPromise::Resolve(Handle<JSPromise> promise,
   DCHECK(
       !reinterpret_cast<v8::Isolate*>(isolate)->GetCurrentContext().IsEmpty());
 
-  isolate->RunPromiseHook(PromiseHookType::kResolve, promise,
-                          isolate->factory()->undefined_value());
+  isolate->RunAllPromiseHooks(PromiseHookType::kResolve, promise,
+                              isolate->factory()->undefined_value());
 
   // 7. If SameValue(resolution, promise) is true, then
   if (promise.is_identical_to(resolution)) {

test/mjsunit/promise-hooks.js

@@ -220,7 +220,7 @@ function optimizerBailout(test, verify) {
   d8.promise.setHooks();
 }
 
-function doTest () {
+if (has_promise_hooks) {
   optimizerBailout(async () => {
     await Promise.resolve();
   }, () => {
@@ -234,19 +234,6 @@ function doTest () {
     assertNextEvent('after', [ 3 ]);
     assertEmptyLog();
   });
-  optimizerBailout(async () => {
-    await Promise.reject();
-  }, () => {
-    assertNextEvent('init', [ 1 ]);
-    assertNextEvent('init', [ 2 ]);
-    assertNextEvent('resolve', [ 2 ]);
-    assertNextEvent('init', [ 3, 2 ]);
-    assertNextEvent('before', [ 3 ]);
-    assertNextEvent('resolve', [ 1 ]);
-    assertNextEvent('resolve', [ 3 ]);
-    assertNextEvent('after', [ 3 ]);
-    assertEmptyLog();
-  });
   optimizerBailout(async () => {
     await { then (cb) { cb() } };
   }, () => {
@@ -262,21 +249,6 @@ function doTest () {
     assertNextEvent('after', [ 3 ]);
     assertEmptyLog();
   });
-  optimizerBailout(async () => {
-    await { then (_, cb) { cb() } };
-  }, () => {
-    assertNextEvent('init', [ 1 ]);
-    assertNextEvent('init', [ 2, 1 ]);
-    assertNextEvent('init', [ 3, 2 ]);
-    assertNextEvent('before', [ 2 ]);
-    assertNextEvent('resolve', [ 2 ]);
-    assertNextEvent('after', [ 2 ]);
-    assertNextEvent('before', [ 3 ]);
-    assertNextEvent('resolve', [ 1 ]);
-    assertNextEvent('resolve', [ 3 ]);
-    assertNextEvent('after', [ 3 ]);
-    assertEmptyLog();
-  });
   basicTest();
   exceptions();
 
@@ -320,9 +292,3 @@ function doTest () {
   });
 
 }
-
-if (has_promise_hooks) {
-  doTest();
-  d8.debugger.enable();
-  doTest();
-}