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

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

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

Committed: https://crrev.com/1f2e5973ebe2f4f9d4acca44129109e20b2109bc
Cr-Commit-Position: refs/heads/master@{#26314}

[caitp (gmail), 2015-01-21 07:44:35]

PTAL, should be a trivial fix

[arv (Not doing code reviews), 2015-01-21 15:05:18]

arv@chromium.org changed reviewers:
+ arv@chromium.org

[arv (Not doing code reviews), 2015-01-21 15:05:18]

https://codereview.chromium.org/856303002/diff/1/src/harmony-array.js
File src/harmony-array.js (right):

https://codereview.chromium.org/856303002/diff/1/src/harmony-array.js#newcode145
src/harmony-array.js:145: var iterable = GetMethod(items, symbolIterator);
We now call items.[[Get]](symbolIterator) twice. The spec mandates that it is
only called once. The spec makes me sad that we cannot use for-of loop here.

https://codereview.chromium.org/856303002/diff/1/src/v8natives.js
File src/v8natives.js (right):

https://codereview.chromium.org/856303002/diff/1/src/v8natives.js#newcode659
src/v8natives.js:659: // Assert: obj is not null or undefined
Remove?

https://codereview.chromium.org/856303002/diff/1/src/v8natives.js#newcode661
src/v8natives.js:661: if (IS_NULL_OR_UNDEFINED(func)) return;
return UNDEFINED;

for clarity?

https://codereview.chromium.org/856303002/diff/1/src/v8natives.js#newcode663
src/v8natives.js:663: throw MakeTypeError('called_non_callable', [typeof func]);
maybe update this to use the new better error message than typeof?

[caitp (gmail), 2015-01-21 17:21:49]

https://codereview.chromium.org/856303002/diff/1/src/harmony-array.js
File src/harmony-array.js (right):

https://codereview.chromium.org/856303002/diff/1/src/harmony-array.js#newcode145
src/harmony-array.js:145: var iterable = GetMethod(items, symbolIterator);
On 2015/01/21 15:05:18, arv wrote:
> We now call items.[[Get]](symbolIterator) twice. The spec mandates that it is
> only called once. The spec makes me sad that we cannot use for-of loop here.

hmm, well it's trivial to replace the for...of with a more explicit loop, but
that is a bit sad.

https://codereview.chromium.org/856303002/diff/1/src/v8natives.js
File src/v8natives.js (right):

https://codereview.chromium.org/856303002/diff/1/src/v8natives.js#newcode659
src/v8natives.js:659: // Assert: obj is not null or undefined
On 2015/01/21 15:05:18, arv wrote:
> Remove?

Acknowledged.

https://codereview.chromium.org/856303002/diff/1/src/v8natives.js#newcode661
src/v8natives.js:661: if (IS_NULL_OR_UNDEFINED(func)) return;
On 2015/01/21 15:05:18, arv wrote:
> return UNDEFINED;
> 
> for clarity?

Acknowledged.

https://codereview.chromium.org/856303002/diff/1/src/v8natives.js#newcode663
src/v8natives.js:663: throw MakeTypeError('called_non_callable', [typeof func]);
On 2015/01/21 15:05:18, arv wrote:
> maybe update this to use the new better error message than typeof?

I had a go at it, but it will end up implicating Array.from rather than the
property itself, which is kind of weird

[arv (Not doing code reviews), 2015-01-21 19:43:05]

Please add a test that counts the number of Get operations so that this does not
regress.

[caitp (gmail), 2015-01-22 19:02:22]

On 2015/01/21 19:43:05, arv wrote:
> Please add a test that counts the number of Get operations so that this does
not
> regress.

done, hows that?

[arv (Not doing code reviews), 2015-01-22 21:14:58]

LGTM with nits

https://codereview.chromium.org/856303002/diff/60001/test/mjsunit/harmony/arr...
File test/mjsunit/harmony/array-from.js (right):

https://codereview.chromium.org/856303002/diff/60001/test/mjsunit/harmony/arr...
test/mjsunit/harmony/array-from.js:114: var arr = [1,2,3];
ws after ,

https://codereview.chromium.org/856303002/diff/60001/test/mjsunit/harmony/arr...
test/mjsunit/harmony/array-from.js:119: if (!called) {
Maybe use assertFalse instead?

https://codereview.chromium.org/856303002/diff/60001/test/mjsunit/harmony/arr...
test/mjsunit/harmony/array-from.js:139:
assertArrayLikeEquals(Array.from.call(thisArg, obj), [1,2,3], constructor);
ws

[caitp (gmail), 2015-01-22 21:21:46]

The CQ bit was checked by caitpotter88@gmail.com

[commit-bot: I haz the power, 2015-01-22 21:26:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/856303002/80001

[commit-bot: I haz the power, 2015-01-22 21:28:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-01-22 21:28:42]

Try jobs failed on following builders:
  v8_presubmit on tryserver.v8
(http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/149)

[caitp (gmail), 2015-01-22 21:37:31]

The CQ bit was checked by caitpotter88@gmail.com

[commit-bot: I haz the power, 2015-01-22 21:38:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/856303002/80001

[commit-bot: I haz the power, 2015-01-22 21:40:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-01-22 21:40:42]

Try jobs failed on following builders:
  v8_presubmit on tryserver.v8
(http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/151)

[caitp (gmail), 2015-01-22 21:45:57]

On 2015/01/22 21:40:42, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   v8_presubmit on tryserver.v8
> (http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/151)

well, I don't get why presubmit is failing :<

[caitp (gmail), 2015-01-22 21:59:25]

Patchset #6 (id:100001) has been deleted

[caitp (gmail), 2015-01-22 22:05:48]

guess it needs an lgtm from an owner, okay.

[caitp (gmail), 2015-01-29 02:34:25]

The CQ bit was checked by caitpotter88@gmail.com

[commit-bot: I haz the power, 2015-01-29 02:34:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/856303002/80001

[commit-bot: I haz the power, 2015-01-29 02:36:11]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-01-29 02:36:31]

Patchset 5 (id:??) landed as
https://crrev.com/1f2e5973ebe2f4f9d4acca44129109e20b2109bc
Cr-Commit-Position: refs/heads/master@{#26314}