1294388 - [e10s-multi] remote PermitUnload messages can get lost for a recently opened tab that use a new content process

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect, P3)

Description

[Gabor Krizsanits (INACTIVE)]

With multiple content processes I get this error:

browser/base/content/test/general/browser_bug380960.js | tab removed immediately - Got [object XULElement], expected null

Comment 1

[Gabor Krizsanits (INACTIVE)]

This seem to happen only on windows and the reason for the failure is that in tabbrowser.xml _beginRemoveTab the browser.permitUnload() call times out.

After some logging it seems like that by the time we add the "PermitUnload" listener in browser-child.js
we already missed the message sent from permitUnload in remote-browser.xml.

We could use the nested event loop to somehow make sure that the browser-child.js frame script is already executed before we use the mm to send the message. Not sure yet if this is the right approach...

Comment 2

[Gabor Krizsanits (INACTIVE)]

Attachment: Increase remote permitUnload timeout for test

It turns out that I don't have to do the extra hack I mentioned in my previous comment, it's enough to increase the timeout for this test (and maybe some others).

I've been debugging this for ages, I simply wanted to force single process to make this test pass, but that didn't work. I thought that there is a subtle bug somewhere and we create processes when we should not, and we're slow here because we start up a content process for the tab opening. But what happens is that simply the testing framework starting up, opening a few window and a tab, which results some extra process creation before we could get to the actual test code that would set the dom.ipc.processCount back to 1. Because of all this (as starting/killing content processes on windows in debug mode with sandbox on is super slow) this particular operation is simply really slow.

In the end I had to increase the timeout, but I don't want this change to affect release builds, 5 seconds there is plenty. I wish our windows debug builds were less horrible in terms of performance.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8791252 [details] [diff] [review]
Increase remote permitUnload timeout for test

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

::: toolkit/content/widgets/remote-browser.xml
@@ +306,5 @@
>            let responded = false;
>            let permitUnload;
>            let id = this._permitUnloadId++;
>            let mm = this.messageManager;
>            let Services = Components.utils.import("resource://gre/modules/Services.jsm", {}).Services;

Pull this Services above getting the pref and use Services.prefs.

Comment 4

[Gabor Krizsanits (INACTIVE)]

This should be fixed by bug 1211637.