Investigate incremental tab destruction when destroying multiple tabs

NEW
Unassigned

Status

()

Firefox
Tabbed Browser
6 years ago
6 years ago

People

(Reporter: pcwalton, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
It'd be nice if we spun the event loop after destroying each tab when the user performs operations like "Close Other Tabs". That way, the UI will stay responsive. We could even instantly destroy the tab in the UI, but hang onto a reference, and drop each reference one at a time.
Instead of spinning the event loop, could we we do the following?

1. hide all tabs in the UI;
2. start a recursive setTimeout or Schedule API-based (see bug 692420) loop to kill the tabs one by one (or a few at a time)
(Reporter)

Comment 2

6 years ago
(In reply to David Rajchenbach Teller [:Yoric] from comment #1)
> Instead of spinning the event loop, could we we do the following?
> 
> 1. hide all tabs in the UI;
> 2. start a recursive setTimeout or Schedule API-based (see bug 692420) loop
> to kill the tabs one by one (or a few at a time)

Right, that's what I meant. I didn't mean nested event loops.
In that case, if there is no hurry, I can try and look at that.
One thing we might be able to do is force a CC after killing each tab.  Or something like the lazy content blaster (bug 730581) may handle this.
"Close Other Tabs" is easy to isolate - it uses tabbrowser's specialized removeAllTabsBut() function. But a more commonly encountered case of multiple tab closing is sessionstore's setBrowserState (which is used when going into private browsing mode, and when restoring sessions after startup), which just uses tabs.forEach(tabbrowser.removeTab). I think we should introduce a tabbrowser API that takes an array of tabs to close, and does it efficiently (per comment 1), and then make setBrowserState use that.

Updated

6 years ago
OS: Mac OS X → All
Hardware: x86 → All
Created attachment 601748 [details] [diff] [review]
patch that you don't want to use

Using setTimeout. This seems more disruptive/annoying than the current behavior.
(Reporter)

Comment 7

6 years ago
Could we do this:

(1) Build up an array of references to the content documents we want to delete;
(2) Call removeTab() to delete each document;
(3) Slowly drop a reference to each content document, by nulling out each element of the array, one by one?
Created attachment 601800 [details] [diff] [review]
patch where I can't tell if it helps

(In reply to Patrick Walton (:pcwalton) from comment #7)
> Could we do this:
> 
> (1) Build up an array of references to the content documents we want to
> delete;
> (2) Call removeTab() to delete each document;
> (3) Slowly drop a reference to each content document, by nulling out each
> element of the array, one by one?

Sure. Can you apply this and test if this improves things as expected?
You need to log in before you can comment on or make changes to this bug.