Refactor ObjectGetOwnPropertyKeys to accept bitmask rather than boolean

Refactor ObjectGetOwnPropertyKeys to accept bitmask rather than boolean

BUG=v8:3549
LOG=Y
R=arv@chromium.org, rossberg@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=25111

[caitp (gmail), 2014-09-09 20:45:13]

caitpotter88@gmail.com changed reviewers:
+ arv@chromium.org

[caitp (gmail), 2014-09-09 20:46:34]

Ping --- so I think semantically it's more or less the same as previously,
however it should now be possible to ask for both kinds of property names.

[arv (Not doing code reviews), 2014-09-09 20:56:53]

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

https://codereview.chromium.org/557023002/diff/1/src/v8natives.js#newcode1092
src/v8natives.js:1092: if (IS_PRIVATE(name)) continue;
IS_PRIVATE is only true if IS_SYMBOL is also true so we can skip this check for
the common, string key case.

https://codereview.chromium.org/557023002/diff/1/src/v8natives.js#newcode1095
src/v8natives.js:1095: continue;
fits on previous line

V8 style is to always use {} if we can't fit on one line.

https://codereview.chromium.org/557023002/diff/1/src/v8natives.js#newcode1099
src/v8natives.js:1099: name = ToSTring(name);
typo

[arv (Not doing code reviews), 2014-09-09 20:57:13]

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

[arv (Not doing code reviews), 2014-09-09 20:57:13]

+ Andreas

[rossberg, 2014-09-10 07:37:59]

https://codereview.chromium.org/557023002/diff/20001/src/v8natives.js
File src/v8natives.js (right):

https://codereview.chromium.org/557023002/diff/20001/src/v8natives.js#newcode...
src/v8natives.js:1043: filter = (filter || PROPERTY_ATTRIBUTES_NONE)
Please remove the defaulting and make sure that it's always called with explicit
arguments.

[caitp (gmail), 2014-09-10 16:54:11]

Updated

https://codereview.chromium.org/557023002/diff/20001/src/v8natives.js
File src/v8natives.js (right):

https://codereview.chromium.org/557023002/diff/20001/src/v8natives.js#newcode...
src/v8natives.js:1043: filter = (filter || PROPERTY_ATTRIBUTES_NONE)
On 2014/09/10 07:37:59, rossberg wrote:
> Please remove the defaulting and make sure that it's always called with
explicit
> arguments.

`filter |= PROPERTY_ATTRIBUTES_PRIVATE_SYMBOL` would be semantically the same as
a default PRIVATE_SATTRIBUTES_NONE --- just that it's not clearly spelled out.

I think we do want to always filter out private symbols, since there's probably
never a time to ask for them this way (not in spec, and I can't think of a good
reason to do that)

I will change to |= just to make it shorter though.

[arv (Not doing code reviews), 2014-09-10 17:35:14]

On 2014/09/10 16:54:11, caitp wrote:

> I think we do want to always filter out private symbols, since there's
probably
> never a time to ask for them this way (not in spec, and I can't think of a
good
> reason to do that)

Yeah. We should never include private symbols here.h.

[caitp (gmail), 2014-11-03 22:21:59]

I've addressed all of the comments, `make check` is all good, so I believe
everything is correct.

[arv (Not doing code reviews), 2014-11-03 22:37:00]

LGTM

[rossberg, 2014-11-04 08:00:20]

lgtm

[arv (Not doing code reviews), 2014-11-04 15:08:29]

Committed patchset #3 (id:40001) manually as 25111 (presubmit successful).