[Mac] Close browsers when quitting is prevented by QuitWithAppsController.

Follow-up to https://codereview.chromium.org/220373003/

This closes all browser windows when quitting, even if Chrome is kept
alive by QuitWithAppsController.

TEST=Enable --apps-keep-chrome-alive in chrome://flags.
Enable session restore in settings ("Continue where you left off").
Start Chrome and open an extra tab and browser window.
Open an app from chrome://apps.
Quit Chrome. i.e. via Dock->Quit, Menu->Quit, or Cmd+Q.
Chrome and the app should stay running but browser windows should close.
TEST=Repeat the first test and then:
Close the app window, Chrome should quit.
TEST=Repeat the first test and then:
Click on the Chrome dock icon, the previous session should be restored.

BUG=333429

Review URL: https://codereview.chromium.org/253383004

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

5 files changed, 84 insertions, 43 deletions

diff --git a/apps/app_shim/app_shim_handler_mac.cc b/apps/app_shim/app_shim_handler_mac.cc
index cfbf4ff..7e67e73 100644
--- a/apps/app_shim/app_shim_handler_mac.cc
+++ b/apps/app_shim/app_shim_handler_mac.cc
@@ -61,7 +61,7 @@ class AppShimHandlerRegistry : public content::NotificationObserver {
   }
 
   void MaybeTerminate() {
-    if (!browser_opened_ever_) {
+    if (!browser_session_running_) {
       // Post this to give AppWindows a chance to remove themselves from the
       // registry.
       base::MessageLoop::current()->PostTask(
@@ -69,16 +69,26 @@ class AppShimHandlerRegistry : public content::NotificationObserver {
     }
   }
 
+  bool ShouldRestoreSession() {
+    return !browser_session_running_;
+  }
+
  private:
   friend struct DefaultSingletonTraits<AppShimHandlerRegistry>;
   typedef std::map<std::string, AppShimHandler*> HandlerMap;
 
   AppShimHandlerRegistry()
       : default_handler_(NULL),
-        browser_opened_ever_(false) {
+        browser_session_running_(false) {
     registrar_.Add(
         this, chrome::NOTIFICATION_BROWSER_OPENED,
         content::NotificationService::AllBrowserContextsAndSources());
+    registrar_.Add(
+        this, chrome::NOTIFICATION_CLOSE_ALL_BROWSERS_REQUEST,
+        content::NotificationService::AllBrowserContextsAndSources());
+    registrar_.Add(
+        this, chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED,
+        content::NotificationService::AllBrowserContextsAndSources());
   }
 
   virtual ~AppShimHandlerRegistry() {}
@@ -88,17 +98,23 @@ class AppShimHandlerRegistry : public content::NotificationObserver {
       int type,
       const content::NotificationSource& source,
       const content::NotificationDetails& details) OVERRIDE {
-    DCHECK_EQ(chrome::NOTIFICATION_BROWSER_OPENED, type);
-    registrar_.Remove(
-        this, chrome::NOTIFICATION_BROWSER_OPENED,
-        content::NotificationService::AllBrowserContextsAndSources());
-    browser_opened_ever_ = true;
+    switch (type) {
+      case chrome::NOTIFICATION_BROWSER_OPENED:
+      case chrome::NOTIFICATION_BROWSER_CLOSE_CANCELLED:
+        browser_session_running_ = true;
+        break;
+      case chrome::NOTIFICATION_CLOSE_ALL_BROWSERS_REQUEST:
+        browser_session_running_ = false;
+        break;
+      default:
+        NOTREACHED();
+    }
   }
 
   HandlerMap handlers_;
   AppShimHandler* default_handler_;
   content::NotificationRegistrar registrar_;
-  bool browser_opened_ever_;
+  bool browser_session_running_;
 
   DISALLOW_COPY_AND_ASSIGN(AppShimHandlerRegistry);
 };
@@ -132,4 +148,9 @@ void AppShimHandler::MaybeTerminate() {
   AppShimHandlerRegistry::GetInstance()->MaybeTerminate();
 }
 
+// static
+bool AppShimHandler::ShouldRestoreSession() {
+  return AppShimHandlerRegistry::GetInstance()->ShouldRestoreSession();
+}
+
 }  // namespace apps
diff --git a/apps/app_shim/app_shim_handler_mac.h b/apps/app_shim/app_shim_handler_mac.h
index 0ecde07..9e53e0e 100644
--- a/apps/app_shim/app_shim_handler_mac.h
+++ b/apps/app_shim/app_shim_handler_mac.h
@@ -56,6 +56,11 @@ class AppShimHandler {
   // shell windows, and the app list is not visible.
   static void MaybeTerminate();
 
+  // Whether browser sessions should be restored right now. This is true if
+  // the browser has been quit but kept alive because Chrome Apps are still
+  // running.
+  static bool ShouldRestoreSession();
+
   // Invoked by the shim host when the shim process is launched. The handler
   // must call OnAppLaunchComplete to inform the shim of the result.
   // |launch_type| indicates the type of launch.
diff --git a/chrome/browser/app_controller_mac.mm b/chrome/browser/app_controller_mac.mm
index b863936..22ec9a9 100644
--- a/chrome/browser/app_controller_mac.mm
+++ b/chrome/browser/app_controller_mac.mm
@@ -372,12 +372,6 @@ class AppControllerProfileObserver : public ProfileInfoCacheObserver {
       ![self shouldQuitWithInProgressDownloads])
     return NO;
 
-  // Check for active apps, and prompt the user if they really want to quit
-  // (and also quit the apps).
-  if (!browser_shutdown::IsTryingToQuit() &&
-      quitWithAppsController_.get() && !quitWithAppsController_->ShouldQuit())
-    return NO;
-
   // 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
@@ -388,6 +382,19 @@ class AppControllerProfileObserver : public ProfileInfoCacheObserver {
       [self applicationShouldTerminate:app] != NSTerminateNow)
     return NO;
 
+  // Check for active apps. If quitting is prevented, only close browsers and
+  // sessions.
+  if (!browser_shutdown::IsTryingToQuit() &&
+      quitWithAppsController_ && !quitWithAppsController_->ShouldQuit()) {
+    content::NotificationService::current()->Notify(
+        chrome::NOTIFICATION_CLOSE_ALL_BROWSERS_REQUEST,
+        content::NotificationService::AllSources(),
+        content::NotificationService::NoDetails());
+    // This will close all browser sessions.
+    chrome::CloseAllBrowsers();
+    return NO;
+  }
+
   size_t num_browsers = chrome::GetTotalBrowserCount();
 
   // Initiate a shutdown (via chrome::CloseAllBrowsersAndQuit()) if we aren't
@@ -1208,16 +1215,16 @@ class AppControllerProfileObserver : public ProfileInfoCacheObserver {
   // Normally, it'd just open a new empty page.
   {
     static BOOL doneOnce = NO;
-    if (!doneOnce) {
-      doneOnce = YES;
-      if (base::mac::WasLaunchedAsHiddenLoginItem()) {
-        SessionService* sessionService =
-            SessionServiceFactory::GetForProfileForSessionRestore(
-                [self lastProfile]);
-        if (sessionService &&
-            sessionService->RestoreIfNecessary(std::vector<GURL>()))
-          return NO;
-      }
+    BOOL attemptRestore = apps::AppShimHandler::ShouldRestoreSession() ||
+        (!doneOnce && base::mac::WasLaunchedAsHiddenLoginItem());
+    doneOnce = YES;
+    if (attemptRestore) {
+      SessionService* sessionService =
+          SessionServiceFactory::GetForProfileForSessionRestore(
+              [self lastProfile]);
+      if (sessionService &&
+          sessionService->RestoreIfNecessary(std::vector<GURL>()))
+        return NO;
     }
   }
 
diff --git a/chrome/browser/ui/cocoa/apps/quit_with_apps_controller_mac_interactive_uitest.cc b/chrome/browser/ui/cocoa/apps/quit_with_apps_controller_mac_interactive_uitest.cc
index 51a8acb..18eb721 100644
--- a/chrome/browser/ui/cocoa/apps/quit_with_apps_controller_mac_interactive_uitest.cc
+++ b/chrome/browser/ui/cocoa/apps/quit_with_apps_controller_mac_interactive_uitest.cc
@@ -7,8 +7,10 @@
 #include "apps/app_window_registry.h"
 #include "apps/ui/native_app_window.h"
 #include "base/command_line.h"
+#include "base/run_loop.h"
 #include "chrome/browser/apps/app_browsertest_util.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/chrome_browser_application_mac.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_test_message_listener.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
@@ -39,14 +41,6 @@ class QuitWithAppsControllerInteractiveTest
     command_line->AppendSwitch(switches::kAppsKeepChromeAlive);
   }
 
-  virtual void SetUpOnMainThread() OVERRIDE {
-    ExtensionBrowserTest::SetUpOnMainThread();
-
-    ExtensionTestMessageListener listener("Launched", false);
-    app_ = InstallAndLaunchPlatformApp("minimal_id");
-    ASSERT_TRUE(listener.WaitUntilSatisfied());
-  }
-
   const extensions::Extension* app_;
 
  private:
@@ -62,6 +56,18 @@ IN_PROC_BROWSER_TEST_F(QuitWithAppsControllerInteractiveTest, QuitBehavior) {
   const Notification* notification;
   message_center::MessageCenter* message_center =
       message_center::MessageCenter::Get();
+
+  // With no app windows open, ShouldQuit returns true.
+  EXPECT_TRUE(controller->ShouldQuit());
+  notification = g_browser_process->notification_ui_manager()->FindById(
+      QuitWithAppsController::kQuitWithAppsNotificationID);
+  EXPECT_EQ(NULL, notification);
+
+  // Open an app window.
+  ExtensionTestMessageListener listener("Launched", false);
+  app_ = InstallAndLaunchPlatformApp("minimal_id");
+  ASSERT_TRUE(listener.WaitUntilSatisfied());
+
   // One browser and one app window at this point.
   EXPECT_FALSE(chrome::BrowserIterator().done());
   EXPECT_TRUE(apps::AppWindowRegistry::IsAppWindowRegisteredInAnyProfile(0));
@@ -97,12 +103,11 @@ IN_PROC_BROWSER_TEST_F(QuitWithAppsControllerInteractiveTest, QuitBehavior) {
   EXPECT_FALSE(chrome::BrowserIterator().done());
   EXPECT_TRUE(apps::AppWindowRegistry::IsAppWindowRegisteredInAnyProfile(0));
 
-  // Normally, quitting would close all browsers, but since we're just
-  // simulating a quit, close it here.
+  // Quitting should not quit but close all browsers
   content::WindowedNotificationObserver observer(
       chrome::NOTIFICATION_BROWSER_CLOSED,
       content::NotificationService::AllSources());
-  chrome::BrowserIterator()->window()->Close();
+  chrome_browser_application_mac::Terminate();
   observer.Wait();
 
   EXPECT_TRUE(chrome::BrowserIterator().done());
@@ -114,14 +119,13 @@ IN_PROC_BROWSER_TEST_F(QuitWithAppsControllerInteractiveTest, QuitBehavior) {
       QuitWithAppsController::kQuitWithAppsNotificationID);
   ASSERT_TRUE(notification);
 
-  // Clicking "Quit All Apps." button closes all app windows.
+  // Clicking "Quit All Apps." button closes all app windows. With no browsers
+  // open, this should also quit Chrome.
+  content::WindowedNotificationObserver quit_observer(
+      chrome::NOTIFICATION_APP_TERMINATING,
+      content::NotificationService::AllSources());
   notification->delegate()->ButtonClick(0);
   message_center->RemoveAllNotifications(false);
   EXPECT_FALSE(apps::AppWindowRegistry::IsAppWindowRegisteredInAnyProfile(0));
-
-  // With no app windows open, ShouldQuit returns true.
-  EXPECT_TRUE(controller->ShouldQuit());
-  notification = g_browser_process->notification_ui_manager()->FindById(
-      QuitWithAppsController::kQuitWithAppsNotificationID);
-  EXPECT_EQ(NULL, notification);
+  quit_observer.Wait();
 }
diff --git a/chrome/browser/ui/cocoa/confirm_quit_panel_controller.mm b/chrome/browser/ui/cocoa/confirm_quit_panel_controller.mm
index 499dc93..ad2c835 100644
--- a/chrome/browser/ui/cocoa/confirm_quit_panel_controller.mm
+++ b/chrome/browser/ui/cocoa/confirm_quit_panel_controller.mm
@@ -15,6 +15,7 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#import "chrome/browser/ui/cocoa/browser_window_controller.h"
 #include "chrome/browser/ui/cocoa/confirm_quit.h"
 #include "chrome/common/pref_names.h"
 #include "grit/generated_resources.h"
@@ -154,7 +155,10 @@ void RegisterLocalState(PrefRegistrySimple* registry) {
 
 - (void)setCurrentProgress:(NSAnimationProgress)progress {
   for (NSWindow* window in [application_ windows]) {
-    [window setAlphaValue:1.0 - progress];
+    if ([[window windowController]
+            isKindOfClass:[BrowserWindowController class]]) {
+      [window setAlphaValue:1.0 - progress];
+    }
   }
 }