Remove _resume_session_once_on_shutdown code from SessionStore

RESOLVED FIXED in Firefox 25

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ttaubert, Assigned: ttaubert)

Tracking

Trunk
Firefox 25
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
This code has been added back in bug 600545:

> // The original "sessionstore.resume_session_once" preference value before it
> // was modified by saveState.  saveState will set the
> // "sessionstore.resume_session_once" to true when the
> // the "sessionstore.resume_from_crash" preference is false (crash recovery
> // is disabled) so that pinned tabs will be restored in the case of a
> // crash.  This variable is used to restore the original value so the
> // previous session is not always restored when
> // "sessionstore.resume_from_crash" is true.
> _resume_session_once_on_shutdown: null,

So the whole _resume_session_once_on_shutdown code is there so that pinned tabs are restored when Firefox crashes for people with resume_from_crash=false. It's quite obvious that we don't need this code as it works fine without it. Looking at nsSessionStartup.js:

> // set the startup type
> if (lastSessionCrashed && resumeFromCrash)
>   this._sessionType = Ci.nsISessionStartup.RECOVER_SESSION;
> else if (!lastSessionCrashed && doResumeSession)
>   this._sessionType = Ci.nsISessionStartup.RESUME_SESSION;
> else if (this._initialState)
>   this._sessionType = Ci.nsISessionStartup.DEFER_SESSION;
> else
>   this._initialState = null; // reset the state

If we have a valid state and we crash with resumeFromCrash=false we will always end up in the third branch and we will have a deferred session. That means only pinned tabs will be restored as desired.
(Assignee)

Comment 1

5 years ago
Created attachment 782146 [details] [diff] [review]
Remove _resume_session_once_on_shutdown code from SessionStore

The logic is a little tricky here. There's lots of legacy code around which doesn't make it easier to understand. I myself had to read for quite a while until I came to the conclusion that we should be able to just remove the code.
Attachment #782146 - Flags: feedback?(smacleod)
(Assignee)

Updated

5 years ago
Blocks: 898775
Comment on attachment 782146 [details] [diff] [review]
Remove _resume_session_once_on_shutdown code from SessionStore

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

Yay for code removal! Looks good to me.
Attachment #782146 - Flags: feedback?(smacleod) → feedback+
(Assignee)

Updated

5 years ago
Attachment #782146 - Flags: review?(dteller)
I'll try and review this on Thursday.
Comment on attachment 782146 [details] [diff] [review]
Remove _resume_session_once_on_shutdown code from SessionStore

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

Looks good to me.
Attachment #782146 - Flags: review?(dteller) → review+
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/mozilla-central/rev/d4e2c685f30f
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25
You need to log in before you can comment on or make changes to this bug.