Commit 300d1e52 authored by stevenjb@google.com's avatar stevenjb@google.com

Fix for regression: Any window of type APP should create APP_POPUP windows.

When code was moved from browser.cc to browser_navigator.cc, the logic for
spawning popup windows was either mis-copied or copied from a different code
path; any browser of TYPE_APP (e.g. TYPE_APP_POPUP) should create popup windows
of TYPE_APP_POPUP.

BUG=chromium-os:11040
TEST=See issue + UI auto tests

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@71876 0039d316-1c4b-4281-b951-d872f2087c98
parent 7d06e052
...@@ -161,9 +161,8 @@ Browser* GetBrowserForDisposition(browser::NavigateParams* params) { ...@@ -161,9 +161,8 @@ Browser* GetBrowserForDisposition(browser::NavigateParams* params) {
// Make a new popup window. Coerce app-style if |params->browser| or the // Make a new popup window. Coerce app-style if |params->browser| or the
// |source| represents an app. // |source| represents an app.
Browser::Type type = Browser::TYPE_POPUP; Browser::Type type = Browser::TYPE_POPUP;
if ((params->browser && params->browser->type() == Browser::TYPE_APP) || if ((params->browser && (params->browser->type() & Browser::TYPE_APP)) ||
(params->source_contents && (params->source_contents && params->source_contents->is_app())) {
params->source_contents->is_app())) {
type = Browser::TYPE_APP_POPUP; type = Browser::TYPE_APP_POPUP;
} }
if (profile) { if (profile) {
......
...@@ -260,6 +260,30 @@ IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, Disposition_NewPopup) { ...@@ -260,6 +260,30 @@ IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, Disposition_NewPopup) {
EXPECT_EQ(1, p.browser->tab_count()); EXPECT_EQ(1, p.browser->tab_count());
} }
// This test verifies that navigating with WindowOpenDisposition = NEW_POPUP
// from a normal popup results in a new Browser with TYPE_POPUP.
IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, Disposition_NewPopupFromPopup) {
// Open a popup.
browser::NavigateParams p1(MakeNavigateParams());
p1.disposition = NEW_POPUP;
browser::Navigate(&p1);
// Open another popup.
browser::NavigateParams p2(MakeNavigateParams(p1.browser));
p2.disposition = NEW_POPUP;
browser::Navigate(&p2);
// Navigate() should have opened a new normal popup window.
EXPECT_NE(p1.browser, p2.browser);
EXPECT_EQ(Browser::TYPE_POPUP, p2.browser->type());
// We should have three windows, the browser() provided by the framework,
// the first popup window, and the second popup window.
EXPECT_EQ(3u, BrowserList::size());
EXPECT_EQ(1, browser()->tab_count());
EXPECT_EQ(1, p1.browser->tab_count());
EXPECT_EQ(1, p2.browser->tab_count());
}
// This test verifies that navigating with WindowOpenDisposition = NEW_POPUP // This test verifies that navigating with WindowOpenDisposition = NEW_POPUP
// from an app frame results in a new Browser with TYPE_APP_POPUP. // from an app frame results in a new Browser with TYPE_APP_POPUP.
IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest,
...@@ -283,6 +307,35 @@ IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, ...@@ -283,6 +307,35 @@ IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest,
EXPECT_EQ(1, p.browser->tab_count()); EXPECT_EQ(1, p.browser->tab_count());
} }
// This test verifies that navigating with WindowOpenDisposition = NEW_POPUP
// from an app popup results in a new Browser also of TYPE_APP_POPUP.
IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest,
Disposition_NewPopupFromAppPopup) {
Browser* app_browser = CreateEmptyBrowserForType(Browser::TYPE_APP,
browser()->profile());
// Open an app popup.
browser::NavigateParams p1(MakeNavigateParams(app_browser));
p1.disposition = NEW_POPUP;
browser::Navigate(&p1);
// Now open another app popup.
browser::NavigateParams p2(MakeNavigateParams(p1.browser));
p2.disposition = NEW_POPUP;
browser::Navigate(&p2);
// Navigate() should have opened a new popup app window.
EXPECT_NE(browser(), p1.browser);
EXPECT_NE(p1.browser, p2.browser);
EXPECT_EQ(Browser::TYPE_APP_POPUP, p2.browser->type());
// We should now have four windows, the app window, the first app popup,
// the second app popup, and the original browser() provided by the framework.
EXPECT_EQ(4u, BrowserList::size());
EXPECT_EQ(1, browser()->tab_count());
EXPECT_EQ(1, app_browser->tab_count());
EXPECT_EQ(1, p1.browser->tab_count());
EXPECT_EQ(1, p2.browser->tab_count());
}
// This test verifies that navigating with WindowOpenDisposition = NEW_POPUP // This test verifies that navigating with WindowOpenDisposition = NEW_POPUP
// from an extension app tab results in a new Browser with TYPE_APP_POPUP. // from an extension app tab results in a new Browser with TYPE_APP_POPUP.
IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment