Bug 1459042: Handle cross-compartment wrappers for async iterator objects. r=jorendorff
authorAndré Bargull <andre.bargull@gmail.com>
Mon, 18 Nov 2019 11:15:47 +0000
changeset 502425 6ddf558231ebee00b3b0828d343e7115c4e975f7
parent 502424 d25c3e100157884a8f83aab9b563e7f0e0316875
child 502426 edb4de0b8fdc194a95c837eaec8bc53b1df1a08d
push id100828
push usercbrindusan@mozilla.com
push dateMon, 18 Nov 2019 13:30:04 +0000
treeherderautoland@6ddf558231eb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs1459042
milestone72.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 1459042: Handle cross-compartment wrappers for async iterator objects. r=jorendorff Differential Revision: https://phabricator.services.mozilla.com/D52366
js/src/builtin/Promise.cpp
js/src/tests/non262/AsyncGenerators/cross-compartment.js
--- a/js/src/builtin/Promise.cpp
+++ b/js/src/builtin/Promise.cpp
@@ -4476,26 +4476,26 @@ static MOZ_MUST_USE bool AsyncGeneratorR
 // 25.5.3.6 AsyncGeneratorEnqueue ( generator, completion )
 MOZ_MUST_USE bool js::AsyncGeneratorEnqueue(JSContext* cx,
                                             HandleValue asyncGenVal,
                                             CompletionKind completionKind,
                                             HandleValue completionValue,
                                             MutableHandleValue result) {
   // Step 1 (implicit).
 
-  // Step 2.
-  Rooted<PromiseObject*> resultPromise(
-      cx, CreatePromiseObjectForAsyncGenerator(cx));
-  if (!resultPromise) {
-    return false;
-  }
-
   // Step 3.
   if (!asyncGenVal.isObject() ||
-      !asyncGenVal.toObject().is<AsyncGeneratorObject>()) {
+      !asyncGenVal.toObject().canUnwrapAs<AsyncGeneratorObject>()) {
+    // Step 2.
+    Rooted<PromiseObject*> resultPromise(
+        cx, CreatePromiseObjectForAsyncGenerator(cx));
+    if (!resultPromise) {
+      return false;
+    }
+
     // Step 3.a.
     RootedValue badGeneratorError(cx);
     if (!GetTypeError(cx, JSMSG_NOT_AN_ASYNC_GENERATOR, &badGeneratorError)) {
       return false;
     }
 
     // Step 3.b.
     if (!RejectPromiseInternal(cx, resultPromise, badGeneratorError)) {
@@ -4503,42 +4503,71 @@ MOZ_MUST_USE bool js::AsyncGeneratorEnqu
     }
 
     // Step 3.c.
     result.setObject(*resultPromise);
     return true;
   }
 
   Rooted<AsyncGeneratorObject*> asyncGenObj(
-      cx, &asyncGenVal.toObject().as<AsyncGeneratorObject>());
-
-  // Step 5 (reordered).
-  Rooted<AsyncGeneratorRequest*> request(
-      cx, AsyncGeneratorObject::createRequest(cx, asyncGenObj, completionKind,
-                                              completionValue, resultPromise));
-  if (!request) {
-    return false;
-  }
-
-  // Steps 4, 6.
-  if (!AsyncGeneratorObject::enqueueRequest(cx, asyncGenObj, request)) {
-    return false;
-  }
-
-  // Step 7.
-  if (!asyncGenObj->isExecuting() && !asyncGenObj->isAwaitingYieldReturn()) {
-    // Step 8.
-    if (!AsyncGeneratorResumeNext(cx, asyncGenObj, ResumeNextKind::Enqueue)) {
+      cx, &asyncGenVal.toObject().unwrapAs<AsyncGeneratorObject>());
+
+  bool wrapResult = false;
+  {
+    // The |resultPromise| must be same-compartment with |asyncGenObj|, because
+    // it is stored in AsyncGeneratorRequest, which in turn is stored in a
+    // reserved slot of |asyncGenObj|.
+    // So we first enter the realm of |asyncGenObj|, then create the result
+    // promise and resume the generator, and finally wrap the result promise to
+    // match the original compartment.
+
+    mozilla::Maybe<AutoRealm> ar;
+    RootedValue completionVal(cx, completionValue);
+    if (asyncGenObj->compartment() != cx->compartment()) {
+      ar.emplace(cx, asyncGenObj);
+      wrapResult = true;
+
+      if (!cx->compartment()->wrap(cx, &completionVal)) {
+        return false;
+      }
+    }
+
+    // Step 2.
+    Rooted<PromiseObject*> resultPromise(
+        cx, CreatePromiseObjectForAsyncGenerator(cx));
+    if (!resultPromise) {
       return false;
     }
-  }
-
-  // Step 9.
-  result.setObject(*resultPromise);
-  return true;
+
+    // Step 5 (reordered).
+    Rooted<AsyncGeneratorRequest*> request(
+        cx, AsyncGeneratorObject::createRequest(cx, asyncGenObj, completionKind,
+                                                completionVal, resultPromise));
+    if (!request) {
+      return false;
+    }
+
+    // Steps 4, 6.
+    if (!AsyncGeneratorObject::enqueueRequest(cx, asyncGenObj, request)) {
+      return false;
+    }
+
+    // Step 7.
+    if (!asyncGenObj->isExecuting() && !asyncGenObj->isAwaitingYieldReturn()) {
+      // Step 8.
+      if (!AsyncGeneratorResumeNext(cx, asyncGenObj, ResumeNextKind::Enqueue)) {
+        return false;
+      }
+    }
+
+    // Step 9.
+    result.setObject(*resultPromise);
+  }
+
+  return !wrapResult || cx->compartment()->wrap(cx, result);
 }
 
 static bool Promise_catch_impl(JSContext* cx, unsigned argc, Value* vp,
                                bool rvalUsed) {
   CallArgs args = CallArgsFromVp(argc, vp);
 
   HandleValue thisVal = args.thisv();
   HandleValue onFulfilled = UndefinedHandleValue;
new file mode 100644
--- /dev/null
+++ b/js/src/tests/non262/AsyncGenerators/cross-compartment.js
@@ -0,0 +1,90 @@
+var g = newGlobal();
+g.mainGlobal = this;
+
+if (typeof isSameCompartment !== "function") {
+  var isSameCompartment = SpecialPowers.Cu.getJSTestingFunctions().isSameCompartment;
+}
+
+var next = async function*(){}.prototype.next;
+
+var f = g.eval(`(async function*() {
+  var x = yield {message: "yield"};
+
+  // Input completion values are correctly wrapped into |f|'s compartment.
+  assertEq(isSameCompartment(x, mainGlobal), true);
+  assertEq(x.message, "continue");
+
+  return {message: "return"};
+})`);
+
+var it = f();
+
+// The async iterator is same-compartment with |f|.
+assertEq(isSameCompartment(it, f), true);
+
+var p1 = next.call(it, {message: "initial yield"});
+
+// The promise object is same-compartment with |f|.
+assertEq(isSameCompartment(p1, f), true);
+
+// Note: This doesn't follow the spec, which requires that only |p1 instanceof Promise| is true.
+assertEq(p1 instanceof Promise || p1 instanceof g.Promise, true);
+
+p1.then(v => {
+  // The iterator result object is same-compartment with |f|.
+  assertEq(isSameCompartment(v, f), true);
+  assertEq(v.done, false);
+
+  assertEq(isSameCompartment(v.value, f), true);
+  assertEq(v.value.message, "yield");
+});
+
+var p2 = next.call(it, {message: "continue"});
+
+// The promise object is same-compartment with |f|.
+assertEq(isSameCompartment(p2, f), true);
+
+// Note: This doesn't follow the spec, which requires that only |p2 instanceof Promise| is true.
+assertEq(p2 instanceof Promise || p2 instanceof g.Promise, true);
+
+p2.then(v => {
+  // The iterator result object is same-compartment with |f|.
+  assertEq(isSameCompartment(v, f), true);
+  assertEq(v.done, true);
+
+  assertEq(isSameCompartment(v.value, f), true);
+  assertEq(v.value.message, "return");
+});
+
+var p3 = next.call(it, {message: "already finished"});
+
+// The promise object is same-compartment with |f|.
+assertEq(isSameCompartment(p3, f), true);
+
+// Note: This doesn't follow the spec, which requires that only |p3 instanceof Promise| is true.
+assertEq(p3 instanceof Promise || p3 instanceof g.Promise, true);
+
+p3.then(v => {
+  // The iterator result object is same-compartment with |f|.
+  assertEq(isSameCompartment(v, f), true);
+  assertEq(v.done, true);
+  assertEq(v.value, undefined);
+});
+
+var p4 = next.call({}, {message: "bad |this| argument"});
+
+// The promise object is same-compartment with |next|.
+assertEq(isSameCompartment(p4, next), true);
+
+// Only in this case we're following the spec and are creating the promise object
+// in the correct realm.
+assertEq(p4 instanceof Promise, true);
+
+p4.then(() => {
+  throw new Error("expected a TypeError");
+}, e => {
+  assertEq(e instanceof TypeError, true);
+});
+
+if (typeof reportCompare === "function")
+  reportCompare(0, 0);