Cmd+Q on an app window quits the app.

In applicationShouldTerminate, if the terminate was triggered by a keyboard
event, and the currently focused window is a ShellWindow, then quit the app
associated with that window and return NSTerminateCancel.
An accessor for ExtensionAppShimHandler is added to
BrowserProcessPlatFormPart to allow manipulating apps from anywhere.
This will also be used for the Quit menu option for app menus. 
Cmd+Q behavior on a browser window remains mostly unchanged. The
only difference is that Cmd+Q when there are no windows quits immediately,
even if confirm-to-quit is enabled.

Note that this does not affect termination due to OSX shutting down. Even if
the shutdown is triggered by a keyboard shortcut (e.g. Ctrl+Cmd+Eject), the
event type will be NSSystemDefined, so Chrome terminates as usual.

BUG=168080,249059,227472
TEST=Start an app.
Cmd+Q with one of the app's windows focused should quit the app.
Quit Chrome and start an app using its shim.
Cmd+Q on the app should quit the app and Chrome.
TEST=Start an app.
Ctrl+Cmd+Eject with the app window focused to shut down OSX.
Chrome should terminate cleanly.

Review URL: https://chromiumcodereview.appspot.com/17448007

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

15 files changed, 243 insertions, 32 deletions

diff --git a/apps/DEPS b/apps/DEPS
index eda580a..44a0dde 100644
--- a/apps/DEPS
+++ b/apps/DEPS
@@ -12,6 +12,7 @@ include_rules = [
   # Temporary allowed includes.
   # TODO(benwells): remove these (http://crbug.com/159366)
   "+chrome/browser/browser_process.h",
+  "+chrome/browser/browser_shutdown.h",
   "+chrome/browser/chrome_notification_types.h",
   "+chrome/browser/extensions",
   "+chrome/browser/lifetime/application_lifetime.h",
@@ -30,6 +31,7 @@ include_rules = [
   "+chrome/common/extensions",
   "+chrome/common/mac/app_mode_common.h",
   "+chrome/installer",
+  "+chrome/test/base/interactive_test_utils.h",
   "+chrome/test/base/testing_profile.h",
   "+grit/generated_resources.h",
 ]
diff --git a/apps/app_shim/app_shim_host_manager_mac.h b/apps/app_shim/app_shim_host_manager_mac.h
index 45d881c..1c088f5 100644
--- a/apps/app_shim/app_shim_host_manager_mac.h
+++ b/apps/app_shim/app_shim_host_manager_mac.h
@@ -18,6 +18,10 @@ class AppShimHostManager
   AppShimHostManager();
   virtual ~AppShimHostManager();
 
+  apps::ExtensionAppShimHandler* extension_app_shim_handler() {
+    return &extension_app_shim_handler_;
+  }
+
  private:
   // IPC::ChannelFactory::Delegate implementation.
   virtual void OnClientConnected(const IPC::ChannelHandle& handle) OVERRIDE;
diff --git a/apps/app_shim/app_shim_quit_interactive_uitest_mac.mm b/apps/app_shim/app_shim_quit_interactive_uitest_mac.mm
new file mode 100644
index 0000000..f16caef
--- /dev/null
+++ b/apps/app_shim/app_shim_quit_interactive_uitest_mac.mm
@@ -0,0 +1,125 @@
+// Copyright 2013 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.
+
+// Tests behavior when quitting apps with app shims.
+
+#import <Cocoa/Cocoa.h>
+
+#include "apps/app_shim/app_shim_host_manager_mac.h"
+#include "apps/app_shim/extension_app_shim_handler_mac.h"
+#include "apps/switches.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/browser_shutdown.h"
+#include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/extension_test_message_listener.h"
+#include "chrome/browser/extensions/platform_app_browsertest_util.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/test/base/interactive_test_utils.h"
+#include "content/public/test/test_utils.h"
+#include "ui/base/test/cocoa_test_event_utils.h"
+
+using extensions::PlatformAppBrowserTest;
+
+namespace apps {
+
+namespace {
+
+class FakeHost : public apps::AppShimHandler::Host {
+ public:
+  FakeHost(const base::FilePath& profile_path,
+           const std::string& app_id,
+           ExtensionAppShimHandler* handler)
+      : profile_path_(profile_path),
+        app_id_(app_id),
+        handler_(handler) {}
+
+  virtual void OnAppLaunchComplete(AppShimLaunchResult result) OVERRIDE {}
+  virtual void OnAppClosed() OVERRIDE {
+    handler_->OnShimClose(this);
+  }
+  virtual base::FilePath GetProfilePath() const OVERRIDE {
+    return profile_path_;
+  }
+  virtual std::string GetAppId() const OVERRIDE { return app_id_; }
+
+ private:
+  base::FilePath profile_path_;
+  std::string app_id_;
+  ExtensionAppShimHandler* handler_;
+
+  DISALLOW_COPY_AND_ASSIGN(FakeHost);
+};
+
+// Starts an app without a browser window using --load_and_launch_app and
+// --silent_launch.
+class AppShimQuitTest : public PlatformAppBrowserTest {
+ protected:
+  AppShimQuitTest() {}
+
+  void SetUpAppShim() {
+    ASSERT_EQ(0u, [[NSApp windows] count]);
+    ExtensionTestMessageListener launched_listener("Launched", false);
+    ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
+    ASSERT_EQ(1u, [[NSApp windows] count]);
+
+    handler_ = g_browser_process->platform_part()->app_shim_host_manager()->
+        extension_app_shim_handler();
+
+    // Attach a host for the app.
+    extension_id_ =
+        GetExtensionByPath(extension_service()->extensions(), app_path_)->id();
+    host_.reset(new FakeHost(profile()->GetPath().BaseName(),
+                             extension_id_,
+                             handler_));
+    handler_->OnShimLaunch(host_.get(), APP_SHIM_LAUNCH_REGISTER_ONLY);
+    EXPECT_EQ(host_.get(), handler_->FindHost(profile(), extension_id_));
+
+    // Focus the app window.
+    NSWindow* window = [[NSApp windows] objectAtIndex:0];
+    EXPECT_TRUE(ui_test_utils::ShowAndFocusNativeWindow(window));
+    content::RunAllPendingInMessageLoop();
+  }
+
+  virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
+    PlatformAppBrowserTest::SetUpCommandLine(command_line);
+    // Simulate an app shim initiated launch, i.e. launch app but not browser.
+    app_path_ = test_data_dir_
+        .AppendASCII("platform_apps")
+        .AppendASCII("minimal");
+    command_line->AppendSwitchNative(apps::kLoadAndLaunchApp,
+                                     app_path_.value());
+    command_line->AppendSwitch(switches::kSilentLaunch);
+  }
+
+  base::FilePath app_path_;
+  ExtensionAppShimHandler* handler_;
+  std::string extension_id_;
+  scoped_ptr<FakeHost> host_;
+
+  DISALLOW_COPY_AND_ASSIGN(AppShimQuitTest);
+};
+
+}  // namespace
+
+// Test that closing an app with Cmd+Q when no browsers have ever been open
+// terminates Chrome.
+IN_PROC_BROWSER_TEST_F(AppShimQuitTest, QuitWithKeyEvent) {
+  SetUpAppShim();
+
+  // Simulate a Cmd+Q event.
+  NSWindow* window = [[NSApp windows] objectAtIndex:0];
+  NSEvent* event = cocoa_test_event_utils::KeyEventWithKeyCode(
+      0, 'q', NSKeyDown, NSCommandKeyMask);
+  [window postEvent:event
+            atStart:NO];
+
+  // This will time out if the event above does not terminate Chrome.
+  content::RunMessageLoop();
+
+  EXPECT_FALSE(handler_->FindHost(profile(), extension_id_));
+  EXPECT_TRUE(browser_shutdown::IsTryingToQuit());
+}
+
+}  // namespace apps
diff --git a/apps/app_shim/extension_app_shim_handler_mac.cc b/apps/app_shim/extension_app_shim_handler_mac.cc
index 1349f91..5491144 100644
--- a/apps/app_shim/extension_app_shim_handler_mac.cc
+++ b/apps/app_shim/extension_app_shim_handler_mac.cc
@@ -5,6 +5,7 @@
 #include "apps/app_shim/extension_app_shim_handler_mac.h"
 
 #include "apps/app_lifetime_monitor_factory.h"
+#include "apps/app_shim/app_shim_host_manager_mac.h"
 #include "apps/app_shim/app_shim_messages.h"
 #include "apps/shell_window.h"
 #include "base/files/file_path.h"
@@ -138,6 +139,29 @@ ExtensionAppShimHandler::ExtensionAppShimHandler()
 
 ExtensionAppShimHandler::~ExtensionAppShimHandler() {}
 
+AppShimHandler::Host* ExtensionAppShimHandler::FindHost(
+    Profile* profile,
+    const std::string& app_id) {
+  HostMap::iterator it = hosts_.find(make_pair(profile, app_id));
+  return it == hosts_.end() ? NULL : it->second;
+}
+
+// static
+void ExtensionAppShimHandler::QuitAppForWindow(ShellWindow* shell_window) {
+  ExtensionAppShimHandler* handler =
+      g_browser_process->platform_part()->app_shim_host_manager()->
+          extension_app_shim_handler();
+  Host* host = handler->FindHost(shell_window->profile(),
+                                 shell_window->extension_id());
+  if (host) {
+    handler->OnShimQuit(host);
+  } else {
+    // App shims might be disabled or the shim is still starting up.
+    extensions::ShellWindowRegistry::Get(shell_window->profile())->
+        CloseAllShellWindowsForApp(shell_window->extension_id());
+  }
+}
+
 void ExtensionAppShimHandler::OnShimLaunch(Host* host,
                                            AppShimLaunchType launch_type) {
   const std::string& app_id = host->GetAppId();
@@ -268,15 +292,16 @@ void ExtensionAppShimHandler::OnShimQuit(Host* host) {
   DCHECK(delegate_->ProfileExistsForPath(host->GetProfilePath()));
   Profile* profile = delegate_->ProfileForPath(host->GetProfilePath());
 
+  const std::string& app_id = host->GetAppId();
   const ShellWindowList windows =
-      delegate_->GetWindows(profile, host->GetAppId());
+      delegate_->GetWindows(profile, app_id);
   for (extensions::ShellWindowRegistry::const_iterator it = windows.begin();
        it != windows.end(); ++it) {
     (*it)->GetBaseWindow()->Close();
   }
 
-  DCHECK_NE(0u, hosts_.count(make_pair(profile, host->GetAppId())));
-  hosts_.find(make_pair(profile, host->GetAppId()))->second->OnAppClosed();
+  DCHECK_NE(0u, hosts_.count(make_pair(profile, app_id)));
+  host->OnAppClosed();
 
   if (!browser_opened_ever_ && hosts_.empty())
     delegate_->MaybeTerminate();
@@ -322,9 +347,9 @@ void ExtensionAppShimHandler::Observe(
     case chrome::NOTIFICATION_EXTENSION_UNINSTALLED: {
       const std::string& app_id =
           content::Details<extensions::Extension>(details).ptr()->id();
-      HostMap::const_iterator it = hosts_.find(make_pair(profile, app_id));
-      if (it != hosts_.end())
-        it->second->OnAppClosed();
+      Host* host = FindHost(profile, app_id);
+      if (host)
+        host->OnAppClosed();
       break;
     }
     default: {
@@ -344,10 +369,10 @@ void ExtensionAppShimHandler::OnAppActivated(Profile* profile,
   if (!extension)
     return;
 
-  HostMap::iterator it = hosts_.find(make_pair(profile, app_id));
-  if (it != hosts_.end()) {
-    it->second->OnAppLaunchComplete(APP_SHIM_LAUNCH_SUCCESS);
-    OnShimFocus(it->second, APP_SHIM_FOCUS_NORMAL);
+  Host* host = FindHost(profile, app_id);
+  if (host) {
+    host->OnAppLaunchComplete(APP_SHIM_LAUNCH_SUCCESS);
+    OnShimFocus(host, APP_SHIM_FOCUS_NORMAL);
     return;
   }
 
diff --git a/apps/app_shim/extension_app_shim_handler_mac.h b/apps/app_shim/extension_app_shim_handler_mac.h
index 27c26fd..616fd78 100644
--- a/apps/app_shim/extension_app_shim_handler_mac.h
+++ b/apps/app_shim/extension_app_shim_handler_mac.h
@@ -63,6 +63,10 @@ class ExtensionAppShimHandler : public AppShimHandler,
   ExtensionAppShimHandler();
   virtual ~ExtensionAppShimHandler();
 
+  AppShimHandler::Host* FindHost(Profile* profile, const std::string& app_id);
+
+  static void QuitAppForWindow(ShellWindow* shell_window);
+
   // AppShimHandler overrides:
   virtual void OnShimLaunch(Host* host, AppShimLaunchType launch_type) OVERRIDE;
   virtual void OnShimClose(Host* host) OVERRIDE;
diff --git a/chrome/browser/app_controller_mac.mm b/chrome/browser/app_controller_mac.mm
index c018058..04590b1 100644
--- a/chrome/browser/app_controller_mac.mm
+++ b/chrome/browser/app_controller_mac.mm
@@ -4,6 +4,7 @@
 
 #import "chrome/browser/app_controller_mac.h"
 
+#include "apps/app_shim/extension_app_shim_handler_mac.h"
 #include "apps/shell_window.h"
 #include "base/auto_reset.h"
 #include "base/bind.h"
@@ -387,6 +388,27 @@ class AppControllerProfileObserver : public ProfileInfoCacheObserver {
 }
 
 - (NSApplicationTerminateReply)applicationShouldTerminate:(NSApplication*)app {
+  using extensions::ShellWindowRegistry;
+
+  // If there are no windows, quit immediately.
+  if (chrome::BrowserIterator().done() &&
+      !ShellWindowRegistry::IsShellWindowRegisteredInAnyProfile(0)) {
+    return NSTerminateNow;
+  }
+
+  // Check if this is a keyboard initiated quit on an app window. If so, quit
+  // the app. This could cause the app to trigger another terminate, but that
+  // will be caught by the no windows condition above.
+  if ([[app currentEvent] type] == NSKeyDown) {
+    apps::ShellWindow* shellWindow =
+        ShellWindowRegistry::GetShellWindowForNativeWindowAnyProfile(
+            [app keyWindow]);
+    if (shellWindow) {
+      apps::ExtensionAppShimHandler::QuitAppForWindow(shellWindow);
+      return NSTerminateCancel;
+    }
+  }
+
   // Check if the preference is turned on.
   const PrefService* prefs = g_browser_process->local_state();
   if (!prefs->GetBoolean(prefs::kConfirmToQuitEnabled)) {
diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc
index 75897f2..82aa66b 100644
--- a/chrome/browser/browser_process_impl.cc
+++ b/chrome/browser/browser_process_impl.cc
@@ -115,11 +115,6 @@
 #include "chrome/browser/plugins/plugins_resource_service.h"
 #endif
 
-#if defined(OS_MACOSX)
-#include "apps/app_shim/app_shim_host_manager_mac.h"
-#include "chrome/browser/ui/app_list/app_list_service.h"
-#endif
-
 #if defined(ENABLE_WEBRTC)
 #include "chrome/browser/media/webrtc_log_uploader.h"
 #endif
@@ -279,10 +274,6 @@ void BrowserProcessImpl::StartTearDown() {
   aura::Env::DeleteInstance();
 #endif
 
-#if defined(OS_MACOSX)
-  app_shim_host_manager_.reset();
-#endif
-
   platform_part()->StartTearDown();
 }
 
@@ -939,10 +930,7 @@ void BrowserProcessImpl::PreMainMessageLoopRun() {
   storage_monitor_.reset(chrome::StorageMonitor::Create());
 #endif
 
-#if defined(OS_MACOSX)
-  app_shim_host_manager_.reset(new AppShimHostManager);
-  AppListService::InitAll(NULL);
-#endif
+  platform_part_->PreMainMessageLoopRun();
 }
 
 void BrowserProcessImpl::CreateIconManager() {
diff --git a/chrome/browser/browser_process_impl.h b/chrome/browser/browser_process_impl.h
index 50a76d1..728c0d9 100644
--- a/chrome/browser/browser_process_impl.h
+++ b/chrome/browser/browser_process_impl.h
@@ -41,10 +41,6 @@ class BrowserPolicyConnector;
 class PolicyService;
 };
 
-#if defined(OS_MACOSX)
-class AppShimHostManager;
-#endif
-
 // Real implementation of BrowserProcess that creates and returns the services.
 class BrowserProcessImpl : public BrowserProcess,
                            public base::NonThreadSafe {
@@ -292,11 +288,6 @@ class BrowserProcessImpl : public BrowserProcess,
   scoped_refptr<PluginsResourceService> plugins_resource_service_;
 #endif
 
-#if defined(OS_MACOSX)
-  // Hosts the IPC channel factory that App Shims connect to on Mac.
-  scoped_ptr<AppShimHostManager> app_shim_host_manager_;
-#endif
-
   scoped_ptr<BrowserProcessPlatformPart> platform_part_;
 
   // TODO(eroman): Remove this when done debugging 113031. This tracks
diff --git a/chrome/browser/browser_process_platform_part_base.cc b/chrome/browser/browser_process_platform_part_base.cc
index 1da8917..0287321 100644
--- a/chrome/browser/browser_process_platform_part_base.cc
+++ b/chrome/browser/browser_process_platform_part_base.cc
@@ -29,3 +29,6 @@ void BrowserProcessPlatformPartBase::AttemptExit() {
   chrome::CloseAllBrowsers();
 #endif
 }
+
+void BrowserProcessPlatformPartBase::PreMainMessageLoopRun() {
+}
diff --git a/chrome/browser/browser_process_platform_part_base.h b/chrome/browser/browser_process_platform_part_base.h
index 1494abc..abf35d3 100644
--- a/chrome/browser/browser_process_platform_part_base.h
+++ b/chrome/browser/browser_process_platform_part_base.h
@@ -27,6 +27,9 @@ class BrowserProcessPlatformPartBase {
   // Called from AttemptExitInternal().
   virtual void AttemptExit();
 
+  // Called at the end of BrowserProcessImpl::PreMainMessageLoopRun().
+  virtual void PreMainMessageLoopRun();
+
  private:
   DISALLOW_COPY_AND_ASSIGN(BrowserProcessPlatformPartBase);
 };
diff --git a/chrome/browser/browser_process_platform_part_mac.h b/chrome/browser/browser_process_platform_part_mac.h
index 2077fef..20d2645 100644
--- a/chrome/browser/browser_process_platform_part_mac.h
+++ b/chrome/browser/browser_process_platform_part_mac.h
@@ -6,17 +6,31 @@
 #define CHROME_BROWSER_BROWSER_PROCESS_PLATFORM_PART_MAC_H_
 
 #include "base/compiler_specific.h"
+#include "base/memory/scoped_ptr.h"
 #include "chrome/browser/browser_process_platform_part_base.h"
 
+class AppShimHostManager;
+
+namespace apps {
+class ExtensionAppShimHandler;
+}
+
 class BrowserProcessPlatformPart : public BrowserProcessPlatformPartBase {
  public:
   BrowserProcessPlatformPart();
   virtual ~BrowserProcessPlatformPart();
 
   // Overridden from BrowserProcessPlatformPartBase:
+  virtual void StartTearDown() OVERRIDE;
   virtual void AttemptExit() OVERRIDE;
+  virtual void PreMainMessageLoopRun() OVERRIDE;
+
+  AppShimHostManager* app_shim_host_manager();
 
  private:
+  // Hosts the IPC channel factory that App Shims connect to on Mac.
+  scoped_ptr<AppShimHostManager> app_shim_host_manager_;
+
   DISALLOW_COPY_AND_ASSIGN(BrowserProcessPlatformPart);
 };
 
diff --git a/chrome/browser/browser_process_platform_part_mac.mm b/chrome/browser/browser_process_platform_part_mac.mm
index a279de6..c83cd5b 100644
--- a/chrome/browser/browser_process_platform_part_mac.mm
+++ b/chrome/browser/browser_process_platform_part_mac.mm
@@ -3,7 +3,10 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/browser_process_platform_part_mac.h"
+
+#include "apps/app_shim/app_shim_host_manager_mac.h"
 #include "chrome/browser/chrome_browser_application_mac.h"
+#include "chrome/browser/ui/app_list/app_list_service.h"
 
 BrowserProcessPlatformPart::BrowserProcessPlatformPart() {
 }
@@ -11,9 +14,22 @@ BrowserProcessPlatformPart::BrowserProcessPlatformPart() {
 BrowserProcessPlatformPart::~BrowserProcessPlatformPart() {
 }
 
+void BrowserProcessPlatformPart::StartTearDown() {
+  app_shim_host_manager_.reset();
+}
+
 void BrowserProcessPlatformPart::AttemptExit() {
   // On the Mac, the application continues to run once all windows are closed.
   // Terminate will result in a CloseAllBrowsers() call, and once (and if)
   // that is done, will cause the application to exit cleanly.
   chrome_browser_application_mac::Terminate();
 }
+
+void BrowserProcessPlatformPart::PreMainMessageLoopRun() {
+  app_shim_host_manager_.reset(new AppShimHostManager);
+  AppListService::InitAll(NULL);
+}
+
+AppShimHostManager* BrowserProcessPlatformPart::app_shim_host_manager() {
+  return app_shim_host_manager_.get();
+}
diff --git a/chrome/browser/extensions/shell_window_registry.cc b/chrome/browser/extensions/shell_window_registry.cc
index 38a8d38..b9c939d 100644
--- a/chrome/browser/extensions/shell_window_registry.cc
+++ b/chrome/browser/extensions/shell_window_registry.cc
@@ -109,6 +109,16 @@ ShellWindowRegistry::ShellWindowList ShellWindowRegistry::GetShellWindowsForApp(
   return app_windows;
 }
 
+void ShellWindowRegistry::CloseAllShellWindowsForApp(
+    const std::string& app_id) {
+  for (ShellWindowList::const_iterator i = shell_windows_.begin();
+       i != shell_windows_.end(); ) {
+    ShellWindow* shell_window = *(i++);
+    if (shell_window->extension_id() == app_id)
+      shell_window->GetBaseWindow()->Close();
+  }
+}
+
 ShellWindow* ShellWindowRegistry::GetShellWindowForRenderViewHost(
     content::RenderViewHost* render_view_host) const {
   for (ShellWindowList::const_iterator i = shell_windows_.begin();
diff --git a/chrome/browser/extensions/shell_window_registry.h b/chrome/browser/extensions/shell_window_registry.h
index da1072c..fb5f48e 100644
--- a/chrome/browser/extensions/shell_window_registry.h
+++ b/chrome/browser/extensions/shell_window_registry.h
@@ -74,6 +74,9 @@ class ShellWindowRegistry : public BrowserContextKeyedService {
   ShellWindowList GetShellWindowsForApp(const std::string& app_id) const;
   const ShellWindowList& shell_windows() const { return shell_windows_; }
 
+  // Close all shell windows associated with an app.
+  void CloseAllShellWindowsForApp(const std::string& app_id);
+
   // Helper functions to find shell windows with particular attributes.
   apps::ShellWindow* GetShellWindowForRenderViewHost(
       content::RenderViewHost* render_view_host) const;
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index fe5ade0..442ef18 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -205,6 +205,7 @@
         'INTERACTIVE_TESTS',
       ],
       'sources': [
+        '../apps/app_shim/app_shim_quit_interactive_uitest_mac.mm',
         'browser/autofill/autofill_interactive_uitest.cc',
         'browser/browser_keyevents_browsertest.cc',
         'browser/extensions/api/omnibox/omnibox_api_interactive_test.cc',