670033 - Persist deferred session across a browser restart

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[Dave Townsend [:mossop]]

In bug 476430 we're going to be directing users to restart almost straight after starting their browser. This will currently lose any deferred session, throwing away whatever they were last viewing.

Comment 1

[Dave Townsend [:mossop]]

Attachment: WIP

This is a WIP that basically seems to work. It works by saving any existing deferred session along with the current session. On startup the previous previous session is thrown away unless we're doing a one-time restore in which case it is loaded into _lastSessionState ready for restoring at the users request.

Don't know whether to pursue this or hold off till the app tabs stuff changes

Comment 2

[Dave Townsend [:mossop]]

Attachment: WIP

Comment 3

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

Comment on attachment 544654 [details] [diff] [review]
WIP

So this _works_ for your case, but I'm not 100% sure it's not setting us up for more confusion. It definitely makes this a possibility...

0. user has enabled quit dialog
1. quit normally (don't "save and quit")
2. startup (restore previous session is enabled)
3. decide you don't need that session, just do normal browsing
4. quit, but set resume_session_once (via quit dialog, or maybe an extension)
5. session is restored, but restore previous session is also enabled, which is actually the previous previous session.

I don't want to do this as is, but apart from new topics like we talked about, there's no great way to do this. feedback:ehh?

Poking Dietrich to get another set of eyes on this

Comment 4

[Dave Townsend [:mossop]]

Attachment: patch rev 1

This should work better, clears the deferred session on shutdowns that aren't restarts. This makes the restore-next-time button in the quit dialog behave better.

Comment 5

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

Comment on attachment 548874 [details] [diff] [review]
patch rev 1

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

Looks good. My only nit is that you add _lastSessionState to the SessionStoreService prototype along with a comment.

Comment 6

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

(In reply to comment #5)

> Looks good. My only nit is that you add _lastSessionState to the
> SessionStoreService prototype along with a comment.

D'oh. That's there already. But do add a comment that it's persisted sometimes (primarily) to make the addon restart case better.

Comment 7

[Dave Townsend [:mossop]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/086328df641d

Comment 8

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/086328df641d