Swap BrowsingInstances on certain browser-initiated navigations.
After this CL, certain cross-site, main frame, browser-initiated
navigations will use a new BrowsingInstance. This includes
navigations initiated from the address bar (typing in the URL or
searching) and bookmarks.
Note that this works even if the navigated frame has an opener, since
these user-initiated navigations indicate that it's ok to break
the opener relationship.
This change helps avoid unneeded process sharing. For example, after
being on a A-embed-B page, any future browser-initiated navigation to
B would've stayed in the same BrowsingInstance, SiteInstance, and
process as the B subframe on the old page, and if the subframe reused
another tab's process due OOPIF process reuse policy, this resulted in
both tabs unnecessarily ending up in that process.
Bug: 803367
Change-Id: I3a2461a67cb626329c20a887f993d4c9a1494236
Reviewed-on: https://chromium-review.googlesource.com/881836
Commit-Queue: Alex Moshchuk <[email protected]>
Reviewed-by: Charlie Reis <[email protected]>
Reviewed-by: Devlin <[email protected]>
Cr-Commit-Position: refs/heads/master@{#540709}diff --git a/chrome/browser/extensions/app_process_apitest.cc b/chrome/browser/extensions/app_process_apitest.cc
index 6eadd740..da67267 100644
--- a/chrome/browser/extensions/app_process_apitest.cc
+++ b/chrome/browser/extensions/app_process_apitest.cc
@@ -798,10 +798,11 @@
ASSERT_TRUE(is_installed);
}
-// Test that a cross-process navigation away from a hosted app stays in the same
-// BrowsingInstance, so that postMessage calls to the app's other windows still
-// work.
-IN_PROC_BROWSER_TEST_F(AppApiTest, SameBrowsingInstanceAfterSwap) {
+// Test that a cross-site renderer-initiated navigation away from a hosted app
+// stays in the same BrowsingInstance, so that postMessage calls to the app's
+// other windows still work, and a cross-site browser-initiated navigation away
+// from a hosted app switches BrowsingInstances.
+IN_PROC_BROWSER_TEST_F(AppApiTest, NavigatePopupFromAppToOutsideApp) {
extensions::ProcessMap* process_map =
extensions::ProcessMap::Get(browser()->profile());
@@ -828,13 +829,40 @@
SiteInstance* popup_instance = popup_contents->GetSiteInstance();
EXPECT_EQ(app_instance, popup_instance);
- // Navigate the popup to another process outside the app.
+ // Do a renderer-initiated navigation in the popup to a URL outside the app.
GURL non_app_url(base_url.Resolve("path3/empty.html"));
- ui_test_utils::NavigateToURL(active_browser_list->get(1), non_app_url);
- SiteInstance* new_instance = popup_contents->GetSiteInstance();
- EXPECT_NE(app_instance, new_instance);
+ content::TestNavigationObserver observer(popup_contents);
+ EXPECT_TRUE(ExecuteScript(
+ popup_contents,
+ base::StringPrintf("location = '%s';", non_app_url.spec().c_str())));
+ observer.Wait();
- // It should still be in the same BrowsingInstance, allowing postMessage to
- // work.
- EXPECT_TRUE(app_instance->IsRelatedSiteInstance(new_instance));
+ // The popup will stay in the same SiteInstance, unless we are in
+ // --site-per-process mode, in which case it will go into a new related
+ // SiteInstance. Either case will allow postMessage to still work.
+ EXPECT_TRUE(
+ app_instance->IsRelatedSiteInstance(popup_contents->GetSiteInstance()));
+ if (content::AreAllSitesIsolatedForTesting())
+ EXPECT_NE(app_instance, popup_contents->GetSiteInstance());
+ else
+ EXPECT_EQ(app_instance, popup_contents->GetSiteInstance());
+
+ // Go back to app process in the popup.
+ {
+ content::TestNavigationObserver observer(popup_contents);
+ popup_contents->GetController().GoBack();
+ observer.Wait();
+ EXPECT_EQ(app_instance, popup_contents->GetSiteInstance());
+ }
+
+ // Do a browser-initiated navigation in the popup to a URL outside the app.
+ // This should swap BrowsingInstances.
+ {
+ content::TestNavigationObserver observer(popup_contents);
+ ui_test_utils::NavigateToURL(active_browser_list->get(1), non_app_url);
+ observer.Wait();
+ EXPECT_NE(app_instance, popup_contents->GetSiteInstance());
+ EXPECT_FALSE(
+ app_instance->IsRelatedSiteInstance(popup_contents->GetSiteInstance()));
+ }
}
