Bug 904723, part 4 - In Array.from, only fetch the @@iterator property once. r=till.

js/src/builtin/Array.js

@@ -660,7 +660,7 @@ function ArrayKeys() {
     return CreateArrayIterator(this, ITEM_KIND_KEY);
 }
 
-/* ES6 rev 24 (2014 April 27) 22.1.2.1 */
+/* ES6 rev 25 (2014 May 22) 22.1.2.1 */
 function ArrayFrom(arrayLike, mapfn=undefined, thisArg=undefined) {
     // Step 1.
     var C = this;
@@ -677,23 +677,36 @@ function ArrayFrom(arrayLike, mapfn=undefined, thisArg=undefined) {
     var attrs = ATTR_CONFIGURABLE | ATTR_ENUMERABLE | ATTR_WRITABLE;
 
     // Steps 6-8.
-    if (items["@@iterator"] !== undefined) {
+    var usingIterator = items["@@iterator"];
+    if (usingIterator !== undefined) {
         // Steps 8.a-c.
         var A = IsConstructor(C) ? new C() : [];
 
+        // Steps 8.d-e.
+        var iterator = callFunction(usingIterator, items);
+
         // Step 8.f.
         var k = 0;
 
-        // Steps 8.d-e and 8.g.i-vi.
-        for (var nextValue of items) {
+        // Steps 8.g.i-vi.
+        // These steps cannot be implemented using a for-of loop.
+        // See <https://bugs.ecmascript.org/show_bug.cgi?id=2883>.
+        var next;
+        while (true) {
+            // Steps 8.g.ii-vi.
+            next = iterator.next();
+            if (!IsObject(next))
+                ThrowError(JSMSG_NEXT_RETURNED_PRIMITIVE);
+            if (next.done)
+                break;  // Substeps of 8.g.iv are implemented below.
+            var nextValue = next.value;
+
             // Steps 8.g.vii-viii.
             var mappedValue = mapping ? callFunction(mapfn, thisArg, nextValue, k) : nextValue;
 
             // Steps 8.g.ix-xi.
             _DefineDataProperty(A, k++, mappedValue, attrs);
         }
-
-        // Here we're at step 8.g.iv.1. Fall through; it's implemented below.
     } else {
         // Step 9 is an assertion: items is not an Iterator. Testing this is
         // literally the very last thing we did, so we don't assert here.

js/src/js.msg

@@ -357,7 +357,7 @@ MSG_DEF(JSMSG_ALREADY_HAS_PRAGMA, 303, 2, JSEXN_ERR,      "{0} is being assigned
 MSG_DEF(JSMSG_PAR_ARRAY_BAD_ARG,      304, 0, JSEXN_RANGEERR, "invalid parallel method argument")
 MSG_DEF(JSMSG_REGEXP_RUNTIME_ERROR,   305, 0, JSEXN_INTERNALERR, "an error occurred while executing regular expression")
 MSG_DEF(JSMSG_DEBUG_OPTIMIZED_OUT,    306, 0, JSEXN_ERR, "variable has been optimized out")
-MSG_DEF(JSMSG_UNUSED307,              307, 0, JSEXN_NONE, "")
+MSG_DEF(JSMSG_NEXT_RETURNED_PRIMITIVE,307, 0, JSEXN_TYPEERR, "iterator.next() returned a non-object value")
 MSG_DEF(JSMSG_PAR_ARRAY_SCATTER_CONFLICT, 308, 0, JSEXN_ERR, "no conflict resolution function provided")
 MSG_DEF(JSMSG_PAR_ARRAY_SCATTER_BOUNDS, 309, 0, JSEXN_ERR, "index in scatter vector out of bounds")
 MSG_DEF(JSMSG_CANT_REPORT_NC_AS_NE,   310, 0, JSEXN_TYPEERR, "proxy can't report a non-configurable own property as non-existent")

js/src/tests/ecma_6/Array/from_errors.js

@@ -130,5 +130,16 @@ assertThrowsValue(() => Array.from.call(C, arrayish, () => { throw exc; }), exc)
 assertEq(log, "lC0");
 assertEq(obj instanceof C, true);
 
+// It's a TypeError if the iterator's .next() method returns a primitive.
+for (var primitive of [undefined, null, 17]) {
+    assertThrowsInstanceOf(
+        () => Array.from({
+            "@@iterator": () => {
+                next: () => primitive
+            }
+        }),
+        TypeError);
+}
+
 if (typeof reportCompare === 'function')
     reportCompare(0, 0);

js/src/tests/ecma_6/Array/from_proxy.js

@@ -36,9 +36,17 @@ assertDeepEq(log, ["define 0", "define 1", "define 2", "set length"]);
 // calls handler.get on it.
 log = [];
 assertDeepEq(Array.from(new LoggingProxy([3, 4, 5])), [3, 4, 5]);
-assertDeepEq(log, ["get @@iterator", "get @@iterator",
+assertDeepEq(log, ["get @@iterator",
                    "get length", "get 0", "get length", "get 1", "get length", "get 2",
                    "get length"]);
 
+// Array-like iteration only gets the length once.
+log = [];
+var arr = [5, 6, 7];
+arr["@@iterator"] = undefined;
+assertDeepEq(Array.from(new LoggingProxy(arr)), [5, 6, 7]);
+assertDeepEq(log, ["get @@iterator",
+                   "get length", "get 0", "get 1", "get 2"]);
+
 if (typeof reportCompare === 'function')
     reportCompare(0, 0);