1174666 - Remove unneeded SessionSaver.clearLastSaveTime()

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[Tim Taubert [:ttaubert] (inactive)]

If SessionStore.initializeWindow() were a little more intelligent we wouldn't need that function. Let's remove it and simplify the SessionSaver code.

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0001-Bug-1174666-Remove-unneeded-SessionSaver.clearLastSa.patch

Comment 2

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

Comment on attachment 8622386 [details] [diff] [review]
0001-Bug-1174666-Remove-unneeded-SessionSaver.clearLastSa.patch

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

::: browser/components/sessionstore/SessionStore.jsm
@@ +912,5 @@
>  
>        // restore a crashed session resp. resume the last session if requested
>        if (aInitialState) {
> +        // Don't write to disk right after startup.
> +        SessionSaver.updateLastSaveTime();

This is something of a hack. I fully support it, but please mention that you're resetting the clock [to avoid writing immediately].

@@ -938,5 @@
>          // Nothing to restore, notify observers things are complete.
>          Services.obs.notifyObservers(null, NOTIFY_WINDOWS_RESTORED, "");
> -
> -        // The next delayed save request should execute immediately.
> -        SessionSaver.clearLastSaveTime();

Mmmh... What did that do, exactly? And why don't we need it anymore?

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

(In reply to David Rajchenbach-Teller [:Yoric] (away June 22 - July 7th, use "needinfo") from comment #2)
> >        // restore a crashed session resp. resume the last session if requested
> >        if (aInitialState) {
> > +        // Don't write to disk right after startup.
> > +        SessionSaver.updateLastSaveTime();
> 
> This is something of a hack. I fully support it, but please mention that
> you're resetting the clock [to avoid writing immediately].

This isn't something new I'm adding, I'm just moving it. Will update the comment.

> @@ -938,5 @@
> >          // Nothing to restore, notify observers things are complete.
> >          Services.obs.notifyObservers(null, NOTIFY_WINDOWS_RESTORED, "");
> > -
> > -        // The next delayed save request should execute immediately.
> > -        SessionSaver.clearLastSaveTime();
> 
> Mmmh... What did that do, exactly? And why don't we need it anymore?

This was setting lastWriteTime=0, basically reverting the updateLastSaveTime() call from above. If we just move that to the proper branch we don't need it.

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0001-Bug-1174666-Remove-unneeded-SessionSaver.clearLastSa.patch, v2

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/8d99fb177495

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8d99fb177495