911146 - nsIAppStartup should be able to tell whether the app was restarted

Status: RESOLVED FIXED

(Toolkit :: Startup and Profile System, defect)

Description

[Tim Taubert [:ttaubert] (inactive)]

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

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Attachment: add nsIAppStartup.wasRestarted

Sorry, not sure who to ask for feedback.

Comment 2

[Benjamin Smedberg]

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?

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

Attachment: add nsIAppStartup.wasRestarted and nsIAppStartup.willRestart

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?

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

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.

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

Landed without the .willRestart attribute:

https://hg.mozilla.org/integration/fx-team/rev/4c2ec5c33ea9

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

Backed out for lots of breakage:

https://hg.mozilla.org/integration/fx-team/rev/51fbc6fcec35

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

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.

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

Attachment: Add nsIAppStartup attribute to tell whether the app was restarted, v2

Same patch without only minimal TimeStamp.cpp changes.

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

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

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

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

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b7a8703a187d