Last Comment Bug 338039 - Provide the easy and right way to restart an application
: Provide the easy and right way to restart an application
Status: RESOLVED FIXED
: dev-doc-complete
Product: Toolkit
Classification: Components
Component: Startup and Profile System (show other bugs)
: Trunk
: All All
: -- minor (vote)
: mozilla1.9.1b1
Assigned To: Simon Bünzli
:
Mentors:
: 352210 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-05-15 12:39 PDT by Simon Bünzli
Modified: 2008-09-07 04:46 PDT (History)
8 users (show)
dao+bmo: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
goRestartApplication as a convenience wrapper for goQuitApplication (2.09 KB, patch)
2006-05-15 12:42 PDT, Simon Bünzli
benjamin: first‑review-
Details | Diff | Splinter Review
goRestartApplication as a convenience wrapper for goQuitApplication (2.36 KB, patch)
2006-05-16 11:54 PDT, Simon Bünzli
no flags Details | Diff | Splinter Review
Application.quit() and Application.restart() (4.70 KB, patch)
2008-08-27 12:06 PDT, Simon Bünzli
enndeakin: review+
Details | Diff | Splinter Review
for check-in (4.75 KB, patch)
2008-08-31 05:26 PDT, Simon Bünzli
no flags Details | Diff | Splinter Review
for check-in (with passing tests) (4.75 KB, patch)
2008-08-31 07:05 PDT, Simon Bünzli
no flags Details | Diff | Splinter Review

Description Simon Bünzli 2006-05-15 12:39:55 PDT
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 2006-05-15 12:42:24 PDT
Created attachment 222080 [details] [diff] [review]
goRestartApplication as a convenience wrapper for goQuitApplication
Comment 2 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2006-05-16 03:15:43 PDT
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 2006-05-16 11:54:01 PDT
Created attachment 222232 [details] [diff] [review]
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 2007-09-02 08:57:08 PDT
Rather a case for FUEL now. -> WONTFIX
Comment 5 Nickolay_Ponomarev 2007-09-02 09:09:54 PDT
Is there a fuel bug about this?
Comment 6 Simon Bünzli 2007-09-02 09:23:38 PDT
I don't know. If there isn't, maybe there's really no demand for such functionality at all...
Comment 7 Nickolay_Ponomarev 2007-09-02 09:27:53 PDT
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?) 2007-09-06 11:18:06 PDT
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 9 Nickolay_Ponomarev 2007-09-15 10:27:12 PDT
*** Bug 352210 has been marked as a duplicate of this bug. ***
Comment 10 Simon Bünzli 2008-08-27 12:06:07 PDT
Created attachment 335742 [details] [diff] [review]
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 2008-08-29 10:06:43 PDT
So is this a single simple way to quit/restart from any code (chrome or js component)?
Comment 12 Simon Bünzli 2008-08-29 10:12:57 PDT
(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: gavin@gavinsharp.com] 2008-08-31 02:02:08 PDT
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 2008-08-31 05:26:29 PDT
Created attachment 336237 [details] [diff] [review]
for check-in

(In reply to comment #13)
Sure.
Comment 15 Mark Finkle (:mfinkle) (use needinfo?) 2008-08-31 06:52:35 PDT
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?) 2008-08-31 06:55:52 PDT
aSubject instanceof Components.interfaces.nsISupportsBoolean

should be:

aSubject instanceof Components.interfaces.nsISupportsPRBool
Comment 17 Mark Finkle (:mfinkle) (use needinfo?) 2008-08-31 07:01:40 PDT
Tests pass with that change
Comment 18 Simon Bünzli 2008-08-31 07:05:19 PDT
Created attachment 336250 [details] [diff] [review]
for check-in (with passing tests)

Ugh, that's embarrassing. Thanks for catching this, Mark!
Comment 19 Mark Finkle (:mfinkle) (use needinfo?) 2008-09-06 20:36:20 PDT
http://hg.mozilla.org/mozilla-central/rev/551b4e4c7813
Comment 20 Mark Finkle (:mfinkle) (use needinfo?) 2008-09-06 20:59:22 PDT
updated http://developer.mozilla.org/En/FUEL/Application

Note You need to log in before you can comment on or make changes to this bug.