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 290119 32a6f67bd8e8f30097e92ef2527446656b31eee1
parent 290118 34046c232ee1f7324ac586cf0b72374764d8f513
child 290120 0dc03a76f51bd963ea4f5a6cfdfbb081efb81ef5
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstill
bugs1256342
milestone48.0a1
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(),