564520 - Use sessionstore.json instead of sessionstore.js

Status: RESOLVED WONTFIX

(Firefox :: Session Restore, enhancement)

Description

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

After bug 387859 gets fixed, we'll actually be saving JSON to disk. As such, we can actually call our file sessionstore.json.

Will require support for loading sessionstore.js if sessionstore.json doesn't exist (as well as removing the file once it's loaded).

Comment 1

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

Attachment: Patch v0.1 (WIP)

Quick 2 minute patch. Took the simple way to read from sessionstore.js if sessionstore.json doesn't exist.

Todo:
* Should probably delete sessionstore.js in _clearDisk as well, in case somebody manages to get into a state where sessionstore.js and .json exist at startup.
* Need to update the tests which look to see when sessionstore.js changes

Comment 2

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

I could also move some of those checks around so that we look at the prefs before trying to move the file (before even doing anything with dirservice). I think anything we can avoid doing on the filesystem is worth it. Also, we can use Services.dirsvc (I either missed that before, or it got added after I switched to using it)

Comment 3

[Simon Bünzli]

Comment on attachment 449373 [details] [diff] [review]
Patch v0.1 (WIP)

Bug 387859 comment #38 reminded me that this change would also prevent Firefox 3.5 and 3.6 from resuming a session after a downgrade.

So what we should do instead for now is either WONTFIX this bug and stick with the filename we're stuck with - or to add code to write both sessionstore.js and sessionstore.json as a transitory measure (and read whichever is available). Then we could drop writing sessionstore.js with Firefox.next.next and drop reading it with Firefox.next^3 or something.

Comment 4

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

(In reply to comment #3)
> (From update of attachment 449373 [details] [diff] [review])
> Bug 387859 comment #38 reminded me that this change would also prevent Firefox
> 3.5 and 3.6 from resuming a session after a downgrade.
> 
> So what we should do instead for now is either WONTFIX this bug and stick with
> the filename we're stuck with - or to add code to write both sessionstore.js
> and sessionstore.json as a transitory measure (and read whichever is
> available). Then we could drop writing sessionstore.js with Firefox.next.next
> and drop reading it with Firefox.next^3 or something.

Bah this sucks. I would be ok writing both at quit, but writing both every time we write seems less than ideal. It seems less likely to happen, but it would be great to add the ability to read sessionstore.json (in the same fashion this does here, only if sessionstore.js doesn't exist) to 3.5/6

As a major release, 4.0 would be a great time to make the change if we're ever going to do it.

To be fair though, being stuck with sessionstore.js isn't that terrible a thing.

Comment 5

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

After a chat with Dietrich, we'll take the WONTFIX here. The extra disk access stuff isn't worth it. Since Firefox 3.5/3.6 read the JSON file just fine, it's not a big deal.