Closed
Bug 955735
Opened 11 years ago
Closed 11 years ago
Re-organize the code in nsScriptLoader to eliminate the last two hazards
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla29
People
(Reporter: terrence, Assigned: terrence)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
The problem here is that we get the ScriptContext and the JS GlobalObject at once since the lookup of the nsIScriptGlobalObject is costly. This makes the JS object live across a bunch of stuff that either may GC, or is hard to make the analysis understand.
However, it looks like the lookup of the JSObject and the ScriptContext off the nsIScriptGlobalObject are trivial, so if we just move the nsIScriptGlobalObject up a level, all our order-of-operations problems can just go away.
This should be the last two exact rooting hazards. Try build is at:
https://tbpl.mozilla.org/?tree=Try&rev=0930152dfc87
Attachment #8354903 -
Flags: review?(bugs)
Comment 1•11 years ago
|
||
Comment on attachment 8354903 [details] [diff] [review]
hazards_nsScriptLoader-v0.diff
>+nsIScriptGlobalObject *
* goes with the type
>+nsScriptLoader::GetScriptGlobalObject()
> {
> nsPIDOMWindow *pwin = mDocument->GetInnerWindow();
> if (!pwin) {
> return nullptr;
> }
>
> nsCOMPtr<nsIScriptGlobalObject> globalObject = do_QueryInterface(pwin);
> NS_ASSERTION(globalObject, "windows must be global objects");
>
> // and make sure we are setup for this type of script.
> nsresult rv = globalObject->EnsureScriptEnvironment();
> if (NS_FAILED(rv)) {
> return nullptr;
> }
>
>- *aGlobal = globalObject->GetGlobalJSObject();
>+ return globalObject;
>+}
Please make this method return already_AddRefed<nsIScriptGlobalObject>
and then return globalObject.forget();
>+nsIScriptContext *
>+nsScriptLoader::GetScriptContext(nsIScriptGlobalObject *globalObject)
>+{
> return globalObject->GetScriptContext();
> }
>
>+JSObject *
>+nsScriptLoader::GetScriptJSGlobalObject(nsIScriptGlobalObject *globalObject)
>+{
>+ return globalObject->GetGlobalJSObject();
>+}
Why we need these two methods?
(and * goes with type and params should be in form aFoo)
Attachment #8354903 -
Flags: review?(bugs) → review-
Assignee | ||
Comment 2•11 years ago
|
||
Gah! I have no idea how that extraneous hunk slipped in. I thought I cut the patch /after/ building and pushing to try, but apparently not. In any case, the try run (which doesn't have the hunk) is green.
Attachment #8354903 -
Attachment is obsolete: true
Attachment #8354971 -
Flags: review?(bugs)
Updated•11 years ago
|
Attachment #8354971 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 3•11 years ago
|
||
Comment 4•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in
before you can comment on or make changes to this bug.
Description
•