Mac: reform our shutdown routine.

Make shutdown be more like other platforms. Moreover:
- Cancelling quit from an onbeforeunload dialog shouldn't mess up the browser.
- Having quit cancelled due to a window pop up on the closure of another window
  shouldn't break the browser. [With this patch, it will result in the browser
  being in a quirky state in which the closure of the last browser window will
  cause a quit. But the browser won't be broken.]

BUG=34384,37813,37927
TEST=See bugs.

Review URL: http://codereview.chromium.org/1520006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@44096 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 61 insertions, 16 deletions

diff --git a/chrome/browser/app_controller_mac.mm b/chrome/browser/app_controller_mac.mm
index 8985761..5da68a3 100644
--- a/chrome/browser/app_controller_mac.mm
+++ b/chrome/browser/app_controller_mac.mm
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -138,6 +138,8 @@ void RecordLastRunAppBundlePath() {
 
 @interface AppController(Private)
 - (void)initMenuState;
+- (void)handleQuitEvent:(NSAppleEventDescriptor*)event
+              withReply:(NSAppleEventDescriptor*)reply;
 - (void)openUrls:(const std::vector<GURL>&)urls;
 - (void)getUrl:(NSAppleEventDescriptor*)event
      withReply:(NSAppleEventDescriptor*)reply;
@@ -217,30 +219,66 @@ void RecordLastRunAppBundlePath() {
   }
 }
 
+// (NSApplicationDelegate protocol) This is the Apple-approved place to override
+// the default handlers.
+- (void)applicationWillFinishLaunching:(NSNotification*)notification {
+  NSAppleEventManager* em = [NSAppleEventManager sharedAppleEventManager];
+  [em setEventHandler:self
+          andSelector:@selector(handleQuitEvent:withReply:)
+        forEventClass:kCoreEventClass
+           andEventID:kAEQuitApplication];
+}
+
+// (NSApplicationDelegate protocol) Our mechanism for application termination
+// does not go through |-applicationShouldTerminate:|, so it should not be
+// called. In a release build, cancelling termination will prevent a crash (but
+// if things go really wrong, the user may have to force-terminate the
+// application).
 - (NSApplicationTerminateReply)applicationShouldTerminate:
-    (NSApplication *)sender {
-  // Check for in-progress downloads, and prompt the user if they really want to
-  // quit (and thus cancel the downloads).
-  if (![self shouldQuitWithInProgressDownloads])
-    return NSTerminateCancel;
+    (NSApplication*)sender {
+  NOTREACHED();
+  return NSTerminateCancel;
+}
+
+- (BOOL)tryToTerminateApplication:(NSApplication*)app {
+  // Set the state to "trying to quit", so that closing all browser windows will
+  // lead to termination.
+  browser_shutdown::SetTryingToQuit(true);
+
+  // TODO(viettrungluu): Remove Apple Event handlers here? (It's safe to leave
+  // them in, but I'm not sure about UX; we'd also want to disable other things
+  // though.) http://crbug.com/40861
+
+  if (!BrowserList::size())
+    return YES;
+
+  // Try to close all the windows.
+  BrowserList::CloseAllBrowsers(true);
+
+  return NO;
+}
+
+- (void)stopTryingToTerminateApplication:(NSApplication*)app {
+  if (browser_shutdown::IsTryingToQuit()) {
+    // Reset the "trying to quit" state, so that closing all browser windows
+    // will no longer lead to termination.
+    browser_shutdown::SetTryingToQuit(false);
 
-  return NSTerminateNow;
+    // TODO(viettrungluu): Were we to remove Apple Event handlers above, we
+    // would have to reinstall them here. http://crbug.com/40861
+  }
 }
 
 // Called when the app is shutting down. Clean-up as appropriate.
-- (void)applicationWillTerminate:(NSNotification *)aNotification {
+- (void)applicationWillTerminate:(NSNotification*)aNotification {
   NSAppleEventManager* em = [NSAppleEventManager sharedAppleEventManager];
   [em removeEventHandlerForEventClass:kInternetEventClass
                            andEventID:kAEGetURL];
   [em removeEventHandlerForEventClass:'WWW!'
                            andEventID:'OURL'];
 
-  // Close all the windows.
-  BrowserList::CloseAllBrowsers(true);
-
-  // On Windows, this is done in Browser::OnWindowClosing, but that's not
-  // appropriate on Mac since we don't shut down when we reach zero windows.
-  browser_shutdown::OnShutdownStarting(browser_shutdown::BROWSER_EXIT);
+  // There better be no browser windows left at this point.
+  CHECK_EQ(BrowserList::size(), 0u);
 
   // Release the reference to the browser process. Once all the browsers get
   // dealloc'd, it will stop the RunLoop and fall back into main().
@@ -395,7 +433,7 @@ void RecordLastRunAppBundlePath() {
   CFPropertyListRef plist = CFPreferencesCopyAppValue(checkInterval, app);
   if (!plist) {
     const float fiveHoursInSeconds = 5.0 * 60.0 * 60.0;
-    NSNumber *value = [NSNumber numberWithFloat:fiveHoursInSeconds];
+    NSNumber* value = [NSNumber numberWithFloat:fiveHoursInSeconds];
     CFPreferencesSetAppValue(checkInterval, value, app);
     CFPreferencesAppSynchronize(app);
   }
@@ -422,7 +460,7 @@ void RecordLastRunAppBundlePath() {
   // call this from awakeFromNib.
   NSMenu* view_menu = [[[NSApp mainMenu] itemWithTag:IDC_VIEW_MENU] submenu];
   NSMenuItem* encoding_menu_item = [view_menu itemWithTag:IDC_ENCODING_MENU];
-  NSMenu *encoding_menu = [encoding_menu_item submenu];
+  NSMenu* encoding_menu = [encoding_menu_item submenu];
   EncodingMenuControllerDelegate::BuildEncodingMenu([self defaultProfile],
                                                     encoding_menu);
 
@@ -802,6 +840,13 @@ void RecordLastRunAppBundlePath() {
   return NULL;
 }
 
+// (Private) Never call |-applicationShouldTerminate:|; just make everything go
+// through |-terminate:|.
+- (void)handleQuitEvent:(NSAppleEventDescriptor*)event
+              withReply:(NSAppleEventDescriptor*)reply {
+  [NSApp terminate:nil];
+}
+
 // Various methods to open URLs that we get in a native fashion. We use
 // BrowserInit here because on the other platforms, URLs to open come through
 // the ProcessSingleton, and it calls BrowserInit. It's best to bottleneck the