338039 - Provide the easy and right way to restart an application

Status: RESOLVED FIXED

(Toolkit :: Startup and Profile System, defect)

Description

[Simon Bünzli]

There's currently no "one true way" how to restart a Toolkit application (see bug 335864 comment #11). Why not add a goRestartApplication method which does whatever necessary for a restart - similar to the existing goQuitApplication for quitting? This might prevent some duplicated code and prevent extensions from accidentally omitting the call to canQuitApplication on which other extensions do rely.

Comment 1

[Simon Bünzli]

Attachment: goRestartApplication as a convenience wrapper for goQuitApplication

Comment 2

[Benjamin Smedberg]

Comment on attachment 222080 [details] [diff] [review]
goRestartApplication as a convenience wrapper for goQuitApplication

>Index: toolkit/content/globalOverlay.js

>+  var nsIAppStartup = Components.interfaces.nsIAppStartup;

const nsIAppStartup?

>+  var appStartup = Components.classes['@mozilla.org/toolkit/app-startup;1']
>+                             .getService(nsIAppStartup);
>+  appStartup.quit(aFlags || nsIAppStartup.eAttemptQuit);

Why not just extend goQuitApplication(true) to mean restart?

mconnor should review this.

Comment 3

[Simon Bünzli]

Attachment: goRestartApplication as a convenience wrapper for goQuitApplication

(In reply to comment #2)
> Why not just extend goQuitApplication(true) to mean restart?

For one, because I don't feel psychic enough to know that we'll never have a third reason/way of shutting down - and for the other that a boolean pref on a method called quitApplication simply doesn't look right (I'd rather expect it to mean: force-quit as opposed to just try-to-quit). And of course, because goRestartApplication is simply easier to understand when used in the code.

Comment 4

[Simon Bünzli]

Rather a case for FUEL now. -> WONTFIX

Comment 5

[Nickolay_Ponomarev]

Is there a fuel bug about this?

Comment 6

[Simon Bünzli]

I don't know. If there isn't, maybe there's really no demand for such functionality at all...

Comment 7

[Nickolay_Ponomarev]

I've seen the question about this asked quite a few times, so I think there is.

CCing mfinkle to comment on this.

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

Application.quit() and Application.restart() sound like good additions to FUEL. I agree that there is not enough emphasis on shutting down correctly using the "canQuitApplication" method.

Unfortunately, FUEL is not in toolkit yet.

Comment 10

[Simon Bünzli]

Attachment: Application.quit() and Application.restart()

This should be the "one true way" of quitting/restarting any toolkit app.

The question remains whether goQuitApplication should just wrap Application.quit(), whether it should go away completely (together with canQuitApplication) or whether it should remain untouched.

Comment 11

[Neil Deakin]

So is this a single simple way to quit/restart from any code (chrome or js component)?

Comment 12

[Simon Bünzli]

(In reply to comment #11)
You could even use this from C++ through XPCOM, so yeah, it'll work from anywhere.

Comment 13

[:Gavin Sharp [email: [e-mail]]]

It would be a good idea to specify what "if nobody objects" means in the IDL, I think (i.e. mention quit-application-requested).

Comment 14

[Simon Bünzli]

Attachment: for check-in

(In reply to comment #13)
Sure.

Comment 15

[Mark Finkle (:mfinkle) (use needinfo?)]

Error when running the browser chrome tests:

* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "'[JavaScript Error: "invalid 'instanceof' operand Components.interfaces.nsISupportsBoolean" {file: "chrome://mochikit/content/browser/browser/fuel/test/browser_ApplicationQuitting.js" line: 5}]' when calling method: [nsIObserver::observe]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: file:///e:/mozilla-trunk/mozilla/firefox-objdir/dist/bin/components/fuelApplication.js :: app__quitWithFlags :: line 1433"  data: yes]

Comment 16

[Mark Finkle (:mfinkle) (use needinfo?)]

aSubject instanceof Components.interfaces.nsISupportsBoolean

should be:

aSubject instanceof Components.interfaces.nsISupportsPRBool

Comment 17

[Mark Finkle (:mfinkle) (use needinfo?)]

Tests pass with that change

Comment 18

[Simon Bünzli]

Attachment: for check-in (with passing tests)

Ugh, that's embarrassing. Thanks for catching this, Mark!

Comment 19

[Mark Finkle (:mfinkle) (use needinfo?)]

http://hg.mozilla.org/mozilla-central/rev/551b4e4c7813

Comment 20

[Mark Finkle (:mfinkle) (use needinfo?)]

updated http://developer.mozilla.org/En/FUEL/Application