Closed Bug 898755 Opened 11 years ago Closed 11 years ago

Remove _resume_session_once_on_shutdown code from SessionStore

Categories

(Firefox :: Session Restore, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 25

People

(Reporter: ttaubert, Assigned: ttaubert)

References

Details

Attachments

(1 file)

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.
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)
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+
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+
https://hg.mozilla.org/mozilla-central/rev/d4e2c685f30f
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: