962449 - Create a choke-point for class initialization

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Bobby Holley (:bholley)]

I have various patches in my queue for this, and it seemed self-contained enough to split out.

At the moment, we have a number of different points where we may potentially invoke a routine to initialize a standard class. In order to implement bug 959012, we need this all to funnel through GlobalObject::ensureConstructor. These patches do that.

Comment 1

[Bobby Holley (:bholley)]

Green try run: https://tbpl.mozilla.org/?tree=Try&rev=196eb1211252

Comment 2

[Bobby Holley (:bholley)]

Attachment: Part 1 - When resolving iterator classes, use JSProto_Iterator instead of JSProto_GeneratorFunction. v1

The former triggers all of the appropriate initialization, whereas the latter
is 'imaginary' in jsprototypes.h, which means that the is no initialization
function associated with it.

Comment 3

[Bobby Holley (:bholley)]

Attachment: Part 2 - Don't directly invoke init functions in GlobalObject::initStandardClasses. v1

Comment 4

[Bobby Holley (:bholley)]

Attachment: Part 3 - Stop manually invoking initialization routines in GlobalObject.h. v1

Comment 5

[Bobby Holley (:bholley)]

Attachment: Part 4 - Remove init functions from JSStdName, and pipe everything through ensureConstructor. v1

Comment 6

[Bobby Holley (:bholley)]

Attachment: Part 5 - Don't call initIteratorClasses directly, and use ensureConstructor. v1

This isn't strictly necessary at this point. The basic issue is that there are
a few new classes (iterator stuff, intl stuff, and typed objects) that use this
kind of constructor bootstrap scheme, and each of them will need something like
this to move to ClassSpecs. But that doesn't actually need to happen until we
convert them. I wrote this patch before I realized this, and I believe it's
correct, so I figured I'd get it landed along with everything. But I'm also ok
to skip it for now.

Comment 7

[Bobby Holley (:bholley)]

Given that we've all talked about this now, switching some of these over to luke to spread the load.

Comment 8

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=7e28bc0a21b7

Comment 9

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=4c5d730696f0

Comment 10

[Bobby Holley (:bholley)]

Oh I see. Rooting analysis is considered to be a separate platform:

https://tbpl.mozilla.org/?tree=Try&rev=1f20ebbd5d06

Comment 11

[Bobby Holley (:bholley)]

Attachment: Part 6 - Make various getOrCreate methods static to efficiently fix hazards. v1

Comment 12

[Jeff Walden [:Waldo]]

Comment on attachment 8368748 [details] [diff] [review]
Part 6 - Make various getOrCreate methods static to efficiently fix hazards. v1

Review of attachment 8368748 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/vm/GlobalObject.h
@@ +471,5 @@
>    public:
> +    static JSObject *getOrCreateIteratorPrototype(JSContext *cx,
> +                                                  Handle<GlobalObject*> global)
> +    {
> +        if (!global->ensureConstructor(cx, JSProto_Iterator))

Does ensureConstructor presumably create a Rooted for the global?  Seems like it should be static too, to avoid that.  Followup.

Comment 13

[Bobby Holley (:bholley)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/a44f13894eb0
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/b4c540663486
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/98c896b888fc
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/91e3fce5bb56
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/8c206e77eb64
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/290a8f359441

Comment 14

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/b4c540663486
https://hg.mozilla.org/mozilla-central/rev/98c896b888fc
https://hg.mozilla.org/mozilla-central/rev/91e3fce5bb56
https://hg.mozilla.org/mozilla-central/rev/8c206e77eb64
https://hg.mozilla.org/mozilla-central/rev/290a8f359441