diff --git a/src/builtins/promise-abstract-operations.tq b/src/builtins/promise-abstract-operations.tq index 8828ab84d266..d5e6ffab6e30 100644 --- a/src/builtins/promise-abstract-operations.tq +++ b/src/builtins/promise-abstract-operations.tq @@ -40,7 +40,8 @@ namespace promise { EnqueueMicrotask(Context, Microtask): Undefined; macro - ExtractHandlerContext(implicit context: Context)(handler: Callable|Undefined): + ExtractHandlerContextInternal(implicit context: Context)(handler: Callable| + Undefined): Context labels NotFound { let iter: JSAny = handler; while (true) { @@ -62,16 +63,25 @@ namespace promise { goto NotFound; } - // According to the HTML specification, we use the handler's context to - // EnqueueJob for Promise resolution. + macro + ExtractHandlerContext(implicit context: Context)(handler: Callable| + Undefined): Context { + try { + return ExtractHandlerContextInternal(handler) otherwise NotFound; + } + label NotFound deferred { + return context; + } + } + macro ExtractHandlerContext(implicit context: Context)( primary: Callable|Undefined, secondary: Callable|Undefined): Context { try { - return ExtractHandlerContext(primary) otherwise NotFound; + return ExtractHandlerContextInternal(primary) otherwise NotFound; } label NotFound deferred { - return ExtractHandlerContext(secondary) otherwise Default; + return ExtractHandlerContextInternal(secondary) otherwise Default; } label Default deferred { return context; @@ -92,6 +102,9 @@ namespace promise { secondaryHandler = promiseReaction.fulfill_handler; } + // According to HTML, we use the context of the appropriate handler as the + // context of the microtask. See step 3 of HTML's EnqueueJob: + // https://html.spec.whatwg.org/C/#enqueuejob(queuename,-job,-arguments) const handlerContext: Context = ExtractHandlerContext(primaryHandler, secondaryHandler); diff --git a/src/builtins/promise-misc.tq b/src/builtins/promise-misc.tq index 7996cc5b3d95..51aa53b7e55d 100644 --- a/src/builtins/promise-misc.tq +++ b/src/builtins/promise-misc.tq @@ -130,10 +130,10 @@ namespace promise { macro NewPromiseResolveThenableJobTask(implicit context: Context)( promiseToResolve: JSPromise, then: JSReceiver, thenable: JSReceiver, - thenableContext: Context): PromiseResolveThenableJobTask { + thenContext: Context): PromiseResolveThenableJobTask { return new PromiseResolveThenableJobTask{ map: PromiseResolveThenableJobTaskMapConstant(), - context: thenableContext, + context: thenContext, promise_to_resolve: promiseToResolve, then: then, thenable: thenable diff --git a/src/builtins/promise-resolve.tq b/src/builtins/promise-resolve.tq index af7dd7afa0ce..0fc98b556b22 100644 --- a/src/builtins/promise-resolve.tq +++ b/src/builtins/promise-resolve.tq @@ -177,7 +177,14 @@ namespace promise { label Enqueue { // 12. Perform EnqueueJob("PromiseJobs", PromiseResolveThenableJob, // «promise, resolution, thenAction»). - const nativeContext = LoadNativeContext(context); + + // 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(then)); + const nativeContext = LoadNativeContext(thenContext); const task = NewPromiseResolveThenableJobTask( promise, UnsafeCast(then), UnsafeCast(resolution), nativeContext); diff --git a/src/objects/objects.cc b/src/objects/objects.cc index 0acf8b0ab873..db8d8db59fcb 100644 --- a/src/objects/objects.cc +++ b/src/objects/objects.cc @@ -6017,10 +6017,20 @@ MaybeHandle JSPromise::Resolve(Handle promise, // 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) + Handle then_context; + if (!JSReceiver::GetContextForMicrotask(Handle::cast(then_action)) + .ToHandle(&then_context)) { + then_context = isolate->native_context(); + } + Handle task = isolate->factory()->NewPromiseResolveThenableJobTask( promise, Handle::cast(then_action), - Handle::cast(resolution), isolate->native_context()); + Handle::cast(resolution), then_context); if (isolate->debug()->is_active() && resolution->IsJSPromise()) { // Mark the dependency of the new {promise} on the {resolution}. Object::SetProperty(isolate, resolution, @@ -6028,8 +6038,7 @@ MaybeHandle JSPromise::Resolve(Handle promise, promise) .Check(); } - MicrotaskQueue* microtask_queue = - isolate->native_context()->microtask_queue(); + MicrotaskQueue* microtask_queue = then_context->microtask_queue(); if (microtask_queue) microtask_queue->EnqueueMicrotask(*task); // 13. Return undefined. @@ -6065,6 +6074,9 @@ Handle JSPromise::TriggerPromiseReactions(Isolate* isolate, Handle reaction = Handle::cast(task); reactions = handle(reaction->next(), isolate); + // According to HTML, we use the context of the appropriate handler as the + // context of the microtask. See step 3 of HTML's EnqueueJob: + // https://html.spec.whatwg.org/C/#enqueuejob(queuename,-job,-arguments) Handle handler_context; Handle primary_handler; diff --git a/test/unittests/execution/microtask-queue-unittest.cc b/test/unittests/execution/microtask-queue-unittest.cc index 304891778186..01e26f1a630a 100644 --- a/test/unittests/execution/microtask-queue-unittest.cc +++ b/test/unittests/execution/microtask-queue-unittest.cc @@ -68,7 +68,15 @@ using TestWithNativeContextAndFinalizationRegistry = // WithSharedIsolateMixin< // ::testing::Test>>>>>; -class MicrotaskQueueTest : public TestWithNativeContextAndFinalizationRegistry { +namespace { + +void DummyPromiseHook(PromiseHookType type, Local promise, + Local parent) {} + +} // namespace + +class MicrotaskQueueTest : public TestWithNativeContextAndFinalizationRegistry, + public ::testing::WithParamInterface { public: template Handle NewMicrotask(F&& f) { @@ -82,6 +90,12 @@ class MicrotaskQueueTest : public TestWithNativeContextAndFinalizationRegistry { void SetUp() override { microtask_queue_ = MicrotaskQueue::New(isolate()); native_context()->set_microtask_queue(microtask_queue()); + + if (GetParam()) { + // Use a PromiseHook to switch the implementation to ResolvePromise + // runtime, instead of ResolvePromise builtin. + v8_isolate()->SetPromiseHook(&DummyPromiseHook); + } } void TearDown() override { @@ -126,7 +140,7 @@ class RecordingVisitor : public RootVisitor { }; // Sanity check. Ensure a microtask is stored in a queue and run. -TEST_F(MicrotaskQueueTest, EnqueueAndRun) { +TEST_P(MicrotaskQueueTest, EnqueueAndRun) { bool ran = false; EXPECT_EQ(0, microtask_queue()->capacity()); EXPECT_EQ(0, microtask_queue()->size()); @@ -142,7 +156,7 @@ TEST_F(MicrotaskQueueTest, EnqueueAndRun) { } // Check for a buffer growth. -TEST_F(MicrotaskQueueTest, BufferGrowth) { +TEST_P(MicrotaskQueueTest, BufferGrowth) { int count = 0; // Enqueue and flush the queue first to have non-zero |start_|. @@ -176,7 +190,7 @@ TEST_F(MicrotaskQueueTest, BufferGrowth) { } // MicrotaskQueue instances form a doubly linked list. -TEST_F(MicrotaskQueueTest, InstanceChain) { +TEST_P(MicrotaskQueueTest, InstanceChain) { ClearTestMicrotaskQueue(); MicrotaskQueue* default_mtq = isolate()->default_microtask_queue(); @@ -207,7 +221,7 @@ TEST_F(MicrotaskQueueTest, InstanceChain) { // Pending Microtasks in MicrotaskQueues are strong roots. Ensure they are // visited exactly once. -TEST_F(MicrotaskQueueTest, VisitRoot) { +TEST_P(MicrotaskQueueTest, VisitRoot) { // Ensure that the ring buffer has separate in-use region. for (int i = 0; i < MicrotaskQueue::kMinimumCapacity / 2 + 1; ++i) { microtask_queue()->EnqueueMicrotask(*NewMicrotask([] {})); @@ -233,7 +247,7 @@ TEST_F(MicrotaskQueueTest, VisitRoot) { EXPECT_EQ(expected, actual); } -TEST_F(MicrotaskQueueTest, PromiseHandlerContext) { +TEST_P(MicrotaskQueueTest, PromiseHandlerContext) { Local v8_context2 = v8::Context::New(v8_isolate()); Local v8_context3 = v8::Context::New(v8_isolate()); Local v8_context4 = v8::Context::New(v8_isolate()); @@ -327,7 +341,7 @@ TEST_F(MicrotaskQueueTest, PromiseHandlerContext) { v8_context2->DetachGlobal(); } -TEST_F(MicrotaskQueueTest, DetachGlobal_Enqueue) { +TEST_P(MicrotaskQueueTest, DetachGlobal_Enqueue) { EXPECT_EQ(0, microtask_queue()->size()); // Detach MicrotaskQueue from the current context. @@ -339,7 +353,7 @@ TEST_F(MicrotaskQueueTest, DetachGlobal_Enqueue) { EXPECT_EQ(0, microtask_queue()->size()); } -TEST_F(MicrotaskQueueTest, DetachGlobal_Run) { +TEST_P(MicrotaskQueueTest, DetachGlobal_Run) { EXPECT_EQ(0, microtask_queue()->size()); // Enqueue microtasks to the current context. @@ -377,18 +391,7 @@ TEST_F(MicrotaskQueueTest, DetachGlobal_Run) { } } -namespace { - -void DummyPromiseHook(PromiseHookType type, Local promise, - Local parent) {} - -} // namespace - -TEST_F(MicrotaskQueueTest, DetachGlobal_PromiseResolveThenableJobTask) { - // Use a PromiseHook to switch the implementation to ResolvePromise runtime, - // instead of ResolvePromise builtin. - v8_isolate()->SetPromiseHook(&DummyPromiseHook); - +TEST_P(MicrotaskQueueTest, DetachGlobal_PromiseResolveThenableJobTask) { RunJS( "var resolve;" "var promise = new Promise(r => { resolve = r; });" @@ -410,7 +413,50 @@ TEST_F(MicrotaskQueueTest, DetachGlobal_PromiseResolveThenableJobTask) { EXPECT_EQ(0, microtask_queue()->size()); } -TEST_F(MicrotaskQueueTest, DetachGlobal_HandlerContext) { +TEST_P(MicrotaskQueueTest, DetachGlobal_ResolveThenableForeignThen) { + Handle result = RunJS( + "let result = [false];" + "result"); + Handle then = RunJS("() => { result[0] = true; }"); + + Handle stale_promise; + + Local sub_context = v8::Context::New(v8_isolate()); + std::unique_ptr sub_microtask_queue = + MicrotaskQueue::New(isolate()); + Utils::OpenHandle(*sub_context) + ->native_context() + .set_microtask_queue(microtask_queue()); + { + v8::Context::Scope scope(sub_context); + CHECK(sub_context->Global() + ->Set(sub_context, NewString("then"), + Utils::ToLocal(Handle::cast(then))) + .FromJust()); + + EXPECT_EQ(0, microtask_queue()->size()); + EXPECT_EQ(0, sub_microtask_queue->size()); + EXPECT_TRUE( + Object::GetElement(isolate(), result, 0).ToHandleChecked()->IsFalse()); + + stale_promise = RunJS("Promise.resolve({ then })"); + + EXPECT_EQ(1, microtask_queue()->size()); + EXPECT_EQ(0, sub_microtask_queue->size()); + EXPECT_TRUE( + Object::GetElement(isolate(), result, 0).ToHandleChecked()->IsFalse()); + } + sub_context->DetachGlobal(); + sub_context.Clear(); + sub_microtask_queue.reset(); + + EXPECT_EQ(1, microtask_queue()->RunMicrotasks(isolate())); + EXPECT_EQ(0, microtask_queue()->size()); + EXPECT_TRUE( + Object::GetElement(isolate(), result, 0).ToHandleChecked()->IsTrue()); +} + +TEST_P(MicrotaskQueueTest, DetachGlobal_HandlerContext) { // EnqueueMicrotask should use the context associated to the handler instead // of the current context. E.g. // // At Context A. @@ -489,7 +535,7 @@ TEST_F(MicrotaskQueueTest, DetachGlobal_HandlerContext) { .FromJust()); } -TEST_F(MicrotaskQueueTest, DetachGlobal_Chain) { +TEST_P(MicrotaskQueueTest, DetachGlobal_Chain) { Handle stale_rejected_promise; Local sub_context = v8::Context::New(v8_isolate()); @@ -516,7 +562,7 @@ TEST_F(MicrotaskQueueTest, DetachGlobal_Chain) { Object::GetElement(isolate(), result, 0).ToHandleChecked()->IsTrue()); } -TEST_F(MicrotaskQueueTest, DetachGlobal_InactiveHandler) { +TEST_P(MicrotaskQueueTest, DetachGlobal_InactiveHandler) { Local sub_context = v8::Context::New(v8_isolate()); Utils::OpenHandle(*sub_context) ->native_context() @@ -558,7 +604,7 @@ TEST_F(MicrotaskQueueTest, DetachGlobal_InactiveHandler) { Object::GetElement(isolate(), result, 1).ToHandleChecked()->IsFalse()); } -TEST_F(MicrotaskQueueTest, MicrotasksScope) { +TEST_P(MicrotaskQueueTest, MicrotasksScope) { ASSERT_NE(isolate()->default_microtask_queue(), microtask_queue()); microtask_queue()->set_microtasks_policy(MicrotasksPolicy::kScoped); @@ -574,5 +620,11 @@ TEST_F(MicrotaskQueueTest, MicrotasksScope) { EXPECT_TRUE(ran); } +INSTANTIATE_TEST_SUITE_P( + , MicrotaskQueueTest, ::testing::Values(false, true), + [](const ::testing::TestParamInfo& info) { + return info.param ? "runtime" : "builtin"; + }); + } // namespace internal } // namespace v8