summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--chrome/browser/chrome_content_browser_client.cc50
-rw-r--r--chrome/browser/extensions/app_process_apitest.cc47
2 files changed, 72 insertions, 25 deletions
diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
index 37faaf8..541c5d9 100644
--- a/chrome/browser/chrome_content_browser_client.cc
+++ b/chrome/browser/chrome_content_browser_client.cc
@@ -1326,26 +1326,8 @@ bool ChromeContentBrowserClient::ShouldSwapBrowsingInstancesForNavigation(
SiteInstance* site_instance,
const GURL& current_url,
const GURL& new_url) {
- if (current_url.is_empty()) {
- // Always choose a new process when navigating to extension URLs. The
- // process grouping logic will combine all of a given extension's pages
- // into the same process.
- if (new_url.SchemeIs(extensions::kExtensionScheme))
- return true;
-
- return false;
- }
-
- // Also, we must switch if one is an extension and the other is not the exact
- // same extension.
- if (current_url.SchemeIs(extensions::kExtensionScheme) ||
- new_url.SchemeIs(extensions::kExtensionScheme)) {
- if (current_url.GetOrigin() != new_url.GetOrigin())
- return true;
- }
-
- // The checks below only matter if we can retrieve which extensions are
- // installed.
+ // If we don't have an ExtensionService, then rely on the SiteInstance logic
+ // in RenderViewHostManager to decide when to swap.
Profile* profile =
Profile::FromBrowserContext(site_instance->GetBrowserContext());
ExtensionService* service =
@@ -1353,23 +1335,41 @@ bool ChromeContentBrowserClient::ShouldSwapBrowsingInstancesForNavigation(
if (!service)
return false;
- // We must swap if the URL is for an extension and we are not using an
- // extension process.
+ // We must use a new BrowsingInstance (forcing a process swap and disabling
+ // scripting by existing tabs) if one of the URLs is an extension and the
+ // other is not the exact same extension.
+ //
+ // We ignore hosted apps here so that other tabs in their BrowsingInstance can
+ // use postMessage with them. (The exception is the Chrome Web Store, which
+ // is a hosted app that requires its own BrowsingInstance.) Navigations
+ // to/from a hosted app will still trigger a SiteInstance swap in
+ // RenderViewHostManager.
+ const Extension* current_extension =
+ service->extensions()->GetExtensionOrAppByURL(current_url);
+ if (current_extension &&
+ current_extension->is_hosted_app() &&
+ current_extension->id() != extension_misc::kWebStoreAppId)
+ current_extension = NULL;
+
const Extension* new_extension =
service->extensions()->GetExtensionOrAppByURL(new_url);
- // Ignore all hosted apps except the Chrome Web Store, since they do not
- // require their own BrowsingInstance (e.g., postMessage is ok).
if (new_extension &&
new_extension->is_hosted_app() &&
new_extension->id() != extension_misc::kWebStoreAppId)
new_extension = NULL;
+
+ // First do a process check. We should force a BrowsingInstance swap if the
+ // current process doesn't know about new_extension, even if current_extension
+ // is somehow the same as new_extension.
if (new_extension &&
site_instance->HasProcess() &&
!service->process_map()->Contains(new_extension->id(),
site_instance->GetProcess()->GetID()))
return true;
- return false;
+ // Otherwise, swap BrowsingInstances if current_extension and new_extension
+ // differ.
+ return current_extension != new_extension;
}
bool ChromeContentBrowserClient::ShouldSwapProcessesForRedirect(
diff --git a/chrome/browser/extensions/app_process_apitest.cc b/chrome/browser/extensions/app_process_apitest.cc
index 6f12316..72e4375 100644
--- a/chrome/browser/extensions/app_process_apitest.cc
+++ b/chrome/browser/extensions/app_process_apitest.cc
@@ -24,6 +24,7 @@
#include "content/public/browser/notification_service.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/render_view_host.h"
+#include "content/public/browser/site_instance.h"
#include "content/public/browser/web_contents.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/test_navigation_observer.h"
@@ -36,6 +37,7 @@
using content::NavigationController;
using content::RenderViewHost;
+using content::SiteInstance;
using content::WebContents;
using extensions::Extension;
@@ -832,3 +834,48 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, MAYBE_ReloadAppAfterCrash) {
&is_installed));
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) {
+ extensions::ProcessMap* process_map = extensions::ExtensionSystem::Get(
+ browser()->profile())->extension_service()->process_map();
+
+ host_resolver()->AddRule("*", "127.0.0.1");
+ ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
+
+ GURL base_url = GetTestBaseURL("app_process");
+
+ // Load app and start URL (in the app).
+ const Extension* app =
+ LoadExtension(test_data_dir_.AppendASCII("app_process"));
+ ASSERT_TRUE(app);
+
+ ui_test_utils::NavigateToURL(browser(),
+ base_url.Resolve("path1/iframe.html"));
+ content::SiteInstance* app_instance =
+ browser()->tab_strip_model()->GetWebContentsAt(0)->GetSiteInstance();
+ EXPECT_TRUE(process_map->Contains(app_instance->GetProcess()->GetID()));
+
+ // Popup window should be in the app's process.
+ const BrowserList* active_browser_list =
+ BrowserList::GetInstance(chrome::GetActiveDesktop());
+ EXPECT_EQ(2U, active_browser_list->size());
+ content::WebContents* popup_contents =
+ active_browser_list->get(1)->tab_strip_model()->GetActiveWebContents();
+ content::WaitForLoadStop(popup_contents);
+
+ SiteInstance* popup_instance = popup_contents->GetSiteInstance();
+ EXPECT_EQ(app_instance, popup_instance);
+
+ // Navigate the popup to another process 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);
+
+ // It should still be in the same BrowsingInstance, allowing postMessage to
+ // work.
+ EXPECT_TRUE(app_instance->IsRelatedSiteInstance(new_instance));
+}