526491 - Define .constructor on DOM objects without needing a resolve hook

Status: RESOLVED WORKSFORME

(Core :: DOM: Core & HTML, defect)

Description

[Peter Van der Beken [:peterv]]

Attachment: v1

Still need to do something about Location, it doesn't have ALLOW_PROP_MODS_TO_PROTOTYPE and so I can't define a constructor property on its prototype. I might be able to just add a constructor attribute on nsIDOMLocation.

Comment 1

[Peter Van der Beken [:peterv]]

Blake, we can't trick hasOwnProperty using JSPROP_PERMANENT | JSPROP_SHARED because the wrapper and the prototype have different classes :-/.

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

So, I don't see an easy way around this... Either we have to bite the bullet and eagerly define .constructor on all nodes or have a resolve hook (they exist explicitly to avoid eagerly defining properties, after all).

Brendan, have I missed something?

Comment 3

[Brendan Eich [:brendan]]

Use a resolve hook if you must. This is all about location, a singleton, so paying up front (once per window/frame) whether in a prototype object or in the direct location object could hurt Tp, I take it. Has anyone measured?

/be

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to comment #3)
> Use a resolve hook if you must. This is all about location, a singleton, so
> paying up front (once per window/frame) whether in a prototype object or in the
> direct location object could hurt Tp, I take it. Has anyone measured?

brendan, the problem is that this isn't just about location. We're talking about the resolve hook on all nodes (currently nsNodeSH::NewResolve). The property in question is .constructor, it's a property that's (probably) rarely used, but it isn't free to define and it has to be hasOwnProperty to the nodes.

Comment 5

[Brendan Eich [:brendan]]

Who ever said constructor has to be "own" for all nodes? Don't tell me some HTML5 or WebAPI crack-spec!

js> d = new Date
(new Date(1258478166310))
js> d.constructor
function Date() {[native code]}
js> d.hasOwnProperty('constructor')
false

Note that constructor is not "own", it is delegated to constructor.prototype. Do that and prosper.

/be

Comment 6

[Peter Van der Beken [:peterv]]

Attachment: v2

This just puts .constructor on the prototype. I also solved the problem for Location by allowing PostCreatePrototype to add a constructor property to a XPCWrappedNativeProto.

Comment 7

[Peter Van der Beken [:peterv]]

Comment on attachment 414310 [details] [diff] [review]
v2

Actually, there's still some mochitest failures with this.

Comment 8

[Peter Van der Beken [:peterv]]

I think our new bindings already do this.