Closed Bug 1224105 Opened 6 years ago Closed 5 years ago

Use a <browser> element for the background page

Categories

(WebExtensions :: Untriaged, defect)

defect
Not set
normal

Tracking

(firefox46 fixed)

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: billm, Assigned: billm)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Right now we render the background page in a windowless docshell, which doesn't have a frameloader. In order to start running extensions out of process, we'll need to use a <browser> element so that we can set the remote attribute. My plan here is to create a windowless chrome docshell and put a content <browser> element inside it that would render the background page.
This is the patch we discussed today on IRC. Right now when you add a content docshell to a chrome docshell that was created with an nsWebBrowser, the content docshell doesn't get a tree owner. This patch creates a new tree owner for it. I tried to go with the simplest possible approach, since no one else is likely to use this code.
Attachment #8686447 - Flags: review?(bugs)
Comment on attachment 8686447 [details] [diff] [review]
Allow windowless chrome docshells containing content docshells

>+nsDocShellTreeOwner::EnsureContentTreeOwner()
>+{
>+  if (mContentTreeOwner)
>+    return;
Always {} with if

>@@ -786,16 +806,20 @@ nsDocShellTreeOwner::WebBrowser(nsWebBrowser* aWebBrowser)
>     RemoveChromeListeners();
>   }
>   if (aWebBrowser != mWebBrowser) {
>     mPrompter = nullptr;
>     mAuthPrompter = nullptr;
>   }
> 
>   mWebBrowser = aWebBrowser;
>+
>+  if (mContentTreeOwner) {
>+    mContentTreeOwner->WebBrowser(aWebBrowser);

make this

  if (mContentTreeOwner) {
    mContentTreeOwner->WebBrowser(aWebBrowser);
    if (!aWebBrowser) {
      mContentTreeOwner = nullptr;
    }
  }
so that we clear the edge when webbrowser->Destroy() is called.
Attachment #8686447 - Flags: review?(bugs) → review+
Kris, are you okay reviewing this?
Attachment #8689321 - Flags: review?(kmaglione+bmo)
Yeah, I don't see anything I'm not familiar with. I'll look at it tomorrow.
I guess the ext-utils.js thing you already fixed.
Comment on attachment 8689321 [details] [diff] [review]
use <browser> element for background page

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

(In reply to Bill McCloskey (:billm) from comment #5)
> I guess the ext-utils.js thing you already fixed.

Yeah. I don't really care which fix we keep, just so long as we don't keep both.
Attachment #8689321 - Flags: review?(kmaglione+bmo) → review+
https://hg.mozilla.org/mozilla-central/rev/4b03b2f843a8
https://hg.mozilla.org/mozilla-central/rev/0cb416a5606a
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.