[typedarrays] Move %TypedArray%.prototype.indexOf to C++

[typedarrays] Move %TypedArray%.prototype.indexOf to C++

- Remove TypedArrayIndexOf in src/js/typedarray.js
- Implement it to C++ using the IndexOfValue in ElementsAccessor
- Add buffer neutering check also for %TypedArray%.prototype.includes

BUG=v8:5929
Review-Url: https://codereview.chromium.org/2733193002
Cr-Commit-Position: refs/heads/master@{#43741}
Committed: https://chromium.googlesource.com/v8/v8/+/b2efe57cdfb87698599c1135581b258ab6fd8aa5

[Choongwoo Han, 2017-03-07 05:50:01]

Description was changed from

==========
[typedarrays] Move %TypedArray%.prototype.indexOf to C++

- Remove TypedArrayIndexOf in src/js/typedarray.js
- Implement it to C++ using the IndexOfValue in ElementsAccessor

BUG=v8:5929
==========

to

==========
[typedarrays] Move %TypedArray%.prototype.indexOf to C++

- Remove TypedArrayIndexOf in src/js/typedarray.js
- Implement it to C++ using the IndexOfValue in ElementsAccessor

BUG=v8:5929
==========

[Choongwoo Han, 2017-03-07 05:50:01]

cwhan.tunz@gmail.com changed reviewers:
+ bmeurer@chromium.org, caitp@igalia.com, cbruni@chromium.org

[Choongwoo Han, 2017-03-07 05:52:22]

PTAL. very similar with TA.p.includes.

[Camillo Bruni, 2017-03-07 08:45:35]

Could you address my comment? Other than that looks good.

https://codereview.chromium.org/2733193002/diff/1/src/builtins/builtins-typed...
File src/builtins/builtins-typedarray.cc (right):

https://codereview.chromium.org/2733193002/diff/1/src/builtins/builtins-typed...
src/builtins/builtins-typedarray.cc:507: 
Could you do the IsNeutered check here? 
Ultimately I'd like to get rid of the check in the ElementsAccessor and assume
that all incoming arguments have been properly converted.
Also, ToInteger can have side-effects and neuter the typed array.

[Choongwoo Han, 2017-03-07 09:29:35]

https://codereview.chromium.org/2733193002/diff/1/src/builtins/builtins-typed...
File src/builtins/builtins-typedarray.cc (right):

https://codereview.chromium.org/2733193002/diff/1/src/builtins/builtins-typed...
src/builtins/builtins-typedarray.cc:507: 
On 2017/03/07 08:45:34, Camillo Bruni wrote:
> Could you do the IsNeutered check here? 
> Ultimately I'd like to get rid of the check in the ElementsAccessor and assume
> that all incoming arguments have been properly converted.
> Also, ToInteger can have side-effects and neuter the typed array.

Done. I also added neutering check for TA.p.includes.

[Choongwoo Han, 2017-03-07 09:30:48]

Description was changed from

==========
[typedarrays] Move %TypedArray%.prototype.indexOf to C++

- Remove TypedArrayIndexOf in src/js/typedarray.js
- Implement it to C++ using the IndexOfValue in ElementsAccessor

BUG=v8:5929
==========

to

==========
[typedarrays] Move %TypedArray%.prototype.indexOf to C++

- Remove TypedArrayIndexOf in src/js/typedarray.js
- Implement it to C++ using the IndexOfValue in ElementsAccessor
- Add buffer neutering check also for %TypedArray%.prototype.includes

BUG=v8:5929
==========

[Camillo Bruni, 2017-03-07 13:59:53]

lgtm

https://codereview.chromium.org/2733193002/diff/40001/src/builtins/builtins-t...
File src/builtins/builtins-typedarray.cc (right):

https://codereview.chromium.org/2733193002/diff/40001/src/builtins/builtins-t...
src/builtins/builtins-typedarray.cc:489: MAYBE_RETURN(result,
isolate->heap()->exception());
Technically not necessary, given that you can only call this builtin for
TypedArrays and the TypedElementsAccessor::IncludesValueImpl cannot throw.

FromJust will just hard crash in case of a pending failure.

[Choongwoo Han, 2017-03-07 14:04:48]

On 2017/03/07 13:59:53, Camillo Bruni wrote:
> lgtm
> 
>
https://codereview.chromium.org/2733193002/diff/40001/src/builtins/builtins-t...
> File src/builtins/builtins-typedarray.cc (right):
> 
>
https://codereview.chromium.org/2733193002/diff/40001/src/builtins/builtins-t...
> src/builtins/builtins-typedarray.cc:489: MAYBE_RETURN(result,
> isolate->heap()->exception());
> Technically not necessary, given that you can only call this builtin for
> TypedArrays and the TypedElementsAccessor::IncludesValueImpl cannot throw.
> 
> FromJust will just hard crash in case of a pending failure.

Yes I know the ElementsAccessor does not throw. I just put it because I cannot
expect future changes in the ElementsAccessor.
Do you think is it better to delete this line?

[Camillo Bruni, 2017-03-07 16:37:06]

On 2017/03/07 at 14:04:48, cwhan.tunz wrote:
> On 2017/03/07 13:59:53, Camillo Bruni wrote:
> > lgtm
> > 
> >
https://codereview.chromium.org/2733193002/diff/40001/src/builtins/builtins-t...
> > File src/builtins/builtins-typedarray.cc (right):
> > 
> >
https://codereview.chromium.org/2733193002/diff/40001/src/builtins/builtins-t...
> > src/builtins/builtins-typedarray.cc:489: MAYBE_RETURN(result,
> > isolate->heap()->exception());
> > Technically not necessary, given that you can only call this builtin for
> > TypedArrays and the TypedElementsAccessor::IncludesValueImpl cannot throw.
> > 
> > FromJust will just hard crash in case of a pending failure.
> 
> Yes I know the ElementsAccessor does not throw. I just put it because I cannot
expect future changes in the ElementsAccessor.
> Do you think is it better to delete this line?

Let's leave it there, much cleaner this way ;)

[Choongwoo Han, 2017-03-08 10:30:24]

The CQ bit was checked by cwhan.tunz@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-08 10:30:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-08 11:01:06]

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

[commit-bot: I haz the power, 2017-03-08 11:01:07]

Dry run: This issue passed the CQ dry run.

[Choongwoo Han, 2017-03-13 09:09:59]

The CQ bit was checked by cwhan.tunz@gmail.com

[commit-bot: I haz the power, 2017-03-13 09:10:07]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-13 09:40:10]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489396199262700,
"parent_rev": "07ee0b175cdf8048ad23b8f58ad24df3d8426b4b", "commit_rev":
"b2efe57cdfb87698599c1135581b258ab6fd8aa5"}

[commit-bot: I haz the power, 2017-03-13 09:40:15]

Description was changed from

==========
[typedarrays] Move %TypedArray%.prototype.indexOf to C++

- Remove TypedArrayIndexOf in src/js/typedarray.js
- Implement it to C++ using the IndexOfValue in ElementsAccessor
- Add buffer neutering check also for %TypedArray%.prototype.includes

BUG=v8:5929
==========

to

==========
[typedarrays] Move %TypedArray%.prototype.indexOf to C++

- Remove TypedArrayIndexOf in src/js/typedarray.js
- Implement it to C++ using the IndexOfValue in ElementsAccessor
- Add buffer neutering check also for %TypedArray%.prototype.includes

BUG=v8:5929

Review-Url: https://codereview.chromium.org/2733193002
Cr-Commit-Position: refs/heads/master@{#43741}
Committed:
https://chromium.googlesource.com/v8/v8/+/b2efe57cdfb87698599c1135581b258ab6f...
==========

[commit-bot: I haz the power, 2017-03-13 09:40:16]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/v8/v8/+/b2efe57cdfb87698599c1135581b258ab6f...