921581 - [Session Restore] Send a notification once the final sessionstore.js has been written to disk

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

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

We need this information to verify whether we have crashed before this point.

Comment 1

[Steven MacLeod [:smacleod]]

Attachment: Patch - Send a notification when the final state is written

This patch introduces a new "sessionstore-shutdown-state-write-complete" notification which will fire after the final session state is written at shutdown.

I moved where we set |this._isClosed| in order to ensure no other writes sneak in between the time we start writing asynchronously, and the time the write finishes.

Comment 2

[Steven MacLeod [:smacleod]]

Try push: https://tbpl.mozilla.org/?tree=Try&rev=c0c43310899f

I also tested this patch by modifying the crashmonitor patch in Bug 888373, and using it with the new notification. Things seem to work.

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 818054 [details] [diff] [review]
Patch - Send a notification when the final state is written

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

LGTM. Would be good to have David review as well, he knows all the shutdown stuff.

::: browser/components/sessionstore/src/_SessionFile.jsm
@@ +252,5 @@
>                         + ": " + ex);
>        }
> +
> +      if (isFinalWrite) {
> +        Services.obs.notifyObservers(null, "sessionstore-shutdown-state-write-complete", "");

Maybe "sessionstore-final-state-write-complete"? Not sure.

Comment 4

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

Comment on attachment 818054 [details] [diff] [review]
Patch - Send a notification when the final state is written

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

LGTM, too.
I have no strong opinion on the topic string, but the one suggested by Tim looks good.

Comment 5

[Steven MacLeod [:smacleod]]

Attachment: Patch - Send a notification when the final state is written v2

Changed the topic string to Tim's suggestion.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/34c0b12b9c10

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/34c0b12b9c10