730402 - Investigate incremental tab destruction when destroying multiple tabs

Status: NEW

(Firefox :: Tabbed Browser, defect)

Description

[Patrick Walton (:pcwalton)]

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.

Comment 1

[David Teller [:Yoric] - still alive but not very active]

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)

Comment 2

[Patrick Walton (:pcwalton)]

(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.

Comment 3

[David Teller [:Yoric] - still alive but not very active]

In that case, if there is no hurry, I can try and look at that.

Comment 4

[Andrew McCreight [:mccr8]]

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.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

"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.

Comment 6

[Dão Gottwald [:dao]]

Attachment: patch that you don't want to use

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

Comment 7

[Patrick Walton (:pcwalton)]

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?

Comment 8

[Dão Gottwald [:dao]]

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