[Webkit-unassigned] [Bug 78409] Move special __proto__ property to Object.prototype

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=78409





--- Comment #5 from Gavin Barraclough <barraclough at apple.com>  2012-02-11 12:35:11 PST ---
(In reply to comment #4)
> > Source/JavaScriptCore/parser/Parser.cpp:-777
> > -        failIfTrueWithMessage(*name == m_globalData->propertyNames->underscoreProto, "Cannot name a function __proto__");
> 
> Removing this should break tests -- i recall it hosing initialization of program code, but maybe i'm wrong?  I suppose global initialization is now purely putDirect, so maybe this is okay?

Yes - see the changes in the patch to LayoutTests/fast/js/parser-syntax-check-expected.txt – these now work.  And I think it makes sense this way, now that we basically no longer need there to be anything magic about __proto__.  (Except, possibly, for object literal behaviour... but even there I wonder if it is really required for web compatibility...)

> > Source/JavaScriptCore/runtime/JSGlobalObject.cpp:210
> > +    protoAccessor->setGetter(exec->globalData(), JSFunction::create(exec, this, 0, Identifier(), globalFuncProtoGetter));
> > +    protoAccessor->setSetter(exec->globalData(), JSFunction::create(exec, this, 0, Identifier(), globalFuncProtoSetter));
> 
> So the decision was anonymous functions?

There is no consensus on the ECMA list at the minute.  This change would be a step in the right direction whichever way the committee goes, since this moves __proto__ to the Object Prototype and makes in configurable (there seems to be agreement on both of these changes).

The only real outstanding question is whether this should cleanly be specified as an accessor, or whether we'll adopt Firefox's evil-magic-data-property hack.  There doesn't seem to be a broad base of support for the latter.  If the spec does go that way, we'd have three options: (1) reintroduce some evil hack into the object prototype classes (2) keep it as an accessor, add a new attribute to let the accessor masquerade as a data descriptor for getOwnPropertyDescriptor, or (3) don't follow the spec – it'll only be a normative-optional annex anyway, and we should ask serious questions before adopting such a bad de-jure spec.  But I don't think it needs get to this point – I think we should just resist any spec that demands this be an ugly hack.

> > Source/JavaScriptCore/runtime/JSObject.cpp:222
> > -    structure()->setHasGetterSetterProperties(true);
> > +    structure()->setHasGetterSetterProperties(propertyName == globalData.propertyNames->underscoreProto);
> 
> I don't get this change at all -- it seems to drop the getter/setter existence in all cases but the one where we're setting __proto__ - this probably passes tests because all test objects probably have the object prototype so this has poisoned everything.  If it doesn't impact performance (presumably the case seeing as you've put this up for review :D )  I don't see any reason __proto__ needs t be special cased.

Please see the changes in Structure.h – I'm now separately tracking whether the object had any accessors at all, and whether it has any accessors other than __proto__.  This means that hasGetterSetterProperties() returns the right thing (true for Object.prototype, due to __proto__), but JSValue::put can still be optimized (checking a combination of hasGetterSetterPropertiesExcludingProto(), and testing whether the identifier is __proto__).

> > Source/JavaScriptCore/runtime/JSObject.h:107
> > +        JS_EXPORT_PRIVATE static bool allowsAccessFrom(JSObject*, ExecState*);
> 
> This is a fairly generic name -- i can imagine in future we'd want this name to be used for cross-origin checks, etc, will that still be okay then?

Yes! – this is the cross-origin check! – see changes in JSDOMWindowBase. :-)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.