Commit 19da16a9 authored by creis@chromium.org's avatar creis@chromium.org

Fix flakiness by removing GetLastActive from extension browser tests.

BUG=108853
TEST=AppApiTest.* and ExtensionBrowserTest.* no longer flaky


Review URL: https://chromiumcodereview.appspot.com/10412047

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@138503 0039d316-1c4b-4281-b951-d872f2087c98
parent 3e31a0ad
...@@ -2,7 +2,6 @@ ...@@ -2,7 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
#include "base/utf_string_conversions.h"
#include "chrome/browser/extensions/extension_apitest.h" #include "chrome/browser/extensions/extension_apitest.h"
#include "chrome/browser/extensions/extension_host.h" #include "chrome/browser/extensions/extension_host.h"
#include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/extension_service.h"
...@@ -32,50 +31,6 @@ using content::RenderViewHost; ...@@ -32,50 +31,6 @@ using content::RenderViewHost;
using content::WebContents; using content::WebContents;
using extensions::Extension; using extensions::Extension;
// Simulates a page calling window.open on an URL, and waits for the navigation.
static void WindowOpenHelper(Browser* browser,
RenderViewHost* opener_host,
const GURL& url,
bool newtab_process_should_equal_opener) {
ui_test_utils::WindowedNotificationObserver observer(
content::NOTIFICATION_LOAD_STOP,
content::NotificationService::AllSources());
ASSERT_TRUE(ui_test_utils::ExecuteJavaScript(
opener_host, L"", L"window.open('" + UTF8ToWide(url.spec()) + L"');"));
// The above window.open call is not user-initiated, it will create
// a popup window instead of a new tab in current window.
// Now the active tab in last active window should be the new tab.
Browser* last_active_browser = BrowserList::GetLastActive();
ASSERT_TRUE(last_active_browser);
WebContents* newtab = last_active_browser->GetSelectedWebContents();
ASSERT_TRUE(newtab);
observer.Wait();
EXPECT_EQ(url, newtab->GetController().GetLastCommittedEntry()->GetURL());
if (newtab_process_should_equal_opener)
EXPECT_EQ(opener_host->GetProcess(), newtab->GetRenderProcessHost());
else
EXPECT_NE(opener_host->GetProcess(), newtab->GetRenderProcessHost());
}
// Simulates a page navigating itself to an URL, and waits for the navigation.
static void NavigateTabHelper(WebContents* contents, const GURL& url) {
bool result = false;
ui_test_utils::WindowedNotificationObserver observer(
content::NOTIFICATION_LOAD_STOP,
content::NotificationService::AllSources());
ASSERT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
contents->GetRenderViewHost(), L"",
L"window.addEventListener('unload', function() {"
L" window.domAutomationController.send(true);"
L"}, false);"
L"window.location = '" + UTF8ToWide(url.spec()) + L"';",
&result));
ASSERT_TRUE(result);
observer.Wait();
EXPECT_EQ(url, contents->GetController().GetLastCommittedEntry()->GetURL());
}
class AppApiTest : public ExtensionApiTest { class AppApiTest : public ExtensionApiTest {
protected: protected:
// Gets the base URL for files for a specific test, making sure that it uses // Gets the base URL for files for a specific test, making sure that it uses
...@@ -146,30 +101,23 @@ class AppApiTest : public ExtensionApiTest { ...@@ -146,30 +101,23 @@ class AppApiTest : public ExtensionApiTest {
// process, since they do not have the background permission. (Thus, we // process, since they do not have the background permission. (Thus, we
// want to separate them to improve responsiveness.) // want to separate them to improve responsiveness.)
ASSERT_EQ(3, browser()->tab_count()); ASSERT_EQ(3, browser()->tab_count());
RenderViewHost* host1 = browser()->GetWebContentsAt(1)->GetRenderViewHost(); WebContents* tab1 = browser()->GetWebContentsAt(1);
RenderViewHost* host2 = browser()->GetWebContentsAt(2)->GetRenderViewHost(); WebContents* tab2 = browser()->GetWebContentsAt(2);
EXPECT_NE(host1->GetProcess(), host2->GetProcess()); EXPECT_NE(tab1->GetRenderProcessHost(), tab2->GetRenderProcessHost());
// Opening tabs with window.open should keep the page in the opener's // Opening tabs with window.open should keep the page in the opener's
// process. // process.
ASSERT_EQ(1u, browser::GetBrowserCount(browser()->profile())); ASSERT_EQ(1u, browser::GetBrowserCount(browser()->profile()));
WindowOpenHelper(browser(), host1, OpenWindow(tab1, base_url.Resolve("path1/empty.html"), true, NULL);
base_url.Resolve("path1/empty.html"), true);
LOG(INFO) << "WindowOpenHelper 1."; LOG(INFO) << "WindowOpenHelper 1.";
WindowOpenHelper(browser(), host2, OpenWindow(tab2, base_url.Resolve("path2/empty.html"), true, NULL);
base_url.Resolve("path2/empty.html"), true);
LOG(INFO) << "End of test."; LOG(INFO) << "End of test.";
} }
}; };
// Tests that hosted apps with the background permission get a process-per-app // Tests that hosted apps with the background permission get a process-per-app
// model, since all pages need to be able to script the background page. // model, since all pages need to be able to script the background page.
#if defined(OS_WIN) IN_PROC_BROWSER_TEST_F(AppApiTest, AppProcess) {
#define MAYBE_AppProcess FLAKY_AppProcess
#else
#define MAYBE_AppProcess AppProcess
#endif
IN_PROC_BROWSER_TEST_F(AppApiTest, MAYBE_AppProcess) {
LOG(INFO) << "Start of test."; LOG(INFO) << "Start of test.";
extensions::ProcessMap* process_map = extensions::ProcessMap* process_map =
...@@ -221,50 +169,47 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, MAYBE_AppProcess) { ...@@ -221,50 +169,47 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, MAYBE_AppProcess) {
// permission, all of its instances are in the same process. Thus two tabs // permission, all of its instances are in the same process. Thus two tabs
// should be part of the extension app and grouped in the same process. // should be part of the extension app and grouped in the same process.
ASSERT_EQ(4, browser()->tab_count()); ASSERT_EQ(4, browser()->tab_count());
RenderViewHost* host = browser()->GetWebContentsAt(1)->GetRenderViewHost(); WebContents* tab = browser()->GetWebContentsAt(1);
EXPECT_EQ(host->GetProcess(), EXPECT_EQ(tab->GetRenderProcessHost(),
browser()->GetWebContentsAt(2)->GetRenderProcessHost()); browser()->GetWebContentsAt(2)->GetRenderProcessHost());
EXPECT_NE(host->GetProcess(), EXPECT_NE(tab->GetRenderProcessHost(),
browser()->GetWebContentsAt(3)->GetRenderProcessHost()); browser()->GetWebContentsAt(3)->GetRenderProcessHost());
// Now let's do the same using window.open. The same should happen. // Now let's do the same using window.open. The same should happen.
ASSERT_EQ(1u, browser::GetBrowserCount(browser()->profile())); ASSERT_EQ(1u, browser::GetBrowserCount(browser()->profile()));
WindowOpenHelper(browser(), host, OpenWindow(tab, base_url.Resolve("path1/empty.html"), true, NULL);
base_url.Resolve("path1/empty.html"), true);
LOG(INFO) << "WindowOpenHelper 1."; LOG(INFO) << "WindowOpenHelper 1.";
WindowOpenHelper(browser(), host, OpenWindow(tab, base_url.Resolve("path2/empty.html"), true, NULL);
base_url.Resolve("path2/empty.html"), true);
LOG(INFO) << "WindowOpenHelper 2."; LOG(INFO) << "WindowOpenHelper 2.";
// TODO(creis): This should open in a new process (i.e., false for the last // TODO(creis): This should open in a new process (i.e., false for the last
// argument), but we temporarily avoid swapping processes away from an app // argument), but we temporarily avoid swapping processes away from an app
// until we're able to support cross-process postMessage calls. // until we're able to support cross-process postMessage calls.
// See crbug.com/59285. // See crbug.com/59285.
WindowOpenHelper(browser(), host, OpenWindow(tab, base_url.Resolve("path3/empty.html"), true, NULL);
base_url.Resolve("path3/empty.html"), true);
LOG(INFO) << "WindowOpenHelper 3."; LOG(INFO) << "WindowOpenHelper 3.";
// Now let's have these pages navigate, into or out of the extension web // Now let's have these pages navigate, into or out of the extension web
// extent. They should switch processes. // extent. They should switch processes.
const GURL& app_url(base_url.Resolve("path1/empty.html")); const GURL& app_url(base_url.Resolve("path1/empty.html"));
const GURL& non_app_url(base_url.Resolve("path3/empty.html")); const GURL& non_app_url(base_url.Resolve("path3/empty.html"));
NavigateTabHelper(browser()->GetWebContentsAt(2), non_app_url); NavigateInRenderer(browser()->GetWebContentsAt(2), non_app_url);
LOG(INFO) << "NavigateTabHelper 1."; LOG(INFO) << "NavigateTabHelper 1.";
NavigateTabHelper(browser()->GetWebContentsAt(3), app_url); NavigateInRenderer(browser()->GetWebContentsAt(3), app_url);
LOG(INFO) << "NavigateTabHelper 2."; LOG(INFO) << "NavigateTabHelper 2.";
// TODO(creis): This should swap out of the app's process (i.e., EXPECT_NE), // TODO(creis): This should swap out of the app's process (i.e., EXPECT_NE),
// but we temporarily avoid swapping away from an app in case the window // but we temporarily avoid swapping away from an app in case the window
// tries to send a postMessage to the app. See crbug.com/59285. // tries to send a postMessage to the app. See crbug.com/59285.
EXPECT_EQ(host->GetProcess(), EXPECT_EQ(tab->GetRenderProcessHost(),
browser()->GetWebContentsAt(2)->GetRenderProcessHost()); browser()->GetWebContentsAt(2)->GetRenderProcessHost());
EXPECT_EQ(host->GetProcess(), EXPECT_EQ(tab->GetRenderProcessHost(),
browser()->GetWebContentsAt(3)->GetRenderProcessHost()); browser()->GetWebContentsAt(3)->GetRenderProcessHost());
// If one of the popup tabs navigates back to the app, window.opener should // If one of the popup tabs navigates back to the app, window.opener should
// be valid. // be valid.
NavigateTabHelper(browser()->GetWebContentsAt(6), app_url); NavigateInRenderer(browser()->GetWebContentsAt(6), app_url);
LOG(INFO) << "NavigateTabHelper 3."; LOG(INFO) << "NavigateTabHelper 3.";
EXPECT_EQ(host->GetProcess(), EXPECT_EQ(tab->GetRenderProcessHost(),
browser()->GetWebContentsAt(6)->GetRenderProcessHost()); browser()->GetWebContentsAt(6)->GetRenderProcessHost());
bool windowOpenerValid = false; bool windowOpenerValid = false;
ASSERT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool( ASSERT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
...@@ -278,35 +223,20 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, MAYBE_AppProcess) { ...@@ -278,35 +223,20 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, MAYBE_AppProcess) {
// Test that hosted apps without the background permission use a process per app // Test that hosted apps without the background permission use a process per app
// instance model, such that separate instances are in separate processes. // instance model, such that separate instances are in separate processes.
#if defined(OS_WIN) IN_PROC_BROWSER_TEST_F(AppApiTest, AppProcessInstances) {
#define MAYBE_AppProcessInstances FLAKY_AppProcessInstances
#else
#define MAYBE_AppProcessInstances AppProcessInstances
#endif
IN_PROC_BROWSER_TEST_F(AppApiTest, MAYBE_AppProcessInstances) {
TestAppInstancesHelper("app_process_instances"); TestAppInstancesHelper("app_process_instances");
} }
// Test that hosted apps with the background permission but that set // Test that hosted apps with the background permission but that set
// allow_js_access to false also use a process per app instance model. // allow_js_access to false also use a process per app instance model.
// Separate instances should be in separate processes. // Separate instances should be in separate processes.
#if defined(OS_WIN) IN_PROC_BROWSER_TEST_F(AppApiTest, AppProcessBackgroundInstances) {
#define MAYBE_AppProcessBackgroundInstances FLAKY_AppProcessBackgroundInstances
#else
#define MAYBE_AppProcessBackgroundInstances AppProcessBackgroundInstances
#endif
IN_PROC_BROWSER_TEST_F(AppApiTest, MAYBE_AppProcessBackgroundInstances) {
TestAppInstancesHelper("app_process_background_instances"); TestAppInstancesHelper("app_process_background_instances");
} }
// Tests that bookmark apps do not use the app process model and are treated // Tests that bookmark apps do not use the app process model and are treated
// like normal web pages instead. http://crbug.com/104636. // like normal web pages instead. http://crbug.com/104636.
#if defined(OS_WIN) IN_PROC_BROWSER_TEST_F(AppApiTest, BookmarkAppGetsNormalProcess) {
#define MAYBE_BookmarkAppGetsNormalProcess FLAKY_BookmarkAppGetsNormalProcess
#else
#define MAYBE_BookmarkAppGetsNormalProcess BookmarkAppGetsNormalProcess
#endif
IN_PROC_BROWSER_TEST_F(AppApiTest, MAYBE_BookmarkAppGetsNormalProcess) {
ExtensionService* service = browser()->profile()->GetExtensionService(); ExtensionService* service = browser()->profile()->GetExtensionService();
extensions::ProcessMap* process_map = service->process_map(); extensions::ProcessMap* process_map = service->process_map();
...@@ -350,26 +280,24 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, MAYBE_BookmarkAppGetsNormalProcess) { ...@@ -350,26 +280,24 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, MAYBE_BookmarkAppGetsNormalProcess) {
// tab, we now have 3 tabs. Because normal pages use the // tab, we now have 3 tabs. Because normal pages use the
// process-per-site-instance model, each should be in its own process. // process-per-site-instance model, each should be in its own process.
ASSERT_EQ(3, browser()->tab_count()); ASSERT_EQ(3, browser()->tab_count());
RenderViewHost* host = browser()->GetWebContentsAt(1)->GetRenderViewHost(); WebContents* tab = browser()->GetWebContentsAt(1);
EXPECT_NE(host->GetProcess(), EXPECT_NE(tab->GetRenderProcessHost(),
browser()->GetWebContentsAt(2)->GetRenderProcessHost()); browser()->GetWebContentsAt(2)->GetRenderProcessHost());
// Now let's do the same using window.open. The same should happen. // Now let's do the same using window.open. The same should happen.
ASSERT_EQ(1u, browser::GetBrowserCount(browser()->profile())); ASSERT_EQ(1u, browser::GetBrowserCount(browser()->profile()));
WindowOpenHelper(browser(), host, OpenWindow(tab, base_url.Resolve("path1/empty.html"), true, NULL);
base_url.Resolve("path1/empty.html"), true); OpenWindow(tab, base_url.Resolve("path2/empty.html"), true, NULL);
WindowOpenHelper(browser(), host,
base_url.Resolve("path2/empty.html"), true);
// Now let's have a tab navigate out of and back into the app's web // Now let's have a tab navigate out of and back into the app's web
// extent. Neither navigation should switch processes. // extent. Neither navigation should switch processes.
const GURL& app_url(base_url.Resolve("path1/empty.html")); const GURL& app_url(base_url.Resolve("path1/empty.html"));
const GURL& non_app_url(base_url.Resolve("path3/empty.html")); const GURL& non_app_url(base_url.Resolve("path3/empty.html"));
RenderViewHost* host2 = browser()->GetWebContentsAt(2)->GetRenderViewHost(); RenderViewHost* host2 = browser()->GetWebContentsAt(2)->GetRenderViewHost();
NavigateTabHelper(browser()->GetWebContentsAt(2), non_app_url); NavigateInRenderer(browser()->GetWebContentsAt(2), non_app_url);
EXPECT_EQ(host2->GetProcess(), EXPECT_EQ(host2->GetProcess(),
browser()->GetWebContentsAt(2)->GetRenderProcessHost()); browser()->GetWebContentsAt(2)->GetRenderProcessHost());
NavigateTabHelper(browser()->GetWebContentsAt(2), app_url); NavigateInRenderer(browser()->GetWebContentsAt(2), app_url);
EXPECT_EQ(host2->GetProcess(), EXPECT_EQ(host2->GetProcess(),
browser()->GetWebContentsAt(2)->GetRenderProcessHost()); browser()->GetWebContentsAt(2)->GetRenderProcessHost());
} }
...@@ -520,12 +448,7 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, MAYBE_ReloadIntoAppProcess) { ...@@ -520,12 +448,7 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, MAYBE_ReloadIntoAppProcess) {
// link from that iframe to a new window to a URL in the app's extent (path1/ // link from that iframe to a new window to a URL in the app's extent (path1/
// empty.html) results in the new window being in an app process. See // empty.html) results in the new window being in an app process. See
// http://crbug.com/89272 for more details. // http://crbug.com/89272 for more details.
#if defined(OS_WIN) IN_PROC_BROWSER_TEST_F(AppApiTest, OpenAppFromIframe) {
#define MAYBE_OpenAppFromIframe FLAKY_OpenAppFromIframe
#else
#define MAYBE_OpenAppFromIframe OpenAppFromIframe
#endif
IN_PROC_BROWSER_TEST_F(AppApiTest, MAYBE_OpenAppFromIframe) {
extensions::ProcessMap* process_map = extensions::ProcessMap* process_map =
browser()->profile()->GetExtensionService()->process_map(); browser()->profile()->GetExtensionService()->process_map();
...@@ -538,37 +461,20 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, MAYBE_OpenAppFromIframe) { ...@@ -538,37 +461,20 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, MAYBE_OpenAppFromIframe) {
const Extension* app = const Extension* app =
LoadExtension(test_data_dir_.AppendASCII("app_process")); LoadExtension(test_data_dir_.AppendASCII("app_process"));
ASSERT_TRUE(app); ASSERT_TRUE(app);
ui_test_utils::NavigateToURLWithDisposition(
browser(), ui_test_utils::WindowedNotificationObserver popup_observer(
base_url.Resolve("path3/container.html"), content::NOTIFICATION_RENDER_VIEW_HOST_CREATED,
CURRENT_TAB, content::NotificationService::AllSources());
ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION | ui_test_utils::NavigateToURL(browser(),
ui_test_utils::BROWSER_TEST_WAIT_FOR_BROWSER); base_url.Resolve("path3/container.html"));
EXPECT_FALSE(process_map->Contains( EXPECT_FALSE(process_map->Contains(
browser()->GetWebContentsAt(0)->GetRenderProcessHost()->GetID())); browser()->GetWebContentsAt(0)->GetRenderProcessHost()->GetID()));
popup_observer.Wait();
// Wait for popup window to appear.
GURL app_url = base_url.Resolve("path1/empty.html");
Browser* last_active_browser = BrowserList::GetLastActive();
EXPECT_TRUE(last_active_browser);
ASSERT_NE(browser(), last_active_browser);
WebContents* newtab = last_active_browser->GetSelectedWebContents();
EXPECT_TRUE(newtab);
if (!newtab->GetController().GetLastCommittedEntry() ||
newtab->GetController().GetLastCommittedEntry()->GetURL() != app_url) {
// TODO(gbillock): This still looks racy. Need to make a custom
// observer to intercept new window creation and then look for
// NAV_ENTRY_COMMITTED on the new tab there.
ui_test_utils::WindowedNotificationObserver observer(
content::NOTIFICATION_NAV_ENTRY_COMMITTED,
content::Source<NavigationController>(&(newtab->GetController())));
observer.Wait();
}
// Popup window should be in the app's process. // Popup window should be in the app's process.
EXPECT_TRUE(process_map->Contains( RenderViewHost* popup_host =
last_active_browser->GetWebContentsAt(0)->GetRenderProcessHost()-> content::Source<RenderViewHost>(popup_observer.source()).ptr();
GetID())); EXPECT_TRUE(process_map->Contains(popup_host->GetProcess()->GetID()));
} }
// Tests that if an extension launches an app via chrome.tabs.create with an URL // Tests that if an extension launches an app via chrome.tabs.create with an URL
...@@ -620,12 +526,7 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, OpenAppFromExtension) { ...@@ -620,12 +526,7 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, OpenAppFromExtension) {
// This is in contrast to OpenAppFromIframe, since here the popup will not be // This is in contrast to OpenAppFromIframe, since here the popup will not be
// missing special permissions and should be scriptable from the iframe. // missing special permissions and should be scriptable from the iframe.
// See http://crbug.com/92669 for more details. // See http://crbug.com/92669 for more details.
#if defined(OS_WIN) IN_PROC_BROWSER_TEST_F(AppApiTest, OpenWebPopupFromWebIframe) {
#define MAYBE_OpenWebPopupFromWebIframe FLAKY_OpenWebPopupFromWebIframe
#else
#define MAYBE_OpenWebPopupFromWebIframe OpenWebPopupFromWebIframe
#endif
IN_PROC_BROWSER_TEST_F(AppApiTest, MAYBE_OpenWebPopupFromWebIframe) {
extensions::ProcessMap* process_map = extensions::ProcessMap* process_map =
browser()->profile()->GetExtensionService()->process_map(); browser()->profile()->GetExtensionService()->process_map();
...@@ -638,40 +539,23 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, MAYBE_OpenWebPopupFromWebIframe) { ...@@ -638,40 +539,23 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, MAYBE_OpenWebPopupFromWebIframe) {
const Extension* app = const Extension* app =
LoadExtension(test_data_dir_.AppendASCII("app_process")); LoadExtension(test_data_dir_.AppendASCII("app_process"));
ASSERT_TRUE(app); ASSERT_TRUE(app);
ui_test_utils::WindowedNotificationObserver observer(
content::NOTIFICATION_LOAD_STOP, ui_test_utils::WindowedNotificationObserver popup_observer(
content::NOTIFICATION_RENDER_VIEW_HOST_CREATED,
content::NotificationService::AllSources()); content::NotificationService::AllSources());
ui_test_utils::NavigateToURLWithDisposition( ui_test_utils::NavigateToURL(browser(),
browser(), base_url.Resolve("path1/container.html"));
base_url.Resolve("path1/container.html"),
CURRENT_TAB,
ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION |
ui_test_utils::BROWSER_TEST_WAIT_FOR_BROWSER);
content::RenderProcessHost* process = content::RenderProcessHost* process =
browser()->GetWebContentsAt(0)->GetRenderProcessHost(); browser()->GetWebContentsAt(0)->GetRenderProcessHost();
EXPECT_TRUE(process_map->Contains(process->GetID())); EXPECT_TRUE(process_map->Contains(process->GetID()));
// Wait for popup window to appear. The new Browser may not have been // Wait for popup window to appear.
// added with SetLastActive, in which case we need to show it first. popup_observer.Wait();
// This is necessary for popup windows without a cross-site transition.
if (browser() == BrowserList::GetLastActive()) {
// Grab the second window and show it.
ASSERT_TRUE(BrowserList::size() == 2);
Browser* popup_browser = *(++BrowserList::begin());
popup_browser->window()->Show();
}
Browser* last_active_browser = BrowserList::GetLastActive();
EXPECT_TRUE(last_active_browser);
ASSERT_NE(browser(), last_active_browser);
WebContents* newtab = last_active_browser->GetSelectedWebContents();
EXPECT_TRUE(newtab);
GURL non_app_url = base_url.Resolve("path3/empty.html");
observer.Wait();
// Popup window should be in the app's process. // Popup window should be in the app's process.
content::RenderProcessHost* popup_process = RenderViewHost* popup_host =
last_active_browser->GetWebContentsAt(0)->GetRenderProcessHost(); content::Source<RenderViewHost>(popup_observer.source()).ptr();
EXPECT_EQ(process, popup_process); EXPECT_EQ(process, popup_host->GetProcess());
} }
// http://crbug.com/118502 // http://crbug.com/118502
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/path_service.h" #include "base/path_service.h"
#include "base/scoped_temp_dir.h" #include "base/scoped_temp_dir.h"
#include "base/string_number_conversions.h" #include "base/string_number_conversions.h"
#include "base/utf_string_conversions.h"
#include "chrome/browser/extensions/app_shortcut_manager.h" #include "chrome/browser/extensions/app_shortcut_manager.h"
#include "chrome/browser/extensions/component_loader.h" #include "chrome/browser/extensions/component_loader.h"
#include "chrome/browser/extensions/crx_installer.h" #include "chrome/browser/extensions/crx_installer.h"
...@@ -28,6 +29,8 @@ ...@@ -28,6 +29,8 @@
#include "chrome/common/chrome_paths.h" #include "chrome/common/chrome_paths.h"
#include "chrome/common/chrome_switches.h" #include "chrome/common/chrome_switches.h"
#include "chrome/test/base/ui_test_utils.h" #include "chrome/test/base/ui_test_utils.h"
#include "content/public/browser/navigation_controller.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/notification_registrar.h" #include "content/public/browser/notification_registrar.h"
#include "content/public/browser/notification_service.h" #include "content/public/browser/notification_service.h"
#include "content/public/browser/render_view_host.h" #include "content/public/browser/render_view_host.h"
...@@ -460,6 +463,53 @@ bool ExtensionBrowserTest::WaitForExtensionCrash( ...@@ -460,6 +463,53 @@ bool ExtensionBrowserTest::WaitForExtensionCrash(
return (service->GetExtensionById(extension_id, true) == NULL); return (service->GetExtensionById(extension_id, true) == NULL);
} }
void ExtensionBrowserTest::OpenWindow(content::WebContents* contents,
const GURL& url,
bool newtab_process_should_equal_opener,
content::WebContents** newtab_result) {
ui_test_utils::WindowedNotificationObserver observer(
content::NOTIFICATION_LOAD_STOP,
content::NotificationService::AllSources());
ASSERT_TRUE(ui_test_utils::ExecuteJavaScript(
contents->GetRenderViewHost(), L"",
L"window.open('" + UTF8ToWide(url.spec()) + L"');"));
// The above window.open call is not user-initiated, so it will create
// a popup window instead of a new tab in current window.
// The stop notification will come from the new tab.
observer.Wait();
content::NavigationController* controller =
content::Source<content::NavigationController>(observer.source()).ptr();
content::WebContents* newtab = controller->GetWebContents();
ASSERT_TRUE(newtab);
EXPECT_EQ(url, controller->GetLastCommittedEntry()->GetURL());
if (newtab_process_should_equal_opener)
EXPECT_EQ(contents->GetRenderProcessHost(), newtab->GetRenderProcessHost());
else
EXPECT_NE(contents->GetRenderProcessHost(), newtab->GetRenderProcessHost());
if (newtab_result)
*newtab_result = newtab;
}
void ExtensionBrowserTest::NavigateInRenderer(content::WebContents* contents,
const GURL& url) {
bool result = false;
ui_test_utils::WindowedNotificationObserver observer(
content::NOTIFICATION_LOAD_STOP,
content::NotificationService::AllSources());
ASSERT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
contents->GetRenderViewHost(), L"",
L"window.addEventListener('unload', function() {"
L" window.domAutomationController.send(true);"
L"}, false);"
L"window.location = '" + UTF8ToWide(url.spec()) + L"';",
&result));
ASSERT_TRUE(result);
observer.Wait();
EXPECT_EQ(url, contents->GetController().GetLastCommittedEntry()->GetURL());
}
void ExtensionBrowserTest::Observe( void ExtensionBrowserTest::Observe(
int type, int type,
const content::NotificationSource& source, const content::NotificationSource& source,
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#include "content/public/browser/notification_details.h" #include "content/public/browser/notification_details.h"
#include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_types.h" #include "content/public/browser/notification_types.h"
#include "content/public/browser/web_contents.h"
namespace extensions { namespace extensions {
class Extension; class Extension;
...@@ -136,6 +137,17 @@ class ExtensionBrowserTest ...@@ -136,6 +137,17 @@ class ExtensionBrowserTest
// crashed. // crashed.
bool WaitForExtensionCrash(const std::string& extension_id); bool WaitForExtensionCrash(const std::string& extension_id);
// Simulates a page calling window.open on an URL and waits for the
// navigation.
void OpenWindow(content::WebContents* contents,
const GURL& url,
bool newtab_process_should_equal_opener,
content::WebContents** newtab_result);
// Simulates a page navigating itself to an URL and waits for the
// navigation.
void NavigateInRenderer(content::WebContents* contents, const GURL& url);
// content::NotificationObserver // content::NotificationObserver
virtual void Observe(int type, virtual void Observe(int type,
const content::NotificationSource& source, const content::NotificationSource& source,
......
...@@ -649,44 +649,18 @@ IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, LastError) { ...@@ -649,44 +649,18 @@ IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, LastError) {
EXPECT_TRUE(result); EXPECT_TRUE(result);
} }
// Helper function for common code shared by the 3 WindowOpen tests below.
static void WindowOpenHelper(Browser* browser, const GURL& start_url,
const std::string& newtab_url,
WebContents** newtab_result) {
ui_test_utils::NavigateToURL(browser, start_url);
ui_test_utils::WindowedNotificationObserver observer(
content::NOTIFICATION_LOAD_STOP,
content::NotificationService::AllSources());
ASSERT_TRUE(ui_test_utils::ExecuteJavaScript(
browser->GetSelectedWebContents()->GetRenderViewHost(), L"",
L"window.open('" + UTF8ToWide(newtab_url) + L"');"));
// Now the active tab in last active window should be the new tab.
Browser* last_active_browser = BrowserList::GetLastActive();
EXPECT_TRUE(last_active_browser);
WebContents* newtab = last_active_browser->GetSelectedWebContents();
EXPECT_TRUE(newtab);
GURL expected_url = start_url.Resolve(newtab_url);
observer.Wait();
EXPECT_EQ(expected_url,
newtab->GetController().GetLastCommittedEntry()->GetURL());
if (newtab_result)
*newtab_result = newtab;
}
// Tests that an extension page can call window.open to an extension URL and // Tests that an extension page can call window.open to an extension URL and
// the new window has extension privileges. // the new window has extension privileges.
IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, WindowOpenExtension) { IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, WindowOpenExtension) {
ASSERT_TRUE(LoadExtension( ASSERT_TRUE(LoadExtension(
test_data_dir_.AppendASCII("uitest").AppendASCII("window_open"))); test_data_dir_.AppendASCII("uitest").AppendASCII("window_open")));
GURL start_url(std::string("chrome-extension://") +
last_loaded_extension_id_ + "/test.html");
ui_test_utils::NavigateToURL(browser(), start_url);
WebContents* newtab; WebContents* newtab;
ASSERT_NO_FATAL_FAILURE(WindowOpenHelper( ASSERT_NO_FATAL_FAILURE(OpenWindow(browser()->GetSelectedWebContents(),
browser(), start_url.Resolve("newtab.html"), true, &newtab));
GURL(std::string("chrome-extension://") + last_loaded_extension_id_ +
"/test.html"),
"newtab.html", &newtab));
bool result = false; bool result = false;
ASSERT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool( ASSERT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
...@@ -700,11 +674,12 @@ IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, WindowOpenInvalidExtension) { ...@@ -700,11 +674,12 @@ IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, WindowOpenInvalidExtension) {
ASSERT_TRUE(LoadExtension( ASSERT_TRUE(LoadExtension(
test_data_dir_.AppendASCII("uitest").AppendASCII("window_open"))); test_data_dir_.AppendASCII("uitest").AppendASCII("window_open")));
ASSERT_NO_FATAL_FAILURE(WindowOpenHelper( GURL start_url(std::string("chrome-extension://") +
browser(), last_loaded_extension_id_ + "/test.html");
GURL(std::string("chrome-extension://") + last_loaded_extension_id_ + ui_test_utils::NavigateToURL(browser(), start_url);
"/test.html"), ASSERT_NO_FATAL_FAILURE(OpenWindow(browser()->GetSelectedWebContents(),
"chrome-extension://thisissurelynotavalidextensionid/newtab.html", NULL)); GURL("chrome-extension://thisissurelynotavalidextensionid/newtab.html"),
false, NULL));
// If we got to this point, we didn't crash, so we're good. // If we got to this point, we didn't crash, so we're good.
} }
...@@ -717,13 +692,11 @@ IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, WindowOpenNoPrivileges) { ...@@ -717,13 +692,11 @@ IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, WindowOpenNoPrivileges) {
ASSERT_TRUE(LoadExtension( ASSERT_TRUE(LoadExtension(
test_data_dir_.AppendASCII("uitest").AppendASCII("window_open"))); test_data_dir_.AppendASCII("uitest").AppendASCII("window_open")));
ui_test_utils::NavigateToURL(browser(), GURL("about:blank"));
WebContents* newtab; WebContents* newtab;
ASSERT_NO_FATAL_FAILURE(WindowOpenHelper( ASSERT_NO_FATAL_FAILURE(OpenWindow(browser()->GetSelectedWebContents(),
browser(), GURL(std::string("chrome-extension://") + last_loaded_extension_id_ +
GURL("about:blank"), "/newtab.html"), false, &newtab));
std::string("chrome-extension://") + last_loaded_extension_id_ +
"/newtab.html",
&newtab));
// Extension API should succeed. // Extension API should succeed.
bool result = false; bool result = false;
......
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