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

RESOLVED INCOMPLETE

Status

()

Firefox
Tabbed Browser
RESOLVED INCOMPLETE
3 years ago
2 years ago

People

(Reporter: Denis, Unassigned)

Tracking

36 Branch
x86
Mac OS X
Points:
3
Dependency tree / graph
Bug Flags:
firefox-backlog +
qe-verify -

Firefox Tracking Flags

(e10s+)

Details

(Reporter)

Description

3 years ago
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.
Dave, considering your work on bug 1093594, can you look into this? :-)
Blocks: 516752
tracking-e10s: --- → ?
Component: Untriaged → Tabbed Browser
Flags: needinfo?(dtownsend+bugmail)
I can but it's likely to be more than two weeks until I can do so
Blocks: 905436
tracking-e10s: ? → +
(Reporter)

Comment 3

3 years ago
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");
(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.
(Reporter)

Comment 5

3 years ago
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.
(Reporter)

Comment 6

3 years ago
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.
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).
Status: UNCONFIRMED → NEW
Points: --- → 3
Ever confirmed: true
Flags: qe-verify-
Flags: needinfo?(dtownsend+bugmail)
Flags: firefox-backlog+
Summary: nsIWebProgressListener.onStateChange() isn't called for a just created tab → Add an event so extensions know that a browser has been re-created when moving from remote to non-remote or vice versa

Comment 8

2 years ago
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.
It looks like we fire a TabRemotenessChange event on the <xul:tab> after switching remoteness. Is that sufficient?
Flags: needinfo?(disya2)

Comment 10

2 years ago
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

2 years ago
Just for reference, it looks like the TabRemotenessChange event was introduced in bug 1118618.
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.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Flags: needinfo?(disya2)
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.