Bug 40613 – The JSObjectSetProperty doesn't overwrite property flags.

• Component: JavaScriptCore
• Status: RESOLVED WONTFIX

[Jędrzej Nowacki]

It seems that the JSObjectSetProperty can't overwrite property flag. It simply ignores attributes argument while replacing property.

[Jędrzej Nowacki]

Created attachment 58759 [details]
Fix v1

[Oliver Hunt]

Comment on attachment 58759 [details]
Fix v1

This has bad behaviour in a number of ways -- the unconditional use of delete if the property exists will destroy the PIC and so hurt performance for cases where the attributes are not being changed.

Additionally you get weird behaviour if have:

proto = {prop: "foo"}
o = Object.create(proto);

and do:
JSObjectSetProperty(o, "prop", ..., ...);

you will delete "prop" from proto, and then add "prop" to o.  Clearly this is less than ideal.

:-(

--Oliver

[Kent Hansen]

(In reply to comment #2)
> (From update of attachment 58759 [details])
> This has bad behaviour in a number of ways -- the unconditional use of delete if the property exists will destroy the PIC and so hurt performance for cases where the attributes are not being changed.
> 
> Additionally you get weird behaviour if have:
> 
> proto = {prop: "foo"}
> o = Object.create(proto);
> 
> and do:
> JSObjectSetProperty(o, "prop", ..., ...);
> 
> you will delete "prop" from proto, and then add "prop" to o.  Clearly this is less than ideal.

The hasProperty() call seems weird. Just because the object has some property in the prototype chain, the attributes you specify will be ignored when you create a property of the same name on your object?
Would calling hasOwnProperty() be sufficient?
Better, by calling getOwnPropertyDescriptor() one can get the attributes, and only delete the property if they differ.
Oh, looks like JSObject::defineOwnProperty() already has such logic. ;)

Still seems like a work-around, though. How about fixing JSObject::putWithAttributes() so it takes care of updating the attributes of an existing entry in case the property already exists (just like it takes care of updating the value)? Then unconditionally call putWithAttributes() from JSObjectSetProperty() when attributes != 0. (Which means there's no way to remove all attributes without recreating the property; oh well.)

[Gavin Barraclough]

Sigh - the problem here is that it's not really clear what JSObjectSetProperty should be doing.  Does it correspond to the spec's [[Put]] functionality? [[DefineOwnProperty]]? - or a mix of the two?

Given the interface we have, if the property already exists on the object being put to, it probably does make sense to make this call defineOwnProperty so the attributes are updated accordingly.

[Gavin Barraclough]

I don't think we're going to change this interface right now.  The API behaviour is

if ([[HasProperty]])
    [[Put]]
else
    [[DefineOwnProperty]]

It might be nice to separate put & defineOwnProperty traps in the API, but JSObjectSetProperty's behaviour isn't going to change now.