[e10s] Exceptions during tests: "gBrowser._finalizeTabSwitch is not a function" and "newBrowser is null"

RESOLVED WORKSFORME

Status

()

Firefox
Tabbed Browser
RESOLVED WORKSFORME
4 years ago
4 years ago

People

(Reporter: billm, Assigned: mconley)

Tracking

Trunk
x86_64
Linux
Points:
---

Firefox Tracking Flags

(e10s?)

Details

Attachments

(1 obsolete attachment)

I see these a lot when running e10s tests. Here's an example command line:

mach mochitest-browser --e10s browser/base/content/test/general/browser_tabs_owner.js

However, most other tests in browser/base/content/test/general trigger these errors as well. I'm guessing it has to do with quickly opening and closing tabs.
tracking-e10s: --- → ?
I was seeing some of this working on a test the other day which was indeed opening and closing tabs quickly.
Assignee: nobody → mconley
The good news is that a number of the problems dealing with quickly closed new tabs causing null "newBrowser" or "toBrowser" references are eliminated by handyman's patch in bug 1059036.

I still need to handle the case where the tabbrowser binding has been destructed though - patch coming up...
Created attachment 8483696 [details] [diff] [review]
Ensure that tabbrowser hasn't destructed by the time MozAfterRemotePaint promises are resolved. r=?

It's possible, especially in tests, for tabs to close very quickly after they open. Opening
a tab in e10s causes us to set up some Promises that resolve when the newly opened browser
is ready to paint. If a tab has closed quickly enough after it has opened, it's possible
for the Promise to resolve, and for the tabbrowser binding to have been destructed,
in which case the calls into the binding from the Promise resolve handler don't work. We
check for a destructed tabbrowser binding in the Promise resolve / reject handlers now.
This needs to be applied on top of handyman's patch, which hasn't yet merged to m-c.

Mossop - this seemed to get rid of all of the warnings for me... can you confirm on your end?
Depends on: 1059036
Flags: needinfo?(dtownsend+bugmail)
I can't reproduce anymore even without this patch, so ... meh?
Flags: needinfo?(dtownsend+bugmail)
Comment on attachment 8483696 [details] [diff] [review]
Ensure that tabbrowser hasn't destructed by the time MozAfterRemotePaint promises are resolved. r=?

Alright, well, I think these sort of checks might still be worth handling, since they're theoretically hittable. Feel OK reviewing tabbrowser stuff?
Attachment #8483696 - Flags: review?(dtownsend+bugmail)
Can you provide a code example hitting this?
Interesting... I can't. I've engineered a test that successfully destroys the tabbrowser binding before a MozAfterRemotePaint gets hit and the Promise resolves, and yet the gBrowser._finalizeTabSwitch method is still defined.

I'm no longer convinced that this is a thing we need to worry about. I'm going to mark this bug as WORKSFORME, unless there are any objections.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → WORKSFORME

Updated

4 years ago
Attachment #8483696 - Attachment is obsolete: true
Attachment #8483696 - Flags: review?(dtownsend+bugmail)
You need to log in before you can comment on or make changes to this bug.