Bug 1111170. Make ArrayIterator and StringIterator next() methods work even with cross-compartment wrappers for those objects as this values. r=waldo
authorBoris Zbarsky <bzbarsky@mit.edu>
Sat, 13 Dec 2014 01:25:25 -0500
changeset 219908 5e513880db47fb34486a946963775b7a3f7684c5
parent 219877 b2649ad7d86e7278268fb33b05911e4864850bf3
child 219909 d1510bd5657793434031489c5c342a2980c0808c
push id27972
push userryanvm@gmail.com
push dateTue, 16 Dec 2014 20:14:14 +0000
treeherdermozilla-central@473ecad73b44 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerswaldo
bugs1111170
milestone37.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 1111170. Make ArrayIterator and StringIterator next() methods work even with cross-compartment wrappers for those objects as this values. r=waldo
js/src/builtin/Array.js
js/src/builtin/String.js
js/src/jit-test/tests/for-of/next-3.js
js/src/tests/ecma_6/Array/iterator_edge_cases.js
js/src/tests/ecma_6/String/iterator_edge_cases.js
js/src/vm/SelfHosting.cpp
--- a/js/src/builtin/Array.js
+++ b/js/src/builtin/Array.js
@@ -646,19 +646,20 @@ function CreateArrayIterator(obj, kind) 
     return CreateArrayIteratorAt(obj, kind, 0);
 }
 
 function ArrayIteratorIdentity() {
     return this;
 }
 
 function ArrayIteratorNext() {
-    // FIXME: Cross-compartment wrapper ArrayIterator objects should pass this test.  Bug 1111170.
-    if (!IsObject(this) || !IsArrayIterator(this))
-        ThrowError(JSMSG_INCOMPATIBLE_METHOD, "ArrayIterator", "next", ToString(this));
+    if (!IsObject(this) || !IsArrayIterator(this)) {
+        return callFunction(CallArrayIteratorMethodIfWrapped, this,
+                            "ArrayIteratorNext");
+    }
 
     var a = UnsafeGetReservedSlot(this, ARRAY_ITERATOR_SLOT_ITERATED_OBJECT);
     var index = UnsafeGetReservedSlot(this, ARRAY_ITERATOR_SLOT_NEXT_INDEX);
     var itemKind = UnsafeGetReservedSlot(this, ARRAY_ITERATOR_SLOT_ITEM_KIND);
     var result = { value: undefined, done: false };
 
     // FIXME: This should be ToLength, which clamps at 2**53.  Bug 924058.
     if (index >= TO_UINT32(a.length)) {
--- a/js/src/builtin/String.js
+++ b/js/src/builtin/String.js
@@ -202,19 +202,20 @@ function String_iterator() {
     return iterator;
 }
 
 function StringIteratorIdentity() {
     return this;
 }
 
 function StringIteratorNext() {
-    // FIXME: Cross-compartment wrapper StringIterator objects should pass this test.  Bug 1111170.
-    if (!IsObject(this) || !IsStringIterator(this))
-        ThrowError(JSMSG_INCOMPATIBLE_METHOD, "StringIterator", "next", ToString(this));
+    if (!IsObject(this) || !IsStringIterator(this)) {
+        return callFunction(CallStringIteratorMethodIfWrapped, this,
+                            "StringIteratorNext");
+    }
 
     var S = UnsafeGetReservedSlot(this, STRING_ITERATOR_SLOT_ITERATED_OBJECT);
     var index = UnsafeGetReservedSlot(this, STRING_ITERATOR_SLOT_NEXT_INDEX);
     var size = S.length;
     var result = { value: undefined, done: false };
 
     if (index >= size) {
         result.done = true;
--- a/js/src/jit-test/tests/for-of/next-3.js
+++ b/js/src/jit-test/tests/for-of/next-3.js
@@ -1,13 +1,10 @@
-// Iterators from another compartment work with their own .next method
-// when called from another compartment, but not with the other
-// compartment's .next method.
-
-// FIXME: 'next' should work cross-realm.  Bug 924059.
+// Iterators from another compartment work with both their own .next method
+// with the other compartment's .next method.
 
 load(libdir + "asserts.js");
 load(libdir + "iteration.js");
 
 var g = newGlobal();
 g.eval(`var it = [1, 2][${uneval(std_iterator)}]();`);
 assertIteratorNext(g.it, 1);
-assertThrowsInstanceOf([][std_iterator]().next.bind(g.it), TypeError)
+assertDeepEq([][std_iterator]().next.call(g.it), { value: 2, done: false })
--- a/js/src/tests/ecma_6/Array/iterator_edge_cases.js
+++ b/js/src/tests/ecma_6/Array/iterator_edge_cases.js
@@ -2,15 +2,26 @@
 // ArrayIterator object.
 function TestArrayIteratorPrototypeConfusion() {
     var iter = [][Symbol.iterator]();
     try {
         iter.next.call(Object.getPrototypeOf(iter))
         throw new Error("Call did not throw");
     } catch (e) {
         assertEq(e instanceof TypeError, true);
-        assertEq(e.message, "ArrayIterator next called on incompatible [object Array Iterator]");
+        assertEq(e.message, "CallArrayIteratorMethodIfWrapped method called on incompatible Array Iterator");
     }
 }
 TestArrayIteratorPrototypeConfusion();
 
+// Tests that we can use %ArrayIteratorPrototype%.next on a
+// cross-compartment iterator.
+function TestArrayIteratorWrappers() {
+    var iter = [][Symbol.iterator]();
+    assertDeepEq(iter.next.call(newGlobal().eval('[5][Symbol.iterator]()')),
+		 { value: 5, done: false })
+}
+if (typeof newGlobal === "function") {
+    TestArrayIteratorWrappers();
+}
+
 if (typeof reportCompare === "function")
   reportCompare(true, true);
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/String/iterator_edge_cases.js
@@ -0,0 +1,27 @@
+// Test that we can't confuse %StringIteratorPrototype% for a
+// StringIterator object.
+function TestStringIteratorPrototypeConfusion() {
+    var iter = ""[Symbol.iterator]();
+    try {
+        iter.next.call(Object.getPrototypeOf(iter))
+        throw new Error("Call did not throw");
+    } catch (e) {
+        assertEq(e instanceof TypeError, true);
+        assertEq(e.message, "CallStringIteratorMethodIfWrapped method called on incompatible String Iterator");
+    }
+}
+TestStringIteratorPrototypeConfusion();
+
+// Tests that we can use %StringIteratorPrototype%.next on a
+// cross-compartment iterator.
+function TestStringIteratorWrappers() {
+    var iter = ""[Symbol.iterator]();
+    assertDeepEq(iter.next.call(newGlobal().eval('"x"[Symbol.iterator]()')),
+		 { value: "x", done: false })
+}
+if (typeof newGlobal === "function") {
+    TestStringIteratorWrappers();
+}
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);
--- a/js/src/vm/SelfHosting.cpp
+++ b/js/src/vm/SelfHosting.cpp
@@ -1097,19 +1097,24 @@ static const JSFunctionSpec intrinsic_fu
     JS_FN("UnsafeGetReservedSlot",   intrinsic_UnsafeGetReservedSlot,   2,0),
     JS_FN("HaveSameClass",           intrinsic_HaveSameClass,           2,0),
     JS_FN("IsPackedArray",           intrinsic_IsPackedArray,           1,0),
 
     JS_FN("GetIteratorPrototype",    intrinsic_GetIteratorPrototype,    0,0),
 
     JS_FN("NewArrayIterator",        intrinsic_NewArrayIterator,        0,0),
     JS_FN("IsArrayIterator",         intrinsic_IsArrayIterator,         1,0),
+    JS_FN("CallArrayIteratorMethodIfWrapped",
+          CallNonGenericSelfhostedMethod<Is<ArrayIteratorObject>>,      2,0),
+
 
     JS_FN("NewStringIterator",       intrinsic_NewStringIterator,       0,0),
     JS_FN("IsStringIterator",        intrinsic_IsStringIterator,        1,0),
+    JS_FN("CallStringIteratorMethodIfWrapped",
+          CallNonGenericSelfhostedMethod<Is<StringIteratorObject>>,     2,0),
 
     JS_FN("IsStarGeneratorObject",   intrinsic_IsStarGeneratorObject,   1,0),
     JS_FN("StarGeneratorObjectIsClosed", intrinsic_StarGeneratorObjectIsClosed, 1,0),
     JS_FN("IsSuspendedStarGenerator",intrinsic_IsSuspendedStarGenerator,1,0),
 
     JS_FN("IsLegacyGeneratorObject", intrinsic_IsLegacyGeneratorObject, 1,0),
     JS_FN("LegacyGeneratorObjectIsClosed", intrinsic_LegacyGeneratorObjectIsClosed, 1,0),
     JS_FN("CloseClosingLegacyGeneratorObject", intrinsic_CloseClosingLegacyGeneratorObject, 1,0),