89165 - File|Exit should not kill -turbo mode

Status: VERIFIED FIXED

(Core Graveyard :: Cmd-line Features, defect)

Description

[Cathleen]

Comment 1

[timeless]

Wrong behavior.  File|Exit should quit mozilla and then spawn a new instance of 
mozilla -turbo. See bugs about skin switching.

Comment 2

[Blake Ross]

I'll do this.  Should be easy...

Comment 3

[Blake Ross]

Ohh, this turned out to be quite fun after all :-)

Here's what I see happening: File | Exit calls nsAppShellService::Quit().  
Quit() enumerates through all of the open windows, closing/destroying each one. 
 nsXULWindow::Destroy calls nsAppShellService::UnregisterTopLevelWindow() for 
each window.  When there are no more windows open, and if the app has decided to 
quit upon closing the last window, and if turbo mode isn't on, 
UnregisterTopLevelWindow() calls Quit() again.  So we will properly bail there 
if we're in turbo mode.  The problem, of course, is that we still have to finish 
out the original Quit() call.

It's somewhat circular logic. An oversimplified model shows: Quit -> Close -> 
Destroy -> Unregister -> Quit {if no more windows && server mode && quit on last 
window}

I see a couple possible solutions, none of which I particularly like:

* Move the server mode check to Quit() and bail early when necessary.
* Have UnregisterTopLevelWindow() set some mCanQuit member variable (this sucks, 
we may as well just move the server mode check if we're going to do this).
* Handle the window enumeration in js, so that we can just call 
UnregisterTopLevelWindow() (which, for some odd reason, is scriptable) from js, 
which would Quit() for us when the conditions are right.
* Some kickass idea that Dan has.

Dan, any suggestions?

Comment 4

[[e-mail]]

Quitting doesn't work properly anyway. Sometimes windows pop up alerts after
other windows have already closed.

Comment 5

[Blake Ross]

Vishy says PDT+

Comment 6

[Blake Ross]

Attachment: patch (sr=ben)

Comment 7

[timeless]

r=timeless but you can't check this into trunk before the theme switching bits 
are checked in.

Comment 8

[Alex Vincent [:WeirdAl]]

I've noticed some pretty heavy bloat in my swapfile after I've closed all
Mozilla windows in turbo mode.  After CTRL-ALT-DEL for Mozilla alone in my Win95
Task Manager, my swapfile gains back about 25MB...

Specifically,

1. Browsing around in Moz0.9.2 for several hours
2. Closing all instances of Mozilla
3. Noting swapfile usage at about 38% (100MB swapfile on my machine)
4. CTRL-ALT-DEL Mozilla, confirm when the dialog pops up 20 seconds later
5. Swapfile jumps 25% to about 61%.

Given you're trying to shutdown the turbo mode for a few moments completely and
have it restart, I figured this might be related.  
What about the Quit() function starting a "transitional restart program", which
will independently and automatically restart Mozilla after Mozilla itself shuts
down?

mozrestart.bat:

@echo off
(sleep about ten seconds)
mozilla -turbo
^Z


Not the most elegant solution, but it would work...

Comment 9

[Ben Goodger (use ben at mozilla dot org for email)]

sr=[e-mail] for the branch, and for the trunk if you fix the strings to 
use brandShortName.

Comment 10

[lchiang]

Update ETA per IM w/blake. Will land when tree opens.

Comment 11

[Blake Ross]

Fixed.

Comment 12

[Conrad Carlen (not reading bugmail)]

This breaks the Mac. This bit at the end of globalOverlay.js:

 if (!nativeAppSupport.isServerMode)

won't work on the Mac because there is no nativeAppSupport.
This patch prevents the Mac from being able to Quit.                           
               

Comment 13

[Blake Ross]

fix checked in. sorry.

Comment 14

[Viswanath Ramachandran]

Conrad - could you verify that the mac works okay now?

Comment 15

[Paul Wyskoczka]

I checked the Mac. The mac worked with yesterday's branch build.  

Comment 16

[Paul Wyskoczka]

Verified on the branch using build 2001071705-0.9.2

Comment 17

[Paul Wyskoczka]

Verified on trunk build 2001071803