560827 - e10s: Add remote support to browser XBL

Status: RESOLVED FIXED

(Toolkit :: XUL Widgets, defect)

Description

[Mark Finkle (:mfinkle) (use needinfo?)]

When <browser remote="true">, IPC is used to create content in a separate process. This state bascially kills all the useful properties and methods of the browser XBL binding.

We need to create a browser binding specifically for remote content.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: wip 1

This patch creates an XBL binding for a remote-aware browser. It extends the normal browser binding, but overrides .webNavigation and .webProgress so we can make many of the other properties remote-aware.

I want to do the same for the "find" properties too.

The patch also starts making changes to browser-ui.js and browser.js, to use the normal properties instead of the current e10s workarounds. I need to do more of this too.

Some parts of the code just start working again, like bookmarking.

Comment 2

[Oleg Romashin (:romaxa)]

Is this patch supposed to contain browser.xml file?

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: wip2 (now with browser.xml and browser.js)

Added missing files

Comment 5

[Benjamin Smedberg]

I don't think I understand. Are you creating a *new* browser.xml binding? Why not just fix the one we already have so that it works OOP or IP?

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #5)
> I don't think I understand. Are you creating a *new* browser.xml binding? Why
> not just fix the one we already have so that it works OOP or IP?

I am creating a binding that extends the current non-IPC binding. It is not completely new. This binding will only be activated when <browser remote="true" ... >

The new binding will live in the current browser.xml file in toolkit widgets.

Comment 7

[Benjamin Stover (:stechz)]

Attachment: Add progress listener support

I cleaned up redundant Fennec stuff (making the overall e10s patch smaller) and added progress listener support.  There's several ways the progress listener dispatcher is different for remote=true:
* aWebProgress and aRequest don't really have anything in them
* isRootWindow has been added to aWebProgress request since DOMWindow isn't accessible
* browser element is added to aWebProgress
* addProgressListener takes an extra parameter so that we can filter out iframe content before it even comes to chrome.  This may be too pre-optimized, but I know onStateChanged gets completely spammed when multiple iframes are being loaded.
* security change has a JSON object returned with it

I don't know if I'm going in the right direction, what do you think?  Since this interface is frozen I don't really know if we can get away with changing web progress with a new argument (see https://developer.mozilla.org/en/nsIWebProgress).

Do you have any ideas?

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 442197 [details] [diff] [review]
Add progress listener support

overall I like this approach in this patch. I am merging it into my current patch.

I am leaving out some of the optimizations with the progress listener for now.

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch

I think we should try to get this patch reviewed and landed so we can build on it. I don't expect this to be the final iteration by any means. The XBL is still in "mobile" while we clean up the fennec front-end code. We can easily move it to toolkit when we feel comfortable with it.

This patch includes:
* Removes more of the original browser XBL binding methods and properties from the new binding. We are inheriting more now, so we don't need the duplication.
* Adds support for basic .contentDocument property 
* Merges in Ben's web progress listener support and the Fennec code cleanup we got from that

With this patch Fennec is at least as functional as the existing e10s version, and probably more so, since bookmarking and other parts are working again.

Comment 10

[Benjamin Stover (:stechz)]

Comment on attachment 442303 [details] [diff] [review]
patch

Looks good!  Everything seems to work properly (back/forward, favicon, reload, bookmark, progress...)

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed to m-e10s:
http://hg.mozilla.org/users/pavlov_mozilla.com/mobile-e10s/rev/ccd4bef82fb2