Bug 1287334 - Properly handle non-Promise objects as dependent promise objects in js::EnqueuePromiseReactionJob. r=efaust
authorTill Schneidereit <till@tillschneidereit.net>
Thu, 21 Jul 2016 10:27:24 +0200
changeset 346079 1e55eeb2395e3255816a94f4f50f4e291c7394d8
parent 346078 79b1ba1f1f4bcc68801315438fa632e6c916549e
child 346080 904a6eb36f1ba6d8d1d79f445f25d6eeef021688
push id6389
push userraliiev@mozilla.com
push dateMon, 19 Sep 2016 13:38:22 +0000
treeherdermozilla-beta@01d67bfe6c81 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersefaust
bugs1287334
milestone50.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1287334 - Properly handle non-Promise objects as dependent promise objects in js::EnqueuePromiseReactionJob. r=efaust
js/src/builtin/Promise.cpp
js/src/tests/ecma_6/Promise/bug-1287334.js
--- a/js/src/builtin/Promise.cpp
+++ b/js/src/builtin/Promise.cpp
@@ -626,22 +626,32 @@ EnqueuePromiseReactionJob(JSContext* cx,
         return false;
 
     // Store the handler and the data array on the reaction job.
     job->setExtendedSlot(ReactionJobSlot_Handler, handler);
     job->setExtendedSlot(ReactionJobSlot_JobData, dataVal);
 
     // When using JS::AddPromiseReactions, no actual promise is created, so we
     // might not have one here.
-    // If we do, Wrap it in case we entered the handler's compartment above,
-    // because we should pass objects from a single compartment to the
-    // enqueuePromiseJob callback.
-    RootedObject promise(cx, promise_);
-    if (!cx->compartment()->wrap(cx, &promise))
-        return false;
+    // Additionally, we might have an object here that isn't an instance of
+    // Promise. This can happen if content overrides the value of
+    // Promise[@@species] (or invokes Promise#then on a Promise subclass
+    // instance with a non-default @@species value on the constructor) with a
+    // function that returns objects that're not Promise (subclass) instances.
+    // In that case, we just pretend we didn't have an object in the first
+    // place.
+    // If after all this we do have an object, wrap it in case we entered the
+    // handler's compartment above, because we should pass objects from a
+    // single compartment to the enqueuePromiseJob callback.
+    RootedObject promise(cx);
+    if (promise_->is<PromiseObject>()) {
+      promise = promise_;
+      if (!cx->compartment()->wrap(cx, &promise))
+          return false;
+    }
 
     // Using objectFromIncumbentGlobal, we can derive the incumbent global by
     // unwrapping and then getting the global. This is very convoluted, but
     // much better than having to store the original global as a private value
     // because we couldn't wrap it to store it as a normal JS value.
     RootedObject global(cx);
     RootedObject objectFromIncumbentGlobal(cx, objectFromIncumbentGlobal_);
     if (objectFromIncumbentGlobal) {
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/Promise/bug-1287334.js
@@ -0,0 +1,12 @@
+if (!this.Promise) {
+    this.reportCompare && reportCompare(true, true);
+    quit(0);
+}
+
+var promise = Promise.resolve(1);
+var FakeCtor = function(exec){ exec(function(){}, function(){}); };
+Object.defineProperty(Promise, Symbol.species, {value: FakeCtor});
+// This just shouldn't crash. It does without bug 1287334 fixed.
+promise.then(function(){});
+
+this.reportCompare && reportCompare(true, true);