Bug 1324140 - Unwrap given Promise in some JSAPI functions. r=bz

To make Promise-related JSAPI functions easier to use, this patch unwraps handed-in Promise objects automatically. Some functions don't unwrap, mostly debugging-related ones and, notably, JS::IsPromiseObject. The latter doesn't unwrap in order to stay conservative: if JSAPI-using code uses IsPromiseObject to verify that an object is a Promise, it should always be fine to say "no". MozReview-Commit-ID: 7DuCqCj95JR --HG-- extra : rebase_source : 86e5b837c68fcbd1c1930dffefc22856b02cf3b1
2017-05-03 10:53:19 -04:00 · 2017-05-03 10:53:19 -04:00 · 4662c498be
--- a/js/src/builtin/Promise.cpp
+++ b/js/src/builtin/Promise.cpp
@ -2167,34 +2167,39 @@ NewReactionRecord(JSContext* cx, HandleObject resultPromise, HandleValue onFulfi
 }

 // ES2016, 25.4.5.3., steps 3-5.
-MOZ_MUST_USE JSObject*
-js::OriginalPromiseThen(JSContext* cx, Handle<PromiseObject*> promise, HandleValue onFulfilled,
-                        HandleValue onRejected)
+MOZ_MUST_USE bool
+js::OriginalPromiseThen(JSContext* cx, Handle<PromiseObject*> promise,
+                        HandleValue onFulfilled, HandleValue onRejected,
+                        MutableHandleObject dependent, bool createDependent)
 {
    RootedObject promiseObj(cx, promise);
    if (promise->compartment() != cx->compartment()) {
        if (!cx->compartment()->wrap(cx, &promiseObj))
-            return nullptr;
+            return false;
    }

-    // Step 3.
-    RootedValue ctorVal(cx);
-    if (!SpeciesConstructor(cx, promiseObj, JSProto_Promise, &ctorVal))
-        return nullptr;
-    RootedObject C(cx, &ctorVal.toObject());
-
-    // Step 4.
    RootedObject resultPromise(cx);
    RootedObject resolve(cx);
    RootedObject reject(cx);
-    if (!NewPromiseCapability(cx, C, &resultPromise, &resolve, &reject, true))
-        return nullptr;
+
+    if (createDependent) {
+        // Step 3.
+        RootedValue ctorVal(cx);
+        if (!SpeciesConstructor(cx, promiseObj, JSProto_Promise, &ctorVal))
+            return false;
+        RootedObject C(cx, &ctorVal.toObject());
+
+        // Step 4.
+        if (!NewPromiseCapability(cx, C, &resultPromise, &resolve, &reject, true))
+            return false;
+    }

    // Step 5.
    if (!PerformPromiseThen(cx, promise, onFulfilled, onRejected, resultPromise, resolve, reject))
-        return nullptr;
+        return false;

-    return resultPromise;
+    dependent.set(resultPromise);
+    return true;
 }

 static MOZ_MUST_USE bool PerformPromiseThenWithReaction(JSContext* cx,
@ -2658,8 +2663,8 @@ js::Promise_then(JSContext* cx, unsigned argc, Value* vp)
    }

    // Steps 3-5.
-    RootedObject resultPromise(cx, OriginalPromiseThen(cx, promise, onFulfilled, onRejected));
-    if (!resultPromise)
+    RootedObject resultPromise(cx);
+    if (!OriginalPromiseThen(cx, promise, onFulfilled, onRejected, &resultPromise, true))
        return false;

    args.rval().setObject(*resultPromise);
@ -2832,8 +2837,10 @@ BlockOnPromise(JSContext* cx, HandleValue promiseVal, HandleObject blockedPromis
    mozilla::Maybe<AutoCompartment> ac;
    if (IsProxy(promiseObj)) {
        unwrappedPromiseObj = CheckedUnwrap(promiseObj);
-        if (!unwrappedPromiseObj)
+        if (!unwrappedPromiseObj) {
+            ReportAccessDenied(cx);
            return false;
+        }
        if (JS_IsDeadWrapper(unwrappedPromiseObj)) {
            JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_DEAD_OBJECT);
            return false;
@ -3086,16 +3093,6 @@ PromiseObject::onSettled(JSContext* cx, Handle<PromiseObject*> promise)
    JS::dbg::onPromiseSettled(cx, promise);
 }

-MOZ_MUST_USE bool
-js::EnqueuePromiseReactions(JSContext* cx, Handle<PromiseObject*> promise,
-                            HandleObject dependentPromise,
-                            HandleValue onFulfilled, HandleValue onRejected)
-{
-    MOZ_ASSERT_IF(dependentPromise, dependentPromise->is<PromiseObject>());
-    return PerformPromiseThen(cx, promise, onFulfilled, onRejected, dependentPromise,
-                              nullptr, nullptr);
-}
-
 PromiseTask::PromiseTask(JSContext* cx, Handle<PromiseObject*> promise)
  : runtime_(cx->runtime()),
    promise_(cx, promise)
--- a/js/src/builtin/Promise.h
+++ b/js/src/builtin/Promise.h
@ -102,26 +102,34 @@ class PromiseObject : public NativeObject
 };

 /**
- * Enqueues resolve/reject reactions in the given Promise's reactions lists
- * in a content-invisible way.
+ * Unforgeable version of the JS builtin Promise.all.
 *
- * Used internally to implement DOM functionality.
+ * Takes an AutoObjectVector of Promise objects and returns a promise that's
+ * resolved with an array of resolution values when all those promises have
+ * been resolved, or rejected with the rejection value of the first rejected
+ * promise.
 *
- * Note: the reactions pushed using this function contain a `promise` field
- * that can contain null. That field is only ever used by devtools, which have
- * to treat these reactions specially.
+ * Asserts that all objects in the `promises` vector are, maybe wrapped,
+ * instances of `Promise` or a subclass of `Promise`.
 */
-MOZ_MUST_USE bool
-EnqueuePromiseReactions(JSContext* cx, Handle<PromiseObject*> promise,
-                        HandleObject dependentPromise,
-                        HandleValue onFulfilled, HandleValue onRejected);
-
 MOZ_MUST_USE JSObject*
 GetWaitForAllPromise(JSContext* cx, const JS::AutoObjectVector& promises);

-MOZ_MUST_USE JSObject*
-OriginalPromiseThen(JSContext* cx, Handle<PromiseObject*> promise, HandleValue onFulfilled,
-                    HandleValue onRejected);
+/**
+ * Enqueues resolve/reject reactions in the given Promise's reactions lists
+ * as though calling the original value of Promise.prototype.then.
+ *
+ * If the `createDependent` flag is not set, no dependent Promise will be
+ * created. This is used internally to implement DOM functionality.
+ * Note: In this case, the reactions pushed using this function contain a
+ * `promise` field that can contain null. That field is only ever used by
+ * devtools, which have to treat these reactions specially.
+ */
+MOZ_MUST_USE bool
+OriginalPromiseThen(JSContext* cx, Handle<PromiseObject*> promise,
+                    HandleValue onFulfilled, HandleValue onRejected,
+                    MutableHandleObject dependent, bool createDependent);
+

 MOZ_MUST_USE PromiseObject*
 CreatePromiseObjectForAsync(JSContext* cx, HandleValue generatorVal);
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@ -4902,9 +4902,13 @@ JS::GetPromisePrototype(JSContext* cx)
 }

 JS_PUBLIC_API(JS::PromiseState)
-JS::GetPromiseState(JS::HandleObject promise)
+JS::GetPromiseState(JS::HandleObject promiseObj_)
 {
-    return promise->as<PromiseObject>().state();
+    JSObject* promiseObj = CheckedUnwrap(promiseObj_);
+    if (!promiseObj || !promiseObj->is<PromiseObject>())
+        return JS::PromiseState::Pending;
+
+    return promiseObj->as<PromiseObject>().state();
 }

 JS_PUBLIC_API(uint64_t)
@ -4977,60 +4981,114 @@ JS::CallOriginalPromiseReject(JSContext* cx, JS::HandleValue rejectionValue)
    return promise;
 }

-JS_PUBLIC_API(bool)
-JS::ResolvePromise(JSContext* cx, JS::HandleObject promiseObj, JS::HandleValue resolutionValue)
+static bool
+ResolveOrRejectPromise(JSContext* cx, JS::HandleObject promiseObj, JS::HandleValue resultOrReason_,
+                       bool reject)
 {
    AssertHeapIsIdle();
    CHECK_REQUEST(cx);
-    assertSameCompartment(cx, promiseObj, resolutionValue);
+    assertSameCompartment(cx, promiseObj, resultOrReason_);

-    Handle<PromiseObject*> promise = promiseObj.as<PromiseObject>();
-    return PromiseObject::resolve(cx, promise, resolutionValue);
+    mozilla::Maybe<AutoCompartment> ac;
+    Rooted<PromiseObject*> promise(cx);
+    RootedValue resultOrReason(cx, resultOrReason_);
+    if (IsWrapper(promiseObj)) {
+        JSObject* unwrappedPromiseObj = CheckedUnwrap(promiseObj);
+        if (!unwrappedPromiseObj) {
+            ReportAccessDenied(cx);
+            return false;
+        }
+        promise = &unwrappedPromiseObj->as<PromiseObject>();
+        ac.emplace(cx, promise);
+        if (!cx->compartment()->wrap(cx, &resultOrReason))
+            return false;
+    } else {
+        promise = promiseObj.as<PromiseObject>();
+    }
+
+    return reject
+           ? PromiseObject::reject(cx, promise, resultOrReason)
+           : PromiseObject::resolve(cx, promise, resultOrReason);
+}
+
+JS_PUBLIC_API(bool)
+JS::ResolvePromise(JSContext* cx, JS::HandleObject promiseObj, JS::HandleValue resolutionValue)
+{
+    return ResolveOrRejectPromise(cx, promiseObj, resolutionValue, false);
 }

 JS_PUBLIC_API(bool)
 JS::RejectPromise(JSContext* cx, JS::HandleObject promiseObj, JS::HandleValue rejectionValue)
+{
+    return ResolveOrRejectPromise(cx, promiseObj, rejectionValue, true);
+}
+
+static bool
+CallOriginalPromiseThenImpl(JSContext* cx, JS::HandleObject promiseObj,
+                            JS::HandleObject onResolvedObj_, JS::HandleObject onRejectedObj_,
+                            JS::MutableHandleObject resultObj, bool createDependent)
 {
    AssertHeapIsIdle();
    CHECK_REQUEST(cx);
-    assertSameCompartment(cx, promiseObj, rejectionValue);
+    assertSameCompartment(cx, promiseObj, onResolvedObj_, onRejectedObj_);
+
+    MOZ_ASSERT_IF(onResolvedObj_, IsCallable(onResolvedObj_));
+    MOZ_ASSERT_IF(onRejectedObj_, IsCallable(onRejectedObj_));
+
+    {
+        mozilla::Maybe<AutoCompartment> ac;
+        Rooted<PromiseObject*> promise(cx);
+        RootedObject onResolvedObj(cx, onResolvedObj_);
+        RootedObject onRejectedObj(cx, onRejectedObj_);
+        if (IsWrapper(promiseObj)) {
+            JSObject* unwrappedPromiseObj = CheckedUnwrap(promiseObj);
+            if (!unwrappedPromiseObj) {
+                ReportAccessDenied(cx);
+                return false;
+            }
+            promise = &unwrappedPromiseObj->as<PromiseObject>();
+            ac.emplace(cx, promise);
+            if (!cx->compartment()->wrap(cx, &onResolvedObj) ||
+                !cx->compartment()->wrap(cx, &onRejectedObj))
+            {
+                return false;
+            }
+        } else {
+            promise = promiseObj.as<PromiseObject>();
+        }
+
+        RootedValue onFulfilled(cx, ObjectOrNullValue(onResolvedObj));
+        RootedValue onRejected(cx, ObjectOrNullValue(onRejectedObj));
+        if (!OriginalPromiseThen(cx, promise, onFulfilled, onRejected, resultObj, createDependent))
+            return false;
+    }
+
+    if (resultObj) {
+        if (!cx->compartment()->wrap(cx, resultObj))
+            return false;
+    }
+    return true;

-    Handle<PromiseObject*> promise = promiseObj.as<PromiseObject>();
-    return PromiseObject::reject(cx, promise, rejectionValue);
 }

 JS_PUBLIC_API(JSObject*)
 JS::CallOriginalPromiseThen(JSContext* cx, JS::HandleObject promiseObj,
-                            JS::HandleObject onResolveObj, JS::HandleObject onRejectObj)
+                            JS::HandleObject onResolvedObj, JS::HandleObject onRejectedObj)
 {
-    AssertHeapIsIdle();
-    CHECK_REQUEST(cx);
-    assertSameCompartment(cx, promiseObj, onResolveObj, onRejectObj);
-
-    MOZ_ASSERT_IF(onResolveObj, IsCallable(onResolveObj));
-    MOZ_ASSERT_IF(onRejectObj, IsCallable(onRejectObj));
-
-    Rooted<PromiseObject*> promise(cx, &promiseObj->as<PromiseObject>());
-    RootedValue onFulfilled(cx, ObjectOrNullValue(onResolveObj));
-    RootedValue onRejected(cx, ObjectOrNullValue(onRejectObj));
-    return OriginalPromiseThen(cx, promise, onFulfilled, onRejected);
+    RootedObject resultPromise(cx);
+    if (!CallOriginalPromiseThenImpl(cx, promiseObj, onResolvedObj, onRejectedObj, &resultPromise, true))
+        return nullptr;
+    return resultPromise;
 }

 JS_PUBLIC_API(bool)
 JS::AddPromiseReactions(JSContext* cx, JS::HandleObject promiseObj,
                        JS::HandleObject onResolvedObj, JS::HandleObject onRejectedObj)
 {
-    AssertHeapIsIdle();
-    CHECK_REQUEST(cx);
-    assertSameCompartment(cx, promiseObj, onResolvedObj, onRejectedObj);
-
-    MOZ_ASSERT(IsCallable(onResolvedObj));
-    MOZ_ASSERT(IsCallable(onRejectedObj));
-
-    Rooted<PromiseObject*> promise(cx, &promiseObj->as<PromiseObject>());
-    RootedValue onResolved(cx, ObjectValue(*onResolvedObj));
-    RootedValue onRejected(cx, ObjectValue(*onRejectedObj));
-    return EnqueuePromiseReactions(cx, promise, nullptr, onResolved, onRejected);
+    RootedObject resultPromise(cx);
+    bool result = CallOriginalPromiseThenImpl(cx, promiseObj, onResolvedObj, onRejectedObj, &resultPromise, false);
+    MOZ_ASSERT(!resultPromise);
+    return result;
 }

 /**
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@ -4608,6 +4608,9 @@ enum class PromiseState {

 /**
 * Returns the given Promise's state as a JS::PromiseState enum value.
+ *
+ * Returns JS::PromiseState::Pending if the given object is a wrapper that
+ * can't safely be unwrapped.
 */
 extern JS_PUBLIC_API(PromiseState)
 GetPromiseState(JS::HandleObject promise);
@ -4706,12 +4709,12 @@ AddPromiseReactions(JSContext* cx, JS::HandleObject promise,
 * Unforgeable version of the JS builtin Promise.all.
 *
 * Takes an AutoObjectVector of Promise objects and returns a promise that's
- * resolved with an array of resolution values when all those promises ahve
+ * resolved with an array of resolution values when all those promises have
 * been resolved, or rejected with the rejection value of the first rejected
 * promise.
 *
- * Asserts if the array isn't dense or one of the entries isn't an unwrapped
- * instance of Promise or a subclass.
+ * Asserts that all objects in the `promises` vector are, maybe wrapped,
+ * instances of `Promise` or a subclass of `Promise`.
 */
 extern JS_PUBLIC_API(JSObject*)
 GetWaitForAllPromise(JSContext* cx, const JS::AutoObjectVector& promises);