chrome.management.launchApp() should choose the window type for the app based on user prefs.

BUG=70191
TEST=ExtensionManagementApiTest.Launch*App

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

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

4 files changed, 191 insertions, 16 deletions

diff --git a/chrome/browser/extensions/extension_management_api.cc b/chrome/browser/extensions/extension_management_api.cc
index 50117c4..d6b5a35 100644
--- a/chrome/browser/extensions/extension_management_api.cc
+++ b/chrome/browser/extensions/extension_management_api.cc
@@ -167,8 +167,13 @@ bool LaunchAppFunction::RunImpl() {
     return false;
   }
 
-  extension_misc::LaunchContainer container = extension->launch_container();
-  Browser::OpenApplication(profile(), extension, container, NULL);
+  // Look at prefs to find the right launch container.
+  // |default_pref_value| is set to LAUNCH_REGULAR so that if
+  // the user has not set a preference, we open the app in a tab.
+  extension_misc::LaunchContainer launch_container =
+      service()->extension_prefs()->GetLaunchContainer(
+          extension, ExtensionPrefs::LAUNCH_DEFAULT);
+  Browser::OpenApplication(profile(), extension, launch_container, NULL);
 
   return true;
 }
diff --git a/chrome/browser/extensions/extension_management_apitest.cc b/chrome/browser/extensions/extension_management_apitest.cc
index 9883f5d..c2c6bbe 100644
--- a/chrome/browser/extensions/extension_management_apitest.cc
+++ b/chrome/browser/extensions/extension_management_apitest.cc
@@ -4,8 +4,28 @@
 
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/extension_test_message_listener.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_list.h"
+#include "chrome/test/ui_test_utils.h"
+
+namespace {
+
+// Find a browser other than |browser|.
+Browser* FindOtherBrowser(Browser* browser) {
+  Browser* found = NULL;
+  for (BrowserList::const_iterator it = BrowserList::begin();
+       it != BrowserList::end(); ++it) {
+    if (*it == browser)
+      continue;
+    found = *it;
+  }
+
+  return found;
+}
+
+}  // namespace
 
 class ExtensionManagementApiTest : public ExtensionApiTest {
  public:
@@ -25,6 +45,19 @@ class ExtensionManagementApiTest : public ExtensionApiTest {
     ASSERT_TRUE(LoadExtension(basedir.AppendASCII("disabled_app")));
     service->DisableExtension(last_loaded_extension_id_);
   }
+
+  // Load an app, and wait for a message from app "management/launch_on_install"
+  // indicating that the new app has been launched.
+  void LoadAndWaitForLaunch(const std::string& app_path,
+                            std::string* out_app_id) {
+    ExtensionTestMessageListener launched_app("launched app", false);
+    ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII(app_path)));
+
+    if (out_app_id)
+      *out_app_id = last_loaded_extension_id_;
+
+    ASSERT_TRUE(launched_app.WaitUntilSatisfied());
+  }
 };
 
 IN_PROC_BROWSER_TEST_F(ExtensionManagementApiTest, Basics) {
@@ -36,3 +69,107 @@ IN_PROC_BROWSER_TEST_F(ExtensionManagementApiTest, Uninstall) {
   InstallExtensions();
   ASSERT_TRUE(RunExtensionSubtest("management/test", "uninstall.html"));
 }
+
+IN_PROC_BROWSER_TEST_F(ExtensionManagementApiTest, LaunchPanelApp) {
+  ExtensionService* service = browser()->profile()->GetExtensionService();
+
+  // Load an extension that calls launchApp() on any app that gets
+  // installed.
+  ExtensionTestMessageListener launcher_loaded("launcher loaded", false);
+  ASSERT_TRUE(LoadExtension(
+      test_data_dir_.AppendASCII("management/launch_on_install")));
+  ASSERT_TRUE(launcher_loaded.WaitUntilSatisfied());
+
+  // Load an app with app.launch.container = "panel".
+  std::string app_id;
+  LoadAndWaitForLaunch("management/launch_app_panel", &app_id);
+  ASSERT_FALSE(HasFatalFailure());  // Stop the test if any ASSERT failed.
+
+  // Find the app's browser.  Check that it is a panel.
+  ASSERT_EQ(2u, BrowserList::GetBrowserCount(browser()->profile()));
+  Browser* app_browser = FindOtherBrowser(browser());
+  ASSERT_EQ(Browser::TYPE_APP_POPUP, app_browser->type());
+
+  // Close the app panel.
+  app_browser->CloseWindow();
+  ui_test_utils::WaitForNotificationFrom(NotificationType::BROWSER_CLOSED,
+                                           Source<Browser>(app_browser));
+
+  // Unload the extension.
+  UninstallExtension(app_id);
+  ASSERT_EQ(1u, BrowserList::GetBrowserCount(browser()->profile()));
+  ASSERT_FALSE(service->GetExtensionById(app_id, true));
+
+  // Set a pref indicating that the user wants to launch in a regular tab.
+  // This should be ignored, because panel apps always load in a panel.
+  service->extension_prefs()->SetLaunchType(
+      app_id, ExtensionPrefs::LAUNCH_REGULAR);
+
+  // Load the extension again.
+  std::string app_id_new;
+  LoadAndWaitForLaunch("management/launch_app_panel", &app_id_new);
+  ASSERT_FALSE(HasFatalFailure());
+
+  // If the ID changed, then the pref will not apply to the app.
+  ASSERT_EQ(app_id, app_id_new);
+
+  // Find the app's browser.  Apps that should load in a panel ignore
+  // prefs, so we should still see the launch in a panel.
+  ASSERT_EQ(2u, BrowserList::GetBrowserCount(browser()->profile()));
+  app_browser = FindOtherBrowser(browser());
+  ASSERT_EQ(Browser::TYPE_APP_POPUP, app_browser->type());
+}
+
+IN_PROC_BROWSER_TEST_F(ExtensionManagementApiTest, LaunchTabApp) {
+  ExtensionService* service = browser()->profile()->GetExtensionService();
+
+  // Load an extension that calls launchApp() on any app that gets
+  // installed.
+  ExtensionTestMessageListener launcher_loaded("launcher loaded", false);
+  ASSERT_TRUE(LoadExtension(
+      test_data_dir_.AppendASCII("management/launch_on_install")));
+  ASSERT_TRUE(launcher_loaded.WaitUntilSatisfied());
+
+  // Code below assumes that the test starts with a single browser window
+  // hosting one tab.
+  ASSERT_EQ(1u, BrowserList::GetBrowserCount(browser()->profile()));
+  ASSERT_EQ(1, browser()->tab_count());
+
+  // Load an app with app.launch.container = "tab".
+  std::string app_id;
+  LoadAndWaitForLaunch("management/launch_app_tab", &app_id);
+  ASSERT_FALSE(HasFatalFailure());
+
+  // Check that the app opened in a new tab of the existing browser.
+  ASSERT_EQ(1u, BrowserList::GetBrowserCount(browser()->profile()));
+  ASSERT_EQ(2, browser()->tab_count());
+
+  // Unload the extension.
+  UninstallExtension(app_id);
+  ASSERT_EQ(1u, BrowserList::GetBrowserCount(browser()->profile()));
+  ASSERT_FALSE(service->GetExtensionById(app_id, true));
+
+  // Set a pref indicating that the user wants to launch in a window.
+  service->extension_prefs()->SetLaunchType(
+      app_id, ExtensionPrefs::LAUNCH_WINDOW);
+
+  std::string app_id_new;
+  LoadAndWaitForLaunch("management/launch_app_tab", &app_id_new);
+  ASSERT_FALSE(HasFatalFailure());
+
+  // If the ID changed, then the pref will not apply to the app.
+  ASSERT_EQ(app_id, app_id_new);
+
+#if defined(OS_MACOSX)
+  // App windows are not yet implemented on mac os.  We should fall back
+  // to a normal tab.
+  ASSERT_EQ(1u, BrowserList::GetBrowserCount(browser()->profile()));
+  ASSERT_EQ(2, browser()->tab_count());
+#else
+  // Find the app's browser.  Opening in a new window will create
+  // a new browser.
+  ASSERT_EQ(2u, BrowserList::GetBrowserCount(browser()->profile()));
+  Browser* app_browser = FindOtherBrowser(browser());
+  ASSERT_EQ(Browser::TYPE_APP, app_browser->type());
+#endif
+}
diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc
index 64a7496..4bc61a8 100644
--- a/chrome/browser/extensions/extension_prefs.cc
+++ b/chrome/browser/extensions/extension_prefs.cc
@@ -611,23 +611,51 @@ ExtensionPrefs::LaunchType ExtensionPrefs::GetLaunchType(
 extension_misc::LaunchContainer ExtensionPrefs::GetLaunchContainer(
     const Extension* extension,
     ExtensionPrefs::LaunchType default_pref_value) {
-  extension_misc::LaunchContainer launch_container =
+  extension_misc::LaunchContainer manifest_launch_container =
       extension->launch_container();
 
-  // Apps with app.launch.container = 'panel' should always
-  // open in a panel.
-  if (launch_container == extension_misc::LAUNCH_PANEL)
-    return extension_misc::LAUNCH_PANEL;
-
-  ExtensionPrefs::LaunchType prefs_launch_type =
-      GetLaunchType(extension->id(), default_pref_value);
+  const extension_misc::LaunchContainer kInvalidLaunchContainer =
+      static_cast<extension_misc::LaunchContainer>(-1);
+
+  extension_misc::LaunchContainer result = kInvalidLaunchContainer;
+
+  if (manifest_launch_container == extension_misc::LAUNCH_PANEL) {
+    // Apps with app.launch.container = 'panel' should always
+    // open in a panel.
+    result = extension_misc::LAUNCH_PANEL;
+
+  } else if (manifest_launch_container == extension_misc::LAUNCH_TAB) {
+    // Look for prefs that indicate the user's choice of launch
+    // container.  The app's menu on the NTP provides a UI to set
+    // this preference.  If no preference is set, |default_pref_value|
+    // is used.
+    ExtensionPrefs::LaunchType prefs_launch_type =
+        GetLaunchType(extension->id(), default_pref_value);
+
+    if (prefs_launch_type == ExtensionPrefs::LAUNCH_WINDOW) {
+      // If the pref is set to launch a window (or no pref is set, and
+      // window opening is the default), make the container a window.
+      result = extension_misc::LAUNCH_WINDOW;
+
+    } else {
+      // All other launch types (tab, pinned, fullscreen) are
+      // implemented as tabs in a window.
+      result = extension_misc::LAUNCH_TAB;
+    }
+  } else {
+    // If a new value for app.launch.container is added, logic
+    // for it should be added here.  extension_misc::LAUNCH_WINDOW
+    // is not present because there is no way to set it in a manifest.
+    NOTREACHED() << manifest_launch_container;
+  }
 
-  // If the user chose to open in a window, then launch in one.
-  if (prefs_launch_type == ExtensionPrefs::LAUNCH_WINDOW)
-    return extension_misc::LAUNCH_WINDOW;
+  // All paths should set |result|.
+  if (result == kInvalidLaunchContainer) {
+    DLOG(FATAL) << "Failed to set a launch container.";
+    result = extension_misc::LAUNCH_TAB;
+  }
 
-  // Otherwise, use the container the extension chose.
-  return launch_container;
+  return result;
 }
 
 void ExtensionPrefs::SetLaunchType(const std::string& extension_id,
diff --git a/chrome/browser/extensions/extension_prefs.h b/chrome/browser/extensions/extension_prefs.h
index 22c0c70..9a8cdac 100644
--- a/chrome/browser/extensions/extension_prefs.h
+++ b/chrome/browser/extensions/extension_prefs.h
@@ -56,7 +56,12 @@ class ExtensionPrefs {
     LAUNCH_PINNED,
     LAUNCH_REGULAR,
     LAUNCH_FULLSCREEN,
-    LAUNCH_WINDOW
+    LAUNCH_WINDOW,
+
+    // Launch an app in the in the way a click on the NTP would,
+    // if no user pref were set.  Update this constant to change
+    // the default for the NTP and chrome.management.launchApp().
+    LAUNCH_DEFAULT = LAUNCH_REGULAR
   };
 
   // Does not assume ownership of |prefs| and |incognito_prefs|.