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

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[Simon Bünzli]

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).

Comment 1

[Simon Bünzli]

Attachment: make writing to disk dependent on resume_from_crash

Comment 2

[Simon Bünzli]

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.

Comment 3

[Dietrich Ayala (: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 browser.startup.page=3 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/BonEcho.app/Contents/MacOS/components/nsSessionStore.js
Line: 1720

Comment 4

[Simon Bünzli]

Attachment: fix v2

Yeah, of course we need a reference to the session files first before trying to remove the referenced files... sorry for that.

Comment 5

[Dietrich Ayala (:dietrich)]

Comment on attachment 250222 [details] [diff] [review]
fix v2

thanks for fixing that, r+.

Comment 6

[Simon Bünzli]

Gavin: may I ask you for another checkin? Thanks.

And while you're at it: bug 340895 could also need some CVS love... ;-)

Comment 7

[Dietrich Ayala (:dietrich)]

(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
done

i'll do 340895 as well.

Comment 8

[Simon Bünzli]

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.

Comment 9

[Jay Patel [:jay]]

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.

Comment 10

[Simon Bünzli]

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.

Comment 11

[Dietrich Ayala (:dietrich)]

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

Comment 12

[Tony Chung [:tchung]]

Please add test steps so QA can regress.  thanks.

Comment 13

[Simon Bünzli]

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

Comment 14

[juan becerra [:juanb]]

Verified using steps in comment #13: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.2pre) Gecko/20070206 BonEcho/2.0.0.2pre

Also tried on Mac and Linux (ubuntu).