[proxies] Make Array.prototype.concat work correctly with proxies.

[proxies] Make Array.prototype.concat work correctly with proxies.

R=rossberg
BUG=v8:1543
LOG=n

Committed: https://crrev.com/d0ef84b34054f12c66e4cab6e03672d02727b979
Cr-Commit-Position: refs/heads/master@{#32900}

[Camillo Bruni, 2015-12-15 16:23:19]

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

[Camillo Bruni, 2015-12-15 16:23:19]

https://codereview.chromium.org/1525983002/diff/1/src/builtins.cc
File src/builtins.cc (right):

https://codereview.chromium.org/1525983002/diff/1/src/builtins.cc#newcode1154
src/builtins.cc:1154:
DCHECK(isolate->IsFastArrayConstructorPrototypeChainIntact());
I don't think this is needed here since you do a full lookup with
JSReceiver::HasProperty

https://codereview.chromium.org/1525983002/diff/1/test/mjsunit/harmony/array-...
File test/mjsunit/harmony/array-concat.js (right):

https://codereview.chromium.org/1525983002/diff/1/test/mjsunit/harmony/array-...
test/mjsunit/harmony/array-concat.js:732: return Reflect[trap](...arguments);
nice :)

[neis, 2015-12-15 16:25:02]

https://codereview.chromium.org/1525983002/diff/1/src/builtins.cc
File src/builtins.cc (right):

https://codereview.chromium.org/1525983002/diff/1/src/builtins.cc#newcode1154
src/builtins.cc:1154:
DCHECK(isolate->IsFastArrayConstructorPrototypeChainIntact());
On 2015/12/15 16:23:19, cbruni wrote:
> I don't think this is needed here since you do a full lookup with
> JSReceiver::HasProperty

It's important that this Has cannot be observed by a proxy.

[rossberg, 2015-12-16 13:14:37]

lgtm

https://codereview.chromium.org/1525983002/diff/1/test/mjsunit/harmony/array-...
File test/mjsunit/harmony/array-concat.js (right):

https://codereview.chromium.org/1525983002/diff/1/test/mjsunit/harmony/array-...
test/mjsunit/harmony/array-concat.js:731: log.push([trap, ...arguments]);
Nit: if you're already using spread, also use rest, i.e., don't use the implicit
`arguments`, but bind an explicit `...args` instead.

[neis, 2015-12-16 13:34:59]

Description was changed from

==========
[proxies] Make Array.prototype.concat work correctly with proxies.

R=rossberg
BUG=
==========

to

==========
[proxies] Make Array.prototype.concat work correctly with proxies.

R=rossberg
BUG=v8:1543
LOG=n
==========

[neis, 2015-12-16 13:36:52]

The CQ bit was checked by neis@chromium.org

[neis, 2015-12-16 13:36:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
rossberg@chromium.org
Link to the patchset: https://codereview.chromium.org/1525983002/#ps20001
(title: " ")

[commit-bot: I haz the power, 2015-12-16 13:37:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1525983002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1525983002/20001

[commit-bot: I haz the power, 2015-12-16 14:17:22]

Description was changed from

==========
[proxies] Make Array.prototype.concat work correctly with proxies.

R=rossberg
BUG=v8:1543
LOG=n
==========

to

==========
[proxies] Make Array.prototype.concat work correctly with proxies.

R=rossberg
BUG=v8:1543
LOG=n
==========

[commit-bot: I haz the power, 2015-12-16 14:17:23]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-12-16 14:17:32]

Description was changed from

==========
[proxies] Make Array.prototype.concat work correctly with proxies.

R=rossberg
BUG=v8:1543
LOG=n
==========

to

==========
[proxies] Make Array.prototype.concat work correctly with proxies.

R=rossberg
BUG=v8:1543
LOG=n

Committed: https://crrev.com/d0ef84b34054f12c66e4cab6e03672d02727b979
Cr-Commit-Position: refs/heads/master@{#32900}
==========

[commit-bot: I haz the power, 2015-12-16 14:17:33]

Patchset 2 (id:??) landed as
https://crrev.com/d0ef84b34054f12c66e4cab6e03672d02727b979
Cr-Commit-Position: refs/heads/master@{#32900}