Closed
Bug 962449
Opened 10 years ago
Closed 10 years ago
Create a choke-point for class initialization
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla29
People
(Reporter: bholley, Assigned: bholley)
References
Details
Attachments
(6 files)
1.78 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
2.28 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
5.70 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
9.39 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
3.68 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
29.79 KB,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•10 years ago
|
||
Green try run: https://tbpl.mozilla.org/?tree=Try&rev=196eb1211252
Assignee | ||
Comment 2•10 years ago
|
||
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.
Attachment #8364722 -
Flags: review?(jorendorff)
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #8364723 -
Flags: review?(jorendorff)
Assignee | ||
Comment 4•10 years ago
|
||
Attachment #8364724 -
Flags: review?(jorendorff)
Assignee | ||
Comment 5•10 years ago
|
||
Attachment #8364725 -
Flags: review?(jorendorff)
Assignee | ||
Comment 6•10 years ago
|
||
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.
Attachment #8364726 -
Flags: review?(jorendorff)
Assignee | ||
Comment 7•10 years ago
|
||
Given that we've all talked about this now, switching some of these over to luke to spread the load.
Assignee | ||
Updated•10 years ago
|
Attachment #8364722 -
Flags: review?(jorendorff) → review?(luke)
Assignee | ||
Updated•10 years ago
|
Attachment #8364723 -
Flags: review?(jorendorff) → review?(luke)
Assignee | ||
Updated•10 years ago
|
Attachment #8364724 -
Flags: review?(jorendorff) → review?(luke)
Assignee | ||
Updated•10 years ago
|
Attachment #8364725 -
Flags: review?(jorendorff) → review?(luke)
Assignee | ||
Updated•10 years ago
|
Attachment #8364726 -
Flags: review?(jorendorff) → review?(luke)
Updated•10 years ago
|
Attachment #8364722 -
Flags: review?(luke) → review+
Updated•10 years ago
|
Attachment #8364723 -
Flags: review?(luke) → review+
Updated•10 years ago
|
Attachment #8364724 -
Flags: review?(luke) → review+
Updated•10 years ago
|
Attachment #8364725 -
Flags: review?(luke) → review+
Updated•10 years ago
|
Attachment #8364726 -
Flags: review?(luke) → review+
Assignee | ||
Comment 8•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=7e28bc0a21b7
Assignee | ||
Comment 9•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=4c5d730696f0
Assignee | ||
Comment 10•10 years ago
|
||
Oh I see. Rooting analysis is considered to be a separate platform: https://tbpl.mozilla.org/?tree=Try&rev=1f20ebbd5d06
Assignee | ||
Comment 11•10 years ago
|
||
Attachment #8368748 -
Flags: review?(jwalden+bmo)
Comment 12•10 years ago
|
||
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.
Attachment #8368748 -
Flags: review?(jwalden+bmo) → review+
Assignee | ||
Comment 13•10 years ago
|
||
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•10 years ago
|
||
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
Assignee: nobody → bobbyholley
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in
before you can comment on or make changes to this bug.
Description
•