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

Status: RESOLVED WORKSFORME

(Firefox :: Tabbed Browser, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

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.

Comment 1

[Dave Townsend [:mossop]]

I was seeing some of this working on a test the other day which was indeed opening and closing tabs quickly.

Comment 2

[Mike Conley (: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...

Comment 3

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

Attachment: 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.

Comment 4

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

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?

Comment 5

[Dave Townsend [:mossop]]

I can't reproduce anymore even without this patch, so ... meh?

Comment 6

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

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?

Comment 7

[Dão Gottwald [:dao]]

Can you provide a code example hitting this?

Comment 8

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

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.