707866 - Fix old sessions and remove 'about:blank' subframes once

Status: RESOLVED WONTFIX

(Firefox :: Session Restore, defect)

Description

[Tim Taubert [:ttaubert] (inactive)]

After bug 705597 has landed we should fix old sessions that possibly contain thousands of useless 'about:blank' subframes. We should only do that once because there certainly can be valid subframes but there is no way for us to distinguish these from 'dummy entries' inserted by session store.

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v1

It's pretty hard to write a test for this when cleaning up on session startup. But I confirmed it works on my local profile that is full of 'about:blank' subframes. Right after startup my sessionstore.js was down to 260KB (from 650+).

Comment 2

[Dão Gottwald [:dao]]

Won't bug 705597 automatically fix up old sessions when saving them anew?

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

No, this will only prevent it for future sessions. Old sessions still have 'about:blank' child entries that are loaded and considered valid (because we can't really tell them apart after they have been restored and re-saved once).

Comment 4

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Comment on attachment 580634 [details] [diff] [review]
patch v1

Review of attachment 580634 [details] [diff] [review]:
-----------------------------------------------------------------

This isn't going to fix _everything_, just open windows & tabs. If we want to be complete and make sure all of them are removed, we need to also look at _closedWindows & _closedTabs.

I'm also wary of adding this to startup. For the pathological cases it will slow down startup more, even if the session isn't going to be restored (and for most people it will be the first startup following a "major" update).

Let's move this to nsSessionStore and only do it if the session is about to be restored. For normal restore cases we're mostly in the same boat of main thread blocking. For deferred restore case, it won't be done until the session is restored.

My other other concern is Session Manager (or the more general case of dropping in an old sessionstore.js). We won't ever fix those other sessions up. So think about that. It may be that we need to add some flags to state.session indicating things like this. That or we don't fix up old sessions.

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

Talked to dietrich, we don't think it's worth to put real effort into that and messing up session store with some fix up code. When force-reloading a tab all those entries will be removed anyway. Even with normal navigation those entries will slowly be pushed out because we only store 50 of them at a max.