Closed Bug 297862 Opened 15 years ago Closed 15 years ago

Provide an API to restart the application

Categories

(Toolkit :: Startup and Profile System, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla1.8beta3

People

(Reporter: darin.moz, Assigned: darin.moz)

References

Details

Attachments

(1 file)

Provide an API to restart the application

I suspect that this involves adding an API to nsIXULRuntime to "schedule" a
restart to happen when nsAppRunner.cpp is shutting down.  Then, someone
interested in restarting the application would use nsICloseAllWindows to cause
the application to shutdown.
This approach basically involves finding a way to set needsRestart to TRUE in
nsAppRunner.cpp and setting a flag to cause it to avoid checking the
NO_EM_RESTART environment variable.
Blocks: 290390
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.8beta3
What's the use-case? I've thought about this several times before (my original
spec for nsICommandLine actually had a restart flag), but I haven't discovered
any uses for it.
I would assume the use-case is the new update stuff, which probably wants to
actually be user friendly and offer to restart the app for the user (except
it'll lose everything they're doing, which I hope it will mention, and wont
force anyone into it ;) ). I can't think that anything else would need it, though.
bsmedberg: The use case is the update system.  Ben wants to be able to offer to
apply an available software update "now," which in the new system means
restarting the app.
I think that Ben also wants extension manager to present the user with similar
UI after they make a change that requires a restart.
OK, makes sense. I'm cautious about the "avoid checking the NO_EM_RESTART" bit:
we should make sure that our tinderbox test builds don't end up doing this restart.

Regarding the impl, I don't think you need to use the closeallwindows impl
yourself (which doesn't do what you expect on mac), and I don't think we should
attach this to nsIXULRuntime; instead, modify nsIAppStartup.quit with an extra
"restart" flag, and use it directly. It will call the closeallwindows impl in
the appropriate situations. You can pass a result code or a PRBool flag from
nsIAppStartup.run() back to nsAppRunner.

Finally, I think you should use the eAttemptQuit version instead of eForceQuit
for both software update and EM "restart now": that way people won't
accidentally lose work-in-progress if they click "cancel" on one of the
"download in progress" or "multiple tabs open" dialogs.
Sounds reasonable.  If closeallwindows is not what I want on Mac, then what is?

By the way, I believe that NO_EM_RESTART is actually not an issue since we clear
NO_EM_RESTART once we load the application for real ;-) 
nsIAppStartup.quit will clean up windows for you (either with warning dialogs
for eAttemptQuit or without for eForceQuit), you don't need to mess with that
manually at all.
Attached patch v1 patchSplinter Review
Attachment #186646 - Flags: first-review?(benjamin)
Comment on attachment 186646 [details] [diff] [review]
v1 patch

Remember to remove extraneous printf (nsAppStartup::Quit).

I also thought that gSavedVars was supposed to have XRE_PROFILE_PATH and
LOCAL_PROFILE_PATH.
Attachment #186646 - Flags: first-review?(benjamin) → first-review+
(In reply to comment #10)
> (From update of attachment 186646 [details] [diff] [review] [edit])
> Remember to remove extraneous printf (nsAppStartup::Quit).

Yes :)


> I also thought that gSavedVars was supposed to have XRE_PROFILE_PATH and
> LOCAL_PROFILE_PATH.

I thought so too, until I realized that we set those env vars when we restart
the app from the bottom of XRE_Main.  So there is no reason to set them twice.
Comment on attachment 186646 [details] [diff] [review]
v1 patch

This is needed by the new software update system.
Attachment #186646 - Flags: approval1.8b3?
Attachment #186646 - Flags: approval1.8b3? → approval1.8b3+
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
With this fixed now would it be possible to add a button to restart Firefox
after you install an extension? Perhaps a restart extension?
Henrik: that is the plan, yes.
(In reply to comment #14)
> With this fixed now would it be possible to add a button to restart Firefox
> after you install an extension? Perhaps a restart extension?

Done by jedbro.
http://forums.mozillazine.org/viewtopic.php?t=319438
One nice feature would be to be able to pass a command line param on restart for
example to create a mock Profile Manager or to restart in Safe Mode, etc.  Thanks.
(In reply to comment #17)
> One nice feature would be to be able to pass a command line param on restart for
> example to create a mock Profile Manager or to restart in Safe Mode, etc.  Thanks.

Agreed.  Please file a new bug for that.  I don't expect it to be fixed in time
for 1.5 though :(
Submitted Command-Line Request here
https://bugzilla.mozilla.org/show_bug.cgi?id=310378
Flags: in-testsuite?
Component: XRE Startup → Startup and Profile System
QA Contact: nobody → startup
You need to log in before you can comment on or make changes to this bug.