Implement Object.getOwnPropertyNames constructor property [ES5 section 15.2.3...

Implement Object.getOwnPropertyNames constructor property [ES5 section 15.2.3.4].

[pfeldman, 2010-01-14 14:48:32]

I just moved some code around (renamed methods I needed from Debug* to Get*).
Does this look as expected?

[Mads Ager (chromium), 2010-01-14 15:01:12]

Pavel, I think you meant to add sgjesse instead of sgk here?  I added Rico as a
reviewer as well since he has been working on ES5 additions.

[Mads Ager (chromium), 2010-01-14 15:02:06]

Oh, and please perform this change in branches/bleeding_edge instead of trunk.

[pfeldman, 2010-01-14 16:52:12]

On 2010/01/14 15:02:06, Mads Ager wrote:
> Oh, and please perform this change in branches/bleeding_edge instead of trunk.

Yes, that was supposed to be Soren and yes, I know it should be against bleeding
edge. Just tell me it looks reasonable (does the right thing, has ok tests,
etc.) and I get it merged.

[Søren Thygesen Gjesse, 2010-01-14 22:05:13]

I have not read ES5 section 15.2.3.4, but recycling the %Debug functions if
getOwnPropertyNames have the same semantic is the right way to go.

Also consider adding a cctest with objects having accessors, callbacks, hidden
properties etc.

http://codereview.chromium.org/549050/diff/1/2
File test/mjsunit/object-get-own-property-names.js (right):

http://codereview.chromium.org/549050/diff/1/2#newcode28
test/mjsunit/object-get-own-property-names.js:28: // Test ES5 sections 15.2.3.4
Object.getOwnPropertyNames.
sections -> section

http://codereview.chromium.org/549050/diff/1/2#newcode29
test/mjsunit/object-get-own-property-names.js:29: 
I think this test should start with some straight forward examples, e.g

  {a:1,b:2}
  {a:function(){},b:function(){}}

and an object which is known to be in slow-case.

  a={a:1,b:2,c:3}
  delete a.c
  a.getOwnPropertyNames()

This is to cover some of the internal states/representations used by V8.

http://codereview.chromium.org/549050/diff/1/2#newcode54
test/mjsunit/object-get-own-property-names.js:54: print(2);
Remove?

http://codereview.chromium.org/549050/diff/1/2#newcode65
test/mjsunit/object-get-own-property-names.js:65: assertEquals("length",
propertyNames[2]);
I have not read ES5 section 15.2.3.4 but is the order of the getOwnPropertyNames
defined?

[pfeldman, 2010-01-15 00:01:10]

Thanks for the review. I will merge into the bleeding edge, address the comments
and update the patch.

To answer your question, order of the getOwnPropertyNames is not defined. I am
sticking to whatever is in WebKit nightly.

[pfeldman, 2010-01-15 14:35:54]

There are couple of drive-by fixes for preconditions on Object constructor
property functions as well.

I looked into the order issue a bit, but then decided that we already have
enough problems with for-in. Spec does not say anything about the order.

http://codereview.chromium.org/549050/diff/1/2
File test/mjsunit/object-get-own-property-names.js (right):

http://codereview.chromium.org/549050/diff/1/2#newcode28
test/mjsunit/object-get-own-property-names.js:28: // Test ES5 sections 15.2.3.4
Object.getOwnPropertyNames.
On 2010/01/14 22:05:13, Søren Gjesse wrote:
> sections -> section

Done.

http://codereview.chromium.org/549050/diff/1/2#newcode29
test/mjsunit/object-get-own-property-names.js:29: 
On 2010/01/14 22:05:13, Søren Gjesse wrote:
> I think this test should start with some straight forward examples, e.g
> 
>   {a:1,b:2}
>   {a:function(){},b:function(){}}
> 
> and an object which is known to be in slow-case.
> 
>   a={a:1,b:2,c:3}
>   delete a.c
>   a.getOwnPropertyNames()
> 
> This is to cover some of the internal states/representations used by V8.

Done.

http://codereview.chromium.org/549050/diff/1/2#newcode54
test/mjsunit/object-get-own-property-names.js:54: print(2);
On 2010/01/14 22:05:13, Søren Gjesse wrote:
> Remove?

Done.

http://codereview.chromium.org/549050/diff/1/2#newcode65
test/mjsunit/object-get-own-property-names.js:65: assertEquals("length",
propertyNames[2]);
On 2010/01/14 22:05:13, Søren Gjesse wrote:
> I have not read ES5 section 15.2.3.4 but is the order of the
getOwnPropertyNames
> defined?

It is not. WebKit returns everything in the order it was added. No one else has
it implemented. In our case it is not defined.

[Søren Thygesen Gjesse, 2010-01-15 14:54:31]

LGTM if committed to bleeding_edge.

[Rico, 2010-01-18 12:37:33]

I have added as5conform test for this method and it fails on most test cases -
but this is mainly because the test compares the expected result array with the
result of the method on a one to one basis, i.e., it does not allow object to
have additional properties. I have made a patch that I will upload to es5conform
- meanwhile I have made FAIL_OK on these in the es5conform.status file -> see
http://codereview.chromium.org/545109