Refactor extension handling code from AppShimHost into ExtensionAppShimHandler.

AppShimHost is now confined to interacting with the app shim and passing
events to AppShimHandlers. ExtensionAppShimHandler becomes the default
handler for app shim events.

BUG=168080
TEST=There should be no behavior change with this refactor.

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

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

apps/app_shim/app_shim_handler_mac.cc

@@ -25,7 +25,7 @@ class AppShimHandlerRegistry {
     if (it != handlers_.end())
       return it->second;
 
-    return NULL;
+    return default_handler_;
   }
 
   bool SetForAppMode(const std::string& app_mode_id, AppShimHandler* handler) {
@@ -36,14 +36,20 @@ class AppShimHandlerRegistry {
     return inserted_or_removed;
   }
 
+  void SetDefaultHandler(AppShimHandler* handler) {
+    DCHECK_NE(default_handler_ == NULL, handler == NULL);
+    default_handler_ = handler;
+  }
+
  private:
   friend struct DefaultSingletonTraits<AppShimHandlerRegistry>;
   typedef std::map<std::string, AppShimHandler*> HandlerMap;
 
-  AppShimHandlerRegistry() {}
+  AppShimHandlerRegistry() : default_handler_(NULL) {}
   ~AppShimHandlerRegistry() {}
 
   HandlerMap handlers_;
+  AppShimHandler* default_handler_;
 
   DISALLOW_COPY_AND_ASSIGN(AppShimHandlerRegistry);
 };
@@ -67,4 +73,9 @@ AppShimHandler* AppShimHandler::GetForAppMode(const std::string& app_mode_id) {
   return AppShimHandlerRegistry::GetInstance()->GetForAppMode(app_mode_id);
 }
 
+// static
+void AppShimHandler::SetDefaultHandler(AppShimHandler* handler) {
+  AppShimHandlerRegistry::GetInstance()->SetDefaultHandler(handler);
+}
+
 }  // namespace apps

apps/app_shim/app_shim_handler_mac.h

@@ -7,6 +7,8 @@
 
 #include <string>
 
+class Profile;
+
 namespace apps {
 
 // Registrar, and interface for services that can handle interactions with OSX
@@ -18,6 +20,10 @@ class AppShimHandler {
     // Invoked when the app is closed in the browser process.
     virtual void OnAppClosed() = 0;
 
+    // Allows the handler to determine which app this host corresponds to.
+    virtual Profile* GetProfile() const = 0;
+    virtual std::string GetAppId() const = 0;
+
    protected:
     virtual ~Host() {}
   };
@@ -29,10 +35,15 @@ class AppShimHandler {
   // Remove a handler for an |app_mode_id|.
   static void RemoveHandler(const std::string& app_mode_id);
 
-  // Returns the handler registered for the given |app_mode_id|, or NULL if none
-  // is registered.
+  // Returns the handler registered for the given |app_mode_id|. If there is
+  // none registered, it returns the default handler or NULL if there is no
+  // default handler.
   static AppShimHandler* GetForAppMode(const std::string& app_mode_id);
 
+  // Sets the default handler to return when there is no app-specific handler.
+  // Setting this to NULL removes the default handler.
+  static void SetDefaultHandler(AppShimHandler* handler);
+
   // Invoked by the shim host when the shim process is launched. The handler
   // must return true if successful, or false to indicate back to the shim
   // process that it should close.

apps/app_shim/app_shim_host_mac.cc

@@ -10,16 +10,9 @@
 #include "base/files/file_path.h"
 #include "base/logging.h"
 #include "chrome/browser/browser_process.h"
-#include "chrome/browser/extensions/extension_host.h"
-#include "chrome/browser/extensions/extension_service.h"
-#include "chrome/browser/extensions/extension_system.h"
-#include "chrome/browser/extensions/shell_window_registry.h"
 #include "chrome/browser/profiles/profile_manager.h"
-#include "chrome/browser/ui/extensions/application_launch.h"
-#include "chrome/browser/ui/extensions/shell_window.h"
-#include "chrome/common/extensions/extension_constants.h"
+#include "content/public/browser/browser_thread.h"
 #include "ipc/ipc_channel_proxy.h"
-#include "ui/base/cocoa/focus_window_set.h"
 
 AppShimHost::AppShimHost()
     : channel_(NULL), profile_(NULL) {
@@ -36,7 +29,16 @@ void AppShimHost::ServeChannel(const IPC::ChannelHandle& handle) {
   DCHECK(CalledOnValidThread());
   DCHECK(!channel_.get());
   channel_.reset(new IPC::ChannelProxy(handle, IPC::Channel::MODE_SERVER, this,
-      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO)));
+      content::BrowserThread::GetMessageLoopProxyForThread(
+          content::BrowserThread::IO)));
+}
+
+Profile* AppShimHost::GetProfile() const {
+  return profile_;
+}
+
+std::string AppShimHost::GetAppId() const {
+  return app_id_;
 }
 
 bool AppShimHost::OnMessageReceived(const IPC::Message& message) {
@@ -62,58 +64,25 @@ bool AppShimHost::Send(IPC::Message* message) {
 
 void AppShimHost::OnLaunchApp(std::string profile_dir, std::string app_id) {
   DCHECK(CalledOnValidThread());
+  DCHECK(!profile_);
+  if (profile_) {
+    // Only one app launch message per channel.
+    Send(new AppShimMsg_LaunchApp_Done(false));
+    return;
+  }
+
+  profile_ = FetchProfileForDirectory(profile_dir);
   app_id_ = app_id;
   apps::AppShimHandler* handler = apps::AppShimHandler::GetForAppMode(app_id_);
-  bool success =
-      handler ? handler->OnShimLaunch(this) : LaunchAppImpl(profile_dir);
+  bool success = handler && handler->OnShimLaunch(this);
   Send(new AppShimMsg_LaunchApp_Done(success));
 }
 
 void AppShimHost::OnFocus() {
   DCHECK(CalledOnValidThread());
   apps::AppShimHandler* handler = apps::AppShimHandler::GetForAppMode(app_id_);
-  if (handler) {
+  if (handler)
     handler->OnShimFocus(this);
-    return;
-  }
-
-  if (!profile_)
-    return;
-  extensions::ShellWindowRegistry* registry =
-      extensions::ShellWindowRegistry::Get(profile_);
-  const extensions::ShellWindowRegistry::ShellWindowList windows =
-      registry->GetShellWindowsForApp(app_id_);
-  std::set<gfx::NativeWindow> native_windows;
-  for (extensions::ShellWindowRegistry::ShellWindowList::const_iterator i =
-           windows.begin();
-       i != windows.end();
-       ++i) {
-    native_windows.insert((*i)->GetNativeWindow());
-  }
-  ui::FocusWindowSet(native_windows);
-}
-
-bool AppShimHost::LaunchAppImpl(const std::string& profile_dir) {
-  DCHECK(CalledOnValidThread());
-  if (profile_) {
-    // Only one app launch message per channel.
-    return false;
-  }
-  if (!extensions::Extension::IdIsValid(app_id_)) {
-    LOG(ERROR) << "Bad app ID from app shim launch message.";
-    return false;
-  }
-  Profile* profile = FetchProfileForDirectory(profile_dir);
-  if (!profile)
-    return false;
-
-  if (!LaunchApp(profile))
-    return false;
-
-  profile_ = profile;
-  registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_HOST_DESTROYED,
-                 content::Source<Profile>(profile_));
-  return true;
 }
 
 Profile* AppShimHost::FetchProfileForDirectory(const std::string& profile_dir) {
@@ -139,50 +108,6 @@ Profile* AppShimHost::FetchProfileForDirectory(const std::string& profile_dir) {
   return profile;
 }
 
-bool AppShimHost::LaunchApp(Profile* profile) {
-  extensions::ExtensionSystem* extension_system =
-      extensions::ExtensionSystem::Get(profile);
-  ExtensionServiceInterface* extension_service =
-      extension_system->extension_service();
-  const extensions::Extension* extension =
-      extension_service->GetExtensionById(
-          app_id_, false);
-  if (!extension) {
-    LOG(ERROR) << "Attempted to launch nonexistent app with id '"
-               << app_id_ << "'.";
-    return false;
-  }
-  // TODO(jeremya): Handle the case that launching the app fails. Probably we
-  // need to watch for 'app successfully launched' or at least 'background page
-  // exists/was created' and time out with failure if we don't see that sign of
-  // life within a certain window.
-  chrome::AppLaunchParams params(profile,
-                                 extension,
-                                 extension_misc::LAUNCH_NONE,
-                                 NEW_WINDOW);
-  chrome::OpenApplication(params);
-  return true;
-}
-
-void AppShimHost::Observe(int type,
-                          const content::NotificationSource& source,
-                          const content::NotificationDetails& details) {
-  DCHECK(CalledOnValidThread());
-  DCHECK(content::Source<Profile>(source).ptr() == profile_);
-  switch (type) {
-    case chrome::NOTIFICATION_EXTENSION_HOST_DESTROYED: {
-      extensions::ExtensionHost* extension_host =
-          content::Details<extensions::ExtensionHost>(details).ptr();
-      if (app_id_ == extension_host->extension_id())
-        Close();
-      break;
-    }
-    default:
-      NOTREACHED() << "Unexpected notification sent.";
-      break;
-  }
-}
-
 void AppShimHost::OnAppClosed() {
   Close();
 }

apps/app_shim/app_shim_host_mac.h

@@ -10,8 +10,6 @@
 #include "apps/app_shim/app_shim_handler_mac.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/threading/non_thread_safe.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
 #include "ipc/ipc_listener.h"
 #include "ipc/ipc_sender.h"
 
@@ -30,7 +28,6 @@ class Message;
 class AppShimHost : public IPC::Listener,
                     public IPC::Sender,
                     public apps::AppShimHandler::Host,
-                    public content::NotificationObserver,
                     public base::NonThreadSafe {
  public:
   AppShimHost();
@@ -42,12 +39,9 @@ class AppShimHost : public IPC::Listener,
   void ServeChannel(const IPC::ChannelHandle& handle);
 
  protected:
-  const std::string& app_id() const { return app_id_; }
-  const Profile* profile() const { return profile_; }
 
   // Used internally; virtual so they can be mocked for testing.
   virtual Profile* FetchProfileForDirectory(const std::string& profile_dir);
-  virtual bool LaunchApp(Profile* profile);
 
   // IPC::Listener implementation.
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
@@ -68,17 +62,10 @@ class AppShimHost : public IPC::Listener,
   // Cmd+Tabbed to it.)
   void OnFocus();
 
-  bool LaunchAppImpl(const std::string& profile_dir);
-
-  // The AppShimHost listens to the NOTIFICATION_EXTENSION_HOST_DESTROYED
-  // message to detect when the app closes. When that happens, the AppShimHost
-  // closes the channel, which causes the app shim process to quit.
-  virtual void Observe(int type,
-                       const content::NotificationSource& source,
-                       const content::NotificationDetails& details) OVERRIDE;
-
-  // apps::AppShimHandler::Host override:
+  // apps::AppShimHandler::Host overrides:
   virtual void OnAppClosed() OVERRIDE;
+  virtual Profile* GetProfile() const OVERRIDE;
+  virtual std::string GetAppId() const OVERRIDE;
 
   // Closes the channel and destroys the AppShimHost.
   void Close();
@@ -86,7 +73,6 @@ class AppShimHost : public IPC::Listener,
   scoped_ptr<IPC::ChannelProxy> channel_;
   std::string app_id_;
   Profile* profile_;
-  content::NotificationRegistrar registrar_;
 };
 
 #endif  // CHROME_BROWSER_WEB_APPLICATIONS_APP_SHIM_HOST_MAC_H_

apps/app_shim/app_shim_host_mac_unittest.cc

@@ -5,6 +5,7 @@
 #include "apps/app_shim/app_shim_host_mac.h"
 
 #include "apps/app_shim/app_shim_messages.h"
+#include "base/basictypes.h"
 #include "base/memory/scoped_vector.h"
 #include "chrome/test/base/testing_profile.h"
 #include "ipc/ipc_message.h"
@@ -31,18 +32,9 @@ class TestingAppShimHost : public AppShimHost {
     fails_launch_ = fails_launch;
   }
 
-  const std::string& GetAppId() const {
-    return app_id();
-  }
-
-  const Profile* GetProfile() const {
-    return profile();
-  }
-
  protected:
   virtual Profile* FetchProfileForDirectory(const std::string& profile_dir)
       OVERRIDE;
-  virtual bool LaunchApp(Profile* profile) OVERRIDE;
   virtual bool Send(IPC::Message* message) OVERRIDE;
 
  private:
@@ -77,10 +69,6 @@ Profile* TestingAppShimHost::FetchProfileForDirectory(
   return fails_profile_ ? NULL : test_profile_;
 }
 
-bool TestingAppShimHost::LaunchApp(Profile* profile) {
-  return !fails_launch_;
-}
-
 class AppShimHostTest : public testing::Test,
                         public apps::AppShimHandler {
  public:
@@ -117,6 +105,7 @@ class AppShimHostTest : public testing::Test,
 
  private:
   virtual void SetUp() OVERRIDE {
+    testing::Test::SetUp();
     profile_.reset(new TestingProfile);
     host_.reset(new TestingAppShimHost(profile()));
   }
@@ -132,19 +121,12 @@ const char kTestProfileDir[] = "Default";
 
 }  // namespace
 
-TEST_F(AppShimHostTest, TestLaunchApp) {
-  host()->ReceiveMessage(
-      new AppShimHostMsg_LaunchApp(kTestProfileDir, kTestAppId));
-  ASSERT_EQ(kTestAppId, host()->GetAppId());
-  ASSERT_EQ(profile(), host()->GetProfile());
-  ASSERT_TRUE(LaunchWasSuccessful());
-}
-
 TEST_F(AppShimHostTest, TestLaunchAppWithHandler) {
   apps::AppShimHandler::RegisterHandler(kTestAppId, this);
   EXPECT_TRUE(host()->ReceiveMessage(
       new AppShimHostMsg_LaunchApp(kTestProfileDir, kTestAppId)));
-  EXPECT_EQ(kTestAppId, host()->GetAppId());
+  EXPECT_EQ(kTestAppId,
+            implicit_cast<apps::AppShimHandler::Host*>(host())->GetAppId());
   EXPECT_TRUE(LaunchWasSuccessful());
   EXPECT_EQ(1, launch_count_);
   EXPECT_EQ(0, focus_count_);

apps/app_shim/app_shim_host_manager_mac.h

@@ -5,9 +5,8 @@
 #ifndef CHROME_BROWSER_WEB_APPLICATIONS_APP_SHIM_HOST_MANAGER_MAC_H_
 #define CHROME_BROWSER_WEB_APPLICATIONS_APP_SHIM_HOST_MANAGER_MAC_H_
 
-#include "base/mac/scoped_cftyperef.h"
+#include "apps/app_shim/extension_app_shim_handler_mac.h"
 #include "base/memory/weak_ptr.h"
-#include "base/threading/thread.h"
 #include "ipc/ipc_channel_factory.h"
 
 // The AppShimHostManager receives connections from app shims on a UNIX
@@ -34,6 +33,8 @@ class AppShimHostManager
 
   scoped_ptr<IPC::ChannelFactory> factory_;
 
+  apps::ExtensionAppShimHandler extension_app_shim_handler_;
+
   DISALLOW_COPY_AND_ASSIGN(AppShimHostManager);
 };
 

apps/app_shim/app_shim_host_manager_mac.mm

@@ -4,6 +4,7 @@
 
 #include "apps/app_shim/app_shim_host_manager_mac.h"
 
+#include "apps/app_shim/app_shim_handler_mac.h"
 #include "apps/app_shim/app_shim_host_mac.h"
 #include "base/bind.h"
 #include "base/files/file_path.h"
@@ -27,6 +28,7 @@ void CreateAppShimHost(const IPC::ChannelHandle& handle) {
 
 AppShimHostManager::AppShimHostManager() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  apps::AppShimHandler::SetDefaultHandler(&extension_app_shim_handler_);
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&AppShimHostManager::InitOnFileThread,
@@ -34,6 +36,7 @@ AppShimHostManager::AppShimHostManager() {
 }
 
 AppShimHostManager::~AppShimHostManager() {
+  apps::AppShimHandler::SetDefaultHandler(NULL);
 }
 
 void AppShimHostManager::InitOnFileThread() {

apps/app_shim/extension_app_shim_handler_mac.cc

+// 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.
+
+#include "apps/app_shim/extension_app_shim_handler_mac.h"
+
+#include "apps/app_shim/app_shim_messages.h"
+#include "base/files/file_path.h"
+#include "base/logging.h"
+#include "chrome/browser/extensions/extension_host.h"
+#include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/extension_system.h"
+#include "chrome/browser/extensions/shell_window_registry.h"
+#include "chrome/browser/ui/extensions/application_launch.h"
+#include "chrome/browser/ui/extensions/shell_window.h"
+#include "ui/base/cocoa/focus_window_set.h"
+
+namespace apps {
+
+ExtensionAppShimHandler::ExtensionAppShimHandler() {}
+
+ExtensionAppShimHandler::~ExtensionAppShimHandler() {
+  for (HostMap::iterator it = hosts_.begin(); it != hosts_.end(); ) {
+    // Increment the iterator first as OnAppClosed may call back to OnShimClose
+    // and invalidate the iterator.
+    it++->second->OnAppClosed();
+  }
+}
+
+bool ExtensionAppShimHandler::OnShimLaunch(Host* host) {
+  Profile* profile = host->GetProfile();
+  DCHECK(profile);
+
+  const std::string& app_id = host->GetAppId();
+  if (!extensions::Extension::IdIsValid(app_id)) {
+    LOG(ERROR) << "Bad app ID from app shim launch message.";
+    return false;
+  }
+
+  if (!LaunchApp(profile, app_id))
+    return false;
+
+  // The first host to claim this (profile, app_id) becomes the main host.
+  // For any others, we launch the app but return false.
+  if (!hosts_.insert(make_pair(make_pair(profile, app_id), host)).second)
+    return false;
+
+  if (!registrar_.IsRegistered(
+      this, chrome::NOTIFICATION_EXTENSION_HOST_DESTROYED,
+      content::Source<Profile>(profile))) {
+    registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_HOST_DESTROYED,
+                   content::Source<Profile>(profile));
+  }
+
+  return true;
+}
+
+void ExtensionAppShimHandler::OnShimClose(Host* host) {
+  HostMap::iterator it = hosts_.find(make_pair(host->GetProfile(),
+                                               host->GetAppId()));
+  // Any hosts other than the main host will still call OnShimClose, so ignore
+  // them.
+  if (it != hosts_.end() && it->second == host)
+    hosts_.erase(it);
+}
+
+void ExtensionAppShimHandler::OnShimFocus(Host* host) {
+  if (!host->GetProfile())
+    return;
+
+  extensions::ShellWindowRegistry* registry =
+      extensions::ShellWindowRegistry::Get(host->GetProfile());
+  const extensions::ShellWindowRegistry::ShellWindowList windows =
+      registry->GetShellWindowsForApp(host->GetAppId());
+  std::set<gfx::NativeWindow> native_windows;
+  for (extensions::ShellWindowRegistry::const_iterator i = windows.begin();
+       i != windows.end(); ++i) {
+    native_windows.insert((*i)->GetNativeWindow());
+  }
+  ui::FocusWindowSet(native_windows);
+}
+
+bool ExtensionAppShimHandler::LaunchApp(Profile* profile,
+                                        const std::string& app_id) {
+  extensions::ExtensionSystem* extension_system =
+      extensions::ExtensionSystem::Get(profile);
+  ExtensionServiceInterface* extension_service =
+      extension_system->extension_service();
+  const extensions::Extension* extension =
+      extension_service->GetExtensionById(app_id, false);
+  if (!extension) {
+    LOG(ERROR) << "Attempted to launch nonexistent app with id '"
+               << app_id << "'.";
+    return false;
+  }
+  // TODO(jeremya): Handle the case that launching the app fails. Probably we
+  // need to watch for 'app successfully launched' or at least 'background page
+  // exists/was created' and time out with failure if we don't see that sign of
+  // life within a certain window.
+  chrome::OpenApplication(chrome::AppLaunchParams(
+      profile, extension, NEW_FOREGROUND_TAB));
+  return true;
+}
+
+void ExtensionAppShimHandler::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  Profile* profile = content::Source<Profile>(source).ptr();
+  switch (type) {
+    case chrome::NOTIFICATION_EXTENSION_HOST_DESTROYED: {
+      extensions::ExtensionHost* extension_host =
+          content::Details<extensions::ExtensionHost>(details).ptr();
+      CloseShim(profile, extension_host->extension_id());
+      break;
+    }
+    default:
+      NOTREACHED();  // Unexpected notification.
+      break;
+  }
+}
+
+void ExtensionAppShimHandler::CloseShim(Profile* profile,
+                                        const std::string& app_id) {
+  HostMap::const_iterator it = hosts_.find(make_pair(profile, app_id));
+  if (it != hosts_.end())
+    it->second->OnAppClosed();
+}
+
+}  // namespace apps

apps/app_shim/extension_app_shim_handler_mac.h

+// 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.
+
+#ifndef APPS_APP_SHIM_EXTENSION_APP_SHIM_HANDLER_H_
+#define APPS_APP_SHIM_EXTENSION_APP_SHIM_HANDLER_H_
+
+#include <map>
+#include <string>
+
+#include "apps/app_shim/app_shim_handler_mac.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
+
+class Profile;
+
+namespace apps {
+
+// This app shim handler that handles events for app shims that correspond to an
+// extension.
+class ExtensionAppShimHandler : public AppShimHandler,
+                                public content::NotificationObserver {
+ public:
+  ExtensionAppShimHandler();
+  virtual ~ExtensionAppShimHandler();
+
+  // AppShimHandler overrides:
+  virtual bool OnShimLaunch(Host* host) OVERRIDE;
+  virtual void OnShimClose(Host* host) OVERRIDE;
+  virtual void OnShimFocus(Host* host) OVERRIDE;
+
+ protected:
+  typedef std::map<std::pair<Profile*, std::string>, AppShimHandler::Host*>
+      HostMap;
+
+  // Exposed for testing.
+  HostMap& hosts() { return hosts_; }
+  content::NotificationRegistrar& registrar() { return registrar_; }
+
+ private:
+  virtual bool LaunchApp(Profile* profile, const std::string& app_id);
+
+  // Listen to the NOTIFICATION_EXTENSION_HOST_DESTROYED message to detect when
+  // an app closes. When that happens, call OnAppClosed on the relevant
+  // AppShimHandler::Host which causes the app shim process to quit.
+  // The host will call OnShimClose on this.
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
+
+  void CloseShim(Profile* profile, const std::string& app_id);
+
+  HostMap hosts_;
+
+  content::NotificationRegistrar registrar_;
+};
+
+}  // namespace apps
+
+#endif  // APPS_APP_SHIM_EXTENSION_APP_SHIM_HANDLER_H_

apps/app_shim/extension_app_shim_handler_mac_unittest.cc

+// 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.
+
+#include "apps/app_shim/extension_app_shim_handler_mac.h"
+
+#include "apps/app_shim/app_shim_host_mac.h"
+#include "base/memory/scoped_ptr.h"
+#include "chrome/common/chrome_notification_types.h"
+#include "chrome/test/base/testing_profile.h"
+#include "content/public/browser/notification_service.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace apps {
+
+class TestingExtensionAppShimHandler : public ExtensionAppShimHandler {
+ public:
+  explicit TestingExtensionAppShimHandler() : fails_launch_(false) {}
+  virtual ~TestingExtensionAppShimHandler() {}
+
+  void set_fails_launch(bool fails_launch) {
+    fails_launch_ = fails_launch;
+  }
+
+  AppShimHandler::Host* FindHost(Profile* profile,
+                                 const std::string& app_id) {
+    HostMap::const_iterator it = hosts().find(make_pair(profile, app_id));
+    return it == hosts().end() ? NULL : it->second;
+  }
+
+  content::NotificationRegistrar& GetRegistrar() { return registrar(); }
+
+ protected:
+  virtual bool LaunchApp(Profile* profile, const std::string& app_id) OVERRIDE {
+    return !fails_launch_;
+  }
+
+ private:
+  bool fails_launch_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestingExtensionAppShimHandler);
+};
+
+const char kTestAppIdA[] = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
+const char kTestAppIdB[] = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb";
+
+class FakeHost : public apps::AppShimHandler::Host {
+ public:
+  FakeHost(Profile* profile,
+           std::string app_id,
+           TestingExtensionAppShimHandler* handler)
+      : profile_(profile),
+        app_id_(app_id),
+        handler_(handler),
+        close_count_(0) {}
+
+  virtual void OnAppClosed() OVERRIDE {
+    handler_->OnShimClose(this);
+    ++close_count_;
+  }
+  virtual Profile* GetProfile() const OVERRIDE { return profile_; }
+  virtual std::string GetAppId() const OVERRIDE { return app_id_; }
+
+  int close_count() { return close_count_; }
+
+ private:
+  Profile* profile_;
+  std::string app_id_;
+  TestingExtensionAppShimHandler* handler_;
+  int close_count_;
+
+  DISALLOW_COPY_AND_ASSIGN(FakeHost);
+};
+
+class ExtensionAppShimHandlerTest : public testing::Test {
+ protected:
+  ExtensionAppShimHandlerTest()
+      : handler_(new TestingExtensionAppShimHandler),
+        host_aa_(&profile_a_, kTestAppIdA, handler_.get()),
+        host_ab_(&profile_a_, kTestAppIdB, handler_.get()),
+        host_bb_(&profile_b_, kTestAppIdB, handler_.get()),
+        host_aa_duplicate_(&profile_a_, kTestAppIdA, handler_.get()) {}
+
+  scoped_ptr<TestingExtensionAppShimHandler> handler_;
+  TestingProfile profile_a_;
+  TestingProfile profile_b_;
+  FakeHost host_aa_;
+  FakeHost host_ab_;
+  FakeHost host_bb_;
+  FakeHost host_aa_duplicate_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ExtensionAppShimHandlerTest);
+};
+
+TEST_F(ExtensionAppShimHandlerTest, LaunchAndCloseShim) {
+  // If launch fails, the host is not added to the map.
+  handler_->set_fails_launch(true);
+  EXPECT_EQ(false, handler_->OnShimLaunch(&host_aa_));
+  EXPECT_FALSE(handler_->FindHost(&profile_a_, kTestAppIdA));
+
+  // Normal startup.
+  handler_->set_fails_launch(false);
+  EXPECT_EQ(true, handler_->OnShimLaunch(&host_aa_));
+  EXPECT_EQ(&host_aa_, handler_->FindHost(&profile_a_, kTestAppIdA));
+  EXPECT_TRUE(handler_->GetRegistrar().IsRegistered(
+      handler_.get(),
+      chrome::NOTIFICATION_EXTENSION_HOST_DESTROYED,
+      content::Source<Profile>(&profile_a_)));
+  EXPECT_EQ(true, handler_->OnShimLaunch(&host_ab_));
+  EXPECT_EQ(&host_ab_, handler_->FindHost(&profile_a_, kTestAppIdB));
+  EXPECT_EQ(true, handler_->OnShimLaunch(&host_bb_));
+  EXPECT_EQ(&host_bb_, handler_->FindHost(&profile_b_, kTestAppIdB));
+  EXPECT_TRUE(handler_->GetRegistrar().IsRegistered(
+      handler_.get(),
+      chrome::NOTIFICATION_EXTENSION_HOST_DESTROYED,
+      content::Source<Profile>(&profile_b_)));
+
+  // Starting and closing a second host does nothing.
+  EXPECT_EQ(false, handler_->OnShimLaunch(&host_aa_duplicate_));
+  EXPECT_EQ(&host_aa_, handler_->FindHost(&profile_a_, kTestAppIdA));
+  handler_->OnShimClose(&host_aa_duplicate_);
+  EXPECT_EQ(&host_aa_, handler_->FindHost(&profile_a_, kTestAppIdA));
+
+  // Normal close.
+  handler_->OnShimClose(&host_aa_);
+  EXPECT_FALSE(handler_->FindHost(&profile_a_, kTestAppIdA));
+
+  // Closing the second host afterward does nothing.
+  handler_->OnShimClose(&host_aa_duplicate_);
+  EXPECT_FALSE(handler_->FindHost(&profile_a_, kTestAppIdA));
+
+  // Destruction sends OnAppClose to remaining hosts.
+  handler_.reset();
+  EXPECT_EQ(1, host_ab_.close_count());
+  EXPECT_EQ(1, host_bb_.close_count());
+}
+
+}  // namespace apps

apps/apps.gypi

@@ -36,6 +36,8 @@
         'app_shim/app_shim_host_mac.h',
         'app_shim/app_shim_host_manager_mac.h',
         'app_shim/app_shim_host_manager_mac.mm',
+        'app_shim/extension_app_shim_handler_mac.h',
+        'app_shim/extension_app_shim_handler_mac.cc',
         'field_trial_names.cc',
         'field_trial_names.h',
         'pref_names.cc',

chrome/browser/ui/app_list/app_list_service_mac_browsertest.mm

@@ -40,11 +40,17 @@ class AppListServiceMacBrowserTest : public InProcessBrowserTest,
     AppShimHandler::GetForAppMode(app_mode::kAppListModeId)->OnShimClose(this);
   }
 
-  // AppShimHandler::Host override:
+  // AppShimHandler::Host overrides:
   virtual void OnAppClosed() OVERRIDE {
     ++close_count_;
     QuitShim();
   }
+  virtual Profile* GetProfile() const OVERRIDE {
+    return NULL;  // Currently unused in this test.
+  }
+  virtual std::string GetAppId() const OVERRIDE {
+    return app_mode::kAppListModeId;
+  }
 
   int close_count_;
   bool running_;

chrome/chrome_tests_unit.gypi

@@ -464,6 +464,7 @@
       'sources': [
         '../apps/app_shim/app_shim_host_mac_unittest.cc',
         '../apps/saved_files_service_unittest.cc',
+        '../apps/app_shim/extension_app_shim_handler_mac_unittest.cc',
         '../apps/shell_window_geometry_cache_unittest.cc',
         '../extensions/browser/file_reader_unittest.cc',
         '../extensions/common/event_filter_unittest.cc',