[E10s] Sort out what object TabParent::GetGlobalJSObject should return and how

RESOLVED FIXED

Status

()

Core
IPC
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: smaug, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

8 years ago
The method could return TabChildGlobal or top level content page's
global object.
Created attachment 435275 [details] [diff] [review]
Patch that sorts things out.

Back to having only one ContextWrapperChild per TabChild.
Attachment #435275 - Flags: review?(Olli.Pettay)
(Reporter)

Comment 2

8 years ago
Comment on attachment 435275 [details] [diff] [review]
Patch that sorts things out.

>Bug 554942 - [E10s] Sort out what object TabParent::GetGlobalJSObject should return and how.
>
>diff --git a/dom/ipc/TabChild.cpp b/dom/ipc/TabChild.cpp
>--- a/dom/ipc/TabChild.cpp
>+++ b/dom/ipc/TabChild.cpp
>@@ -471,17 +471,25 @@ TabChild::RecvloadURL(const nsCString& u
> 
>     nsresult rv = mWebNav->LoadURI(NS_ConvertUTF8toUTF16(uri).get(),
>                                    nsIWebNavigation::LOAD_FLAGS_NONE,
>                                    NULL, NULL, NULL);
>     if (NS_FAILED(rv)) {
>         NS_WARNING("mWebNav->LoadURI failed. Eating exception, what else can I do?");
>     }
> 
>-    SendPContextWrapperConstructor()->SendPObjectWrapperConstructor(true);
>+    NS_ASSERTION(mContextWrapper, "Should have created mContextWrapper already");
>+    if (mContextWrapper) {
>+        JSContext* cx = mContextWrapper->GetContext();
>+        JSAutoRequest ar(cx);
>+        jsval cval;
>+        if (JS_GetProperty(cx, JS_GetGlobalObject(cx), "content", &cval) &&
>+            JSVAL_IS_OBJECT(cval))
>+            mContextWrapper->GetOrCreateWrapper(JSVAL_TO_OBJECT(cval), true);
>+    }

Do we really want to do this every time a new page is started to load?
Could we do this all just when mContextWrapper is created?
Docshell should be creating the global object when needed.
(Reporter)

Updated

8 years ago
Attachment #435275 - Flags: review?(Olli.Pettay) → review-
Created attachment 435672 [details] [diff] [review]
Simpler patch that sorts things out.

(In reply to comment #2)
> Do we really want to do this every time a new page is started to load?
> Could we do this all just when mContextWrapper is created?
> Docshell should be creating the global object when needed.

Yes, yes.  Much simpler that way!  Thanks.
Attachment #435275 - Attachment is obsolete: true
Attachment #435672 - Flags: review?(Olli.Pettay)
(Reporter)

Comment 4

8 years ago
Comment on attachment 435672 [details] [diff] [review]
Simpler patch that sorts things out.

If you have tested this all works ;)  r=me.
Attachment #435672 - Flags: review?(Olli.Pettay) → review+
Pushed to projects/electrolysis:
http://hg.mozilla.org/projects/electrolysis/rev/f22ca0221b4e
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.