diff --git a/js/src/builtin/Promise.cpp b/js/src/builtin/Promise.cpp index a6f7a29d10..133f18db86 100644 --- a/js/src/builtin/Promise.cpp +++ b/js/src/builtin/Promise.cpp @@ -540,14 +540,14 @@ const Class PromiseReactionRecord::class_ = { static void AddPromiseFlags(PromiseObject& promise, int32_t flag) { - int32_t flags = promise.getFixedSlot(PromiseSlot_Flags).toInt32(); + int32_t flags = promise.flags(); promise.setFixedSlot(PromiseSlot_Flags, Int32Value(flags | flag)); } static bool PromiseHasAnyFlag(PromiseObject& promise, int32_t flag) { - return promise.getFixedSlot(PromiseSlot_Flags).toInt32() & flag; + return promise.flags() & flag; } static bool ResolvePromiseFunction(JSContext* cx, unsigned argc, Value* vp); @@ -922,7 +922,7 @@ ResolvePromise(JSContext* cx, Handle promise, HandleValue valueO // instead of getting the right list of reactions, we determine the // resolution type to retrieve the right information from the // reaction records. - RootedValue reactionsVal(cx, promise->getFixedSlot(PromiseSlot_ReactionsOrResult)); + RootedValue reactionsVal(cx, promise->reactions()); // Steps 3-5. // The same slot is used for the reactions list and the result, so setting @@ -930,7 +930,7 @@ ResolvePromise(JSContext* cx, Handle promise, HandleValue valueO promise->setFixedSlot(PromiseSlot_ReactionsOrResult, valueOrReason); // Step 6. - int32_t flags = promise->getFixedSlot(PromiseSlot_Flags).toInt32(); + int32_t flags = promise->flags(); flags |= PROMISE_FLAG_RESOLVED; if (state == JS::PromiseState::Fulfilled) flags |= PROMISE_FLAG_FULFILLED; @@ -994,8 +994,7 @@ CreatePromiseObjectWithoutResolutionFunctions(JSContext* cx) if (!promise) return nullptr; - AddPromiseFlags(*promise, PROMISE_FLAG_DEFAULT_RESOLVE_FUNCTION | - PROMISE_FLAG_DEFAULT_REJECT_FUNCTION); + AddPromiseFlags(*promise, PROMISE_FLAG_DEFAULT_RESOLVING_FUNCTIONS); return promise; } @@ -1643,9 +1642,9 @@ static MOZ_MUST_USE bool AddPromiseReaction(JSContext* cx, Handle promise, Handle reaction); -static MOZ_MUST_USE bool BlockOnPromise(JSContext* cx, HandleValue promise, - HandleObject blockedPromise, - HandleValue onFulfilled, HandleValue onRejected); +static MOZ_MUST_USE bool +BlockOnPromise(JSContext* cx, HandleValue promise, HandleObject blockedPromise, + HandleValue onFulfilled, HandleValue onRejected, bool onFulfilledReturnsUndefined); static JSFunction* GetResolveFunctionFromReject(JSFunction* reject) @@ -2210,14 +2209,12 @@ RunResolutionFunction(JSContext *cx, HandleObject resolutionFun, HandleValue res if (promise->state() != JS::PromiseState::Pending) return true; - if (mode == ResolveMode) { - if (!PromiseHasAnyFlag(*promise, PROMISE_FLAG_DEFAULT_RESOLVE_FUNCTION)) - return true; - return ResolvePromiseInternal(cx, promise, result); - } - - if (!PromiseHasAnyFlag(*promise, PROMISE_FLAG_DEFAULT_REJECT_FUNCTION)) + if (!PromiseHasAnyFlag(*promise, PROMISE_FLAG_DEFAULT_RESOLVING_FUNCTIONS)) return true; + + if (mode == ResolveMode) + return ResolvePromiseInternal(cx, promise, result); + return RejectMaybeWrappedPromise(cx, promiseObj, result); } @@ -2364,7 +2361,7 @@ PerformPromiseAll(JSContext *cx, JS::ForOfIterator& iterator, HandleObject C, // Step q. RootedValue resolveFunVal(cx, ObjectValue(*resolveFunc)); - if (!BlockOnPromise(cx, nextPromise, promiseObj, resolveFunVal, rejectFunVal)) + if (!BlockOnPromise(cx, nextPromise, promiseObj, resolveFunVal, rejectFunVal, true)) return false; // Step r. @@ -2479,7 +2476,7 @@ PerformPromiseAllSettled(JSContext *cx, JS::ForOfIterator& iterator, HandleObjec RootedValue resolveFunVal(cx, ObjectValue(*resolveFunc)); RootedValue rejectFunVal(cx, ObjectValue(*rejectFunc)); - if (!BlockOnPromise(cx, nextPromise, promiseObj, resolveFunVal, rejectFunVal)) + if (!BlockOnPromise(cx, nextPromise, promiseObj, resolveFunVal, rejectFunVal, true)) return false; index++; @@ -2785,6 +2782,11 @@ PerformPromiseRace(JSContext *cx, JS::ForOfIterator& iterator, HandleObject C, MOZ_ASSERT(C->isConstructor()); RootedValue CVal(cx, ObjectValue(*C)); + // BlockOnPromise fast path requires the passed onFulfilled function + // doesn't return an object value, because otherwise the skipped promise + // creation is detectable due to missing property lookups. + bool isDefaultResolveFn = IsNativeFunction(resolve, ResolvePromiseFunction); + RootedValue nextValue(cx); RootedValue resolveFunVal(cx, ObjectValue(*resolve)); RootedValue rejectFunVal(cx, ObjectValue(*reject)); @@ -2821,8 +2823,11 @@ PerformPromiseRace(JSContext *cx, JS::ForOfIterator& iterator, HandleObject C, return false; // Step i. - if (!BlockOnPromise(cx, nextPromise, promiseObj, resolveFunVal, rejectFunVal)) + if (!BlockOnPromise(cx, nextPromise, promiseObj, resolveFunVal, rejectFunVal, + isDefaultResolveFn)) + { return false; + } } MOZ_ASSERT_UNREACHABLE("Shouldn't reach the end of PerformPromiseRace"); @@ -3826,7 +3831,7 @@ PerformPromiseThenWithReaction(JSContext* cx, Handle promise, Handle reaction) { JS::PromiseState state = promise->state(); - int32_t flags = promise->getFixedSlot(PromiseSlot_Flags).toInt32(); + int32_t flags = promise->flags(); if (state == JS::PromiseState::Pending) { // Steps 5,6 (reordered). // Instead of creating separate reaction records for fulfillment and @@ -3842,7 +3847,7 @@ PerformPromiseThenWithReaction(JSContext* cx, Handle promise, MOZ_ASSERT_IF(state != JS::PromiseState::Fulfilled, state == JS::PromiseState::Rejected); // Step 8.a. / 9.b. - RootedValue valueOrReason(cx, promise->getFixedSlot(PromiseSlot_ReactionsOrResult)); + RootedValue valueOrReason(cx, promise->valueOrReason()); // We might be operating on a promise from another compartment. In // that case, we need to wrap the result/reason value before using it. @@ -3876,7 +3881,7 @@ PerformPromiseThenWithReaction(JSContext* cx, Handle promise, */ static MOZ_MUST_USE bool BlockOnPromise(JSContext* cx, HandleValue promiseVal, HandleObject blockedPromise_, - HandleValue onFulfilled, HandleValue onRejected) + HandleValue onFulfilled, HandleValue onRejected, bool onFulfilledReturnsUndefined) { RootedObject promiseObj(cx, ToObject(cx, promiseVal)); if (!promiseObj) @@ -3898,7 +3903,7 @@ BlockOnPromise(JSContext* cx, HandleValue promiseVal, HandleObject blockedPromis if (!C) return false; - RootedObject resultPromise(cx, blockedPromise_); + RootedObject resultPromise(cx); RootedObject resolveFun(cx); RootedObject rejectFun(cx); @@ -3906,7 +3911,21 @@ BlockOnPromise(JSContext* cx, HandleValue promiseVal, HandleObject blockedPromis // rejected promises list. bool addToDependent = true; - if (C == PromiseCtor && resultPromise->is()) { + // Skip the creation of a built-in Promise object if: + // 1. `C` is the built-in Promise constructor. + // 2. The `onFulfilled` handler doesn't return an object, which + // ensures no side-effects take place in ResolvePromiseInternal. + // 3. The blocked promise is a built-in Promise object. + // 4. The blocked promise doesn't use the default resolving functions, + // which in turn means RunResolutionFunction when called from + // PromiseRectionJob won't try to resolve the promise. + if (C == PromiseCtor && + onFulfilledReturnsUndefined && + blockedPromise_->is() && + !PromiseHasAnyFlag(blockedPromise_->as(), + PROMISE_FLAG_DEFAULT_RESOLVING_FUNCTIONS)) + { + resultPromise.set(blockedPromise_); addToDependent = false; } else { // 25.4.5.3., step 4. @@ -3997,7 +4016,7 @@ AddPromiseReaction(JSContext* cx, Handle promise, } // 25.4.5.3.1 steps 7.a,b. - RootedValue reactionsVal(cx, promise->getFixedSlot(PromiseSlot_ReactionsOrResult)); + RootedValue reactionsVal(cx, promise->reactions()); RootedNativeObject reactions(cx); if (reactionsVal.isUndefined()) { @@ -4093,7 +4112,7 @@ PromiseObject::dependentPromises(JSContext* cx, MutableHandle> v if (state() != JS::PromiseState::Pending) return true; - RootedValue reactionsVal(cx, getFixedSlot(PromiseSlot_ReactionsOrResult)); + RootedValue reactionsVal(cx, reactions()); // If no reactions are pending, we don't have list and are done. if (reactionsVal.isNullOrUndefined()) @@ -4143,7 +4162,7 @@ PromiseObject::resolve(JSContext* cx, Handle promise, HandleValu if (promise->state() != JS::PromiseState::Pending) return true; - if (PromiseHasAnyFlag(*promise, PROMISE_FLAG_DEFAULT_RESOLVE_FUNCTION)) + if (PromiseHasAnyFlag(*promise, PROMISE_FLAG_DEFAULT_RESOLVING_FUNCTIONS)) return ResolvePromiseInternal(cx, promise, resolutionValue); RootedObject resolveFun(cx, GetResolveFunctionFromPromise(promise)); @@ -4172,7 +4191,7 @@ PromiseObject::reject(JSContext* cx, Handle promise, HandleValue if (promise->state() != JS::PromiseState::Pending) return true; - if (PromiseHasAnyFlag(*promise, PROMISE_FLAG_DEFAULT_REJECT_FUNCTION)) + if (PromiseHasAnyFlag(*promise, PROMISE_FLAG_DEFAULT_RESOLVING_FUNCTIONS)) return RejectMaybeWrappedPromise(cx, promise, rejectionValue); RootedValue funVal(cx, promise->getFixedSlot(PromiseSlot_RejectFunction)); diff --git a/js/src/builtin/Promise.h b/js/src/builtin/Promise.h index 811e8efb94..1fe9b060ad 100644 --- a/js/src/builtin/Promise.h +++ b/js/src/builtin/Promise.h @@ -24,9 +24,8 @@ enum PromiseSlots { #define PROMISE_FLAG_FULFILLED 0x2 #define PROMISE_FLAG_HANDLED 0x4 #define PROMISE_FLAG_REPORTED 0x8 -#define PROMISE_FLAG_DEFAULT_RESOLVE_FUNCTION 0x10 -#define PROMISE_FLAG_DEFAULT_REJECT_FUNCTION 0x20 -#define PROMISE_FLAG_ASYNC 0x40 +#define PROMISE_FLAG_DEFAULT_RESOLVING_FUNCTIONS 0x10 +#define PROMISE_FLAG_ASYNC 0x20 class AutoSetNewObjectMetadata; @@ -44,8 +43,11 @@ class PromiseObject : public NativeObject static JSObject* unforgeableResolve(JSContext* cx, HandleValue value); static JSObject* unforgeableReject(JSContext* cx, HandleValue value); + int32_t flags() { + return getFixedSlot(PromiseSlot_Flags).toInt32(); + } JS::PromiseState state() { - int32_t flags = getFixedSlot(PromiseSlot_Flags).toInt32(); + int32_t flags = this->flags(); if (!(flags & PROMISE_FLAG_RESOLVED)) { MOZ_ASSERT(!(flags & PROMISE_FLAG_FULFILLED)); return JS::PromiseState::Pending; @@ -54,6 +56,10 @@ class PromiseObject : public NativeObject return JS::PromiseState::Fulfilled; return JS::PromiseState::Rejected; } + Value reactions() { + MOZ_ASSERT(state() == JS::PromiseState::Pending); + return getFixedSlot(PromiseSlot_ReactionsOrResult); + } Value value() { MOZ_ASSERT(state() == JS::PromiseState::Fulfilled); return getFixedSlot(PromiseSlot_ReactionsOrResult); @@ -62,6 +68,10 @@ class PromiseObject : public NativeObject MOZ_ASSERT(state() == JS::PromiseState::Rejected); return getFixedSlot(PromiseSlot_ReactionsOrResult); } + Value valueOrReason() { + MOZ_ASSERT(state() != JS::PromiseState::Pending); + return getFixedSlot(PromiseSlot_ReactionsOrResult); + } static MOZ_MUST_USE bool resolve(JSContext* cx, Handle promise, HandleValue resolutionValue); @@ -83,7 +93,7 @@ class PromiseObject : public NativeObject uint64_t getID(); bool isUnhandled() { MOZ_ASSERT(state() == JS::PromiseState::Rejected); - return !(getFixedSlot(PromiseSlot_Flags).toInt32() & PROMISE_FLAG_HANDLED); + return !(flags() & PROMISE_FLAG_HANDLED); } void markAsReported() { MOZ_ASSERT(isUnhandled()); diff --git a/js/src/builtin/TestingFunctions.cpp b/js/src/builtin/TestingFunctions.cpp index 914d3b989b..95c724bf90 100644 --- a/js/src/builtin/TestingFunctions.cpp +++ b/js/src/builtin/TestingFunctions.cpp @@ -1334,8 +1334,8 @@ SettlePromiseNow(JSContext* cx, unsigned argc, Value* vp) return false; } - RootedNativeObject promise(cx, &args[0].toObject().as()); - int32_t flags = promise->getFixedSlot(PromiseSlot_Flags).toInt32(); + Rooted promise(cx, &args[0].toObject().as()); + int32_t flags = promise->flags(); promise->setFixedSlot(PromiseSlot_Flags, Int32Value(flags | PROMISE_FLAG_RESOLVED | PROMISE_FLAG_FULFILLED)); promise->setFixedSlot(PromiseSlot_ReactionsOrResult, UndefinedValue()); diff --git a/js/src/jit-test/tests/promise/promise-race-with-default-resolving-internal.js b/js/src/jit-test/tests/promise/promise-race-with-default-resolving-internal.js new file mode 100644 index 0000000000..815124c46f --- /dev/null +++ b/js/src/jit-test/tests/promise/promise-race-with-default-resolving-internal.js @@ -0,0 +1,54 @@ +function newPromiseCapability() { + let resolve, reject, promise = new Promise(function(r1, r2) { + resolve = r1; + reject = r2; + }); + return {promise, resolve, reject}; +} + +function neverCalled() { + // Quit with non-zero exit code to ensure a test suite error is shown, + // even when this function is called within promise handlers which normally + // swallow any exceptions. + quit(1); +} + +var c = 0; +var g_resolve; + +class P extends Promise { + constructor(executor) { + // Only the very first object created through this constructor gets + // special treatment, all other invocations create built-in Promise + // objects. + if (c++ > 1) { + return new Promise(executor); + } + + // Pass a native ResolvePromiseFunction function as the resolve handler. + // (It's okay that the promise of this promise capability is never used.) + executor(newPromiseCapability().resolve, neverCalled); + + let {promise, resolve} = newPromiseCapability(); + g_resolve = resolve; + + // Use an async function to create a Promise without resolving functions. + return async function(){ await promise; return 456; }(); + } + + // Ensure we don't take the (spec) fast path in Promise.resolve and instead + // create a new promise object. (We could not provide an override at all + // and rely on the default behaviour, but giving an explicit definition + // may help to interpret this test case.) + static resolve(v) { + return super.resolve(v); + } +} + +let {promise: alwaysPending} = newPromiseCapability(); + +P.race([alwaysPending]).then(neverCalled, neverCalled); + +g_resolve(123); + +drainJobQueue(); diff --git a/js/src/jit-test/tests/promise/promise-race-with-non-default-resolving.js b/js/src/jit-test/tests/promise/promise-race-with-non-default-resolving.js new file mode 100644 index 0000000000..23d4b95bc3 --- /dev/null +++ b/js/src/jit-test/tests/promise/promise-race-with-non-default-resolving.js @@ -0,0 +1,56 @@ +function newPromiseCapability() { + var resolve, reject, promise = new Promise(function(r1, r2) { + resolve = r1; + reject = r2; + }); + return {promise, resolve, reject}; +} + +function neverCalled() { + // Quit with non-zero exit code to ensure a test suite error is shown, + // even when this function is called within promise handlers which normally + // swallow any exceptions. + quit(1); +} + +var {promise, resolve} = newPromiseCapability(); + +var getterCount = 0; + +class P extends Promise { + constructor(executor) { + var {promise, resolve, reject} = newPromiseCapability(); + + executor(function(v) { + // Resolve the promise. + resolve(v); + + // But then return an object from the resolve function. This object + // must be treated as the resolution value for the otherwise + // skipped promise which gets created when Promise.prototype.then is + // called in PerformPromiseRace. + return { + get then() { + getterCount++; + } + }; + }, neverCalled); + + return promise; + } + + // Default to the standard Promise.resolve function, so we don't create + // another instance of this class when resolving the passed promise objects + // in Promise.race. + static resolve(v) { + return Promise.resolve(v); + } +} + +P.race([promise]); + +resolve(0); + +drainJobQueue(); + +assertEq(getterCount, 1);