Use non-primary promise handler as a source of fallback microtask context

A microtask requires a non-detached Context to trigger, and the Context
is usually pulled from the primary handler.
On an example below, |on_rejected| is primary, as the attached promise
is rejected and |on_rejected| will be called as the reaction.

  Promise.reject().then(on_fulfilled, on_rejected);

If the primary handler is undefined or invalid, we used to use the
promise's context as the fallback. E.g. the primary handler is undefined
on the examlpe below, and the context of |promise| was used.

  let promise = Promise.reject();
  promise.then(on_fulfilled);

However, that causes a non-intuitive behavior around a detached
context:

  let DeadPromise = iframe.contentWindow.Promise;
  iframe.src = "http://example.com"; // navigate away.
  // DeadPromise's Context is detached state now.

  let p = DeadPromise.reject();

  // |on_rejected| is called, as the context is pulled from |on_rejected|.
  p.then(on_fulfilled, on_rejected);

  // |on_rejected| was NOT called, as a microtask to settle |q| does not
  // run due to the detached context.
  let q = p.then(on_fulfilled);
  q.catch(on_rejected);

After this CL, we use non-primary handler as a source of fallback context.
On the last example above, the Context is pulled from |on_fullfilled|,
so that |q| is settled using that context.

Bug: chromium:941271
Change-Id: Iff71acf7c3617f3493d100abcd2c5c36bd1bbfd1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1535916Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60499}

src/builtins/builtins-promise-gen.cc

@@ -417,6 +417,8 @@ void PromiseBuiltinsAssembler::PerformPromiseThen(
   {
     VARIABLE(var_map, MachineRepresentation::kTagged);
     VARIABLE(var_handler, MachineRepresentation::kTagged);
+    VARIABLE(var_handler_context, MachineRepresentation::kTagged,
+             UndefinedConstant());
     Label if_fulfilled(this), if_rejected(this, Label::kDeferred),
         enqueue(this);
     Branch(IsPromiseStatus(status, v8::Promise::kFulfilled), &if_fulfilled,
@@ -426,6 +428,17 @@ void PromiseBuiltinsAssembler::PerformPromiseThen(
     {
       var_map.Bind(LoadRoot(RootIndex::kPromiseFulfillReactionJobTaskMap));
       var_handler.Bind(on_fulfilled);
+
+      Label use_fallback(this, Label::kDeferred), done(this);
+      ExtractHandlerContext(on_fulfilled, &var_handler_context);
+      Branch(IsUndefined(var_handler_context.value()), &use_fallback, &done);
+
+      BIND(&use_fallback);
+      var_handler_context.Bind(context);
+      ExtractHandlerContext(on_rejected, &var_handler_context);
+      Goto(&done);
+
+      BIND(&done);
       Goto(&enqueue);
     }
 
@@ -434,6 +447,17 @@ void PromiseBuiltinsAssembler::PerformPromiseThen(
       CSA_ASSERT(this, IsPromiseStatus(status, v8::Promise::kRejected));
       var_map.Bind(LoadRoot(RootIndex::kPromiseRejectReactionJobTaskMap));
       var_handler.Bind(on_rejected);
+
+      Label use_fallback(this, Label::kDeferred), done(this);
+      ExtractHandlerContext(on_rejected, &var_handler_context);
+      Branch(IsUndefined(var_handler_context.value()), &use_fallback, &done);
+
+      BIND(&use_fallback);
+      var_handler_context.Bind(context);
+      ExtractHandlerContext(on_fulfilled, &var_handler_context);
+      Goto(&done);
+      BIND(&done);
+
       GotoIf(PromiseHasHandler(promise), &enqueue);
       CallRuntime(Runtime::kPromiseRevokeReject, context, promise);
       Goto(&enqueue);
@@ -441,9 +465,6 @@ void PromiseBuiltinsAssembler::PerformPromiseThen(
 
     BIND(&enqueue);
     {
-      VARIABLE(var_handler_context, MachineRepresentation::kTagged, context);
-      ExtractHandlerContext(var_handler.value(), &var_handler_context);
-
       Node* argument =
           LoadObjectField(promise, JSPromise::kReactionsOrResultOffset);
       Node* microtask = AllocatePromiseReactionJobTask(
@@ -585,7 +606,36 @@ Node* PromiseBuiltinsAssembler::TriggerPromiseReactions(
       GotoIf(TaggedIsSmi(current), &done_loop);
       var_current.Bind(LoadObjectField(current, PromiseReaction::kNextOffset));
 
-      VARIABLE(var_context, MachineRepresentation::kTagged, context);
+      VARIABLE(var_context, MachineRepresentation::kTagged,
+               UndefinedConstant());
+
+      Node* primary_handler;
+      Node* secondary_handler;
+      if (type == PromiseReaction::kFulfill) {
+        primary_handler =
+            LoadObjectField(current, PromiseReaction::kFulfillHandlerOffset);
+        secondary_handler =
+            LoadObjectField(current, PromiseReaction::kRejectHandlerOffset);
+      } else {
+        primary_handler =
+            LoadObjectField(current, PromiseReaction::kRejectHandlerOffset);
+        secondary_handler =
+            LoadObjectField(current, PromiseReaction::kFulfillHandlerOffset);
+      }
+
+      {
+        Label use_fallback(this, Label::kDeferred), done(this);
+        ExtractHandlerContext(primary_handler, &var_context);
+        Branch(IsUndefined(var_context.value()), &use_fallback, &done);
+
+        BIND(&use_fallback);
+        var_context.Bind(context);
+        ExtractHandlerContext(secondary_handler, &var_context);
+        CSA_ASSERT(this, IsNotUndefined(var_context.value()));
+        Goto(&done);
+
+        BIND(&done);
+      }
 
       // Morph {current} from a PromiseReaction into a PromiseReactionJobTask
       // and schedule that on the microtask queue. We try to minimize the number
@@ -593,9 +643,6 @@ Node* PromiseBuiltinsAssembler::TriggerPromiseReactions(
       STATIC_ASSERT(static_cast<int>(PromiseReaction::kSize) ==
                     static_cast<int>(PromiseReactionJobTask::kSize));
       if (type == PromiseReaction::kFulfill) {
-        Node* handler =
-            LoadObjectField(current, PromiseReaction::kFulfillHandlerOffset);
-        ExtractHandlerContext(handler, &var_context);
         StoreMapNoWriteBarrier(current,
                                RootIndex::kPromiseFulfillReactionJobTaskMap);
         StoreObjectField(current, PromiseReactionJobTask::kArgumentOffset,
@@ -610,9 +657,6 @@ Node* PromiseBuiltinsAssembler::TriggerPromiseReactions(
             static_cast<int>(
                 PromiseReactionJobTask::kPromiseOrCapabilityOffset));
       } else {
-        Node* handler =
-            LoadObjectField(current, PromiseReaction::kRejectHandlerOffset);
-        ExtractHandlerContext(handler, &var_context);
         StoreMapNoWriteBarrier(current,
                                RootIndex::kPromiseRejectReactionJobTaskMap);
         StoreObjectField(current, PromiseReactionJobTask::kArgumentOffset,
@@ -620,7 +664,7 @@ Node* PromiseBuiltinsAssembler::TriggerPromiseReactions(
         StoreObjectField(current, PromiseReactionJobTask::kContextOffset,
                          var_context.value());
         StoreObjectField(current, PromiseReactionJobTask::kHandlerOffset,
-                         handler);
+                         primary_handler);
         STATIC_ASSERT(
             static_cast<int>(PromiseReaction::kPromiseOrCapabilityOffset) ==
             static_cast<int>(

src/objects.cc

@@ -6145,17 +6145,31 @@ Handle<Object> JSPromise::TriggerPromiseReactions(Isolate* isolate,
 
     Handle<NativeContext> handler_context;
 
-    STATIC_ASSERT(static_cast<int>(PromiseReaction::kSize) ==
-                  static_cast<int>(PromiseReactionJobTask::kSize));
+    Handle<HeapObject> primary_handler;
+    Handle<HeapObject> secondary_handler;
     if (type == PromiseReaction::kFulfill) {
-      Handle<HeapObject> handler = handle(reaction->fulfill_handler(), isolate);
-      if (handler->IsJSReceiver()) {
-        JSReceiver::GetContextForMicrotask(Handle<JSReceiver>::cast(handler))
+      primary_handler = handle(reaction->fulfill_handler(), isolate);
+      secondary_handler = handle(reaction->reject_handler(), isolate);
+    } else {
+      primary_handler = handle(reaction->reject_handler(), isolate);
+      secondary_handler = handle(reaction->fulfill_handler(), isolate);
+    }
+
+    if (primary_handler->IsJSReceiver()) {
+      JSReceiver::GetContextForMicrotask(
+          Handle<JSReceiver>::cast(primary_handler))
+          .ToHandle(&handler_context);
+    }
+    if (handler_context.is_null() && secondary_handler->IsJSReceiver()) {
+      JSReceiver::GetContextForMicrotask(
+          Handle<JSReceiver>::cast(secondary_handler))
           .ToHandle(&handler_context);
     }
-      if (handler_context.is_null())
-        handler_context = isolate->native_context();
+    if (handler_context.is_null()) handler_context = isolate->native_context();
 
+    STATIC_ASSERT(static_cast<int>(PromiseReaction::kSize) ==
+                  static_cast<int>(PromiseReactionJobTask::kSize));
+    if (type == PromiseReaction::kFulfill) {
       task->synchronized_set_map(
           ReadOnlyRoots(isolate).promise_fulfill_reaction_job_task_map());
       Handle<PromiseFulfillReactionJobTask>::cast(task)->set_argument(
@@ -6171,19 +6185,13 @@ Handle<Object> JSPromise::TriggerPromiseReactions(Isolate* isolate,
               PromiseFulfillReactionJobTask::kPromiseOrCapabilityOffset));
     } else {
       DisallowHeapAllocation no_gc;
-      Handle<HeapObject> handler = handle(reaction->reject_handler(), isolate);
-      if (handler->IsJSReceiver()) {
-        JSReceiver::GetContextForMicrotask(Handle<JSReceiver>::cast(handler))
-            .ToHandle(&handler_context);
-      }
-      if (handler_context.is_null())
-        handler_context = isolate->native_context();
       task->synchronized_set_map(
           ReadOnlyRoots(isolate).promise_reject_reaction_job_task_map());
       Handle<PromiseRejectReactionJobTask>::cast(task)->set_argument(*argument);
       Handle<PromiseRejectReactionJobTask>::cast(task)->set_context(
           *handler_context);
-      Handle<PromiseRejectReactionJobTask>::cast(task)->set_handler(*handler);
+      Handle<PromiseRejectReactionJobTask>::cast(task)->set_handler(
+          *primary_handler);
       STATIC_ASSERT(
           static_cast<int>(PromiseReaction::kPromiseOrCapabilityOffset) ==
           static_cast<int>(

test/unittests/microtask-queue-unittest.cc

@@ -514,5 +514,32 @@ TEST_F(MicrotaskQueueTest, DetachGlobal_HandlerContext) {
           .FromJust());
 }
 
+TEST_F(MicrotaskQueueTest, DetachGlobal_Chain) {
+  Handle<JSPromise> stale_rejected_promise;
+
+  Local<v8::Context> sub_context = v8::Context::New(v8_isolate());
+  {
+    v8::Context::Scope scope(sub_context);
+    stale_rejected_promise = RunJS<JSPromise>("Promise.reject()");
+  }
+  sub_context->DetachGlobal();
+  sub_context.Clear();
+
+  SetGlobalProperty(
+      "stale_rejected_promise",
+      Utils::ToLocal(Handle<JSReceiver>::cast(stale_rejected_promise)));
+  Handle<JSArray> result = RunJS<JSArray>(
+      "let result = [false];"
+      "stale_rejected_promise"
+      "  .then(() => {})"
+      "  .catch(() => {"
+      "    result[0] = true;"
+      "  });"
+      "result");
+  microtask_queue()->RunMicrotasks(isolate());
+  EXPECT_TRUE(
+      Object::GetElement(isolate(), result, 0).ToHandleChecked()->IsTrue());
+}
+
 }  // namespace internal
 }  // namespace v8