Ensure extensions and the Chrome Web Store are loaded in new BrowsingInstances.

chrome/browser/chrome_content_browser_client.cc

Index: chrome/browser/chrome_content_browser_client.cc
diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
index a3c33ea39d4992561c60de925c81ef2156909ca2..fb9987ed0dc9796d460e345d7d0552a515908497 100644
--- a/chrome/browser/chrome_content_browser_client.cc
+++ b/chrome/browser/chrome_content_browser_client.cc
@@ -1016,8 +1016,16 @@ void ChromeContentBrowserClient::SiteInstanceDeleting(
 }
 
 bool ChromeContentBrowserClient::ShouldSwapProcessesForNavigation(
+    SiteInstance* site_instance,
     const GURL& current_url,
     const GURL& new_url) {
+  Profile* profile =
+      Profile::FromBrowserContext(site_instance->GetBrowserContext());
+  ExtensionService* service =
+      extensions::ExtensionSystem::Get(profile)->extension_service();
+  if (!service)

[nasko, 2013/02/19 19:20:42]

nit: Put a comment on why it is fine to return early here. Based on my
understanding, this function only deals with extension URLs, so if
ExtensionService is not present, the rest need not be executed.

[Charlie Reis, 2013/02/22 05:08:40]

On second thought, I'm going to reduce the behavior delta here.  I was trying to
make the merge with my 123007 fix easier, but there's no reason to force a
change in behavior for the two existing checks below.

I've included a comment but moved this below them for now.

+    return false;
+
   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
@@ -1036,6 +1044,21 @@ bool ChromeContentBrowserClient::ShouldSwapProcessesForNavigation(
       return true;
   }
 
+  // We must swap if the URL is for an extension and we are not using an
+  // extension process.
+  const Extension* new_extension =
+      service->extensions()->GetExtensionOrAppByURL(ExtensionURLInfo(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;
+  if (new_extension &&
+      site_instance->HasProcess() &&

[nasko, 2013/02/19 19:20:42]

It shouldn't be possible for the site_instance not to have a process at this
point in time, should it?

[Charlie Reis, 2013/02/22 05:08:40]

Actually, I'm not certain about that, so I'd prefer to keep the check.  It's
referring to the RenderProcessHost of the currently visible page, so you could
be typing in a new URL while looking at a sad tab.  In my manual test, it looks
like HasProcess() still returns true after I've killed the tab, but there may be
cases it's gone.

+      !service->process_map()->Contains(site_instance->GetProcess()->GetID()))

[nasko, 2013/02/19 19:20:42]

Why not check with Contains(extension_id, process_id)?

[Charlie Reis, 2013/02/22 05:08:40]

Good call.  Done.

+    return true;
+
   return false;
 }