Closed Bug 364972 Opened 18 years ago Closed 18 years ago

[SessionStore] allow SessionStore to work without writing data to disk


(Firefox :: Session Restore, defect)

2.0 Branch
Not set





(Reporter: zeniko, Assigned: zeniko)


(Keywords: privacy, verified1.8.1.2)


(1 file, 1 obsolete file)

sessionstore.js should only be written when either (1) resume_from_crash is true or (2) we need the data for session resuming.

This allows privacy concerned people to disable crash recovering while keeping full Undo Close Tab functionality, i.e. the recommended way to disable Session Restore would become setting sessionstore.resume_from_crash to |false| and _not_ setting sessionstore.enabled to |false| (which really is only for extensions providing the same functionality in a different way).
Assignee: nobody → zeniko
Attachment #249625 - Flags: review?(
Branch drivers: fixing this bug on the branch would help those users who don't want crash recovery (for privacy reasons) but still want to be able to reopen accidentally closed tabs.
Flags: blocking1.8.1.2?
Keywords: privacy
Flags: blocking1.8.1.2? → blocking1.8.1.2+
Attachment #249625 - Flags: review?(dietrich)
Comment on attachment 249625 [details] [diff] [review]
make writing to disk dependent on resume_from_crash

This works ok, assuming default prefs. However, after applying this patch, if and resume_from_crash=false, sessions are not restored and this error is in the console:

Error: this._sessionFile has no properties
Source File: file:///Users/dayala/moz/branch/build/dist/
Line: 1720
Attachment #249625 - Flags: review?(dietrich) → review-
Attached patch fix v2Splinter Review
Yeah, of course we need a reference to the session files first before trying to remove the referenced files... sorry for that.
Attachment #249625 - Attachment is obsolete: true
Attachment #250222 - Flags: review?(dietrich)
Attachment #249625 - Flags: review?(
Comment on attachment 250222 [details] [diff] [review]
fix v2

thanks for fixing that, r+.
Attachment #250222 - Flags: review?(dietrich) → review+
Whiteboard: [checkin needed]
Gavin: may I ask you for another checkin? Thanks.

And while you're at it: bug 340895 could also need some CVS love... ;-)
(In reply to comment #6)
> Gavin: may I ask you for another checkin? Thanks.
> And while you're at it: bug 340895 could also need some CVS love... ;-)

i checked this in:

Checking in browser/components/sessionstore/src/nsSessionStore.js;
/cvsroot/mozilla/browser/components/sessionstore/src/nsSessionStore.js,v  <--  nsSessionStore.js
new revision: 1.56; previous revision: 1.55

i'll do 340895 as well.
Whiteboard: [checkin needed] → [baking]
Closed: 18 years ago
Resolution: --- → FIXED
Comment on attachment 250222 [details] [diff] [review]
fix v2

Drivers: See comment #2 for why this low-risk patch should be included on the branch.
Attachment #250222 - Flags: approval1.8.1.2?
Comment on attachment 250222 [details] [diff] [review]
fix v2

Approved for the 1.8 branch, a=jay for drivers.  Let's get this landed asap and get QA some time to test it thoroughly.
Attachment #250222 - Flags: approval1.8.1.2? → approval1.8.1.2+
Dietrich: the patch should directly apply to the branch as well. Could you please check it in and mark this bug fixed1.8.1.2? Thanks.
Whiteboard: [baking] → [checkin needed (1.8 branch)]
Checking in browser/components/sessionstore/src/nsSessionStore.js;
/cvsroot/mozilla/browser/components/sessionstore/src/nsSessionStore.js,v  <--  nsSessionStore.js
new revision:; previous revision:
Keywords: fixed1.8.1.2
Whiteboard: [checkin needed (1.8 branch)]
Please add test steps so QA can regress.  thanks.
Whiteboard: [need testcase]
Steps to reproduce:
1. Set the hidden pref browser.sessionstore.resume_from_crash to false
2. Make sure that there's no file sessionstore.js in your profile folder
3. Set Firefox to load the previously opened windows and tabs at startup
4. Close Firefox
5. Verify that sessionstore.js exists
6. Restart Firefox
7. sessionstore.js should be gone again
8. Set the pref from step 1 to true again
9. sessionstore.js exists once more
Component: General → Session Restore
QA Contact: general → session.restore
Verified using steps in comment #13: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20070206 BonEcho/

Also tried on Mac and Linux (ubuntu).
Whiteboard: [need testcase]
You need to log in before you can comment on or make changes to this bug.