Fixes bug in Object.freeze and Object.seal causing them to misbehave when Arr...

Fixes bug in Object.freeze and Object.seal causing them to misbehave when Array.prototype has changed.

Object.freeze and Object.seal uses GetOwnPropertyNames which
returns an array with local property names. This array will also have
the additional properties defined on Array.prototype or
Object.prototype.

Note that, the implementation of GetOwnPropertyNames (when used as
Object.getOwnPropertyNames) is correct, since the spec says to create
a new Array (which would also have these properties).


Committed: http://code.google.com/p/v8/source/detail?r=5350

[Mark Lam, 2010-08-26 06:55:11]

http://codereview.chromium.org/3137041/diff/2001/3001
File src/v8natives.js (right):

http://codereview.chromium.org/3137041/diff/2001/3001#newcode751
src/v8natives.js:751: if (!IS_UNDEFINED(desc)) {
Drive by comment: is there no other internal way to get own property names
without including extensions to Object and Array?  While this fix works, it does
necessitate iterating through more names than it needs to only to skip them.  I
could be wrong but it seems to me like there should be a way to just iterate
over own property names, right?

On the other hand, the cost of skipping over whatever added extensions might not
be significant enough to warrant cooking up an alternate mechanism to iterate
the names.  Just thought I'd ask in case there's already a way to achieve this
so as to avoid this wasted work.

Thanks.

[Rico, 2010-08-26 07:13:09]

Thanks for the comments, I will consider doing another change later on
optimizing this (and other new ES5 functions).

http://codereview.chromium.org/3137041/diff/2001/3001
File src/v8natives.js (right):

http://codereview.chromium.org/3137041/diff/2001/3001#newcode751
src/v8natives.js:751: if (!IS_UNDEFINED(desc)) {
On 2010/08/26 06:55:11, polarjs wrote:
> Drive by comment: is there no other internal way to get own property names
> without including extensions to Object and Array?  While this fix works, it
does
> necessitate iterating through more names than it needs to only to skip them. 
I
> could be wrong but it seems to me like there should be a way to just iterate
> over own property names, right?
> 
> On the other hand, the cost of skipping over whatever added extensions might
not
> be significant enough to warrant cooking up an alternate mechanism to iterate
> the names.  Just thought I'd ask in case there's already a way to achieve this
> so as to avoid this wasted work.
> 
> Thanks. 

First, I am pretty sure that the normal impact of this will be very small since
the work done in this check is very limited compared to the work done in
DefineOwnProperty.

On the other hand, it might be worthwhile to actually implement all the logic
from getOwnPropertyNames in a single C++ runtime function (that would speed up
that method as well). That would, however, also require, Seal, Freeze, IsSealed,
and IsFrozen to be implemented in runtime functions (since the returned object
would also have the added properties).
We have not been looking hard into optimizing these new ES5 additions yet, first
priority is to get a complete (correct) implementation.

[Rico, 2010-08-26 08:09:01]

Erik: As discussed offline, I changed the "for var in" loop to a loop using the
length of the array. The length is not impacted by properties on the prototype
(even array indexed properties on the prototype).

Polarjs: This also fixes your concerns :-)

[Erik Corry, 2010-08-26 08:17:16]

LGTM.

See also http://jsperf.com/for-vs-each-hash-vs-array for why this is a good
change