Provide the easy and right way to restart an application

RESOLVED FIXED in mozilla1.9.1b1

Status

()

Toolkit
Startup and Profile System
--
minor
RESOLVED FIXED
11 years ago
9 years ago

People

(Reporter: Simon Bünzli, Assigned: Simon Bünzli)

Tracking

({dev-doc-complete})

Trunk
mozilla1.9.1b1
dev-doc-complete
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

11 years ago
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.
(Assignee)

Comment 1

11 years ago
Created attachment 222080 [details] [diff] [review]
goRestartApplication as a convenience wrapper for goQuitApplication
Assignee: nobody → zeniko
Status: NEW → ASSIGNED
Attachment #222080 - Flags: first-review?(benjamin)
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.
Attachment #222080 - Flags: first-review?(benjamin) → first-review-
(Assignee)

Comment 3

11 years ago
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.
Attachment #222080 - Attachment is obsolete: true
Attachment #222232 - Flags: first-review?(mconnor)
(Assignee)

Updated

10 years ago
Attachment #222232 - Flags: first-review?(mconnor)
(Assignee)

Updated

10 years ago
Assignee: zeniko → nobody
Status: ASSIGNED → NEW
(Assignee)

Comment 4

10 years ago
Rather a case for FUEL now. -> WONTFIX
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → WONTFIX

Comment 5

10 years ago
Is there a fuel bug about this?
(Assignee)

Comment 6

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

Comment 7

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

CCing mfinkle to comment on this.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Summary: Add goRestartApplication to globalOverlay.js → Provide the easy and right way to restart an application
Version: unspecified → Trunk
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.

Updated

10 years ago
Duplicate of this bug: 352210

Updated

9 years ago
Component: XRE Startup → Startup and Profile System
QA Contact: xre.startup → startup
(Assignee)

Comment 10

9 years ago
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.
Attachment #222232 - Attachment is obsolete: true
Attachment #335742 - Flags: review?(enndeakin)
So is this a single simple way to quit/restart from any code (chrome or js component)?
(Assignee)

Comment 12

9 years ago
(In reply to comment #11)
You could even use this from C++ through XPCOM, so yeah, it'll work from anywhere.
Attachment #335742 - Flags: review?(enndeakin) → review+
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
(Assignee)

Updated

9 years ago
Assignee: nobody → zeniko
Status: REOPENED → ASSIGNED
Keywords: dev-doc-needed
It would be a good idea to specify what "if nobody objects" means in the IDL, I think (i.e. mention quit-application-requested).
(Assignee)

Comment 14

9 years ago
Created attachment 336237 [details] [diff] [review]
for check-in

(In reply to comment #13)
Sure.
Attachment #335742 - Attachment is obsolete: true
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]
aSubject instanceof Components.interfaces.nsISupportsBoolean

should be:

aSubject instanceof Components.interfaces.nsISupportsPRBool
Tests pass with that change
(Assignee)

Comment 18

9 years ago
Created attachment 336250 [details] [diff] [review]
for check-in (with passing tests)

Ugh, that's embarrassing. Thanks for catching this, Mark!
Attachment #336237 - Attachment is obsolete: true
http://hg.mozilla.org/mozilla-central/rev/551b4e4c7813
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
updated http://developer.mozilla.org/En/FUEL/Application
Keywords: dev-doc-needed

Updated

9 years ago
Keywords: dev-doc-complete

Updated

9 years ago
Flags: in-testsuite+

Updated

9 years ago
Target Milestone: --- → mozilla1.9.1b1
You need to log in before you can comment on or make changes to this bug.