1438440 - updateCurrentBrowser() tries to mutate being destroyed docshell

Status: RESOLVED INVALID

(Firefox :: Tabbed Browser, defect)

Description

[Hiroyuki Ikezoe (:hiro)]

In bug 1432396, I am going to add a bunch of assertions that does not allow to mutate docshell which is being destroyed.  On non-E10S, about-blank-replacement.https.html test hits the one of the assertions.

https://treeherder.mozilla.org/logviewer.html#?job_id=162293181&repo=try&lineNumber=20713

It turned out updateCurrentBrowser() tries to mutate being destroyed docshell.

Call stack;
0 updateCurrentBrowser() ["chrome://browser/content/tabbrowser.xml":1250]
    this = [object XULElement]
1 onselect(event = [object Event]) ["chrome://browser/content/browser.xul":1]
    this = [object XULElement]
2 set_selectedIndex(val = 1) ["chrome://global/content/bindings/tabbox.xml":667]
    this = [object XULElement]
3 set_selectedPanel(val = [object XULElement]) ["chrome://global/content/bindings/tabbox.xml":686]
    this = [object XULElement]
4 set_selectedIndex(val = 1) ["chrome://global/content/bindings/tabbox.xml":392]
    this = [object XULElement]
5 set_selectedItem(val = [object XULElement]) ["chrome://global/content/bindings/tabbox.xml":424]
    this = [object XULElement]
6 set_selectedTab(val = [object XULElement]) ["chrome://global/content/bindings/tabbox.xml":100]
    this = [object XULElement]
7 set_selectedTab(val = [object XULElement]) ["chrome://browser/content/tabbrowser.xml":3966]
    this = [object XULElement]
8 _blurTab(aTab = [object XULElement]) ["chrome://browser/content/tabbrowser.xml":3513]
    this = [object XULElement]
9 _beginRemoveTab(aTab = [object XULElement], aAdoptedByTab = null, aCloseWindowWithLastTab = null, aCloseWindowFastpath = true, aSkipPermitUnload = true) ["chrome://browser/content/tabbrowser.xml":3227]
    this = [object XULElement]
10 removeTab(aTab = [object XULElement], aParams = [object Object]) ["chrome://browser/content/tabbrowser.xml":3130]
    this = [object XULElement]
11 onxblDOMWindowClose(event = [object Event]) ["chrome://browser/content/tabbrowser.xml":6130]
    this = [object XULElement]
12 anonymous(evt = [object Event]) ["https://web-platform.test:8443/service-workers/service-worker/resources/about-blank-replacement-popup-frame.py":14]
13 doAsyncTest() ["https://web-platform.test:8443/service-workers/service-worker/about-blank-replacement.https.html":98]
    this = [object Window]

I suspect _findTabToBlurTo returns the destroying tab.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #0)
> Call stack;
> 0 updateCurrentBrowser() ["chrome://browser/content/tabbrowser.xml":1250]
>     this = [object XULElement]

The top frame is at 'newBrowser.setAttribute("primary", "true");' [1].  I am trying to understand why it's called on non-E10S.  A patch in bug 1009628 comment 65 didn't yet have E10S check,  bug 1009628 comment 66 did have E10S check.

mconley, would you mind explaining why we do the setAttribute() call only for non-E10S?  Does it mean these processes are done synchronously on non-E10S whereas asynchronously on E10S?  If we can't avoid the call for non-E10S, I'll have to drop an assertion nsDocShell.cpp.  
 
[1] https://hg.mozilla.org/mozilla-central/file/026401920e32/browser/base/content/tabbrowser.xml#l1249

Comment 2

[Hiroyuki Ikezoe (:hiro)]

I am still curious why it's necessary only for non-E10S, but I believe this bug is invalid now.

Comment 3

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

(In reply to Hiroyuki Ikezoe (:hiro) from comment #2)
> I am still curious why it's necessary only for non-E10S, but I believe this
> bug is invalid now.

As you noticed, when gMultiProcessBrowser is false, we skip this codepath. This is because we do the work earlier in the async tab switcher over here:

https://searchfox.org/mozilla-central/rev/bd05e3853c6e982e2a35c1cc404b987b2bc914d6/browser/base/content/tabbrowser.xml#5176

in requestTab, which occurs before we fire the select event which normally calls updateCurrentBrowser:

https://searchfox.org/mozilla-central/rev/bd05e3853c6e982e2a35c1cc404b987b2bc914d6/browser/base/content/tabbrowser.xml#8622