diff options
-rw-r--r-- | chrome/browser/chrome_content_browser_client.cc | 50 | ||||
-rw-r--r-- | chrome/browser/extensions/app_process_apitest.cc | 47 |
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)); +} |