Closed Bug 911146 Opened 6 years ago Closed 6 years ago

nsIAppStartup should be able to tell whether the app was restarted

Categories

(Toolkit :: Startup and Profile System, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: ttaubert, Assigned: ttaubert)

References

Details

(Keywords: dev-doc-needed)

Attachments

(1 file, 2 obsolete files)

It would be great if nsIAppStartup had a 'wasRestarted' flag that we could use to tell whether the current startup is the one following a restart.

http://hg.mozilla.org/mozilla-central/file/c7459bc8e449/xpcom/ds/TimeStamp.cpp#l60

TimeStamp has its own implementation. SessionStore does something similar (it sets a pref that is reset on next startup) to resume the session automatically.

http://hg.mozilla.org/mozilla-central/file/065727546e13/browser/components/sessionstore/src/SessionStore.jsm#l1025
Attached patch add nsIAppStartup.wasRestarted (obsolete) — Splinter Review
Sorry, not sure who to ask for feedback.
Attachment #797803 - Flags: feedback?(dtownsend+bugmail)
Attachment #797803 - Flags: feedback?(benjamin)
Attachment #797803 - Flags: feedback?(dtownsend+bugmail) → feedback+
Comment on attachment 797803 [details] [diff] [review]
add nsIAppStartup.wasRestarted

I think the IDL comment needs to be more specific: that this is a restart initiated via the eRestart flag. It isn't for "the user shut down the app and restarted it" ;-)

Do we have any tests for this mess?
Attachment #797803 - Flags: feedback?(benjamin) → feedback+
This patch clarifies comments in the IDL file and also adds a 'willRestart' attribute. That turned out to be a helpful addition to 'shuttingDown' especially for SessionStore.

Unfortunately I don't think that there's a way to test this stuff. I couldn't find any tests calling nsIAppStartup.quit() and I don't think any of our test suites supports that.

We could maybe ask the MozMill folks and check if they would like to write a test in a follow-up?
Assignee: nobody → ttaubert
Attachment #797803 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #801463 - Flags: review?(dtownsend+bugmail)
Attachment #801463 - Flags: review?(benjamin)
Blocks: 914062
Attachment #801463 - Flags: review?(benjamin) → review+
Attachment #801463 - Flags: review?(dtownsend+bugmail)
Wow, just as I wanted to land this I saw some UUID conflict caused by bug 882142. Turns out that .restarting is now implemented already.
Depends on: 882142
Landed without the .willRestart attribute:

https://hg.mozilla.org/integration/fx-team/rev/4c2ec5c33ea9
It seems that the nsIAppStartup service isn't available that early in the startup phase when we would set sProcessCreation in xpcom/ds/TimeStamp.cpp.

The environment variable would be available immediately, and unset after reading it once. Looks like we'll have to introduce a second env variable and leave the TimeStamp code untouched.
Same patch without only minimal TimeStamp.cpp changes.
Attachment #801463 - Attachment is obsolete: true
Attachment #802948 - Flags: review?(benjamin)
Attachment #802948 - Flags: review?(benjamin) → review+
https://hg.mozilla.org/mozilla-central/rev/b7a8703a187d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.