[proxies] Implement Symbol/DONT_ENUM filtering for GetKeys()

[proxies] Implement Symbol/DONT_ENUM filtering for GetKeys()

And use it to fix Object.keys() for proxies.

BUG=v8:1543
LOG=n
R=cbruni@chromium.org

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

[Jakob Kummerow, 2015-12-01 14:03:05]

Camillo: PTAL.

Georg, Toon: FYI.

[Camillo Bruni, 2015-12-01 14:39:39]

lgtm

https://codereview.chromium.org/1488873003/diff/1/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1488873003/diff/1/src/objects.cc#newcode8319
src/objects.cc:8319: keys->Shrink(store_position);
we could actually avoid shrinking it by passing the definite size to
AddKeysFromProxy then again we don't care too much for proxies I guess :)

https://codereview.chromium.org/1488873003/diff/1/src/objects.cc#newcode8400
src/objects.cc:8400: target_keys->set(i, Smi::FromInt(0));
I'd prefer working on target_configurable_keys instead since we don't use
target_keys afterwards. Same would apply further up.

https://codereview.chromium.org/1488873003/diff/1/src/objects.cc#newcode8415
src/objects.cc:8415: false);
I wonder whether it would not make more sense to do the filtering in
GetKeys_Internal?

[Jakob Kummerow, 2015-12-02 09:42:57]

https://codereview.chromium.org/1488873003/diff/1/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1488873003/diff/1/src/objects.cc#newcode8319
src/objects.cc:8319: keys->Shrink(store_position);
On 2015/12/01 14:39:39, cbruni wrote:
> we could actually avoid shrinking it by passing the definite size to
> AddKeysFromProxy then again we don't care too much for proxies I guess :)

Acknowledged.

https://codereview.chromium.org/1488873003/diff/1/src/objects.cc#newcode8400
src/objects.cc:8400: target_keys->set(i, Smi::FromInt(0));
On 2015/12/01 14:39:39, cbruni wrote:
> I'd prefer working on target_configurable_keys instead since we don't use
> target_keys afterwards. Same would apply further up.

As discussed, I disagree. At this point we're iterating over all the target
keys. The distinction between configurable and nonconfigurable keys only becomes
valid after this loop.

https://codereview.chromium.org/1488873003/diff/1/src/objects.cc#newcode8415
src/objects.cc:8415: false);
On 2015/12/01 14:39:39, cbruni wrote:
> I wonder whether it would not make more sense to do the filtering in
> GetKeys_Internal?

Done -- moved it into the AddKeysFromProxy call right below. (There's no way we
could do it from GetKeys_Internal.)

[Camillo Bruni, 2015-12-02 09:44:05]

lgtm

[Jakob Kummerow, 2015-12-02 09:44:14]

The CQ bit was checked by jkummerow@chromium.org

[commit-bot: I haz the power, 2015-12-02 09:44:21]

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

[commit-bot: I haz the power, 2015-12-02 10:19:43]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-12-02 10:20:01]

Description was changed from

==========
[proxies] Implement Symbol/DONT_ENUM filtering for GetKeys()

And use it to fix Object.keys() for proxies.

BUG=v8:1543
LOG=n
R=cbruni@chromium.org
==========

to

==========
[proxies] Implement Symbol/DONT_ENUM filtering for GetKeys()

And use it to fix Object.keys() for proxies.

BUG=v8:1543
LOG=n
R=cbruni@chromium.org

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

[commit-bot: I haz the power, 2015-12-02 10:20:02]

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