Closed Bug 921581 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox :: Session Restore, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 27

People

(Reporter: Yoric, Assigned: smacleod)

References

Details

Attachments

(1 file, 1 obsolete file)

We need this information to verify whether we have crashed before this point.
Assignee: nobody → smacleod
Status: NEW → ASSIGNED
Depends on: 922785
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.
Attachment #818054 - Flags: review?(ttaubert)
Attachment #818054 - Flags: feedback?(dteller)
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 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.
Attachment #818054 - Flags: review?(ttaubert)
Attachment #818054 - Flags: review?(dteller)
Attachment #818054 - Flags: review+
Attachment #818054 - Flags: feedback?(dteller)
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.
Attachment #818054 - Flags: review?(dteller) → review+
Changed the topic string to Tim's suggestion.
Attachment #818054 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/34c0b12b9c10
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 27
You need to log in before you can comment on or make changes to this bug.