Teach JSReceiver::GetKeys() how to include symbols

Teach JSReceiver::GetKeys() how to include symbols

No users of that functionality yet, those will come separately.

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

[Jakob Kummerow, 2015-09-29 14:52:35]

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

[Jakob Kummerow, 2015-09-29 14:52:35]

PTAL.

[Camillo Bruni, 2015-09-29 16:27:20]

lgmt

Some comments for future improvements.

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

https://codereview.chromium.org/1375003002/diff/1/src/objects.cc#newcode6920
src/objects.cc:6920: current->GetOwnPropertyNames(*property_keys, 0, filter);
Not sure how much work this would be, but essentially you could add the property
names directly to the accumulator and avoid the property_keys temporary. Might
be harder for the dict paths in GetOwnPropertyNames.

https://codereview.chromium.org/1375003002/diff/1/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/1375003002/diff/1/src/objects.h#newcode1853
src/objects.h:1853: enum IncludeSymbolNamedProperties { SKIP_SYMBOLS,
INCLUDE_SYMBOLS };
Maybe use FixedArray::NON_SYMBOL_KEYS instead (or move the enum from FixedArray
to the global space or JSReceiver);

[Camillo Bruni, 2015-09-29 16:28:19]

lgtm

[Jakob Kummerow, 2015-09-30 13:59:30]

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

https://codereview.chromium.org/1375003002/diff/1/src/objects.cc#newcode6920
src/objects.cc:6920: current->GetOwnPropertyNames(*property_keys, 0, filter);
On 2015/09/29 16:27:20, cbruni wrote:
> Not sure how much work this would be, but essentially you could add the
property
> names directly to the accumulator and avoid the property_keys temporary. Might
> be harder for the dict paths in GetOwnPropertyNames.

I'm leaving that to you :-)

https://codereview.chromium.org/1375003002/diff/1/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/1375003002/diff/1/src/objects.h#newcode1853
src/objects.h:1853: enum IncludeSymbolNamedProperties { SKIP_SYMBOLS,
INCLUDE_SYMBOLS };
On 2015/09/29 16:27:20, cbruni wrote:
> Maybe use FixedArray::NON_SYMBOL_KEYS instead (or move the enum from
FixedArray
> to the global space or JSReceiver);

Done.

[Camillo Bruni, 2015-09-30 14:09:14]

lgtm

[Jakob Kummerow, 2015-09-30 14:09:40]

The CQ bit was checked by jkummerow@chromium.org

[commit-bot: I haz the power, 2015-09-30 14:09:54]

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

[commit-bot: I haz the power, 2015-09-30 14:11:06]

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

[commit-bot: I haz the power, 2015-09-30 14:11:07]

Try jobs failed on following builders:
  v8_linux64_asan_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel/builds/...)
  v8_linux64_avx2_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel/builds/...)
  v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/1...)
  v8_linux_mips64el_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)
  v8_mac_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel/builds/10228)

[Jakob Kummerow, 2015-09-30 14:33:02]

The CQ bit was checked by jkummerow@chromium.org

[Jakob Kummerow, 2015-09-30 14:33:02]

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

[commit-bot: I haz the power, 2015-09-30 14:33:09]

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

[commit-bot: I haz the power, 2015-09-30 15:00:27]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-09-30 15:00:42]

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