Bug 310076 (scary version) - Use the considerquitstopper to control when to run the event loop, not window enumerators r=darin

toolkit/components/startup/public/nsIAppStartup.idl

@@ -51,7 +51,9 @@ interface nsIAppStartup : nsISupports
 
     /**
      * Runs an application event loop: normally the main event pump which
-     * defines the lifetime of the application.
+     * defines the lifetime of the application. If there are no windows open
+     * and no outstanding calls to enterLastWindowClosingSurvivalArea this
+     * method will exit immediately.
      *
      * @returnCode NS_SUCCESS_RESTART_APP
      *             This return code indicates that the application should be
@@ -60,12 +62,10 @@ interface nsIAppStartup : nsISupports
     void run();
 
     /**
-     * During application startup (and at other times!) we may temporarily
+     * There are situations where all application windows will be
-     * encounter a situation where all application windows will be closed
+     * closed but we don't want to take this as a signal to quit the
-     * but we don't want to take this as a signal to quit the app. Bracket
+     * app. Bracket the code where the last window could close with
-     * the code where the last window could close with these.
+     * these.
-     * (And at application startup, on platforms that don't normally quit
-     * when the last window has closed, call Enter once, but not Exit)
      */
     void enterLastWindowClosingSurvivalArea();
     void exitLastWindowClosingSurvivalArea();

toolkit/components/startup/src/nsAppStartup.cpp

@@ -144,7 +144,18 @@ nsAppStartup::CreateHiddenWindow()
 NS_IMETHODIMP
 nsAppStartup::Run(void)
 {
-  if (!mShuttingDown) {
+  NS_ASSERTION(!mRunning, "Reentrant appstartup->Run()");
+
+  // If we have no windows open and no explicit calls to
+  // enterLastWindowClosingSurvivalArea, or somebody has explicitly called
+  // quit, don't bother running the event loop which would probably leave us
+  // with a zombie process.
+
+  if (!mShuttingDown && mConsiderQuitStopper != 0) {
+#ifdef XP_MACOSX
+    EnterLastWindowClosingSurvivalArea();
+#endif
+
     mRunning = PR_TRUE;
 
     nsresult rv = mAppShell->Run();
@@ -187,18 +198,7 @@ nsAppStartup::Quit(PRUint32 aMode)
 
   if (ferocity == eConsiderQuit && mConsiderQuitStopper == 0) {
     // attempt quit if the last window has been unregistered/closed
-
+    ferocity = eAttemptQuit;
-    PRBool windowsRemain = PR_TRUE;
-
-    if (mediator) {
-      nsCOMPtr<nsISimpleEnumerator> windowEnumerator;
-      mediator->GetEnumerator(nsnull, getter_AddRefs(windowEnumerator));
-      if (windowEnumerator)
-        windowEnumerator->HasMoreElements(&windowsRemain);
-    }
-    if (!windowsRemain) {
-      ferocity = eAttemptQuit;
-    }
   }
 
   /* Currently ferocity can never have the value of eForceQuit here.
@@ -296,33 +296,28 @@ nsAppStartup::Quit(PRUint32 aMode)
       // no matter what, make sure we send the exit event.  If
       // worst comes to worst, we'll do a leaky shutdown but we WILL
       // shut down. Well, assuming that all *this* stuff works ;-).
-      nsCOMPtr<nsIEventQueueService> svc = do_GetService(NS_EVENTQUEUESERVICE_CONTRACTID, &rv);
+      nsCOMPtr<nsIEventQueue> queue;
+      rv = NS_GetMainEventQ(getter_AddRefs(queue));
       if (NS_SUCCEEDED(rv)) {
+        PLEvent* event = new PLEvent;
+        if (event) {
+          NS_ADDREF_THIS();
+          PL_InitEvent(event,
+                       this,
+                       HandleExitEvent,
+                       DestroyExitEvent);
 
-        nsCOMPtr<nsIEventQueue> queue;
+          rv = queue->PostEvent(event);
-        rv = NS_GetMainEventQ(getter_AddRefs(queue));
+          if (NS_SUCCEEDED(rv)) {
-        if (NS_SUCCEEDED(rv)) {
+            postedExitEvent = PR_TRUE;
-
-          PLEvent* event = new PLEvent;
-          if (event) {
-            NS_ADDREF_THIS();
-            PL_InitEvent(event,
-                         this,
-                         HandleExitEvent,
-                         DestroyExitEvent);
-
-            rv = queue->PostEvent(event);
-            if (NS_SUCCEEDED(rv)) {
-              postedExitEvent = PR_TRUE;
-            }
-            else {
-              PL_DestroyEvent(event);
-            }
           }
           else {
-            rv = NS_ERROR_OUT_OF_MEMORY;
+            PL_DestroyEvent(event);
           }
         }
+        else {
+          rv = NS_ERROR_OUT_OF_MEMORY;
+        }
       }
     }
   }
@@ -341,21 +336,19 @@ nsAppStartup::Quit(PRUint32 aMode)
 void
 nsAppStartup::AttemptingQuit(PRBool aAttempt)
 {
-#if defined(XP_MAC) || defined(XP_MACOSX)
+#ifdef XP_MACOSX
   if (aAttempt) {
     // now even the Mac wants to quit when the last window is closed
     if (!mAttemptingQuit)
       ExitLastWindowClosingSurvivalArea();
-    mAttemptingQuit = PR_TRUE;
   } else {
     // changed our mind. back to normal.
     if (mAttemptingQuit)
       EnterLastWindowClosingSurvivalArea();
-    mAttemptingQuit = PR_FALSE;
   }
-#else
-  mAttemptingQuit = aAttempt;
 #endif
+
+  mAttemptingQuit = aAttempt;
 }
 
 NS_IMETHODIMP
@@ -371,6 +364,10 @@ nsAppStartup::ExitLastWindowClosingSurvivalArea(void)
 {
   NS_ASSERTION(mConsiderQuitStopper > 0, "consider quit stopper out of bounds");
   --mConsiderQuitStopper;
+
+  if (mRunning && mConsiderQuitStopper == 0)
+    Quit(eAttemptQuit);
+
   return NS_OK;
 }
 
@@ -517,9 +514,10 @@ nsAppStartup::Observe(nsISupports *aSubject,
     }
     ExitLastWindowClosingSurvivalArea();
   } else if (!strcmp(aTopic, "xul-window-registered")) {
+    EnterLastWindowClosingSurvivalArea();
     AttemptingQuit(PR_FALSE);
   } else if (!strcmp(aTopic, "xul-window-destroyed")) {
-    Quit(eConsiderQuit);
+    ExitLastWindowClosingSurvivalArea();
   } else {
     NS_ERROR("Unexpected observer topic.");
   }

toolkit/xre/nsAppRunner.cpp

@@ -1332,8 +1332,6 @@ ShowProfileManager(nsIToolkitProfileService* aProfileSvc,
         (do_GetService(NS_APPSTARTUP_CONTRACTID));
       NS_ENSURE_TRUE(appStartup, NS_ERROR_FAILURE);
 
-      appStartup->EnterLastWindowClosingSurvivalArea();
-
       nsCOMPtr<nsIDOMWindow> newWindow;
       rv = windowWatcher->OpenWindow(nsnull,
                                      kProfileManagerURL,
@@ -1342,8 +1340,6 @@ ShowProfileManager(nsIToolkitProfileService* aProfileSvc,
                                      ioParamBlock,
                                      getter_AddRefs(newWindow));
 
-      appStartup->ExitLastWindowClosingSurvivalArea();
-
       NS_ENSURE_SUCCESS_LOG(rv, rv);
 
       aProfileSvc->Flush();
@@ -2165,9 +2161,6 @@ XRE_main(int argc, char* argv[], const nsXREAppData* aAppData)
         (do_GetService(NS_APPSTARTUP_CONTRACTID));
       NS_ENSURE_TRUE(appStartup, 1);
 
-      // So we can open and close windows during startup
-      appStartup->EnterLastWindowClosingSurvivalArea();
-
       // Profile Migration
       if (gAppData->flags & NS_XRE_ENABLE_PROFILE_MIGRATOR && gDoMigration) {
         gDoMigration = PR_FALSE;
@@ -2271,62 +2264,39 @@ XRE_main(int argc, char* argv[], const nsXREAppData* aAppData)
         rv = cmdLine->Run();
         NS_ENSURE_SUCCESS_LOG(rv, 1);
 
-        nsCOMPtr<nsIWindowMediator> windowMediator
+#ifdef MOZ_ENABLE_XREMOTE
-          (do_GetService(NS_WINDOWMEDIATOR_CONTRACTID, &rv));
+        // if we have X remote support, start listening for requests on the
-        NS_ENSURE_SUCCESS(rv, 1);
+        // proxy window.
+        nsCOMPtr<nsIRemoteService> remoteService;
+        remoteService = do_GetService("@mozilla.org/toolkit/remote-service;1");
+        if (remoteService)
+          remoteService->Startup(gAppData->name, nsnull);
+#endif /* MOZ_ENABLE_XREMOTE */
 
-        // Make sure there exists at least 1 window.
+        // enable win32 DDE responses and Mac appleevents responses
-        nsCOMPtr<nsISimpleEnumerator> windowEnumerator;
+        nativeApp->Enable();
-        rv = windowMediator->GetEnumerator(nsnull, getter_AddRefs(windowEnumerator));
-        NS_ENSURE_SUCCESS(rv, 1);
 
-        PRBool more;
+        NS_TIMELINE_ENTER("appStartup->Run");
-        windowEnumerator->HasMoreElements(&more);
+        rv = appStartup->Run();
-        if (!more) {
+        NS_TIMELINE_LEAVE("appStartup->Run");
-          // We didn't open any windows. This is normally not a good thing,
+        if (NS_FAILED(rv)) {
-          // so we force console logging to file.
+          NS_ERROR("failed to run appstartup");
           gLogConsoleErrors = PR_TRUE;
         }
-        else {
-#ifndef XP_MACOSX
-          appStartup->ExitLastWindowClosingSurvivalArea();
-#endif
 
-#ifdef MOZ_ENABLE_XREMOTE
+        // Check for an application initiated restart.  This is one that
-          // if we have X remote support and we have our one window up and
+        // corresponds to nsIAppStartup.quit(eRestart)
-          // running start listening for requests on the proxy window.
+        if (rv == NS_SUCCESS_RESTART_APP) {
-          nsCOMPtr<nsIRemoteService> remoteService;
+          needsRestart = PR_TRUE;
-          remoteService = do_GetService("@mozilla.org/toolkit/remote-service;1");
+          appInitiatedRestart = PR_TRUE;
-          if (remoteService)
-            remoteService->Startup(gAppData->name, nsnull);
-#endif /* MOZ_ENABLE_XREMOTE */
-
-          // enable win32 DDE responses and Mac appleevents responses
-          nativeApp->Enable();
-
-          // Start main event loop
-          NS_TIMELINE_ENTER("appStartup->Run");
-          rv = appStartup->Run();
-          NS_TIMELINE_LEAVE("appStartup->Run");
-          if (NS_FAILED(rv)) {
-            NS_ERROR("failed to run appstartup");
-            gLogConsoleErrors = PR_TRUE;
-          }
-
-          // Check for an application initiated restart.  This is one that
-          // corresponds to nsIAppStartup.quit(eRestart)
-          if (rv == NS_SUCCESS_RESTART_APP) {
-            needsRestart = PR_TRUE;
-            appInitiatedRestart = PR_TRUE;
-          }
-
-#ifdef MOZ_ENABLE_XREMOTE
-          // shut down the x remote proxy window
-          if (remoteService)
-            remoteService->Shutdown();
-#endif /* MOZ_ENABLE_XREMOTE */
         }
 
+#ifdef MOZ_ENABLE_XREMOTE
+        // shut down the x remote proxy window
+        if (remoteService)
+          remoteService->Shutdown();
+#endif /* MOZ_ENABLE_XREMOTE */
+
 #ifdef MOZ_TIMELINE
         // Make sure we print this out even if timeline is runtime disabled
         if (NS_FAILED(NS_TIMELINE_LEAVE("main1")))

xpfe/appshell/src/nsXULWindow.cpp

@@ -538,17 +538,19 @@ NS_IMETHODIMP nsXULWindow::Destroy()
       mWindow = nsnull;
    }
 
-   /* Inform appstartup we've destroyed this window and it could
+   if (!mIsHiddenWindow) {
-      quit now if it wanted. This must happen at least after mDocShell
+     /* Inform appstartup we've destroyed this window and it could
-      is destroyed, because onunload handlers fire then, and those being
+        quit now if it wanted. This must happen at least after mDocShell
-      script, anything could happen. A new window could open, even.
+        is destroyed, because onunload handlers fire then, and those being
-      See bug 130719. */
+        script, anything could happen. A new window could open, even.
-   nsCOMPtr<nsIObserverService> obssvc =
+        See bug 130719. */
-     do_GetService("@mozilla.org/observer-service;1");
+     nsCOMPtr<nsIObserverService> obssvc =
-   NS_ASSERTION(obssvc, "Couldn't get observer service?");
+       do_GetService("@mozilla.org/observer-service;1");
+     NS_ASSERTION(obssvc, "Couldn't get observer service?");
 
-   if (obssvc)
+     if (obssvc)
-     obssvc->NotifyObservers(nsnull, "xul-window-destroyed", nsnull);
+       obssvc->NotifyObservers(nsnull, "xul-window-destroyed", nsnull);
+   }
 
    return NS_OK;
 }