[promises] Make ResolvePromise match up with spec

[1] has changed the layering between JavaScript and HTML substantially
with regards to queuing promise-related microtasks. This fixes up the
comments and parameter orders so that they match up with the current
spec.

[1]: https://github.com/tc39/ecma262/commit/c59502090e2c250cd7e457b5506b92db6b21d153

Change-Id: I75650f7dc1c0b1d1c2b67aaf19d9791a0391a06d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2106997Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Shu-yu Guo <syg@chromium.org>
Commit-Queue: Timothy Gu <timothygu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67130}

src/builtins/promise-misc.tq

@@ -143,15 +143,31 @@ namespace promise {
 
   extern macro PromiseResolveThenableJobTaskMapConstant(): Map;
 
+  // https://tc39.es/ecma262/#sec-newpromiseresolvethenablejob
   macro NewPromiseResolveThenableJobTask(implicit context: Context)(
-      promiseToResolve: JSPromise, then: JSReceiver, thenable: JSReceiver,
-      thenContext: Context): PromiseResolveThenableJobTask {
+      promiseToResolve: JSPromise, thenable: JSReceiver,
+      then: Callable): PromiseResolveThenableJobTask {
+    // 2. Let getThenRealmResult be GetFunctionRealm(then).
+    // 3. If getThenRealmResult is a normal completion, then let thenRealm be
+    //    getThenRealmResult.[[Value]].
+    // 4. Otherwise, let thenRealm be null.
+    //
+    // The only cases where |thenRealm| can be null is when |then| is a revoked
+    // Proxy object, which would throw when it is called anyway. So instead of
+    // setting the context to null as the spec does, we just use the current
+    // realm.
+    const thenContext: Context = ExtractHandlerContext(then);
+    const nativeContext = LoadNativeContext(thenContext);
+
+    // 1. Let job be a new Job abstract closure with no parameters that
+    //    captures promiseToResolve, thenable, and then...
+    // 5. Return { [[Job]]: job, [[Realm]]: thenRealm }.
     return new PromiseResolveThenableJobTask{
       map: PromiseResolveThenableJobTaskMapConstant(),
-      context: thenContext,
+      context: nativeContext,
       promise_to_resolve: promiseToResolve,
-      then: then,
-      thenable: thenable
+      thenable,
+      then
     };
   }
 

src/builtins/promise-resolve.tq

@@ -85,10 +85,11 @@ namespace promise {
 
   const kThenString: String = ThenStringConstant();
 
+  // https://tc39.es/ecma262/#sec-promise-resolve-functions
   transitioning builtin
   ResolvePromise(implicit context:
                      Context)(promise: JSPromise, resolution: JSAny): JSAny {
-    // 6. If SameValue(resolution, promise) is true, then
+    // 7. If SameValue(resolution, promise) is true, then
     // If promise hook is enabled or the debugger is active, let
     // the runtime handle this operation, which greatly reduces
     // the complexity here and also avoids a couple of back and
@@ -104,8 +105,8 @@ namespace promise {
 
     let then: Object = Undefined;
     try {
-      // 7. If Type(resolution) is not Object, then
-      // 7.b Return FulfillPromise(promise, resolution).
+      // 8. If Type(resolution) is not Object, then
+      // 8.a Return FulfillPromise(promise, resolution).
       if (TaggedIsSmi(resolution)) {
         return FulfillPromise(promise, resolution);
       }
@@ -155,16 +156,17 @@ namespace promise {
       goto Slow;
     }
     label Slow deferred {
-      // 8. Let then be Get(resolution, "then").
-      // 9. If then is an abrupt completion, then
-      // 9.a Return RejectPromise(promise, then.[[Value]]).
+      // 9. Let then be Get(resolution, "then").
+      // 10. If then is an abrupt completion, then
       try {
         then = GetProperty(resolution, kThenString);
       } catch (e) {
+        // a. Return RejectPromise(promise, then.[[Value]]).
         return RejectPromise(promise, e, False);
       }
 
-      // 11. If IsCallable(thenAction) is false, then
+      // 11. Let thenAction be then.[[Value]].
+      // 12. If IsCallable(thenAction) is false, then
       if (!Is<Callable>(then)) {
         // a. Return FulfillPromise(promise, resolution).
         return FulfillPromise(promise, resolution);
@@ -172,20 +174,15 @@ namespace promise {
       goto Enqueue;
     }
     label Enqueue {
-      // 12. Perform EnqueueJob("PromiseJobs", PromiseResolveThenableJob,
-      //                        «promise, resolution, thenAction»).
-
-      // According to HTML, we use the context of the then function
-      // (|thenAction|) as the context of the microtask. See step 3 of HTML's
-      // EnqueueJob:
-      // https://html.spec.whatwg.org/C/#enqueuejob(queuename,-job,-arguments)
-      const thenContext: Context =
-          ExtractHandlerContext(UnsafeCast<Callable>(then));
-      const nativeContext = LoadNativeContext(thenContext);
+      // 13. Let job be NewPromiseResolveThenableJob(promise, resolution,
+      //                                             thenAction).
       const task = NewPromiseResolveThenableJobTask(
-          promise, UnsafeCast<JSReceiver>(then),
-          UnsafeCast<JSReceiver>(resolution), nativeContext);
-      return EnqueueMicrotask(nativeContext, task);
+          promise, UnsafeCast<JSReceiver>(resolution),
+          UnsafeCast<Callable>(then));
+
+      // 14. Perform HostEnqueuePromiseJob(job.[[Job]], job.[[Realm]]).
+      // 15. Return undefined.
+      return EnqueueMicrotask(task.context, task);
     }
   }
 }

src/heap/factory.cc

@@ -1272,15 +1272,15 @@ Handle<CallbackTask> Factory::NewCallbackTask(Handle<Foreign> callback,
 }
 
 Handle<PromiseResolveThenableJobTask> Factory::NewPromiseResolveThenableJobTask(
-    Handle<JSPromise> promise_to_resolve, Handle<JSReceiver> then,
-    Handle<JSReceiver> thenable, Handle<Context> context) {
+    Handle<JSPromise> promise_to_resolve, Handle<JSReceiver> thenable,
+    Handle<JSReceiver> then, Handle<Context> context) {
   DCHECK(then->IsCallable());
   Handle<PromiseResolveThenableJobTask> microtask =
       Handle<PromiseResolveThenableJobTask>::cast(
           NewStruct(PROMISE_RESOLVE_THENABLE_JOB_TASK_TYPE));
   microtask->set_promise_to_resolve(*promise_to_resolve);
-  microtask->set_then(*then);
   microtask->set_thenable(*thenable);
+  microtask->set_then(*then);
   microtask->set_context(*context);
   return microtask;
 }

src/heap/factory.h

@@ -370,8 +370,8 @@ class V8_EXPORT_PRIVATE Factory : public FactoryBase<Factory> {
   Handle<CallbackTask> NewCallbackTask(Handle<Foreign> callback,
                                        Handle<Foreign> data);
   Handle<PromiseResolveThenableJobTask> NewPromiseResolveThenableJobTask(
-      Handle<JSPromise> promise_to_resolve, Handle<JSReceiver> then,
-      Handle<JSReceiver> thenable, Handle<Context> context);
+      Handle<JSPromise> promise_to_resolve, Handle<JSReceiver> thenable,
+      Handle<JSReceiver> then, Handle<Context> context);
 
   // Foreign objects are pretenured when allocated by the bootstrapper.
   Handle<Foreign> NewForeign(Address addr);

src/objects/objects.cc

@@ -5983,6 +5983,7 @@ Handle<Object> JSPromise::Reject(Handle<JSPromise> promise,
                                  PromiseReaction::kReject);
 }
 
+// https://tc39.es/ecma262/#sec-promise-resolve-functions
 // static
 MaybeHandle<Object> JSPromise::Resolve(Handle<JSPromise> promise,
                                        Handle<Object> resolution) {
@@ -5991,7 +5992,7 @@ MaybeHandle<Object> JSPromise::Resolve(Handle<JSPromise> promise,
   isolate->RunPromiseHook(PromiseHookType::kResolve, promise,
                           isolate->factory()->undefined_value());
 
-  // 6. If SameValue(resolution, promise) is true, then
+  // 7. If SameValue(resolution, promise) is true, then
   if (promise.is_identical_to(resolution)) {
     // a. Let selfResolutionError be a newly created TypeError object.
     Handle<Object> self_resolution_error = isolate->factory()->NewTypeError(
@@ -6000,13 +6001,13 @@ MaybeHandle<Object> JSPromise::Resolve(Handle<JSPromise> promise,
     return Reject(promise, self_resolution_error);
   }
 
-  // 7. If Type(resolution) is not Object, then
+  // 8. If Type(resolution) is not Object, then
   if (!resolution->IsJSReceiver()) {
     // a. Return FulfillPromise(promise, resolution).
     return Fulfill(promise, resolution);
   }
 
-  // 8. Let then be Get(resolution, "then").
+  // 9. Let then be Get(resolution, "then").
   MaybeHandle<Object> then;
   Handle<JSReceiver> receiver(Handle<JSReceiver>::cast(resolution));
 
@@ -6029,7 +6030,7 @@ MaybeHandle<Object> JSPromise::Resolve(Handle<JSPromise> promise,
                                    isolate->factory()->then_string());
   }
 
-  // 9. If then is an abrupt completion, then
+  // 10. If then is an abrupt completion, then
   Handle<Object> then_action;
   if (!then.ToHandle(&then_action)) {
     // a. Return RejectPromise(promise, then.[[Value]]).
@@ -6038,19 +6039,15 @@ MaybeHandle<Object> JSPromise::Resolve(Handle<JSPromise> promise,
     return Reject(promise, reason, false);
   }
 
-  // 10. Let thenAction be then.[[Value]].
-  // 11. If IsCallable(thenAction) is false, then
+  // 11. Let thenAction be then.[[Value]].
+  // 12. If IsCallable(thenAction) is false, then
   if (!then_action->IsCallable()) {
     // a. Return FulfillPromise(promise, resolution).
     return Fulfill(promise, resolution);
   }
 
-  // 12. Perform EnqueueJob("PromiseJobs", PromiseResolveThenableJob,
-  //                        «promise, resolution, thenAction»).
-
-  // According to HTML, we use the context of the then function (|thenAction|)
-  // as the context of the microtask. See step 3 of HTML's EnqueueJob:
-  // https://html.spec.whatwg.org/C/#enqueuejob(queuename,-job,-arguments)
+  // 13. Let job be NewPromiseResolveThenableJob(promise, resolution,
+  //                                             thenAction).
   Handle<NativeContext> then_context;
   if (!JSReceiver::GetContextForMicrotask(Handle<JSReceiver>::cast(then_action))
            .ToHandle(&then_context)) {
@@ -6059,8 +6056,8 @@ MaybeHandle<Object> JSPromise::Resolve(Handle<JSPromise> promise,
 
   Handle<PromiseResolveThenableJobTask> task =
       isolate->factory()->NewPromiseResolveThenableJobTask(
-          promise, Handle<JSReceiver>::cast(then_action),
-          Handle<JSReceiver>::cast(resolution), then_context);
+          promise, Handle<JSReceiver>::cast(resolution),
+          Handle<JSReceiver>::cast(then_action), then_context);
   if (isolate->debug()->is_active() && resolution->IsJSPromise()) {
     // Mark the dependency of the new {promise} on the {resolution}.
     Object::SetProperty(isolate, resolution,
@@ -6071,7 +6068,7 @@ MaybeHandle<Object> JSPromise::Resolve(Handle<JSPromise> promise,
   MicrotaskQueue* microtask_queue = then_context->microtask_queue();
   if (microtask_queue) microtask_queue->EnqueueMicrotask(*task);
 
-  // 13. Return undefined.
+  // 15. Return undefined.
   return isolate->factory()->undefined_value();
 }
 

src/objects/promise.tq

@@ -72,6 +72,6 @@ extern class PromiseRejectReactionJobTask extends PromiseReactionJobTask {
 extern class PromiseResolveThenableJobTask extends Microtask {
   context: Context;
   promise_to_resolve: JSPromise;
-  then: JSReceiver;
   thenable: JSReceiver;
+  then: JSReceiver;
 }