Open Bug 637378 Opened 13 years ago Updated 2 years ago

Update meta-object protocol to be more ES5-like

Categories

(Core :: JavaScript Engine, defect)

defect

Tracking

()

People

(Reporter: Waldo, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

With getOwnPropertyDescriptor or somesuch and the like.
See the proxy code, which follows ES5 already.

/be
Blocks: 408416
Depends on: 638997
I'm in favor of this bug being fixed, however: Allen (ES5 editor) may have some guidance, since ES5 is neither "perfect" in some ideal-MOP sense (no MOP is), nor exactly where we are going with Harmony.

We can keep on morphing our MOP to track spec and balance real vs. ideal (in more than one dimension!) but it could well be worth a conversation or a read through Allen's latest spreadsheets (or other docs I don't know about) that he has written to try to make sense of the spec's MOP and the use-cases for its internal methods and properties.

/be
The spec formulation implements property lookup as a [[GetOwnProperty]] call on each level of the prototype chain, more or less.  If you have this you can do sensible things like have object classes represent externally that they have arbitrary sets of properties.  For example, Arrays could say they have a length property and expose it with the proper attributes and all.  (The counterpart is a [[DefineProperty]] hook for classes, roughly, in order to complete that.)  This is kind of like what lookupProperty is, but [[GetOwnProperty]] is more the union of that and the resolve hook, I think.

This patch adds ObjectOps::getOwnProperty but doesn't actually use it -- there are obstacles in the way of doing that, to be pondered, discussed, addressed, and so on.  Still, I want to get this up at least as a small start.  Also, I haven't built this in several weeks, so it's possible it's bitrotted and won't build, but at the worst it's not far off.
This would switch out the guts of the lookup-property code to use the getOwnProperty hook.  But it runs into JSCLASS_NEW_RESOLVE_GETS_START, which I think you can only do with the getOwnProperty formulation if you were to have such classes implement a custom lookupProperty instead.

JSCLASS_NEW_RESOLVE_GETS_START is only used one place: for XBL prototype objects.  These are the objects munged into the prototype chain of DOM nodes associated with bindings.  Since this is already in the resolve-property slow path, the idea I had for working around this would be to add code to the DOM node resolve implementation to check for has-associated-XBL and do the property resolution itself, rather than having the prototype munging happen.  I haven't actually started work on this yet, but I've discussed it (for some level of "discussion") a couple times with bz.

Without some adjustment to the XBL stuff, however, this mostly breaks opening a browser, so this is more being posted for interestingness than for actually being reviewable, let alone pushable.
So there is another option.  Right now, what we do is we store the fields off to the side and define the props on resolve.  What we _used_ to do before bug 372769 was fixed was evaluate them at binding install time and define the property on the element.

There is a middle ground.  That is, at binding install time, define an accesor property whose getter and setter evaluate the field and reconfigure it to the actual value property we want.  The getter would then return the value.  The setter would set the value (and if we do this right the value prop being readonly if that flag is set would still work).

Does that seem feasible?  The hardest part, in some ways, is that we would need the getter/setter to effectively close over the XBL prototype (a C++ object) that installed the field in question so that it can find the actual field off said prototype.

We _could_ also do things in the classinfo resolve hook, but:

1)  We're trying to get rid of it as much as we can, I believe.
2)  That would possibly have even more issues with finding the right XBL prototype for a
    given field.

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Assignee: jwalden → nobody
Status: ASSIGNED → NEW
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: