From 7429891baf4400df3396396e8c60ecdeef3152ee Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Wed, 25 Nov 2015 15:48:10 -0500 Subject: [PATCH] Bug 1170760 part 13. Add subclassing support to Promise::Then/Catch. r=baku,efaust --- dom/base/DOMRequest.cpp | 14 +- dom/base/DOMRequest.h | 6 +- dom/bindings/Codegen.py | 12 + dom/promise/Promise.cpp | 237 ++++++++++++++++-- dom/promise/Promise.h | 18 +- dom/promise/PromiseCallback.cpp | 184 +++++++++++++- dom/promise/PromiseCallback.h | 50 +++- dom/promise/tests/test_promise_xrays.html | 49 ++++ dom/webidl/DOMRequest.webidl | 7 +- dom/webidl/Promise.webidl | 13 +- .../js/builtins/Promise-subclassing.html | 65 ++++- 11 files changed, 595 insertions(+), 60 deletions(-) diff --git a/dom/base/DOMRequest.cpp b/dom/base/DOMRequest.cpp index a51cdce769f..dfc4732a53c 100644 --- a/dom/base/DOMRequest.cpp +++ b/dom/base/DOMRequest.cpp @@ -13,6 +13,7 @@ #include "mozilla/dom/Event.h" #include "mozilla/dom/Promise.h" #include "mozilla/dom/ScriptSettings.h" +#include "jsfriendapi.h" using mozilla::dom::AnyCallback; using mozilla::dom::DOMError; @@ -206,14 +207,16 @@ DOMRequest::RootResultVal() mozilla::HoldJSObjects(this); } -already_AddRefed +void DOMRequest::Then(JSContext* aCx, AnyCallback* aResolveCallback, - AnyCallback* aRejectCallback, mozilla::ErrorResult& aRv) + AnyCallback* aRejectCallback, + JS::MutableHandle aRetval, + mozilla::ErrorResult& aRv) { if (!mPromise) { mPromise = Promise::Create(DOMEventTargetHelper::GetParentObject(), aRv); if (aRv.Failed()) { - return nullptr; + return; } if (mDone) { // Since we create mPromise lazily, it's possible that the DOMRequest object @@ -228,7 +231,10 @@ DOMRequest::Then(JSContext* aCx, AnyCallback* aResolveCallback, } } - return mPromise->Then(aCx, aResolveCallback, aRejectCallback, aRv); + // Just use the global of the Promise itself as the callee global. + JS::Rooted global(aCx, mPromise->GetWrapper()); + global = js::GetGlobalForObjectCrossCompartment(global); + mPromise->Then(aCx, global, aResolveCallback, aRejectCallback, aRetval, aRv); } NS_IMPL_ISUPPORTS(DOMRequestService, nsIDOMRequestService) diff --git a/dom/base/DOMRequest.h b/dom/base/DOMRequest.h index 9c32ff172dc..dfe3c194b32 100644 --- a/dom/base/DOMRequest.h +++ b/dom/base/DOMRequest.h @@ -74,9 +74,11 @@ public: IMPL_EVENT_HANDLER(success) IMPL_EVENT_HANDLER(error) - already_AddRefed + void Then(JSContext* aCx, AnyCallback* aResolveCallback, - AnyCallback* aRejectCallback, mozilla::ErrorResult& aRv); + AnyCallback* aRejectCallback, + JS::MutableHandle aRetval, + mozilla::ErrorResult& aRv); void FireSuccess(JS::Handle aResult); void FireError(const nsAString& aError); diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py index 28852ab8884..a147bb776e4 100644 --- a/dom/bindings/Codegen.py +++ b/dom/bindings/Codegen.py @@ -7147,6 +7147,18 @@ class CGPerSignatureCall(CGThing): if needsCx: argsPre.append("cx") + # Hack for making Promise.prototype.then work well over Xrays. + if (not static and + (descriptor.name == "Promise" or + descriptor.name == "MozAbortablePromise") and + idlNode.isMethod() and + idlNode.identifier.name == "then"): + cgThings.append(CGGeneric(dedent( + """ + JS::Rooted calleeGlobal(cx, xpc::XrayAwareCalleeGlobal(&args.callee())); + """))) + argsPre.append("calleeGlobal") + needsUnwrap = False argsPost = [] if isConstructor: diff --git a/dom/promise/Promise.cpp b/dom/promise/Promise.cpp index d2b3313ac36..e94351012b4 100644 --- a/dom/promise/Promise.cpp +++ b/dom/promise/Promise.cpp @@ -1115,35 +1115,236 @@ Promise::Reject(nsIGlobalObject* aGlobal, JSContext* aCx, return promise.forget(); } -already_AddRefed -Promise::Then(JSContext* aCx, AnyCallback* aResolveCallback, - AnyCallback* aRejectCallback, ErrorResult& aRv) +namespace { +void +SpeciesConstructor(JSContext* aCx, + JS::Handle promise, + JS::Handle defaultCtor, + JS::MutableHandle ctor, + ErrorResult& aRv) { - RefPtr promise = Create(GetParentObject(), aRv); - if (aRv.Failed()) { - return nullptr; + // Implements + // http://www.ecma-international.org/ecma-262/6.0/#sec-speciesconstructor + + // Step 1. + MOZ_ASSERT(promise); + + // Step 2. + JS::Rooted constructorVal(aCx); + if (!JS_GetProperty(aCx, promise, "constructor", &constructorVal)) { + // Step 3. + aRv.NoteJSContextException(); + return; } + // Step 4. + if (constructorVal.isUndefined()) { + ctor.set(defaultCtor); + return; + } + + // Step 5. + if (!constructorVal.isObject()) { + aRv.ThrowTypeError(); + return; + } + + // Step 6. + JS::Rooted species(aCx, + SYMBOL_TO_JSID(JS::GetWellKnownSymbol(aCx, JS::SymbolCode::species))); + JS::Rooted speciesVal(aCx); + JS::Rooted constructorObj(aCx, &constructorVal.toObject()); + if (!JS_GetPropertyById(aCx, constructorObj, species, &speciesVal)) { + // Step 7. + aRv.NoteJSContextException(); + return; + } + + // Step 8. + if (speciesVal.isNullOrUndefined()) { + ctor.set(defaultCtor); + return; + } + + // Step 9. + if (speciesVal.isObject() && JS::IsConstructor(&speciesVal.toObject())) { + ctor.set(speciesVal); + return; + } + + // Step 10. + aRv.ThrowTypeError(); +} +} // anonymous namespace + +void +Promise::Then(JSContext* aCx, JS::Handle aCalleeGlobal, + AnyCallback* aResolveCallback, AnyCallback* aRejectCallback, + JS::MutableHandle aRetval, ErrorResult& aRv) +{ + // Implements + // http://www.ecma-international.org/ecma-262/6.0/#sec-promise.prototype.then + + // Step 1. + JS::Rooted promiseVal(aCx, JS::ObjectValue(*GetWrapper())); + if (!MaybeWrapObjectValue(aCx, &promiseVal)) { + aRv.NoteJSContextException(); + return; + } + JS::Rooted promiseObj(aCx, &promiseVal.toObject()); + MOZ_ASSERT(promiseObj); + + // Step 2 was done by the bindings. + + // Step 3. We want to use aCalleeGlobal here because it will do the + // right thing for us via Xrays (where we won't find @@species on + // our promise constructor for now). + JS::Rooted calleeGlobal(aCx, aCalleeGlobal); + JS::Rooted defaultCtorVal(aCx); + { // Scope for JSAutoCompartment + JSAutoCompartment ac(aCx, aCalleeGlobal); + JSObject* defaultCtor = + PromiseBinding::GetConstructorObject(aCx, calleeGlobal); + if (!defaultCtor) { + aRv.NoteJSContextException(); + return; + } + defaultCtorVal.setObject(*defaultCtor); + } + if (!MaybeWrapObjectValue(aCx, &defaultCtorVal)) { + aRv.NoteJSContextException(); + return; + } + + JS::Rooted constructor(aCx); + SpeciesConstructor(aCx, promiseObj, defaultCtorVal, &constructor, aRv); + if (aRv.Failed()) { + // Step 4. + return; + } + + // Step 5. + GlobalObject globalObj(aCx, GetWrapper()); + if (globalObj.Failed()) { + aRv.NoteJSContextException(); + return; + } + nsCOMPtr globalObject = + do_QueryInterface(globalObj.GetAsSupports()); + if (!globalObject) { + aRv.Throw(NS_ERROR_UNEXPECTED); + return; + } + PromiseCapability capability(aCx); + NewPromiseCapability(aCx, globalObject, constructor, false, capability, aRv); + if (aRv.Failed()) { + // Step 6. + return; + } + + // Now step 7: start + // http://www.ecma-international.org/ecma-262/6.0/#sec-performpromisethen + + // Step 1 and step 2 are just assertions. + + // Step 3 and step 4 are kinda handled for us already; we use null + // to represent "Identity" and "Thrower". + + // Steps 5 and 6. These branch based on whether we know we have a + // vanilla Promise or not. JS::Rooted global(aCx, JS::CurrentGlobalOrNull(aCx)); + if (capability.mNativePromise) { + Promise* promise = capability.mNativePromise; - RefPtr resolveCb = - PromiseCallback::Factory(promise, global, aResolveCallback, - PromiseCallback::Resolve); + RefPtr resolveCb = + PromiseCallback::Factory(promise, global, aResolveCallback, + PromiseCallback::Resolve); - RefPtr rejectCb = - PromiseCallback::Factory(promise, global, aRejectCallback, - PromiseCallback::Reject); + RefPtr rejectCb = + PromiseCallback::Factory(promise, global, aRejectCallback, + PromiseCallback::Reject); - AppendCallbacks(resolveCb, rejectCb); + AppendCallbacks(resolveCb, rejectCb); + } else { + JS::Rooted resolveObj(aCx, &capability.mResolve.toObject()); + RefPtr resolveFunc = + new AnyCallback(aCx, resolveObj, GetIncumbentGlobal()); - return promise.forget(); + JS::Rooted rejectObj(aCx, &capability.mReject.toObject()); + RefPtr rejectFunc = + new AnyCallback(aCx, rejectObj, GetIncumbentGlobal()); + + if (!capability.mPromise.isObject()) { + aRv.ThrowTypeError(); + return; + } + JS::Rooted newPromiseObj(aCx, &capability.mPromise.toObject()); + // We want to store the reflector itself. + newPromiseObj = js::CheckedUnwrap(newPromiseObj); + if (!newPromiseObj) { + // Just throw something. + aRv.ThrowTypeError(); + return; + } + + RefPtr resolveCb; + if (aResolveCallback) { + resolveCb = new WrapperPromiseCallback(global, aResolveCallback, + newPromiseObj, + resolveFunc, rejectFunc); + } else { + resolveCb = new InvokePromiseFuncCallback(global, newPromiseObj, + resolveFunc); + } + + RefPtr rejectCb; + if (aRejectCallback) { + rejectCb = new WrapperPromiseCallback(global, aRejectCallback, + newPromiseObj, + resolveFunc, rejectFunc); + } else { + rejectCb = new InvokePromiseFuncCallback(global, newPromiseObj, + rejectFunc); + } + + AppendCallbacks(resolveCb, rejectCb); + } + + aRetval.set(capability.PromiseValue()); } -already_AddRefed -Promise::Catch(JSContext* aCx, AnyCallback* aRejectCallback, ErrorResult& aRv) +void +Promise::Catch(JSContext* aCx, AnyCallback* aRejectCallback, + JS::MutableHandle aRetval, + ErrorResult& aRv) { - RefPtr resolveCb; - return Then(aCx, resolveCb, aRejectCallback, aRv); + // Implements + // http://www.ecma-international.org/ecma-262/6.0/#sec-promise.prototype.catch + + // We can't call Promise::Then directly, because someone might have + // overridden Promise.prototype.then. + JS::Rooted promiseVal(aCx, JS::ObjectValue(*GetWrapper())); + if (!MaybeWrapObjectValue(aCx, &promiseVal)) { + aRv.NoteJSContextException(); + return; + } + JS::Rooted promiseObj(aCx, &promiseVal.toObject()); + MOZ_ASSERT(promiseObj); + JS::AutoValueArray<2> callbacks(aCx); + callbacks[0].setUndefined(); + if (aRejectCallback) { + callbacks[1].setObject(*aRejectCallback->Callable()); + // It could be in any compartment, so put it in ours. + if (!MaybeWrapObjectValue(aCx, callbacks[1])) { + aRv.NoteJSContextException(); + return; + } + } else { + callbacks[1].setNull(); + } + if (!JS_CallFunctionName(aCx, promiseObj, "then", callbacks, aRetval)) { + aRv.NoteJSContextException(); + } } /** diff --git a/dom/promise/Promise.h b/dom/promise/Promise.h index 5e7a371c0c8..0090674355c 100644 --- a/dom/promise/Promise.h +++ b/dom/promise/Promise.h @@ -181,12 +181,20 @@ public: Reject(nsIGlobalObject* aGlobal, JSContext* aCx, JS::Handle aValue, ErrorResult& aRv); - already_AddRefed - Then(JSContext* aCx, AnyCallback* aResolveCallback, - AnyCallback* aRejectCallback, ErrorResult& aRv); + void + Then(JSContext* aCx, + // aCalleeGlobal may not be in the compartment of aCx, when called over + // Xrays. + JS::Handle aCalleeGlobal, + AnyCallback* aResolveCallback, AnyCallback* aRejectCallback, + JS::MutableHandle aRetval, + ErrorResult& aRv); - already_AddRefed - Catch(JSContext* aCx, AnyCallback* aRejectCallback, ErrorResult& aRv); + void + Catch(JSContext* aCx, + AnyCallback* aRejectCallback, + JS::MutableHandle aRetval, + ErrorResult& aRv); static void All(const GlobalObject& aGlobal, JS::Handle aThisv, diff --git a/dom/promise/PromiseCallback.cpp b/dom/promise/PromiseCallback.cpp index e8506cac0a7..8081234b152 100644 --- a/dom/promise/PromiseCallback.cpp +++ b/dom/promise/PromiseCallback.cpp @@ -156,25 +156,113 @@ RejectPromiseCallback::Call(JSContext* aCx, return NS_OK; } +// InvokePromiseFuncCallback + +NS_IMPL_CYCLE_COLLECTION_CLASS(InvokePromiseFuncCallback) + +NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(InvokePromiseFuncCallback, + PromiseCallback) + NS_IMPL_CYCLE_COLLECTION_UNLINK(mPromiseFunc) + tmp->mGlobal = nullptr; + tmp->mNextPromiseObj = nullptr; +NS_IMPL_CYCLE_COLLECTION_UNLINK_END + +NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(InvokePromiseFuncCallback, + PromiseCallback) + NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPromiseFunc) + NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS +NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END + +NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(InvokePromiseFuncCallback) + NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mGlobal) + NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mNextPromiseObj) +NS_IMPL_CYCLE_COLLECTION_TRACE_END + +NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_INHERITED(InvokePromiseFuncCallback) +NS_INTERFACE_MAP_END_INHERITING(PromiseCallback) + +NS_IMPL_ADDREF_INHERITED(InvokePromiseFuncCallback, PromiseCallback) +NS_IMPL_RELEASE_INHERITED(InvokePromiseFuncCallback, PromiseCallback) + +InvokePromiseFuncCallback::InvokePromiseFuncCallback(JS::Handle aGlobal, + JS::Handle aNextPromiseObj, + AnyCallback* aPromiseFunc) + : mGlobal(aGlobal) + , mNextPromiseObj(aNextPromiseObj) + , mPromiseFunc(aPromiseFunc) +{ + MOZ_ASSERT(aGlobal); + MOZ_ASSERT(aNextPromiseObj); + MOZ_ASSERT(aPromiseFunc); + HoldJSObjects(this); +} + +InvokePromiseFuncCallback::~InvokePromiseFuncCallback() +{ + DropJSObjects(this); +} + +nsresult +InvokePromiseFuncCallback::Call(JSContext* aCx, + JS::Handle aValue) +{ + // Run resolver's algorithm with value and the synchronous flag set. + + JS::ExposeObjectToActiveJS(mGlobal); + JS::ExposeValueToActiveJS(aValue); + + JSAutoCompartment ac(aCx, mGlobal); + JS::Rooted value(aCx, aValue); + if (!JS_WrapValue(aCx, &value)) { + NS_WARNING("Failed to wrap value into the right compartment."); + return NS_ERROR_FAILURE; + } + + ErrorResult rv; + JS::Rooted ignored(aCx); + mPromiseFunc->Call(value, &ignored, rv); + // Useful exceptions already got reported. + rv.SuppressException(); + return NS_OK; +} + +Promise* +InvokePromiseFuncCallback::GetDependentPromise() +{ + Promise* promise; + if (NS_SUCCEEDED(UNWRAP_OBJECT(Promise, mNextPromiseObj, promise))) { + return promise; + } + + // Oh, well. + return nullptr; +} + // WrapperPromiseCallback NS_IMPL_CYCLE_COLLECTION_CLASS(WrapperPromiseCallback) NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(WrapperPromiseCallback, PromiseCallback) NS_IMPL_CYCLE_COLLECTION_UNLINK(mNextPromise) + NS_IMPL_CYCLE_COLLECTION_UNLINK(mResolveFunc) + NS_IMPL_CYCLE_COLLECTION_UNLINK(mRejectFunc) NS_IMPL_CYCLE_COLLECTION_UNLINK(mCallback) tmp->mGlobal = nullptr; + tmp->mNextPromiseObj = nullptr; NS_IMPL_CYCLE_COLLECTION_UNLINK_END NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(WrapperPromiseCallback, PromiseCallback) NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mNextPromise) + NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mResolveFunc) + NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mRejectFunc) NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mCallback) NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(WrapperPromiseCallback) NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mGlobal) + NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mNextPromiseObj) NS_IMPL_CYCLE_COLLECTION_TRACE_END NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_INHERITED(WrapperPromiseCallback) @@ -195,6 +283,24 @@ WrapperPromiseCallback::WrapperPromiseCallback(Promise* aNextPromise, HoldJSObjects(this); } +WrapperPromiseCallback::WrapperPromiseCallback(JS::Handle aGlobal, + AnyCallback* aCallback, + JS::Handle aNextPromiseObj, + AnyCallback* aResolveFunc, + AnyCallback* aRejectFunc) + : mNextPromiseObj(aNextPromiseObj) + , mResolveFunc(aResolveFunc) + , mRejectFunc(aRejectFunc) + , mGlobal(aGlobal) + , mCallback(aCallback) +{ + MOZ_ASSERT(mNextPromiseObj); + MOZ_ASSERT(aResolveFunc); + MOZ_ASSERT(aRejectFunc); + MOZ_ASSERT(aGlobal); + HoldJSObjects(this); +} + WrapperPromiseCallback::~WrapperPromiseCallback() { DropJSObjects(this); @@ -218,9 +324,16 @@ WrapperPromiseCallback::Call(JSContext* aCx, // PromiseReactionTask step 6 JS::Rooted retValue(aCx); + JSCompartment* compartment; + if (mNextPromise) { + compartment = mNextPromise->Compartment(); + } else { + MOZ_ASSERT(mNextPromiseObj); + compartment = js::GetObjectCompartment(mNextPromiseObj); + } mCallback->Call(value, &retValue, rv, "promise callback", CallbackObject::eRethrowExceptions, - mNextPromise->Compartment()); + compartment); rv.WouldReportJSException(); @@ -231,23 +344,43 @@ WrapperPromiseCallback::Call(JSContext* aCx, // Convert the ErrorResult to a JS exception object that we can reject // ourselves with. This will be exactly the exception that would get // thrown from a binding method whose ErrorResult ended up with whatever - // is on "rv" right now. - JSAutoCompartment ac(aCx, mNextPromise->GlobalJSObject()); + // is on "rv" right now. Do this in the promise reflector compartment. + Maybe ac; + if (mNextPromise) { + ac.emplace(aCx, mNextPromise->GlobalJSObject()); + } else { + ac.emplace(aCx, mNextPromiseObj); + } DebugOnly conversionResult = ToJSValue(aCx, rv, &value); MOZ_ASSERT(conversionResult); } - mNextPromise->RejectInternal(aCx, value); + if (mNextPromise) { + mNextPromise->RejectInternal(aCx, value); + } else { + JS::Rooted ignored(aCx); + ErrorResult rejectRv; + mRejectFunc->Call(value, &ignored, rejectRv); + // This reported any JS exceptions; we just have a pointless exception on + // there now. + rejectRv.SuppressException(); + } return NS_OK; } // If the return value is the same as the promise itself, throw TypeError. if (retValue.isObject()) { JS::Rooted valueObj(aCx, &retValue.toObject()); - Promise* returnedPromise; - nsresult r = UNWRAP_OBJECT(Promise, valueObj, returnedPromise); - - if (NS_SUCCEEDED(r) && returnedPromise == mNextPromise) { + valueObj = js::CheckedUnwrap(valueObj); + JS::Rooted nextPromiseObj(aCx); + if (mNextPromise) { + nextPromiseObj = mNextPromise->GetWrapper(); + } else { + MOZ_ASSERT(mNextPromiseObj); + nextPromiseObj = mNextPromiseObj; + } + // XXXbz shouldn't this check be over in ResolveInternal anyway? + if (valueObj == nextPromiseObj) { const char* fileName = nullptr; uint32_t lineNumber = 0; @@ -296,7 +429,16 @@ WrapperPromiseCallback::Call(JSContext* aCx, return NS_ERROR_OUT_OF_MEMORY; } - mNextPromise->RejectInternal(aCx, typeError); + if (mNextPromise) { + mNextPromise->RejectInternal(aCx, typeError); + } else { + JS::Rooted ignored(aCx); + ErrorResult rejectRv; + mRejectFunc->Call(typeError, &ignored, rejectRv); + // This reported any JS exceptions; we just have a pointless exception + // on there now. + rejectRv.SuppressException(); + } return NS_OK; } } @@ -307,7 +449,17 @@ WrapperPromiseCallback::Call(JSContext* aCx, return NS_ERROR_FAILURE; } - mNextPromise->ResolveInternal(aCx, retValue); + if (mNextPromise) { + mNextPromise->ResolveInternal(aCx, retValue); + } else { + JS::Rooted ignored(aCx); + ErrorResult resolveRv; + mResolveFunc->Call(retValue, &ignored, resolveRv); + // This reported any JS exceptions; we just have a pointless exception + // on there now. + resolveRv.SuppressException(); + } + return NS_OK; } @@ -334,7 +486,17 @@ WrapperPromiseCallback::GetDependentPromise() return promise; } - return mNextPromise; + if (mNextPromise) { + return mNextPromise; + } + + Promise* promise; + if (NS_SUCCEEDED(UNWRAP_OBJECT(Promise, mNextPromiseObj, promise))) { + return promise; + } + + // Oh, well. + return nullptr; } // NativePromiseCallback diff --git a/dom/promise/PromiseCallback.h b/dom/promise/PromiseCallback.h index 1f1f6700a78..3cb98e15431 100644 --- a/dom/promise/PromiseCallback.h +++ b/dom/promise/PromiseCallback.h @@ -45,8 +45,13 @@ public: }; // WrapperPromiseCallback execs a JS Callback with a value, and then the return -// value is sent to the aNextPromise->ResolveFunction() or to -// aNextPromise->RejectFunction() if the JS Callback throws. +// value is sent to either: +// a) If aNextPromise is non-null, the aNextPromise->ResolveFunction() or to +// aNextPromise->RejectFunction() if the JS Callback throws. +// or +// b) If aNextPromise is null, in which case aResolveFunc and aRejectFunc must +// be non-null, then to aResolveFunc, unless aCallback threw, in which case +// aRejectFunc. class WrapperPromiseCallback final : public PromiseCallback { public: @@ -59,13 +64,28 @@ public: Promise* GetDependentPromise() override; + // Constructor for when we know we have a vanilla Promise. WrapperPromiseCallback(Promise* aNextPromise, JS::Handle aGlobal, AnyCallback* aCallback); + // Constructor for when all we have to work with are resolve/reject functions. + WrapperPromiseCallback(JS::Handle aGlobal, + AnyCallback* aCallback, + JS::Handle mNextPromiseObj, + AnyCallback* aResolveFunc, + AnyCallback* aRejectFunc); + private: ~WrapperPromiseCallback(); + // Either mNextPromise is non-null or all three of mNextPromiseObj, + // mResolveFund and mRejectFunc must are non-null. RefPtr mNextPromise; + // mNextPromiseObj is the reflector itself; it may not be in the + // same compartment as anything else we have. + JS::Heap mNextPromiseObj; + RefPtr mResolveFunc; + RefPtr mRejectFunc; JS::Heap mGlobal; RefPtr mCallback; }; @@ -122,6 +142,32 @@ private: JS::Heap mGlobal; }; +// InvokePromiseFuncCallback calls the given function with the value +// received by Call(). +class InvokePromiseFuncCallback final : public PromiseCallback +{ +public: + NS_DECL_ISUPPORTS_INHERITED + NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS_INHERITED(InvokePromiseFuncCallback, + PromiseCallback) + + nsresult Call(JSContext* aCx, + JS::Handle aValue) override; + + Promise* GetDependentPromise() override; + + InvokePromiseFuncCallback(JS::Handle aGlobal, + JS::Handle aNextPromiseObj, + AnyCallback* aPromiseFunc); + +private: + ~InvokePromiseFuncCallback(); + + JS::Heap mGlobal; + JS::Heap mNextPromiseObj; + RefPtr mPromiseFunc; +}; + // NativePromiseCallback wraps a PromiseNativeHandler. class NativePromiseCallback final : public PromiseCallback { diff --git a/dom/promise/tests/test_promise_xrays.html b/dom/promise/tests/test_promise_xrays.html index e04d705bc9f..a246243eda2 100644 --- a/dom/promise/tests/test_promise_xrays.html +++ b/dom/promise/tests/test_promise_xrays.html @@ -218,6 +218,52 @@ function testReject1() { ).then(nextTest); } +function testThen1() { + var p = win.Promise.resolve(5); + var q = p.then((x) => x*x); + ok(q instanceof win.Promise, + "Promise.then should return a promise from the right global"); + q.then( + function(arg) { + is(arg, 25, "Promise.then should work"); + }, + function(e) { + ok(false, "Promise.then should not fail"); + } + ).then(nextTest); +} + +function testThen2() { + var p = win.Promise.resolve(5); + var q = p.then((x) => Promise.resolve(x*x)); + ok(q instanceof win.Promise, + "Promise.then should return a promise from the right global"); + q.then( + function(arg) { + is(arg, 25, "Promise.then resolved with chrome promise should work"); + }, + function(e) { + ok(false, "Promise.then resolved with chrome promise should not fail"); + } + ).then(nextTest); +} + +function testCatch1() { + var p = win.Promise.reject(5); + ok(p instanceof win.Promise, "Promise.resolve should return a promise"); + var q = p.catch((x) => x*x); + ok(q instanceof win.Promise, + "Promise.catch should return a promise from the right global"); + q.then( + function(arg) { + is(arg, 25, "Promise.catch should work"); + }, + function(e) { + ok(false, "Promise.catch should not fail"); + } + ).then(nextTest); +} + var tests = [ testLoadComplete, testHaveXray, @@ -234,6 +280,9 @@ var tests = [ testResolve2, testResolve3, testReject1, + testThen1, + testThen2, + testCatch1, ]; function nextTest() { diff --git a/dom/webidl/DOMRequest.webidl b/dom/webidl/DOMRequest.webidl index 74a656d661a..23450387b19 100644 --- a/dom/webidl/DOMRequest.webidl +++ b/dom/webidl/DOMRequest.webidl @@ -20,9 +20,10 @@ interface DOMRequestShared { interface DOMRequest : EventTarget { // The [TreatNonCallableAsNull] annotation is required since then() should do // nothing instead of throwing errors when non-callable arguments are passed. - [NewObject] - Promise then([TreatNonCallableAsNull] optional AnyCallback? fulfillCallback = null, - [TreatNonCallableAsNull] optional AnyCallback? rejectCallback = null); + // See documentation for Promise.then to see why we return "any". + [NewObject, Throws] + any then([TreatNonCallableAsNull] optional AnyCallback? fulfillCallback = null, + [TreatNonCallableAsNull] optional AnyCallback? rejectCallback = null); }; DOMRequest implements DOMRequestShared; diff --git a/dom/webidl/Promise.webidl b/dom/webidl/Promise.webidl index ac5d9f86e34..328affcd3b8 100644 --- a/dom/webidl/Promise.webidl +++ b/dom/webidl/Promise.webidl @@ -30,12 +30,15 @@ interface _Promise { // The [TreatNonCallableAsNull] annotation is required since then() should do // nothing instead of throwing errors when non-callable arguments are passed. - [NewObject] - Promise then([TreatNonCallableAsNull] optional AnyCallback? fulfillCallback = null, - [TreatNonCallableAsNull] optional AnyCallback? rejectCallback = null); + // Have to use "any" (or "object", but "any" is simpler) as the type to + // support the subclassing behavior, since nothing actually requires the + // return value of PromiseSubclass.then/catch to be a Promise object. + [NewObject, Throws] + any then([TreatNonCallableAsNull] optional AnyCallback? fulfillCallback = null, + [TreatNonCallableAsNull] optional AnyCallback? rejectCallback = null); - [NewObject] - Promise catch([TreatNonCallableAsNull] optional AnyCallback? rejectCallback = null); + [NewObject, Throws] + any catch([TreatNonCallableAsNull] optional AnyCallback? rejectCallback = null); // Have to use "any" (or "object", but "any" is simpler) as the type to // support the subclassing behavior, since nothing actually requires the diff --git a/testing/web-platform/tests/js/builtins/Promise-subclassing.html b/testing/web-platform/tests/js/builtins/Promise-subclassing.html index b1207edb529..7264f4661aa 100644 --- a/testing/web-platform/tests/js/builtins/Promise-subclassing.html +++ b/testing/web-platform/tests/js/builtins/Promise-subclassing.html @@ -9,6 +9,7 @@ var theLog = []; var speciesGets = 0; var speciesCalls = 0; var constructorCalls = 0; +var constructorGets = 0; var resolveCalls = 0; var rejectCalls = 0; var thenCalls = 0; @@ -22,7 +23,7 @@ function takeLog() { theLog = []; speciesGets = speciesCalls = constructorCalls = resolveCalls = rejectCalls = thenCalls = catchCalls = allCalls = raceCalls = - nextCalls = 0; + nextCalls = constructorGets = 0; return oldLog; } @@ -37,6 +38,14 @@ function log(str) { class LoggingPromise extends Promise { constructor(func) { super(func); + Object.defineProperty(this, "constructor", + { + get: function() { + ++constructorGets; + log(`Constructor get ${constructorGets}`); + return Object.getPrototypeOf(this).constructor; + } + }); ++constructorCalls; log(`Constructor ${constructorCalls}`); } @@ -128,9 +137,9 @@ promise_test(function testPromiseRace() { var log = takeLog(); assert_array_equals(log, ["Race 1", "Constructor 1", "Next 1", "Resolve 1", "Constructor 2", - "Then 1", - "Next 2", "Resolve 2", "Constructor 3", - "Then 2", + "Then 1", "Constructor get 1", "Species get 1", "Species call 1", "Constructor 3", + "Next 2", "Resolve 2", "Constructor 4", + "Then 2", "Constructor get 2", "Species get 2", "Species call 2", "Constructor 5", "Next 3"]); assert_true(p instanceof LoggingPromise); return p.then(function(arg) { @@ -144,9 +153,9 @@ promise_test(function testPromiseRaceNoSpecies() { var log = takeLog(); assert_array_equals(log, ["Race 1", "Constructor 1", "Next 1", "Resolve 1", "Constructor 2", - "Then 1", + "Then 1", "Constructor get 1", "Next 2", "Resolve 2", "Constructor 3", - "Then 2", + "Then 2", "Constructor get 2", "Next 3"]); assert_true(p instanceof SpeciesLessPromise); return p.then(function(arg) { @@ -160,9 +169,9 @@ promise_test(function testPromiseAll() { var log = takeLog(); assert_array_equals(log, ["All 1", "Constructor 1", "Next 1", "Resolve 1", "Constructor 2", - "Then 1", - "Next 2", "Resolve 2", "Constructor 3", - "Then 2", + "Then 1", "Constructor get 1", "Species get 1", "Species call 1", "Constructor 3", + "Next 2", "Resolve 2", "Constructor 4", + "Then 2", "Constructor get 2", "Species get 2", "Species call 2", "Constructor 5", "Next 3"]); assert_true(p instanceof LoggingPromise); return p.then(function(arg) { @@ -179,9 +188,11 @@ promise_test(function testPromiseResolve() { assert_equals(p, q, "Promise.resolve with same constructor should preserve identity"); log = takeLog(); - assert_array_equals(log, ["Resolve 1"]); + assert_array_equals(log, ["Resolve 1", "Constructor get 1"]); var r = Promise.resolve(p); + log = takeLog(); + assert_array_equals(log, ["Constructor get 1"]); assert_not_equals(p, r, "Promise.resolve with different constructor should " + "create a new Promise instance (1)") @@ -217,4 +228,38 @@ promise_test(function testPromiseReject() { }); }, "Promise.reject behavior"); +promise_test(function testPromiseThen() { + clearLog(); + var p = LoggingPromise.resolve(5); + var log = takeLog(); + assert_array_equals(log, ["Resolve 1", "Constructor 1"]); + + var q = p.then((x) => x*x); + log = takeLog(); + assert_array_equals(log, ["Then 1", "Constructor get 1", "Species get 1", + "Species call 1", "Constructor 1"]); + assert_true(q instanceof LoggingPromise); + + return q.then(function(arg) { + assert_equals(arg, 25); + }); +}, "Promise.then behavior"); + +promise_test(function testPromiseCatch() { + clearLog(); + var p = LoggingPromise.reject(5); + var log = takeLog(); + assert_array_equals(log, ["Reject 1", "Constructor 1"]); + + var q = p.catch((x) => x*x); + log = takeLog(); + assert_array_equals(log, ["Catch 1", "Then 1", "Constructor get 1", + "Species get 1", "Species call 1", "Constructor 1"]); + assert_true(q instanceof LoggingPromise); + + return q.then(function(arg) { + assert_equals(arg, 25); + }); +}, "Promise.catch behavior"); +