Closed Bug 911146 Opened 6 years ago Closed 6 years ago

nsIAppStartup should be able to tell whether the app was restarted


(Toolkit :: Startup and Profile System, defect)

Not set





(Reporter: ttaubert, Assigned: ttaubert)



(Keywords: dev-doc-needed)


(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.

TimeStamp has its own implementation. SessionStore does something similar (it sets a pref that is reset on next startup) to resume the session automatically.
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
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:
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+
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.