Closed Bug 1126018 Opened 5 years ago Closed 3 years ago

[e10s] Miscellaneous additions to add-on shims

Categories

(Firefox :: Extension Compatibility, defect)

34 Branch
x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: billm, Assigned: billm)

References

Details

Attachments

(4 files)

I've been doing more add-on testing and I've found some more issues. These are all pretty small things.
One problem I ran into is that an add-on will ask for window.content and then look for the associated browser via getBrowserForContentWindow. If window.content returns the dummy window, it won't find an associated browser and an exception will be thrown. Sadly this happens a lot in Jetpack, which still uses a great deal of CPOWs.

There's a pretty easy fix though. We can just set browser._contentWindow to the dummy window when we return it. Then getBrowserForContentWindow will return the correct browser.

I also added some properties to make the dummy window look like a real window. These are also useful to Jetpack.
Attachment #8554819 - Flags: review?(mconley)
Attached patch _content-shimSplinter Review
This patch fixes a very old add-on that uses window._content instead of window.content.
Attachment #8554821 - Flags: review?(mconley)
This patch adds a shim for gBrowser.docShell.
Attachment #8554823 - Flags: review?(mconley)
These are some additional observer topics that fire in content. One problem we're going to have here is that window IDs are not unique across processes. I filed bug 1126042 about this. For now I think we'll probably be okay because we only have one content process. But we do need to fix this eventually.
Attachment #8554842 - Flags: review?(mconley)
Comment on attachment 8554819 [details] [diff] [review]
dummy-window-shim-fixes

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

::: toolkit/components/addoncompat/RemoteAddonsParent.jsm
@@ +739,4 @@
>    };
> +  dummyContentWindow.top = dummyContentWindow;
> +  dummyContentWindow.document.defaultView = dummyContentWindow;
> +  browser._contentWindow = dummyContentWindow;

Kinda underhanded, poking a value into this private member. But I guess these shims are just generally underhanded already. :)
Attachment #8554819 - Flags: review?(mconley) → review+
Comment on attachment 8554821 [details] [diff] [review]
_content-shim

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

::: toolkit/components/addoncompat/RemoteAddonsParent.jsm
@@ +788,5 @@
>  let ChromeWindowInterposition = new Interposition("ChromeWindowInterposition",
>                                                    EventTargetInterposition);
>  
> +ChromeWindowInterposition.getters.content =
> +ChromeWindowInterposition.getters._content = function(addon, target) {

Would you mind documenting which add-on this is for? Or at least mentioning here that we're special-casing an old add-on using _content?
Attachment #8554821 - Flags: review?(mconley) → review+
Depends on: 1151335
Depends on: 1159616
No longer depends on: 1159616
Not planning to add the docshell shim, so closing.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
You need to log in before you can comment on or make changes to this bug.