310076 - calling app restart from nsBrowserGlue causes failure to restart the app properly

Status: RESOLVED FIXED

(Toolkit :: Startup and Profile System, defect)

Description

[Mike Connor [:mconnor]]

restart is called, dialog closes, browser window opens.  Once that window is
closed, app restarts.  Benjamin says he can fix this today.

Comment 1

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

When I was testing this, I was launching a modal dialog from nsBrowserGlue's
_onProfileStartup ("profile-after-change" observer) and upon dismissal of the
dialog the app would restart (which brought up the dialog again, etc). This only
happened if I caused a startup restart by using my profile in another build
(which I think is another bug). I guess this is because the EM and browserglue
try to do things at the same time?

Comment 2

[Benjamin Smedberg]

Attachment: Skip nsIAppStartup.run if we've already called .quit

This will skip the event loop on nsIAppStartup.run if we've already called
.quit relatively successfully.

Comment 3

[Benjamin Smedberg]

Attachment: More involved patch that uses considerquitstopper to control when to run the eventloop

This patch is more involved than the first in that it uses the
considerquitstopper to determine when we should run the event loop and when to
try to exit, instead of merely checking for XUL window open/close. This is
something I've been wanting to do, and is necessary to fix bug 281847 properly.
This is not really branch material, so I'm going to look for a temporary hack
for the branch for that bug.

Comment 4

[Benjamin Smedberg]

Comment on attachment 197747 [details] [diff] [review]
More involved patch that uses considerquitstopper to control when to run the eventloop

Needs another little bit. Please note that the review request on the first
patch (targeted at the branch) is still valid.

Comment 5

[Benjamin Smedberg]

Attachment: More involved patch that uses considerquitstopper to control when to run the eventloop, rev. 2

The additional change here involves making sure that the hidden window does not
affect consider-quit stoppage.

Comment 6

[Benjamin Smedberg]

Please ignore the NS_ERROR()s I added for debugging, I will remove them before
checkin.

Comment 7

[Darin Fisher]

Comment on attachment 197576 [details] [diff] [review]
Skip nsIAppStartup.run if we've already called .quit

>Index: toolkit/components/startup/src/nsAppStartup.cpp

>+      nsCOMPtr<nsIEventQueueService> svc = do_GetService(NS_EVENTQUEUESERVICE_CONTRACTID, &rv);
>       if (NS_SUCCEEDED(rv)) {
> 
>+        nsCOMPtr<nsIEventQueue> queue;
>+        rv = svc->GetThreadEventQueue(NS_CURRENT_THREAD, getter_AddRefs(queue));
>+        if (NS_SUCCEEDED(rv)) {
> 

While your here, you might want to replace this code with just:

	  nsCOMPtr<nsIEventQueue> queue;
	  rv = NS_GetMainEventQ(getter_AddRefs(queue));

You'll need to #include "nsEventQueueUtils.h"


r=darin

Comment 8

[Benjamin Smedberg]

Attachment: Update with eventq change

Comment 9

[Benjamin Smedberg]

Attachment 197900 [details] [diff] checked in on trunk. I'm going to leave this open for the more
extensive considerquitstopper patch.

Comment 10

[Benjamin Smedberg]

The simple change was committed this morning on the branch.

Comment 11

[Darin Fisher]

Comment on attachment 197848 [details] [diff] [review]
More involved patch that uses considerquitstopper to control when to run the eventloop, rev. 2

r=darin w/ NS_GetCurrentEventQ.

Comment 12

[Benjamin Smedberg]

Fixed on trunk.

Comment 13

[Stefan Sitter [:ssitter]]

Benjamin:
Please check if the trunk patch caused bug 311234 (Clicking 'Done' after 
customize toolbars exits Firefox/Sunbird trunk builds).

Comment 14

[David :Bienvenu]

The trunk version of this patch caused bug 312135 in Thunderbird - it seems to count "xul-window-registered" and "xul-window-destroyed" notifications, and only quits when they match. The problem is that Thunderbird (and SM, afaik) caches a compose window, because bringing up a compose window was (and most likely still is) slow. Thus, if you bring up a compose window, the count never goes to zero.

There might be some way for Thunderbird to detect when the last open window is closed, and in that case, close the cached compose window. But, is there a chance the nsAppStartup code could use some other mechanism that keeps track of open windows?

Comment 15

[Benjamin Smedberg]

Let's discuss in bug 312135.