Make app panels use popup type. Nuke refocus code and tests.

BUG=55943
TEST=Manual.

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

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

3 files changed, 15 insertions, 237 deletions

diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index d596ec4c..dea3cb9 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -338,8 +338,12 @@ Browser* Browser::CreateForApp(const std::string& app_name,
                                bool is_panel) {
   Browser::Type type = TYPE_APP;
 
-  if (is_panel)
-    type = TYPE_APP_PANEL;
+  if (is_panel) {
+    // TYPE_APP_PANEL is the logical choice.  However, the panel UI
+    // is not fully implemented.  See crbug/55943.
+    type = TYPE_APP_POPUP;
+  }
+
   else if (extension)
     type = TYPE_EXTENSION_APP;
 
@@ -468,43 +472,6 @@ void Browser::OpenURLOffTheRecord(Profile* profile, const GURL& url) {
 }
 
 // static
-Browser* Browser::FindAppWindowOrPanel(Profile* profile,
-                                       Extension* extension_app) {
-  // Test the focused browser first.
-  Browser* browser = BrowserList::GetLastActive();
-  if (browser && BrowserHostsExtensionApp(browser, profile, extension_app))
-    return browser;
-
-  BrowserList::const_iterator browser_all;
-  for (browser_all = BrowserList::begin();
-       browser_all != BrowserList::end();
-       ++browser_all) {
-    if (BrowserHostsExtensionApp(*browser_all, profile, extension_app))
-      return *browser_all;
-  }
-  return NULL;
-}
-
-// static
-TabContents* Browser::FindAppTab(Browser* browser, Extension* extension_app) {
-  if (browser->type() != Browser::TYPE_NORMAL)
-    return NULL;
-
-  for (int tab_idx = 0; tab_idx < browser->tab_count(); ++tab_idx) {
-    TabContents* tab_contents = browser->GetTabContentsAt(tab_idx);
-    if (!tab_contents)
-      continue;
-
-    if (tab_contents->extension_app() != extension_app)
-      continue;
-
-    return tab_contents;
-  }
-
-  return NULL;
-}
-
-// static
 // TODO(erikkay): There are multiple reasons why this could fail.  Should
 // this function return an error reason as well so that callers can show
 // reasonable errors?
@@ -530,34 +497,15 @@ TabContents* Browser::OpenApplication(
     extension_misc::LaunchContainer container) {
   TabContents* tab = NULL;
 
-  // If the app is loaded in an existing window or panel, focus it.
-  Browser* browser = FindAppWindowOrPanel(profile, extension);
-  if (browser) {
-    browser->window()->Show();
-    return browser->GetSelectedTabContents();
-  }
-
-  // If an app is loaded in an app tab in the focused browser, select it.
-  browser = BrowserList::GetLastActive();
-  if (browser && browser->profile() == profile) {
-    tab = Browser::FindAppTab(browser, extension);
-    if (tab) {
-      int tab_idx = browser->tabstrip_model()->GetIndexOfTabContents(tab);
-      DCHECK(tab_idx != TabStripModel::kNoTab);
-      browser->SelectTabContentsAt(tab_idx, false);
-      return tab;
-    }
-  }
-
   // The app is not yet open.  Load it.
   switch (container) {
     case extension_misc::LAUNCH_WINDOW:
     case extension_misc::LAUNCH_PANEL:
       tab = Browser::OpenApplicationWindow(profile, extension, container,
-                                           GURL(), &browser);
+                                           GURL(), NULL);
       break;
     case extension_misc::LAUNCH_TAB: {
-      tab = Browser::OpenApplicationTab(profile, extension, &browser);
+      tab = Browser::OpenApplicationTab(profile, extension, NULL);
       break;
     }
     default:
diff --git a/chrome/browser/browser.h b/chrome/browser/browser.h
index 0982959..9b4bdc3 100644
--- a/chrome/browser/browser.h
+++ b/chrome/browser/browser.h
@@ -71,12 +71,16 @@ class Browser : public TabStripModelDelegate,
     TYPE_APP = 4,
     // The new-style app created by installing a crx. This kinda needs to be
     // separate because we require larger icons and an application name that
-    // are found in the crx. If we ever decide to create this kind of app using
-    // some other system (eg some web standard), maybe we should generalize this
-    // name to TYPE_MULTITAB or something.
+    // are found in the crx. If we ever decide to create this kind of app
+    // using some other system (eg some web standard), maybe we should
+    // generalize this name to TYPE_MULTITAB or something.
     TYPE_EXTENSION_APP = 8,
     TYPE_APP_POPUP = TYPE_APP | TYPE_POPUP,
     TYPE_DEVTOOLS = TYPE_APP | 16,
+
+    // TODO(skerner): crbug/56776: Until the panel UI is complete on all
+    // platforms, apps that set app.launch.container = "panel" have type
+    // APP_POPUP.
     TYPE_APP_PANEL = TYPE_APP | 32,
     TYPE_ANY = TYPE_NORMAL |
                TYPE_POPUP |
@@ -212,13 +216,6 @@ class Browser : public TabStripModelDelegate,
   // |profile|, that session is re-used.
   static void OpenURLOffTheRecord(Profile* profile, const GURL& url);
 
-  // Finds an app tab running a given app in a browser.
-  static TabContents* FindAppTab(Browser* browser, Extension* extension_app);
-
-  // Finds a browser running an app as an app window or panel.
-  static Browser* FindAppWindowOrPanel(Profile* profile,
-                                       Extension* extension_app);
-
   // Open an application specified by |app_id| in the appropriate launch
   // container.  Returns NULL if the app_id is invalid or if ExtensionsService
   // isn't ready/available.
diff --git a/chrome/browser/browser_browsertest.cc b/chrome/browser/browser_browsertest.cc
index 2ccc444..fa60fef 100644
--- a/chrome/browser/browser_browsertest.cc
+++ b/chrome/browser/browser_browsertest.cc
@@ -608,173 +608,6 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, RestorePinnedTabs) {
 }
 #endif  // !defined(OS_CHROMEOS)
 
-class BrowserAppRefocusTest : public ExtensionBrowserTest {
- public:
-  BrowserAppRefocusTest()
-      : extension_app_(NULL),
-        profile_(NULL) {}
-
- protected:
-  // Common setup for all tests.  Can't use SetUpInProcessBrowserTestFixture
-  // because starting the http server crashes if called from that function.
-  // The IO thread is not set up at that point.
-  virtual void SetUpExtensionApp() {
-    // The web URL of the example app we load has a host of
-    // www.example.com .
-    ASSERT_TRUE(test_server()->Start());
-    host_resolver()->AddRule("www.example.com", "127.0.0.1");
-
-    profile_ = browser()->profile();
-    ASSERT_TRUE(profile_);
-
-    ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII("app/")));
-
-    // Save a pointer to the loaded extension in |extension_app_|.
-    const ExtensionList* extensions =
-        profile_->GetExtensionsService()->extensions();
-
-    for (size_t i = 0; i < extensions->size(); ++i) {
-      if ((*extensions)[i]->name() == "App Test")
-        extension_app_ =(*extensions)[i];
-    }
-    ASSERT_TRUE(extension_app_) << "App Test extension not loaded.";
-  }
-
-  // Given a tab, wait for navigation in the tab, then test that it is
-  // selected.  If this function returns false, an error was logged.
-  bool WaitForTab(TabContents* tab) WARN_UNUSED_RESULT {
-    if (!tab) {
-      LOG(ERROR) << "|tab| should not be NULL.";
-      return false;
-    }
-    ui_test_utils::WaitForNavigation(&(tab->controller()));
-    if (tab != browser()->GetSelectedTabContents()) {
-      LOG(ERROR) << "Tab was not selected.";
-      return false;
-    }
-    return true;
-  }
-
-  Extension* extension_app_;
-  Profile* profile_;
-};
-
-#if (defined(OS_LINUX) && defined(TOOLKIT_VIEWS)) || defined(OS_WIN)
-
-// Tests fail on Chrome OS:  http://crbug.com/43061
-#define MAYBE_OpenTab DISABLED_OpenTab
-#define MAYBE_OpenPanel DISABLED_OpenPanel
-#define MAYBE_PanelBeforeTab DISABLED_PanelBeforeTab
-
-#else
-
-#define MAYBE_OpenTab OpenTab
-#define MAYBE_OpenPanel OpenPanel
-#define MAYBE_PanelBeforeTab PanelBeforeTab
-
-#endif
-
-// Test that launching an app refocuses a tab already hosting the app.
-IN_PROC_BROWSER_TEST_F(BrowserAppRefocusTest, MAYBE_OpenTab) {
-  SetUpExtensionApp();
-
-  ASSERT_EQ(1, browser()->tab_count());
-  ASSERT_EQ(NULL, Browser::FindAppTab(browser(), extension_app_));
-
-  // Open a tab with the app.
-  TabContents* tab = Browser::OpenApplicationTab(profile_,
-                                                 extension_app_,
-                                                 NULL);
-  ASSERT_TRUE(WaitForTab(tab));
-  ASSERT_EQ(2, browser()->tab_count());
-
-  int app_tab_index = browser()->selected_index();
-  ASSERT_EQ(0, app_tab_index) << "App tab should be the left most tab.";
-
-  // Open the same app.  The existing tab should stay focused.
-  tab = Browser::OpenApplication(profile_, extension_app_->id());
-
-  // No need to wait for navigation, because the tab already exists,
-  // and no navigation should take place.
-  ASSERT_TRUE(tab != NULL);
-  ASSERT_EQ(3, browser()->tab_count());
-  ASSERT_EQ(app_tab_index + 1, browser()->selected_index());
-
-  // Open a second browser window, and open the app in a tab.
-  Browser* second_browser = CreateBrowser(profile_);
-  second_browser->window()->Show();
-
-  Browser::OpenApplication(profile_, extension_app_,
-                           extension_misc::LAUNCH_TAB);
-  ASSERT_EQ(2, second_browser->tab_count()) <<
-      "Expect the app to open in a tab under |second_browser|.";
-  int second_app_tab_index = second_browser->selected_index();
-  ASSERT_EQ(0, second_app_tab_index) <<
-      "Second app tab should be the left most tab.";
-}
-
-// Test that launching an app refocuses a panel running the app.
-IN_PROC_BROWSER_TEST_F(BrowserAppRefocusTest, MAYBE_OpenPanel) {
-  SetUpExtensionApp();
-
-  ASSERT_EQ(1, browser()->tab_count());
-  ASSERT_EQ(NULL, Browser::FindAppWindowOrPanel(profile_, extension_app_));
-
-  // Open the app in a panel.
-  Browser::OpenApplicationWindow(profile_, extension_app_,
-                                 extension_misc::LAUNCH_PANEL, GURL(), NULL);
-  Browser* app_panel = BrowserList::GetLastActive();
-  ASSERT_TRUE(app_panel);
-  ASSERT_NE(app_panel, browser()) << "New browser should have opened.";
-  ASSERT_EQ(app_panel, BrowserList::GetLastActive());
-  ASSERT_EQ(app_panel,
-            Browser::FindAppWindowOrPanel(profile_, extension_app_));
-
-  // Focus the initial browser.
-  browser()->window()->Show();
-  ASSERT_EQ(browser(), BrowserList::GetLastActive());
-
-  // Open the app.
-  Browser::OpenApplication(profile_, extension_app_->id());
-
-  // Focus should move to the panel.
-  ASSERT_EQ(app_panel, BrowserList::GetLastActive());
-  ASSERT_EQ(app_panel,
-            Browser::FindAppWindowOrPanel(profile_, extension_app_));
-
-  // No new tab should have been created in the initial browser.
-  ASSERT_EQ(1, browser()->tab_count());
-}
-
-// Test that if an app is opened while running in a panel and a tab,
-// the panel is focused.
-IN_PROC_BROWSER_TEST_F(BrowserAppRefocusTest, MAYBE_PanelBeforeTab) {
-  SetUpExtensionApp();
-
-  ASSERT_EQ(1, browser()->tab_count());
-
-  // Open a tab with the app.
-  TabContents* tab = Browser::OpenApplicationTab(profile_, extension_app_,
-                                                 NULL);
-  ASSERT_TRUE(WaitForTab(tab));
-  ASSERT_EQ(2, browser()->tab_count());
-  int app_tab_index = browser()->selected_index();
-  ASSERT_EQ(0, app_tab_index) << "App tab should be the left most tab.";
-
-  // Open a panel with the app.
-  Browser::OpenApplicationWindow(profile_, extension_app_,
-                                 extension_misc::LAUNCH_PANEL, GURL(), NULL);
-  Browser* app_panel = BrowserList::GetLastActive();
-  ASSERT_TRUE(app_panel);
-  ASSERT_NE(app_panel, browser()) << "New browser should have opened.";
-
-  // Focus the initial browser.
-  browser()->window()->Show();
-
-  // Open the app.  Focus should move to the panel.
-  Browser::OpenApplication(profile_, extension_app_->id());
-  ASSERT_EQ(app_panel, BrowserList::GetLastActive());
-}
 
 // TODO(ben): this test was never enabled. It has bit-rotted since being added.
 // It originally lived in browser_unittest.cc, but has been moved here to make