From af8e8bebc96eb401a011b355872f77aeede9326c Mon Sep 17 00:00:00 2001 From: Nikhil Marathe Date: Thu, 23 Jan 2014 10:47:29 -0800 Subject: [PATCH] Bug 879245 - Implement thenables for Promises. r=bz --HG-- extra : rebase_source : cde27792ae58b13c88367df7a99fc8981097eedd --- dom/bindings/BindingUtils.h | 19 +++ dom/promise/Promise.cpp | 200 ++++++++++++++++++++++++++-- dom/promise/Promise.h | 14 ++ dom/promise/tests/test_promise.html | 104 +++++++++++++++ 4 files changed, 326 insertions(+), 11 deletions(-) diff --git a/dom/bindings/BindingUtils.h b/dom/bindings/BindingUtils.h index 06d7e66e17c1..0e732d2e1842 100644 --- a/dom/bindings/BindingUtils.h +++ b/dom/bindings/BindingUtils.h @@ -1316,6 +1316,25 @@ WrapCallThisObject(JSContext* cx, JS::Handle scope, const T& p) return obj; } +/* + * This specialized function simply wraps a JS::Rooted<> since + * WrapNativeParent() is not applicable for JS objects. + */ +template<> +inline JSObject* +WrapCallThisObject>(JSContext* cx, + JS::Handle scope, + const JS::Rooted& p) +{ + JS::Rooted obj(cx, p); + + if (!JS_WrapObject(cx, &obj)) { + return nullptr; + } + + return obj; +} + // Helper for calling WrapNewBindingObject with smart pointers // (nsAutoPtr/nsRefPtr/nsCOMPtr) or references. template ::Value> diff --git a/dom/promise/Promise.cpp b/dom/promise/Promise.cpp index d6cccdef42ce..94c5a4d5c4e5 100644 --- a/dom/promise/Promise.cpp +++ b/dom/promise/Promise.cpp @@ -293,11 +293,11 @@ EnterCompartment(Maybe& aAc, JSContext* aCx, enum { SLOT_PROMISE = 0, - SLOT_TASK + SLOT_DATA }; /* static */ bool -Promise::JSCallback(JSContext *aCx, unsigned aArgc, JS::Value *aVp) +Promise::JSCallback(JSContext* aCx, unsigned aArgc, JS::Value* aVp) { JS::CallArgs args = CallArgsFromVp(aArgc, aVp); @@ -311,7 +311,7 @@ Promise::JSCallback(JSContext *aCx, unsigned aArgc, JS::Value *aVp) return Throw(aCx, NS_ERROR_UNEXPECTED); } - v = js::GetFunctionNativeReserved(&args.callee(), SLOT_TASK); + v = js::GetFunctionNativeReserved(&args.callee(), SLOT_DATA); PromiseCallback::Task task = static_cast(v.toInt32()); if (task == PromiseCallback::Resolve) { @@ -323,6 +323,103 @@ Promise::JSCallback(JSContext *aCx, unsigned aArgc, JS::Value *aVp) return true; } +/* + * Utilities for thenable callbacks. + * + * A thenable is a { then: function(resolve, reject) { } }. + * `then` is called with a resolve and reject callback pair. + * Since only one of these should be called at most once (first call wins), the + * two keep a reference to each other in SLOT_DATA. When either of them is + * called, the references are cleared. Further calls are ignored. + */ +namespace { +void +LinkThenableCallables(JSContext* aCx, JS::Handle aResolveFunc, + JS::Handle aRejectFunc) +{ + js::SetFunctionNativeReserved(aResolveFunc, SLOT_DATA, + JS::ObjectValue(*aRejectFunc)); + js::SetFunctionNativeReserved(aRejectFunc, SLOT_DATA, + JS::ObjectValue(*aResolveFunc)); +} + +/* + * Returns false if callback was already called before, otherwise breaks the + * links and returns true. + */ +bool +MarkAsCalledIfNotCalledBefore(JSContext* aCx, JS::Handle aFunc) +{ + JS::Value otherFuncVal = js::GetFunctionNativeReserved(aFunc, SLOT_DATA); + + if (!otherFuncVal.isObject()) { + return false; + } + + JSObject* otherFuncObj = &otherFuncVal.toObject(); + MOZ_ASSERT(js::GetFunctionNativeReserved(otherFuncObj, SLOT_DATA).isObject()); + + // Break both references. + js::SetFunctionNativeReserved(aFunc, SLOT_DATA, JS::UndefinedValue()); + js::SetFunctionNativeReserved(otherFuncObj, SLOT_DATA, JS::UndefinedValue()); + + return true; +} + +Promise* +GetPromise(JSContext* aCx, JS::Handle aFunc) +{ + JS::Value promiseVal = js::GetFunctionNativeReserved(aFunc, SLOT_PROMISE); + + MOZ_ASSERT(promiseVal.isObject()); + + Promise* promise; + UNWRAP_OBJECT(Promise, &promiseVal.toObject(), promise); + return promise; +} +}; + +/* + * Common bits of (JSCallbackThenableResolver/JSCallbackThenableRejecter). + * Resolves/rejects the Promise if it is ok to do so, based on whether either of + * the callbacks have been called before or not. + */ +/* static */ bool +Promise::ThenableResolverCommon(JSContext* aCx, uint32_t aTask, + unsigned aArgc, JS::Value* aVp) +{ + JS::CallArgs args = CallArgsFromVp(aArgc, aVp); + JS::Rooted thisFunc(aCx, &args.callee()); + if (!MarkAsCalledIfNotCalledBefore(aCx, thisFunc)) { + // A function from this pair has been called before. + return true; + } + + Promise* promise = GetPromise(aCx, thisFunc); + MOZ_ASSERT(promise); + + if (aTask == PromiseCallback::Resolve) { + promise->ResolveInternal(aCx, args.get(0), SyncTask); + } else { + promise->RejectInternal(aCx, args.get(0), SyncTask); + } + return true; +} + +/* static */ bool +Promise::JSCallbackThenableResolver(JSContext* aCx, + unsigned aArgc, JS::Value* aVp) +{ + return ThenableResolverCommon(aCx, PromiseCallback::Resolve, aArgc, aVp); +} + +/* static */ bool +Promise::JSCallbackThenableRejecter(JSContext* aCx, + unsigned aArgc, JS::Value* aVp) +{ + return ThenableResolverCommon(aCx, PromiseCallback::Reject, aArgc, aVp); +} + /* static */ JSObject* Promise::CreateFunction(JSContext* aCx, JSObject* aParent, Promise* aPromise, int32_t aTask) @@ -342,7 +439,33 @@ Promise::CreateFunction(JSContext* aCx, JSObject* aParent, Promise* aPromise, } js::SetFunctionNativeReserved(obj, SLOT_PROMISE, promiseObj); - js::SetFunctionNativeReserved(obj, SLOT_TASK, JS::Int32Value(aTask)); + js::SetFunctionNativeReserved(obj, SLOT_DATA, JS::Int32Value(aTask)); + + return obj; +} + +/* static */ JSObject* +Promise::CreateThenableFunction(JSContext* aCx, Promise* aPromise, uint32_t aTask) +{ + JSNative whichFunc = + aTask == PromiseCallback::Resolve ? JSCallbackThenableResolver : + JSCallbackThenableRejecter ; + + JSFunction* func = js::NewFunctionWithReserved(aCx, whichFunc, + 1 /* nargs */, 0 /* flags */, + nullptr, nullptr); + if (!func) { + return nullptr; + } + + JS::Rooted obj(aCx, JS_GetFunctionObject(func)); + + JS::Rooted promiseObj(aCx); + if (!dom::WrapNewBindingObject(aCx, obj, aPromise, &promiseObj)) { + return nullptr; + } + + js::SetFunctionNativeReserved(obj, SLOT_PROMISE, promiseObj); return obj; } @@ -615,6 +738,16 @@ Promise::MaybeRejectInternal(JSContext* aCx, RejectInternal(aCx, aValue, aAsynchronous); } +void +Promise::HandleException(JSContext* aCx) +{ + JS::Rooted exn(aCx); + if (JS_GetPendingException(aCx, &exn)) { + JS_ClearPendingException(aCx); + RejectInternal(aCx, exn, SyncTask); + } +} + void Promise::ResolveInternal(JSContext* aCx, JS::Handle aValue, @@ -622,16 +755,61 @@ Promise::ResolveInternal(JSContext* aCx, { mResolvePending = true; - // TODO: Bug 879245 - Then-able objects if (aValue.isObject()) { JS::Rooted valueObj(aCx, &aValue.toObject()); - Promise* nextPromise; - nsresult rv = UNWRAP_OBJECT(Promise, valueObj, nextPromise); - if (NS_SUCCEEDED(rv)) { - nsRefPtr resolveCb = new ResolvePromiseCallback(this); - nsRefPtr rejectCb = new RejectPromiseCallback(this); - nextPromise->AppendCallbacks(resolveCb, rejectCb); + // Thenables. + JS::Rooted then(aCx); + if (!JS_GetProperty(aCx, valueObj, "then", &then)) { + HandleException(aCx); + return; + } + + if (then.isObject() && JS_ObjectIsCallable(aCx, &then.toObject())) { + JS::Rooted resolveFunc(aCx, + CreateThenableFunction(aCx, this, PromiseCallback::Resolve)); + + if (!resolveFunc) { + HandleException(aCx); + return; + } + + JS::Rooted rejectFunc(aCx, + CreateThenableFunction(aCx, this, PromiseCallback::Reject)); + if (!rejectFunc) { + HandleException(aCx); + return; + } + + LinkThenableCallables(aCx, resolveFunc, rejectFunc); + + JS::Rooted thenObj(aCx, &then.toObject()); + nsRefPtr thenCallback = + new PromiseInit(thenObj, mozilla::dom::GetIncumbentGlobal()); + + ErrorResult rv; + thenCallback->Call(valueObj, resolveFunc, rejectFunc, + rv, CallbackObject::eRethrowExceptions); + rv.WouldReportJSException(); + + if (rv.IsJSException()) { + JS::Rooted exn(aCx); + rv.StealJSException(aCx, &exn); + + bool couldMarkAsCalled = MarkAsCalledIfNotCalledBefore(aCx, resolveFunc); + + // If we could mark as called, neither of the callbacks had been called + // when the exception was thrown. So we can reject the Promise. + if (couldMarkAsCalled) { + Maybe ac; + EnterCompartment(ac, aCx, exn); + RejectInternal(aCx, exn, Promise::SyncTask); + } + // At least one of resolveFunc or rejectFunc have been called, so ignore + // the exception. FIXME(nsm): This should be reported to the error + // console though, for debugging. + } + return; } } diff --git a/dom/promise/Promise.h b/dom/promise/Promise.h index 7177dab50a3e..059d3a306c2d 100644 --- a/dom/promise/Promise.h +++ b/dom/promise/Promise.h @@ -151,10 +151,24 @@ private: // Static methods for the PromiseInit functions. static bool JSCallback(JSContext *aCx, unsigned aArgc, JS::Value *aVp); + + static bool + ThenableResolverCommon(JSContext* aCx, uint32_t /* PromiseCallback::Task */ aTask, + unsigned aArgc, JS::Value* aVp); + static bool + JSCallbackThenableResolver(JSContext *aCx, unsigned aArgc, JS::Value *aVp); + static bool + JSCallbackThenableRejecter(JSContext *aCx, unsigned aArgc, JS::Value *aVp); + static JSObject* CreateFunction(JSContext* aCx, JSObject* aParent, Promise* aPromise, int32_t aTask); + static JSObject* + CreateThenableFunction(JSContext* aCx, Promise* aPromise, uint32_t aTask); + + void HandleException(JSContext* aCx); + nsRefPtr mWindow; nsTArray > mResolveCallbacks; diff --git a/dom/promise/tests/test_promise.html b/dom/promise/tests/test_promise.html index a0a5363a8395..18d58c98c2b9 100644 --- a/dom/promise/tests/test_promise.html +++ b/dom/promise/tests/test_promise.html @@ -466,6 +466,104 @@ function promiseResolveNestedPromise() { }); } +function promiseSimpleThenableResolve() { + var thenable = { then: function(resolve) { resolve(5); } }; + var promise = new Promise(function(resolve, reject) { + resolve(thenable); + }); + + promise.then(function(v) { + ok(v === 5, "promiseSimpleThenableResolve"); + runTest(); + }, function(e) { + ok(false, "promiseSimpleThenableResolve: Should not reject"); + }); +} + +function promiseSimpleThenableReject() { + var thenable = { then: function(resolve, reject) { reject(5); } }; + var promise = new Promise(function(resolve, reject) { + resolve(thenable); + }); + + promise.then(function() { + ok(false, "promiseSimpleThenableReject: Should not resolve"); + runTest(); + }, function(e) { + ok(e === 5, "promiseSimpleThenableReject"); + runTest(); + }); +} + +function promiseThenableThrowsBeforeCallback() { + var thenable = { then: function(resolve) { + throw new TypeError("Hi there"); + resolve(5); + }}; + + var promise = Promise.resolve(thenable); + promise.then(function(v) { + ok(false, "promiseThenableThrowsBeforeCallback: Should've rejected"); + runTest(); + }, function(e) { + ok(e instanceof TypeError, "promiseThenableThrowsBeforeCallback"); + runTest(); + }); +} + +function promiseThenableThrowsAfterCallback() { + var thenable = { then: function(resolve) { + resolve(5); + throw new TypeError("Hi there"); + }}; + + var promise = Promise.resolve(thenable); + promise.then(function(v) { + ok(v === 5, "promiseThenableThrowsAfterCallback"); + runTest(); + }, function(e) { + ok(false, "promiseThenableThrowsAfterCallback: Should've resolved"); + runTest(); + }); +} + +function promiseThenableRejectThenResolve() { + var thenable = { then: function(resolve, reject) { + reject(new TypeError("Hi there")); + resolve(5); + }}; + + var promise = Promise.resolve(thenable); + promise.then(function(v) { + ok(false, "promiseThenableRejectThenResolve should have rejected"); + runTest(); + }, function(e) { + ok(e instanceof TypeError, "promiseThenableRejectThenResolve"); + runTest(); + }); +} + +function promiseWithThenReplaced() { + // Ensure that we call the 'then' on the promise and not the internal then. + var promise = new Promise(function(resolve, reject) { + resolve(5); + }); + + // Rogue `then` always rejects. + promise.then = function(onFulfill, onReject) { + onReject(new TypeError("Foo")); + } + + var promise2 = Promise.resolve(promise); + promise2.then(function(v) { + ok(false, "promiseWithThenReplaced: Should've rejected"); + runTest(); + }, function(e) { + ok(e instanceof TypeError, "promiseWithThenReplaced"); + runTest(); + }); +} + var tests = [ promiseResolve, promiseReject, promiseException, promiseGC, promiseAsync, promiseDoubleThen, promiseThenException, @@ -485,6 +583,12 @@ var tests = [ promiseResolve, promiseReject, promiseThenNullResolveFunction, promiseCatchNoArg, promiseRejectNoHandler, + promiseSimpleThenableResolve, + promiseSimpleThenableReject, + promiseThenableThrowsBeforeCallback, + promiseThenableThrowsAfterCallback, + promiseThenableRejectThenResolve, + promiseWithThenReplaced, ]; function runTest() {