Don't take iterable path in ArrayFrom if items[@@iterator] is null

BUG=v8:3833
LOG=N
R=arv@chromium.org

Review URL: https://codereview.chromium.org/856303002

Cr-Commit-Position: refs/heads/master@{#26314}

src/harmony-array.js

@@ -142,7 +142,7 @@ function ArrayFrom(arrayLike, mapfn, receiver) {
     }
   }
 
-  var iterable = ToIterable(items);
+  var iterable = GetMethod(items, symbolIterator);
   var k;
   var result;
   var mappedValue;
@@ -151,23 +151,40 @@ function ArrayFrom(arrayLike, mapfn, receiver) {
   if (!IS_UNDEFINED(iterable)) {
     result = %IsConstructor(this) ? new this() : [];
 
+    var iterator = GetIterator(items, iterable);
+
     k = 0;
-    for (nextValue of items) {
-      if (mapping) mappedValue = %_CallFunction(receiver, nextValue, k, mapfn);
-      else mappedValue = nextValue;
+    while (true) {
+      var next = iterator.next();
+
+      if (!IS_OBJECT(next)) {
+        throw MakeTypeError("iterator_result_not_an_object", [next]);
+      }
+
+      if (next.done) {
+        result.length = k;
+        return result;
+      }
+
+      nextValue = next.value;
+      if (mapping) {
+        mappedValue = %_CallFunction(receiver, nextValue, k, mapfn);
+      } else {
+        mappedValue = nextValue;
+      }
       %AddElement(result, k++, mappedValue, NONE);
     }
-
-    result.length = k;
-    return result;
   } else {
     var len = ToLength(items.length);
     result = %IsConstructor(this) ? new this(len) : new $Array(len);
 
     for (k = 0; k < len; ++k) {
       nextValue = items[k];
-      if (mapping) mappedValue = %_CallFunction(receiver, nextValue, k, mapfn);
-      else mappedValue = nextValue;
+      if (mapping) {
+        mappedValue = %_CallFunction(receiver, nextValue, k, mapfn);
+      } else {
+        mappedValue = nextValue;
+      }
       %AddElement(result, k, mappedValue, NONE);
     }
 

src/v8natives.js

@@ -654,6 +654,15 @@ function Delete(obj, p, should_throw) {
 }
 
 
+// ES6, draft 12-24-14, section 7.3.8
+function GetMethod(obj, p) {
+  var func = obj[p];
+  if (IS_NULL_OR_UNDEFINED(func)) return UNDEFINED;
+  if (IS_SPEC_FUNCTION(func)) return func;
+  throw MakeTypeError('called_non_callable', [typeof func]);
+}
+
+
 // Harmony proxies.
 function DefineProxyProperty(obj, p, attributes, should_throw) {
   // TODO(rossberg): adjust once there is a story for symbols vs proxies.

test/mjsunit/harmony/array-from.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --harmony-arrays --harmony-generators
+// Flags: --harmony-arrays
 (function() {
 
 assertEquals(1, Array.from.length);
@@ -91,21 +91,48 @@ function testArrayFrom(thisArg, constructor) {
     return x.toUpperCase();
   }), ['T', 'E', 'S', 'T'], constructor);
 
-  this.thisArg = thisArg;
-  assertThrows('Array.from.call(thisArg, null)', TypeError);
-  assertThrows('Array.from.call(thisArg, undefined)', TypeError);
-  assertThrows('Array.from.call(thisArg, [], null)', TypeError);
-  assertThrows('Array.from.call(thisArg, [], "noncallable")', TypeError);
+  assertThrows(function() { Array.from.call(thisArg, null); }, TypeError);
+  assertThrows(function() { Array.from.call(thisArg, undefined); }, TypeError);
+  assertThrows(function() { Array.from.call(thisArg, [], null); }, TypeError);
+  assertThrows(function() { Array.from.call(thisArg, [], "noncallable"); },
+               TypeError);
 
-  this.nullIterator = {};
+  var nullIterator = {};
   nullIterator[Symbol.iterator] = null;
-  assertThrows('Array.from.call(thisArg, nullIterator)', TypeError);
+  assertArrayLikeEquals(Array.from.call(thisArg, nullIterator), [],
+                        constructor);
 
-  this.nonObjIterator = {};
+  var nonObjIterator = {};
   nonObjIterator[Symbol.iterator] = function() { return "nonObject"; };
-  assertThrows('Array.from.call(thisArg, nonObjIterator)', TypeError);
-
-  assertThrows('Array.from.call(thisArg, [], null)', TypeError);
+  assertThrows(function() { Array.from.call(thisArg, nonObjIterator); },
+               TypeError);
+
+  assertThrows(function() { Array.from.call(thisArg, [], null); }, TypeError);
+
+  // Ensure iterator is only accessed once, and only invoked once
+  var called = false;
+  var arr = [1, 2, 3];
+  var obj = {};
+
+  // Test order --- only get iterator method once
+  function testIterator() {
+    assertFalse(called, "@@iterator should be called only once");
+    called = true;
+    assertEquals(obj, this);
+    return arr[Symbol.iterator]();
+  }
+  var getCalled = false;
+  Object.defineProperty(obj, Symbol.iterator, {
+    get: function() {
+      assertFalse(getCalled, "@@iterator should be accessed only once");
+      getCalled = true;
+      return testIterator;
+    },
+    set: function() {
+      assertUnreachable("@@iterator should not be set");
+    }
+  });
+  assertArrayLikeEquals(Array.from.call(thisArg, obj), [1, 2, 3], constructor);
 }
 
 function Other() {}