Modify --isolate-extensions to not isolate hosted apps.

chrome/browser/site_details.cc

Index: chrome/browser/site_details.cc
diff --git a/chrome/browser/site_details.cc b/chrome/browser/site_details.cc
index 54970b6b6acc152fecba150ade24c4d3fbcb58ca..bf9d9478200dd518a863c3300248ff3232bb2eee 100644
--- a/chrome/browser/site_details.cc
+++ b/chrome/browser/site_details.cc
@@ -7,7 +7,12 @@
 #include "base/metrics/histogram.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_process_host.h"
+
+#if defined(ENABLE_EXTENSIONS)
+#include "extensions/browser/extension_registry.h"
 #include "extensions/common/constants.h"
+#include "extensions/common/extension.h"
+#endif
 
 using content::BrowserThread;
 using content::RenderProcessHost;
@@ -16,7 +21,9 @@ using content::WebContents;
 
 namespace {
 
-bool ShouldIsolate(IsolationScenarioType policy, const GURL& site) {
+bool ShouldIsolate(content::BrowserContext* browser_context,
+                   IsolationScenarioType policy,
+                   const GURL& site) {
   switch (policy) {
     case ISOLATE_ALL_SITES:
       return true;
@@ -26,8 +33,19 @@ bool ShouldIsolate(IsolationScenarioType policy, const GURL& site) {
       // the New Tab Page gets counted as two processes under this policy, and
       // extensions are isolated as well.
       return !site.SchemeIs(url::kHttpScheme);
-    case ISOLATE_EXTENSIONS:
-      return site.SchemeIs(extensions::kExtensionScheme);
+    case ISOLATE_EXTENSIONS: {
+#if !defined(ENABLE_EXTENSIONS)
+      return false;
+#else
+      if (!site.SchemeIs(extensions::kExtensionScheme))
+        return false;
+      extensions::ExtensionRegistry* registry =
+          extensions::ExtensionRegistry::Get(browser_context);
+      const extensions::Extension* extension =
+          registry->enabled_extensions().GetExtensionOrAppByURL(site);
+      return extension && !extension->is_hosted_app();

[Charlie Reis, 2015/10/12 23:13:28]

This is fairly similar to
ChromeContentBrowserClientExtensionsPart::DoesSiteRequireDedicatedProcess, but I
think I'm ok with having separate copies rather than sharing an implementation. 
This one can't depend on the command line flag, and otherwise we're just
checking if it's a hosted app.

[ncarter (slow), 2015/10/13 20:22:11]

I'd thought about this too, and think that duplication is the right tradeoff,
for three reasons:

(1) There's a behavioral difference, in that DoesSiteRequireDedicatedProcess
also looks at the command line flag values. Theoretically, though, you could
have the flag state passed as a parameter to the
ChromeContentBrowserClientExtensionsPart function.

(2) I think it would be pretty weird for
ChromeContentBrowserClientExtensionsPart to be invoked without the content layer
originating the call (it's flat out impossible for chrome to call into
ChromeContentBrowser, for example).

(3) I think of this file, really, as a mock implementation of SiteInstanceImpl,
so duplication of logic just isn't that evil.

+#endif
+    }
   }
   NOTREACHED();
   return true;
@@ -41,7 +59,10 @@ IsolationScenario::~IsolationScenario() {}
 
 void IsolationScenario::CollectSiteInfoForScenario(SiteInstance* primary,
                                                    const GURL& site) {
-  const GURL& isolated = ShouldIsolate(policy, site) ? site : GURL("http://");
+  const GURL& isolated =
+      ShouldIsolate(primary->GetBrowserContext(), policy, site)
+          ? site
+          : GURL("http://");
   sites.insert(isolated);
   browsing_instance_site_map[primary->GetId()].insert(isolated);
 }