diff options
Diffstat (limited to 'chrome')
-rw-r--r-- | chrome/browser/browser.cc | 68 | ||||
-rw-r--r-- | chrome/browser/browser.h | 17 | ||||
-rw-r--r-- | chrome/browser/browser_browsertest.cc | 167 |
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 |