Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1185)

Unified Diff: chrome/browser/extensions/app_process_apitest.cc

Issue 5766003: Temporarily avoid process swaps for renderer navigations away from an app. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Only swap for non-app URLs Created 10 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/extensions/app_process_apitest.cc
diff --git a/chrome/browser/extensions/app_process_apitest.cc b/chrome/browser/extensions/app_process_apitest.cc
index af623f108a435155c94d28078179227e9615b73e..f8e12d3b54eae0146e03774596f90c93207f89ff 100644
--- a/chrome/browser/extensions/app_process_apitest.cc
+++ b/chrome/browser/extensions/app_process_apitest.cc
@@ -111,8 +111,12 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, MAYBE_AppProcess) {
base_url.Resolve("path1/empty.html"), true);
WindowOpenHelper(browser(), host,
base_url.Resolve("path2/empty.html"), true);
+ // TODO(creis): This should open in a new process (i.e., false for the last
+ // argument), but we temporarily keep pages with a valid window.opener in the
+ // same process until we're able to restore window.opener if the page later
+ // returns to an in-app URL. See crbug.com/65953.
WindowOpenHelper(browser(), host,
- base_url.Resolve("path3/empty.html"), false);
+ base_url.Resolve("path3/empty.html"), true);
// Now let's have these pages navigate, into or out of the extension web
// extent. They should switch processes.
@@ -120,8 +124,23 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, MAYBE_AppProcess) {
const GURL& non_app_url(base_url.Resolve("path3/empty.html"));
NavigateTabHelper(browser()->GetTabContentsAt(2), non_app_url);
NavigateTabHelper(browser()->GetTabContentsAt(3), app_url);
- EXPECT_NE(host->process(),
+ // 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 it needs to
+ // communicate with window.opener later. See crbug.com/65953.
+ EXPECT_EQ(host->process(),
browser()->GetTabContentsAt(2)->render_view_host()->process());
EXPECT_EQ(host->process(),
browser()->GetTabContentsAt(3)->render_view_host()->process());
+
+ // If one of the popup tabs navigates back to the app, window.opener should
+ // be valid.
+ NavigateTabHelper(browser()->GetTabContentsAt(6), app_url);
+ EXPECT_EQ(host->process(),
+ browser()->GetTabContentsAt(6)->render_view_host()->process());
+ bool windowOpenerValid = false;
+ ASSERT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
+ browser()->GetTabContentsAt(6)->render_view_host(), L"",
+ L"window.domAutomationController.send(window.opener != null)",
+ &windowOpenerValid));
+ ASSERT_TRUE(windowOpenerValid);
}
« no previous file with comments | « no previous file | chrome/browser/extensions/extension_browsertests_misc.cc » ('j') | chrome/renderer/render_view.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698