Restructure ExtensionAppShimHandler testing.

apps/app_shim/extension_app_shim_handler_mac_unittest.cc

Index: apps/app_shim/extension_app_shim_handler_mac_unittest.cc
diff --git a/apps/app_shim/extension_app_shim_handler_mac_unittest.cc b/apps/app_shim/extension_app_shim_handler_mac_unittest.cc
index 6f650e6ca5f4b8192324d439a31d99cc34b8eef8..2fa41380d9a008dad442a423c72b719bfed42882 100644
--- a/apps/app_shim/extension_app_shim_handler_mac_unittest.cc
+++ b/apps/app_shim/extension_app_shim_handler_mac_unittest.cc
@@ -7,6 +7,7 @@
 #include "apps/app_shim/app_shim_host_mac.h"
 #include "base/memory/scoped_ptr.h"
 #include "chrome/common/chrome_notification_types.h"
+#include "chrome/common/extensions/extension.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/browser/notification_service.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -14,27 +15,36 @@
 
 namespace apps {
 
+using extensions::Extension;
+typedef extensions::ShellWindowRegistry::ShellWindowList ShellWindowList;
+
+using ::testing::_;
 using ::testing::Return;
 
-class MockProfileManagerFacade
-    : public ExtensionAppShimHandler::ProfileManagerFacade {
+class MockDelegate
+    : public ExtensionAppShimHandler::Delegate {

[tapted, 2013/06/21 04:30:47]

nit: one line?

[jackhou1, 2013/06/21 05:46:27]

Done.

  public:
-  virtual ~MockProfileManagerFacade() {}
+  virtual ~MockDelegate() {}
+
+  MOCK_METHOD1(ProfileExistsForPath, bool(const base::FilePath&));
+  MOCK_METHOD1(ProfileForPath, Profile*(const base::FilePath&));
+
+  MOCK_METHOD2(GetWindows, ShellWindowList(Profile*, const std::string&));
+
+  MOCK_METHOD2(GetExtension, const Extension*(Profile*, const std::string&));
+  MOCK_METHOD2(LaunchApp, content::WebContents*(Profile*, const Extension*));
+  MOCK_METHOD2(LaunchShim, void(Profile*, const Extension*));
 
-  MOCK_METHOD1(ProfileExistsForPath, bool(const base::FilePath& path));
-  MOCK_METHOD1(ProfileForPath, Profile*(const base::FilePath& path));
 };
 
 class TestingExtensionAppShimHandler : public ExtensionAppShimHandler {
  public:
-  TestingExtensionAppShimHandler(ProfileManagerFacade* profile_manager_facade) {
-    set_profile_manager_facade(profile_manager_facade);
+  TestingExtensionAppShimHandler(Delegate* dependency_facade) {
+    set_dependency_facade(dependency_facade);
   }
   virtual ~TestingExtensionAppShimHandler() {}
 
-  MOCK_METHOD3(LaunchApp, bool(Profile*,
-                               const std::string&,
-                               AppShimLaunchType));
+  MOCK_METHOD1(OnShimFocus, void(Host* host));
 
   AppShimHandler::Host* FindHost(Profile* profile,
                                  const std::string& app_id) {
@@ -84,25 +94,54 @@ class FakeHost : public apps::AppShimHandler::Host {
 class ExtensionAppShimHandlerTest : public testing::Test {
  protected:
   ExtensionAppShimHandlerTest()
-      : profile_manager_facade_(new MockProfileManagerFacade),
-        handler_(new TestingExtensionAppShimHandler(profile_manager_facade_)),
+      : delegate_(new MockDelegate),
+        handler_(new TestingExtensionAppShimHandler(delegate_)),
         profile_path_a_("Profile A"),
         profile_path_b_("Profile B"),
         host_aa_(profile_path_a_, kTestAppIdA, handler_.get()),
         host_ab_(profile_path_a_, kTestAppIdB, handler_.get()),
         host_bb_(profile_path_b_, kTestAppIdB, handler_.get()),
         host_aa_duplicate_(profile_path_a_, kTestAppIdA, handler_.get()) {
-    EXPECT_CALL(*profile_manager_facade_, ProfileExistsForPath(profile_path_a_))
+    base::FilePath extension_path("/fake/path");
+    base::DictionaryValue manifest;
+    manifest.SetString("name", "Fake Name");
+    manifest.SetString("version", "1");
+    std::string error;
+    extension_a_ = Extension::Create(
+        extension_path, extensions::Manifest::INTERNAL, manifest,
+        Extension::NO_FLAGS, kTestAppIdA, &error);
+    EXPECT_TRUE(extension_a_.get()) << error;
+
+    extension_b_ = Extension::Create(
+        extension_path, extensions::Manifest::INTERNAL, manifest,
+        Extension::NO_FLAGS, kTestAppIdB, &error);
+    EXPECT_TRUE(extension_b_.get()) << error;
+
+    EXPECT_CALL(*delegate_, ProfileExistsForPath(profile_path_a_))
         .WillRepeatedly(Return(true));
-    EXPECT_CALL(*profile_manager_facade_, ProfileForPath(profile_path_a_))
+    EXPECT_CALL(*delegate_, ProfileForPath(profile_path_a_))
         .WillRepeatedly(Return(&profile_a_));
-    EXPECT_CALL(*profile_manager_facade_, ProfileExistsForPath(profile_path_b_))
+    EXPECT_CALL(*delegate_, ProfileExistsForPath(profile_path_b_))
         .WillRepeatedly(Return(true));
-    EXPECT_CALL(*profile_manager_facade_, ProfileForPath(profile_path_b_))
+    EXPECT_CALL(*delegate_, ProfileForPath(profile_path_b_))
         .WillRepeatedly(Return(&profile_b_));
+
+    // In most tests, we don't care about the result of GetWindows, it just
+    // needs to be non-empty.
+    ShellWindowList shell_window_list;
+    shell_window_list.push_back(static_cast<ShellWindow*>(NULL));
+    EXPECT_CALL(*delegate_, GetWindows(_, _))
+        .WillRepeatedly(Return(shell_window_list));
+
+    EXPECT_CALL(*delegate_, GetExtension(_, kTestAppIdA))
+        .WillRepeatedly(Return(extension_a_.get()));
+    EXPECT_CALL(*delegate_, GetExtension(_, kTestAppIdB))
+        .WillRepeatedly(Return(extension_b_.get()));
+    EXPECT_CALL(*delegate_, LaunchApp(_, _))
+        .WillRepeatedly(Return(static_cast<content::WebContents*>(NULL)));
   }
 
-  MockProfileManagerFacade* profile_manager_facade_;
+  MockDelegate* delegate_;
   scoped_ptr<TestingExtensionAppShimHandler> handler_;
   base::FilePath profile_path_a_;
   base::FilePath profile_path_b_;
@@ -112,6 +151,8 @@ class ExtensionAppShimHandlerTest : public testing::Test {
   FakeHost host_ab_;
   FakeHost host_bb_;
   FakeHost host_aa_duplicate_;
+  scoped_refptr<Extension> extension_a_;
+  scoped_refptr<Extension> extension_b_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ExtensionAppShimHandlerTest);
@@ -121,30 +162,24 @@ TEST_F(ExtensionAppShimHandlerTest, LaunchAndCloseShim) {
   const AppShimLaunchType normal_launch = APP_SHIM_LAUNCH_NORMAL;
 
   // If launch fails, the host is not added to the map.
-  EXPECT_CALL(*handler_, LaunchApp(&profile_a_, kTestAppIdA, normal_launch))
-      .WillOnce(Return(false));
+  EXPECT_CALL(*delegate_, GetExtension(&profile_a_, kTestAppIdA))
+      .WillOnce(Return(static_cast<const Extension*>(NULL)))
+      .WillRepeatedly(Return(extension_a_.get()));
   EXPECT_EQ(false, handler_->OnShimLaunch(&host_aa_, normal_launch));
   EXPECT_FALSE(handler_->FindHost(&profile_a_, kTestAppIdA));
 
   // Normal startup.
-  EXPECT_CALL(*handler_, LaunchApp(&profile_a_, kTestAppIdA, normal_launch))
-      .WillOnce(Return(true));
   EXPECT_EQ(true, handler_->OnShimLaunch(&host_aa_, normal_launch));
   EXPECT_EQ(&host_aa_, handler_->FindHost(&profile_a_, kTestAppIdA));
 
-  EXPECT_CALL(*handler_, LaunchApp(&profile_a_, kTestAppIdB, normal_launch))
-      .WillOnce(Return(true));
   EXPECT_EQ(true, handler_->OnShimLaunch(&host_ab_, normal_launch));
   EXPECT_EQ(&host_ab_, handler_->FindHost(&profile_a_, kTestAppIdB));
 
-  EXPECT_CALL(*handler_, LaunchApp(&profile_b_, kTestAppIdB, normal_launch))
-      .WillOnce(Return(true));
   EXPECT_EQ(true, handler_->OnShimLaunch(&host_bb_, normal_launch));
   EXPECT_EQ(&host_bb_, handler_->FindHost(&profile_b_, kTestAppIdB));
 
-  // Starting and closing a second host does nothing.
-  EXPECT_CALL(*handler_, LaunchApp(&profile_a_, kTestAppIdA, normal_launch))
-      .WillOnce(Return(false));
+  // Starting and closing a second host just focuses the app.
+  EXPECT_CALL(*handler_, OnShimFocus(&host_aa_duplicate_));
   EXPECT_EQ(false, handler_->OnShimLaunch(&host_aa_duplicate_, normal_launch));
   EXPECT_EQ(&host_aa_, handler_->FindHost(&profile_a_, kTestAppIdA));
   handler_->OnShimClose(&host_aa_duplicate_);
@@ -159,4 +194,39 @@ TEST_F(ExtensionAppShimHandlerTest, LaunchAndCloseShim) {
   EXPECT_FALSE(handler_->FindHost(&profile_a_, kTestAppIdA));
 }
 
+TEST_F(ExtensionAppShimHandlerTest, AppLifetime) {
+  EXPECT_CALL(*delegate_, LaunchShim(&profile_a_, extension_a_.get()));
+  handler_->OnAppActivated(&profile_a_, kTestAppIdA);
+
+  // Launch the shim, adding an entry in the map.
+  EXPECT_EQ(true, handler_->OnShimLaunch(&host_aa_, APP_SHIM_LAUNCH_NORMAL));
+  EXPECT_EQ(&host_aa_, handler_->FindHost(&profile_a_, kTestAppIdA));
+
+  handler_->OnAppDeactivated(&profile_a_, kTestAppIdA);
+  EXPECT_EQ(1, host_aa_.close_count());
+}
+
+TEST_F(ExtensionAppShimHandlerTest, RegisterOnly) {
+  // For an APP_SHIM_LAUNCH_REGISTER_ONLY, don't launch the app.
+  EXPECT_CALL(*delegate_, LaunchApp(_, _))
+      .Times(0);
+  EXPECT_EQ(true, handler_->OnShimLaunch(&host_aa_,
+                                          APP_SHIM_LAUNCH_REGISTER_ONLY));
+  EXPECT_TRUE(handler_->FindHost(&profile_a_, kTestAppIdA));
+
+  // Close the shim, removing the entry in the map.
+  handler_->OnShimClose(&host_aa_);
+  EXPECT_FALSE(handler_->FindHost(&profile_a_, kTestAppIdA));
+
+  // Don't register if there are no windows open for the app. This can happen if
+  // the app shim was launched in response to an app being activated, but the
+  // app was deactivated before the shim is registered.
+  ShellWindowList shell_window_list;
+  EXPECT_CALL(*delegate_, GetWindows(&profile_a_, kTestAppIdA))
+      .WillRepeatedly(Return(shell_window_list));
+  EXPECT_EQ(false, handler_->OnShimLaunch(&host_aa_,
+                                          APP_SHIM_LAUNCH_REGISTER_ONLY));
+  EXPECT_FALSE(handler_->FindHost(&profile_a_, kTestAppIdA));
+}
+
 }  // namespace apps