Bug 1287334 - Properly handle non-Promise objects as dependent promise objects in js::EnqueuePromiseReactionJob. r=efaust
☠☠ backed out by 8ccb505a1cfc ☠ ☠
authorTill Schneidereit <till@tillschneidereit.net>
Wed, 20 Jul 2016 23:25:48 +0200
changeset 345978 ff1ffc0835711a1ab930ad1477e0ad5635cf7099
parent 345977 59409d62be6199125942fad9f5a1250f9dc3cd25
child 345979 ad246624295ef37f633bed2bd2500690dbf3c7c2
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,7 @@
+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);