1102373 - Add an event so extensions know that a browser has been re-created when moving from remote to non-remote or vice versa

Status: RESOLVED INCOMPLETE

(Firefox :: Tabbed Browser, defect)

Description

[Denis]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/39.0.2171.62 Safari/537.36

Steps to reproduce:

I'm trying to migrate out add-on to e10s thing. To monitor page load we use gBrowser.selectedBrowser.addProgressListener where listener implements onStateChange() method.  After adding the listener the add-on navigates to some page using the browser's loadURI() method.


Actual results:

It works fine if a tab corresponding to selectedBrowser has already some web-page loaded but if it is a just created tab onStateChange() never called.


Expected results:

onStateChange() should be called with appropriate flags set.

Comment 1

[:Gijs (he/him)]

Dave, considering your work on bug 1093594, can you look into this? :-)

Comment 2

[Dave Townsend [:mossop]]

I can but it's likely to be more than two weeks until I can do so

Comment 3

[Denis]

I've just been trying to make a test case and found that it works now in Nightly 36.0a1 (2014-11-25). 

I'm not sure if it was fixed along with something else or it was caused by some bug in my code.

Here is a test case I used just now:
gBrowser.selectedTab = gBrowser.addTab();
gBrowser.selectedBrowser.addProgressListener({
    onStateChange: function (webProgress, req, flags, status) {
	console.log("onStateChange, flags=%d, status=%d", flags, status);
    },
    QueryInterface: function(iid) {
	if (iid.equals(Components.interfaces.nsIWebProgressListener) ||
	    iid.equals(Components.interfaces.nsISupportsWeakReference) ||
	    iid.equals(Components.interfaces.nsISupports))
	    return this;
	throw Components.results.NS_NOINTERFACE;
    }
});
gBrowser.selectedBrowser.loadURI("http://mozilla.org");

Comment 4

[Dave Townsend [:mossop]]

(In reply to Denis from comment #3)
> I've just been trying to make a test case and found that it works now in
> Nightly 36.0a1 (2014-11-25). 

Do you have e10s enabled? It was disabled by default a few days ago.

Comment 5

[Denis]

Yes, I think it is enabled since tab's titles are underlined.

(In reply to Dave Townsend [:mossop] (Out till December 1st) from comment #4)
> (In reply to Denis from comment #3)
> > I've just been trying to make a test case and found that it works now in
> > Nightly 36.0a1 (2014-11-25). 
> 
> Do you have e10s enabled? It was disabled by default a few days ago.

Comment 6

[Denis]

I think I narrowed the issue. onStateChange() isn't called when I manually click on "+" to open a new tab. That is if the .addProgressListener is called for a browser corresponding to "about:newtab". Steps to reproduce the bug:

1. Manually create a new tab or navigate to "about:newtab".
2. Execute the following code

gBrowser.selectedBrowser.addProgressListener({
    onStateChange: function (webProgress, req, flags, status) {
	console.log("onStateChange, flags=%d, status=%d", flags, status);
    },
    QueryInterface: function(iid) {
	if (iid.equals(Components.interfaces.nsIWebProgressListener) ||
	    iid.equals(Components.interfaces.nsISupportsWeakReference) ||
	    iid.equals(Components.interfaces.nsISupports))
	    return this;
	throw Components.results.NS_NOINTERFACE;
    }
});
gBrowser.selectedBrowser.loadURI("http://mozilla.org");


Expected result: messages in console corresponding to state change
Actual result: onStateChange() isn't called at all

The first step is important because when I call gBrowser.addTab("about:newtab") in the code the things start working for some reason.

Comment 7

[Dave Townsend [:mossop]]

Oh I bet this is the hack we use when we switch from a non-remote page (about:newtab) to a remote page (any website) in the browser. We currently recreate the browser which will throw away any registered progress listeners.

It's not impossible for us to cache the progress listeners and re-register them automatically but my preferred choice here would be to add some event that extensions can listen to so they know to re-add anything important (progress listeners, framescripts and message listeners all get lost right now).

Comment 8

[Ian Nartowicz]

A very simple test case is to open a new window, or just the first window when Firefox starts.  Add a web progress listener to the selected browser, then load a web page.  The listener doesn't get any calls.

Adding the listener to gBrowser works.  It reconnects its progress listener when a browser's remoteness changes.  It would be nice if it could transfer the listeners of that browser as well as its own listener.  Failing that expose the change, but that makes things pretty clunky for callers.  For example, adding a specific listener just to listen to one page load, then finding it doesn't fire, isn't too nice.

Comment 9

[Mike Conley (:mconley) (:⚙️)]

It looks like we fire a TabRemotenessChange event on the <xul:tab> after switching remoteness. Is that sufficient?

Comment 10

[Ian Nartowicz]

That looks like a possibility, but it still begs the question: why make dozens of consumers all create exactly the same event listener?  It is hard to imagine a progress listener on a browser element that wouldn't want to keep listening when tab remoteness changes.  So if they all want it, why not make it happen automatically?  At the very least, a wrapper in browser.xml would stop the proliferation of identikit code.

I ran some quick tests with a one-line event listener on TabRemotenessChange.  It seemed to work, but the timing is critical.  I guess if it happens synchronously at the right moment, it will always be OK.  Latest nightly is giving me grief, so I'll run some more tests when I have an e10s browser that works.

Comment 11

[Ian Nartowicz]

Just for reference, it looks like the TabRemotenessChange event was introduced in bug 1118618.

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

Clearing the needinfo and closing the bug as incomplete due to lack of response from the reporter. Please reopen if you still need this behavior.