Fast startup should allow restarts and cope with non "-faststart" starts

VERIFIED FIXED

Status

Toolkit Graveyard
XULRunner
--
major
VERIFIED FIXED
9 years ago
2 years ago

People

(Reporter: Brian Crowder, Assigned: Brian Crowder)

Tracking

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

9 years ago
+++ This bug was initially created as a clone of Bug #512330 +++

The faststartup component does need to listen to quit-application-requested and exitLastWindowClosingSurvivalArea() so that the app can shutdown/restart.  Also, if the faststart component is installed, we assume you always want the faststart behavior.  "-faststart" only means that you don't wish to open the default window on startup, now.  This means that when you close that window, the "daemon" will continue running.
(Assignee)

Comment 1

9 years ago
Created attachment 396756 [details] [diff] [review]
better handling of restarts
Attachment #396756 - Flags: review?(gavin.sharp)
(Assignee)

Updated

9 years ago
Blocks: 509249
No longer depends on: 512330
(Assignee)

Comment 2

9 years ago
Created attachment 396759 [details] [diff] [review]
less dumps
Attachment #396756 - Attachment is obsolete: true
Attachment #396759 - Flags: review?(gavin.sharp)
Attachment #396756 - Flags: review?(gavin.sharp)
(Assignee)

Updated

9 years ago
Summary: Fast startup shouldn't show splash screens after automatic restarts → Fast startup should allow restarts and cope with non "-faststart" starts
Comment on attachment 396759 [details] [diff] [review]
less dumps

>diff --git a/toolkit/components/faststart/FastStartup.js b/toolkit/components/faststart/FastStartup.js

>-  this.QueryInterface = XPCOMUtils.generateQI([Ci.nsIObserver]);
>+  this.QueryInterface = XPCOMUtils.generateQI([Ci.nsIObserver, Ci.nsISupportsWeakReference]);

I guess we're relying on nsWindowWatcher::RegisterNotification passing "false"
for aOwnsWeak on its AddObserver calls, here, to avoid being GCed. That was
true before, but with this change it's theoretically possible for that to turn
into a weak ref if someone ends up changing that method in the future. Granted
that's unlikely, but perhaps it would be worth a comment?

"-faststart" is a bit of a misleading parameter name, I think, especially given its new meaning. Can we change it to something clearer? "-hide", maybe?
Attachment #396759 - Flags: review?(gavin.sharp) → review+
(Assignee)

Comment 4

9 years ago
Created attachment 396786 [details] [diff] [review]
with comment and -faststart changed to -faststart-hidden

Carrying over gavin's r+.  This is what I will land.
Attachment #396759 - Attachment is obsolete: true
Attachment #396786 - Flags: review+
(Assignee)

Comment 5

9 years ago
Created attachment 396794 [details] [diff] [review]
granted, not requested

exitBlahBlahSurvivalArea() in quit-app-requested is erroneous, it must happen in quit-app-granted.
Attachment #396786 - Attachment is obsolete: true
Attachment #396794 - Flags: review+
(Assignee)

Comment 6

9 years ago
http://hg.mozilla.org/mozilla-central/rev/3a3f9a03e210
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
verfied with 20090827 nightly trunk build of fennec.  As a note, the startup time after the addon triggered reboot is much longer, but it does restart.  It also has the splashscreen during that restart.  Future restarts are fast and as expected.

Great work!
Status: RESOLVED → VERIFIED
Attachment #396794 - Flags: approval1.9.2.1?
(Assignee)

Comment 8

8 years ago
Comment on attachment 396794 [details] [diff] [review]
granted, not requested

This has already landed in 192, sorry for missing clearing the flag.
Attachment #396794 - Flags: approval1.9.2.1?
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.