Update process sharing rules for hosted and isolated apps.

Hosted apps should only share with each other, and an isolated app should
only share with the exact same app.

BUG=102533
BUG=105322
TEST=Open many pages, extensions, and apps, and check sharing relationships.

Review URL: http://codereview.chromium.org/8677026

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@113043 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/chrome_content_browser_client.cc

@@ -160,8 +160,9 @@ bool HandleWebUI(GURL* url, content::BrowserContext* browser_context) {
 // sure URLs are served by hosts with the right set of privileges.
 enum RenderProcessHostPrivilege {
   PRIV_NORMAL,
-  PRIV_EXTENSION,
+  PRIV_HOSTED,
   PRIV_ISOLATED,
+  PRIV_EXTENSION,
 };
 
 RenderProcessHostPrivilege GetPrivilegeRequiredByUrl(
@@ -178,9 +179,10 @@ RenderProcessHostPrivilege GetPrivilegeRequiredByUrl(
 
   if (url.SchemeIs(chrome::kExtensionScheme)) {
     const Extension* extension = service->GetExtensionByURL(url);
-    if (extension && extension->is_storage_isolated()) {
+    if (extension && extension->is_storage_isolated())
       return PRIV_ISOLATED;
-    }
+    if (extension && extension->is_hosted_app())
+      return PRIV_HOSTED;
 
     return PRIV_EXTENSION;
   }
@@ -192,8 +194,6 @@ RenderProcessHostPrivilege GetProcessPrivilege(
     content::RenderProcessHost* process_host,
     extensions::ProcessMap* process_map,
     ExtensionService* service) {
-  // TODO(aa): It seems like hosted apps should be grouped separately from
-  // extensions: crbug.com/102533.
   std::set<std::string> extension_ids =
       process_map->GetExtensionsInProcess(process_host->GetID());
   if (extension_ids.empty())
@@ -204,11 +204,34 @@ RenderProcessHostPrivilege GetProcessPrivilege(
     const Extension* extension = service->GetExtensionById(*iter, false);
     if (extension && extension->is_storage_isolated())
       return PRIV_ISOLATED;
+    if (extension && extension->is_hosted_app())
+      return PRIV_HOSTED;
   }
 
   return PRIV_EXTENSION;
 }
 
+bool IsIsolatedAppInProcess(const GURL& site_url,
+                            content::RenderProcessHost* process_host,
+                            extensions::ProcessMap* process_map,
+                            ExtensionService* service) {
+  std::set<std::string> extension_ids =
+      process_map->GetExtensionsInProcess(process_host->GetID());
+  if (extension_ids.empty())
+    return false;
+
+  for (std::set<std::string>::iterator iter = extension_ids.begin();
+       iter != extension_ids.end(); ++iter) {
+    const Extension* extension = service->GetExtensionById(*iter, false);
+    if (extension &&
+        extension->is_storage_isolated() &&
+        extension->url() == site_url)
+      return true;
+  }
+
+  return false;
+}
+
 bool CertMatchesFilter(const net::X509Certificate& cert,
                        const base::DictionaryValue& filter) {
   // TODO(markusheintz): This is the minimal required filter implementation.
@@ -436,8 +459,19 @@ bool ChromeContentBrowserClient::IsSuitableHost(
   if (command_line.HasSwitch(switches::kEnableStrictSiteIsolation))
     return false;
 
-  return GetProcessPrivilege(process_host, process_map, service) ==
+  // An isolated app is only allowed to share with the exact same app in order
+  // to provide complete renderer process isolation.  This also works around
+  // issue http://crbug.com/85588, where different isolated apps in the same
+  // process would end up using the first app's storage contexts.
+  RenderProcessHostPrivilege privilege_required =
       GetPrivilegeRequiredByUrl(site_url, service);
+  if (privilege_required == PRIV_ISOLATED)
+    return IsIsolatedAppInProcess(site_url, process_host, process_map, service);
+
+  // Otherwise, just make sure the process privilege matches the privilege
+  // required by the site.
+  return GetProcessPrivilege(process_host, process_map, service) ==
+      privilege_required;
 }
 
 void ChromeContentBrowserClient::SiteInstanceGotProcess(

chrome/browser/extensions/isolated_app_browsertest.cc

@@ -177,6 +177,8 @@ IN_PROC_BROWSER_TEST_F(IsolatedAppTest, ProcessOverflow) {
   ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII("isolated_apps/app1")));
   ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII("isolated_apps/app2")));
   ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII("hosted_app")));
+  ASSERT_TRUE(
+      LoadExtension(test_data_dir_.AppendASCII("api_test/app_process")));
 
   // The app under test acts on URLs whose host is "localhost",
   // so the URLs we navigate to must have host "localhost".
@@ -187,7 +189,13 @@ IN_PROC_BROWSER_TEST_F(IsolatedAppTest, ProcessOverflow) {
   replace_host.SetHostStr(host_str);
   base_url = base_url.ReplaceComponents(replace_host);
 
-  // Create a tab for each type of renderer that might exist.
+  // Load an extension before adding tabs.
+  const Extension* extension1 = LoadExtension(
+      test_data_dir_.AppendASCII("api_test/browser_action/basics"));
+  ASSERT_TRUE(extension1);
+  GURL extension1_url = extension1->url();
+
+  // Create multiple tabs for each type of renderer that might exist.
   ui_test_utils::NavigateToURLWithDisposition(
       browser(), base_url.Resolve("isolated_apps/app1/main.html"),
       CURRENT_TAB, ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
@@ -200,32 +208,90 @@ IN_PROC_BROWSER_TEST_F(IsolatedAppTest, ProcessOverflow) {
   ui_test_utils::NavigateToURLWithDisposition(
       browser(), base_url.Resolve("test_file.html"),
       NEW_FOREGROUND_TAB, ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+
   ui_test_utils::NavigateToURLWithDisposition(
       browser(), base_url.Resolve("isolated_apps/app2/main.html"),
       NEW_FOREGROUND_TAB, ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), GURL(chrome::kChromeUINewTabURL),
+      NEW_FOREGROUND_TAB, ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), base_url.Resolve("api_test/app_process/path1/empty.html"),
+      NEW_FOREGROUND_TAB, ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), base_url.Resolve("test_file_with_body.html"),
+      NEW_FOREGROUND_TAB, ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
 
-  ASSERT_EQ(5, browser()->tab_count());
+  // Load another copy of isolated app 1.
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), base_url.Resolve("isolated_apps/app1/main.html"),
+      NEW_FOREGROUND_TAB, ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+
+  // Load another extension.
+  const Extension* extension2 = LoadExtension(
+      test_data_dir_.AppendASCII("api_test/browser_action/close_background"));
+  ASSERT_TRUE(extension2);
+  GURL extension2_url = extension2->url();
+
+  // Get tab processes.
+  ASSERT_EQ(9, browser()->tab_count());
   content::RenderProcessHost* isolated1_host =
       browser()->GetTabContentsAt(0)->GetRenderProcessHost();
-  content::RenderProcessHost* ntp_host =
+  content::RenderProcessHost* ntp1_host =
       browser()->GetTabContentsAt(1)->GetRenderProcessHost();
-  content::RenderProcessHost* normal_extension_host =
+  content::RenderProcessHost* hosted1_host =
       browser()->GetTabContentsAt(2)->GetRenderProcessHost();
-  content::RenderProcessHost* web_host =
+  content::RenderProcessHost* web1_host =
       browser()->GetTabContentsAt(3)->GetRenderProcessHost();
+
   content::RenderProcessHost* isolated2_host =
       browser()->GetTabContentsAt(4)->GetRenderProcessHost();
+  content::RenderProcessHost* ntp2_host =
+      browser()->GetTabContentsAt(5)->GetRenderProcessHost();
+  content::RenderProcessHost* hosted2_host =
+      browser()->GetTabContentsAt(6)->GetRenderProcessHost();
+  content::RenderProcessHost* web2_host =
+      browser()->GetTabContentsAt(7)->GetRenderProcessHost();
+
+  content::RenderProcessHost* second_isolated1_host =
+      browser()->GetTabContentsAt(8)->GetRenderProcessHost();
+
+  // Get extension processes.
+  ExtensionProcessManager* process_manager =
+    browser()->GetProfile()->GetExtensionProcessManager();
+  content::RenderProcessHost* extension1_host =
+      process_manager->GetSiteInstanceForURL(extension1_url)->GetProcess();
+  content::RenderProcessHost* extension2_host =
+      process_manager->GetSiteInstanceForURL(extension2_url)->GetProcess();
+
+  // An isolated app only shares with other instances of itself, not other
+  // isolated apps or anything else.
+  EXPECT_EQ(isolated1_host, second_isolated1_host);
+  EXPECT_NE(isolated1_host, isolated2_host);
+  EXPECT_NE(isolated1_host, ntp1_host);
+  EXPECT_NE(isolated1_host, hosted1_host);
+  EXPECT_NE(isolated1_host, web1_host);
+  EXPECT_NE(isolated1_host, extension1_host);
+  EXPECT_NE(isolated2_host, ntp1_host);
+  EXPECT_NE(isolated2_host, hosted1_host);
+  EXPECT_NE(isolated2_host, web1_host);
+  EXPECT_NE(isolated2_host, extension1_host);
+
+  // Everything else is clannish.  WebUI only shares with other WebUI.
+  EXPECT_EQ(ntp1_host, ntp2_host);
+  EXPECT_NE(ntp1_host, hosted1_host);
+  EXPECT_NE(ntp1_host, web1_host);
+  EXPECT_NE(ntp1_host, extension1_host);
+
+  // Hosted apps only share with each other.
+  EXPECT_EQ(hosted1_host, hosted2_host);
+  EXPECT_NE(hosted1_host, web1_host);
+  EXPECT_NE(hosted1_host, extension1_host);
+
+  // Web pages only share with each other.
+  EXPECT_EQ(web1_host, web2_host);
+  EXPECT_NE(web1_host, extension1_host);
 
-  // Isolated apps shared with each other, but no one else.  They're clannish
-  // like that.
-  ASSERT_EQ(isolated1_host, isolated2_host);
-  ASSERT_NE(isolated1_host, ntp_host);
-  ASSERT_NE(isolated1_host, normal_extension_host);
-  ASSERT_NE(isolated1_host, web_host);
-
-  // And cause we're here, make sure everyone else is also clannish.  This could
-  // technially go in another test, but not worth the extra setup overhead.
-  ASSERT_NE(web_host, ntp_host);
-  ASSERT_NE(web_host, normal_extension_host);
-  ASSERT_NE(normal_extension_host, ntp_host);
+  // Extensions only share with each other.
+  EXPECT_EQ(extension1_host, extension2_host);
 }