When closing multiple tabs, the PermitUnload message should be sent to each tab in parallel

NEW
Unassigned

Status

()

Firefox
Tabbed Browser
P3
normal
4 months ago
a month ago

People

(Reporter: florian, Unassigned)

Tracking

(Blocks: 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

4 months ago
Currently the message is sent to one tab, then we spin the event loop until we have a reply: http://searchfox.org/mozilla-central/rev/f1472e5b57fea8d4a7bbdd81c44e46958fe3c1ce/toolkit/content/widgets/remote-browser.xml#344

This is very slow when closing lots of tabs, especially when the reason for closing many tabs is that too much memory is being used, because in that case the content processes will be most likely busy gc'ing. We'll have to wait until it is done GC'ing before getting a reply, but it would be nice if it could reply at once for all the tabs that it owns.

Here is a profile where closing ~160 tabs spent 5.5+s in permitUnload: https://perfht.ml/2toSPHw
Here's how we might approach this:

1) For the set of closed tabs, find which ones have beforeunload set (via the nsITabParent hint), and close those immediately.
2) For the remaining tabs, group them by which process they're in, and send a single message to each process with some kind of unique identifier for each tab that's closing.
3) Spin the event loop until either:
  a) We've heard back from each content process, and can choose whether or not to abort the unload for one or more tabs
  b) Some very small timeout has been exceeded

Inside the content process, we'll need something to receive that single message, find the associated top-level windows, and run the permitUnload function on each, serially. I think it might be better to message the parent as soon as we find a page that wants to block unloading, instead of waiting until all pages have run (since we can't be sure how long running them all will take).

Another thing we could do is continue to close tabs while the user is trying to decide whether or not to allow the tab / window to close.
(In reply to Mike Conley (:mconley) - Digging out of needinfo / review backlog. from comment #1)
> Another thing we could do is continue to close tabs while the user is trying
> to decide whether or not to allow the tab / window to close.

To be clear, I mean, if we have tabs A, B, C and D, all running in the same content process, and B wants to block unloading, we might want to do:

1) Run A's permitUnload, find that it doesn't want to block, move onto the next...
2) Run B's permitUnload, find that it _DOES_ want to block, message parent immediately, and then...
3) Run C and D's permitUnload without waiting for any kind of response from the parent. Keep B open in the meantime.
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.