966253 - [Session Restore] Shutdown blocker doesn't work

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[David Teller [:Yoric] - still alive but not very active]

At http://dxr.mozilla.org/mozilla-central/source/browser/components/sessionstore/src/SessionFile.jsm?from=SessionFile.jsm#236

this should be SessionFileInternal._latestWrite, rather than SessionFile._latestWrite.

Comment 1

[David Teller [:Yoric] - still alive but not very active]

Attachment: Fixing SessionFile blocker

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8368539 [details] [diff] [review]
Fixing SessionFile blocker

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

Great find, we should make sure to get this into Fx 28+29. Actually, it would need to go into Fx 27 as well, which isn't going to happen...

Comment 3

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8368539 [details] [diff] [review]
Fixing SessionFile blocker

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 914581
User impact if declined: Possible sessionstore dataloss during shutdown.
Testing completed (on m-c, etc.): None, but it's a one-liner patch fixing a typo.
Risk to taking this patch (and alternatives if risky): None.
String or IDL/UUID changes made by this patch: None.

Comment 4

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8368539 [details] [diff] [review]
Fixing SessionFile blocker

It is indeed to late for 27 (currently the beta branch) but no worries for aurora.
(coucou David ;)

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/2b69d9881038

Any way to test this?

Comment 6

[David Teller [:Yoric] - still alive but not very active]

Ryan: Right now, I can't think of any.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2b69d9881038

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/fe4da45c3b86

Comment 9

[u279076]

David, does this have steps to reproduce that QA can use to verify this is fixed?

Comment 10

[David Teller [:Yoric] - still alive but not very active]

(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #9)
> David, does this have steps to reproduce that QA can use to verify this is
> fixed?

I can't think of anything reliable, I'm afraid.

Comment 11

[u279076]

(In reply to David Rajchenbach Teller [:Yoric] (please use "needinfo?") from comment #10)
> (In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #9)
> > David, does this have steps to reproduce that QA can use to verify this is
> > fixed?
> 
> I can't think of anything reliable, I'm afraid.

Okay, thank you.

Comment 12

[:Gavin Sharp [email: [e-mail]]]

What are the practical effects of this bug?

Comment 13

[Tim Taubert [:ttaubert] (inactive)]

The effect is that we might not wait for the SessionWorker to finish writing on shutdown. Which in turn might lead to data loss if there's data that hasn't made it to disk yet.

Comment 14

[David Teller [:Yoric] - still alive but not very active]

In the worst case, we might end up with:
- a session store representing the browsing state as it was ~15 seconds ago;
- some intriguing console warnings.