Bug 1256342. Fix typed array iteration to work correctly over Xrays. r=till
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 22 Mar 2016 13:49:36 -0400
changeset 290060 32a6f67bd8e8f30097e92ef2527446656b31eee1
parent 290059 34046c232ee1f7324ac586cf0b72374764d8f513
child 290061 0dc03a76f51bd963ea4f5a6cfdfbb081efb81ef5
push id30114
push usercbook@mozilla.com
push dateThu, 24 Mar 2016 15:15:54 +0000
treeherdermozilla-central@24c5fbde4488 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstill
bugs1256342
milestone48.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 1256342. Fix typed array iteration to work correctly over Xrays. r=till
js/src/builtin/TypedArray.js
js/src/tests/ecma_6/TypedArray/entries.js
js/xpconnect/tests/chrome/test_xrayToJS.xul
--- a/js/src/builtin/TypedArray.js
+++ b/js/src/builtin/TypedArray.js
@@ -114,24 +114,37 @@ function TypedArrayCopyWithin(target, st
     return obj;
 }
 
 // ES6 draft rev30 (2014/12/24) 22.2.3.6 %TypedArray%.prototype.entries()
 function TypedArrayEntries() {
     // Step 1.
     var O = this;
 
+    // We need to be a bit careful here, because in the Xray case we want to
+    // create the iterator in our current compartment.
+    //
+    // Before doing that, though, we want to check that we have a typed
+    // array and it does not have a detached array buffer.  We do the latter by
+    // just calling GetAttachedArrayBuffer() and letting it throw if there isn't
+    // one.  In the case when we're not sure we have a typed array (e.g. we
+    // might have a cross-compartment wrapper for one), we can go ahead and call
+    // GetAttachedArrayBuffer via CallTypedArrayMethodIfWrapped; that will throw
+    // if we're not actually a wrapped typed array, or if we have a detached
+    // array buffer.
+
     // Step 2-3.
     if (!IsObject(O) || !IsTypedArray(O)) {
-        return callFunction(CallTypedArrayMethodIfWrapped, O, "TypedArrayEntries");
+        // And also step 4-6.
+        callFunction(CallTypedArrayMethodIfWrapped, O, O, "GetAttachedArrayBuffer");
+    } else {
+        // Step 4-6.
+        GetAttachedArrayBuffer(O);
     }
 
-    // Step 4-6.
-    GetAttachedArrayBuffer(O);
-
     // Step 7.
     return CreateArrayIterator(O, ITEM_KIND_KEY_AND_VALUE);
 }
 
 // ES6 draft rev30 (2014/12/24) 22.2.3.7 %TypedArray%.prototype.every(callbackfn[, thisArg]).
 function TypedArrayEvery(callbackfn, thisArg = undefined) {
     // This function is not generic.
     if (!IsObject(this) || !IsTypedArray(this)) {
@@ -495,22 +508,23 @@ function TypedArrayJoin(separator) {
     return R;
 }
 
 // ES6 draft (2016/1/11) 22.2.3.15 %TypedArray%.prototype.keys()
 function TypedArrayKeys() {
     // Step 1.
     var O = this;
 
+    // See the big comment in TypedArrayEntries for what we're doing here.
+
     // Step 2.
-    if (!IsObject(O) || !IsTypedArray(O)) {
-        return callFunction(CallTypedArrayMethodIfWrapped, O, "TypedArrayKeys");
-    }
-
-    GetAttachedArrayBuffer(O);
+    if (!IsObject(O) || !IsTypedArray(O))
+        callFunction(CallTypedArrayMethodIfWrapped, O, O, "GetAttachedArrayBuffer");
+    else
+        GetAttachedArrayBuffer(O);
 
     // Step 3.
     return CreateArrayIterator(O, ITEM_KIND_KEY);
 }
 
 // ES6 draft rev29 (2014/12/06) 22.2.3.16 %TypedArray%.prototype.lastIndexOf(searchElement [,fromIndex]).
 function TypedArrayLastIndexOf(searchElement, fromIndex = undefined) {
     // This function is not generic.
@@ -1069,24 +1083,27 @@ function TypedArraySubarray(begin, end) 
     return new constructor(buffer, beginByteOffset, newLength);
 }
 
 // ES6 draft rev30 (2014/12/24) 22.2.3.30 %TypedArray%.prototype.values()
 function TypedArrayValues() {
     // Step 1.
     var O = this;
 
-    // Step 2-3.
+    // See the big comment in TypedArrayEntries for what we're doing here.
+
+    // Steps 2-6.
     if (!IsObject(O) || !IsTypedArray(O)) {
-        return callFunction(CallTypedArrayMethodIfWrapped, O, "TypedArrayValues");
+        // And also steps 4-6.
+        callFunction(CallTypedArrayMethodIfWrapped, O, O, "GetAttachedArrayBuffer");
+    } else {
+        // Steps 4-6.
+        GetAttachedArrayBuffer(O);
     }
 
-    // Step 4-6.
-    GetAttachedArrayBuffer(O);
-
     // Step 7.
     return CreateArrayIterator(O, ITEM_KIND_VALUE);
 }
 _SetCanonicalName(TypedArrayValues, "values");
 
 // Proposed for ES7:
 // https://github.com/tc39/Array.prototype.includes/blob/7c023c19a0/spec.md
 function TypedArrayIncludes(searchElement, fromIndex = 0) {
--- a/js/src/tests/ecma_6/TypedArray/entries.js
+++ b/js/src/tests/ecma_6/TypedArray/entries.js
@@ -32,18 +32,20 @@ for (var constructor of constructors) {
     var iterator = arr.entries();
     assertDeepEq(iterator.next(), {value: [0, 1], done: false});
     assertDeepEq(iterator.next(), {value: [1, 2], done: false});
     assertDeepEq(iterator.next(), {value: [2, 3], done: false});
     assertDeepEq(iterator.next(), {value: undefined, done: true});
 
     // Called from other globals.
     if (typeof newGlobal === "function" && !isSharedConstructor(constructor)) {
-        var entries = newGlobal()[constructor.name].prototype.entries;
-        assertDeepEq([...entries.call(new constructor(2))], [[0, 0], [1, 0]]);
+        var otherGlobal = newGlobal();
+        var entries = otherGlobal[constructor.name].prototype.entries;
+        assertDeepEq([...entries.call(new constructor(2))],
+                     [new otherGlobal.Array(0, 0), new otherGlobal.Array(1, 0)]);
         arr = new (newGlobal()[constructor.name])(2);
         assertEq([...constructor.prototype.entries.call(arr)].toString(), "0,0,1,0");
     }
 
     // Throws if `this` isn't a TypedArray.
     var invalidReceivers = [undefined, null, 1, false, "", Symbol(), [], {}, /./,
                             new Proxy(new constructor(), {})];
     invalidReceivers.forEach(invalidReceiver => {
--- a/js/xpconnect/tests/chrome/test_xrayToJS.xul
+++ b/js/xpconnect/tests/chrome/test_xrayToJS.xul
@@ -409,16 +409,44 @@ https://bugzilla.mozilla.org/show_bug.cg
     testCtorCallables(ctorCallables, xrayCtor, localCtor);
     is(Object.getOwnPropertyNames(xrayCtor).sort().toSource(),
        ctorProps.toSource(), "getOwnPropertyNames works on Xrayed ctors");
     is(Object.getOwnPropertySymbols(xrayCtor).map(uneval).sort().toSource(),
        gConstructorProperties[classname].filter(id => typeof id !== "string").map(uneval).sort().toSource(),
        "getOwnPropertySymbols works on Xrayed ctors");
   }
 
+  // We will need arraysEqual and testArrayIterators both in this global scope
+  // and in sandboxes, so define them as strings up front.
+  var arraysEqualSource = `function arraysEqual(arr1, arr2, reason) {
+    is(arr1.length, arr2.length, \`\${reason}; lengths should be equal\`)
+    for (var i = 0; i < arr1.length; ++i) {
+      if (Array.isArray(arr2[i])) {
+        arraysEqual(arr1[i], arr2[i], \`\${reason}; item at index \${i}\`);
+      } else {
+        is(arr1[i], arr2[i], \`\${reason}; item at index \${i} should be equal\`);
+      }
+    }
+  }`;
+  eval(arraysEqualSource);
+
+  var testArrayIteratorsSource = `
+    function testArrayIterators(arrayLike, equivalentArray, reason) {
+    arraysEqual([...arrayLike], equivalentArray, \`\${reason}; spread operator\`);
+    arraysEqual([...arrayLike.entries()], [...equivalentArray.entries()],
+                \`\${reason}; entries\`);
+    arraysEqual([...arrayLike.keys()], [...equivalentArray.keys()],
+                \`\${reason}; keys\`);
+    if (arrayLike.values) {
+      arraysEqual([...arrayLike.values()], equivalentArray,
+                  \`\${reason}; values\`);
+    }
+  }`;
+  eval(testArrayIteratorsSource);
+
   function testDate() {
     // toGMTString is handled oddly in the engine. We don't bother to support
     // it over Xrays.
     let propsToSkip = ['toGMTString'];
 
     testXray('Date', new iwin.Date(), new iwin.Date(), propsToSkip);
 
     // Test the self-hosted toLocaleString.
@@ -511,16 +539,18 @@ https://bugzilla.mozilla.org/show_bug.cg
     is(trickyArray.length, 0, "Setting length works over Xray");
     is(trickyArray[11], undefined, "Setting length truncates over Xray");
     Object.defineProperty(trickyArray, 'length', { configurable: false, enumerable: false, writable: false, value: 0 });
     trickyArray[1] = "hi";
     is(trickyArray.length, 0, "Length remains non-writable");
     is(trickyArray[1], undefined, "Frozen length forbids new properties");
 
     testTrickyObject(trickyArray);
+
+    testArrayIterators(new iwin.Array(1, 1, 2, 3, 5), [1, 1, 2, 3, 5]);
   }
 
   // Parts of this function are kind of specific to testing Object, but we factor
   // it out so that we can re-use the trickyObject stuff on Arrays.
   function testTrickyObject(trickyObject) {
 
     // Make sure it looks right under the hood.
     is(trickyObject.wrappedJSObject.getterProp, 2, "Underlying object has getter");
@@ -662,16 +692,29 @@ https://bugzilla.mozilla.org/show_bug.cg
       Object.defineProperty(t.wrappedJSObject, 'length', {value: 42});
       is(t.wrappedJSObject.length, 42, "Set tricky expando")
       is(t.length, 10, "Length accessor works over Xrays")
       is(t.byteLength, t.length * window[c].prototype.BYTES_PER_ELEMENT, "byteLength accessor works over Xrays")
 
       var xray = new iwin[c](0);
       var xrayTypedArrayProto = Object.getPrototypeOf(Object.getPrototypeOf(xray));
       testProtoCallables(inheritedCallables, new iwin[c](0), xrayTypedArrayProto, typedArrayProto);
+
+      // When testing iterators, make sure to do so from inside our web
+      // extension sandbox, since from chrome we can't poke their indices.  Note
+      // that we have to actually recreate our functions that touch typed array
+      // indices inside the sandbox, not just export them, because otherwise
+      // they'll just run with our principal anyway.
+      //
+      // But we do want to export is(), since we want ours called.
+      wesb.eval(arraysEqualSource);
+      wesb.eval(testArrayIteratorsSource);
+      Cu.exportFunction(is, wesb,
+                        { defineAs: "is" });
+      wesb.eval('testArrayIterators(t, [0, 0, 3, 0, 0, 0, 0, 0, 0, 0])');
     }
   }
 
   function testErrorObjects() {
     // We only invoke testXray with Error, because that function isn't set up
     // to deal with dependent classes and fixing it up is more trouble than
     // it's worth.
     testXray('Error', new iwin.Error('some error message'), new iwin.Error(),