Closed Bug 1453280 Opened 2 years ago Closed 2 years ago

Adopting the last tab of a ChromeWindow should cleanup both _tabListeners and _tabFilters and the old progress listener should be destroyed

Categories

(Firefox :: Tabbed Browser, defect, P3)

defect

Tracking

()

RESOLVED FIXED
Firefox 61
Tracking Status
firefox61 --- fixed

People

(Reporter: rpl, Assigned: rpl)

Details

Attachments

(1 file)

I notice this issue while I was looking into Bug 1443221 and Bug 1423705,
but the issue is actually unrelated to both those bugs and it doesn't need an extension to be triggered. 

The STR is:

- open Firefox
- create a new Firefox window
- move the the tab from the first open Firefox window into the new Firefox window
  (which is going to adopt that tab into the new window, and then the first Firefox window is closed because that was its only tab)
- load any webpage into the adopted tab

The following TypeError errors are being logged from the tabbrowser progress listeners on every "load" triggered in that tab (and also a bunch of "RemoteWebProgress failed to call" exceptions raised because of those errors):

    TypeError: this.mBrowser.didStartLoadSinceLastUserTyping is not a function[Learn More] tabbrowser.js:4476:11

    TypeError: this.mBrowser.urlbarChangeTracker is undefined[Learn More] tabbrowser.js:4321:11

    TypeError: this.mBrowser.didStartLoadSinceLastUserTyping is not a function[Learn More] tabbrowser.js:4476:11

    TypeError: this.mBrowser.urlbarChangeTracker is undefined[Learn More] tabbrowser.js:4410:11

By repeating the above STR again with the same tab, the number of times these errors are logged increase accordingly (and that's not actually surprising).

I've investigated it a bit, and it seems that the underlying reason for these errors is that when we are adopting the tab from a Firefox window that is going to be closed, we are clearing the tab's filter for that tab, but we are not also clearing (and destroying) the corresponding tab's listener here:

https://searchfox.org/mozilla-central/rev/b55e1a1cbcaee34878e133fbac20c4c2af6e11b5/browser/base/content/tabbrowser.js#2741

On the contrary if the adopted tab wasn't the last one in its previous window, it seems that the listener is going to be cleared and destroyed here:

https://searchfox.org/mozilla-central/rev/b55e1a1cbcaee34878e133fbac20c4c2af6e11b5/browser/base/content/tabbrowser.js#2794-2796
Did you mean to request review on this patch?
Flags: needinfo?(lgreco)
Priority: -- → P3
Attachment #8966948 - Flags: review?(dao+bmo)
Flags: needinfo?(lgreco)
Comment on attachment 8966948 [details]
Bug 1453280 - Clear and destroy tabbrowser tab's listener when adopting the last tab of a window.

https://reviewboard.mozilla.org/r/235614/#review243360

Thanks!
Attachment #8966948 - Flags: review?(dao+bmo) → review+
Assignee: nobody → lgreco
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/67cef646951c
Clear and destroy tabbrowser tab's listener when adopting the last tab of a window. r=dao
https://hg.mozilla.org/mozilla-central/rev/67cef646951c
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
You need to log in before you can comment on or make changes to this bug.