Keep web popups opened from extensions in same BrowsingInstance.

chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc

Index: chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc
diff --git a/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc b/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc
index ca0879d192fed96008233ff83eb019cbf965ead5..5e82dc8ca03c6ad2fe257eebb486b4c5a0431d52 100644
--- a/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc
+++ b/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc
@@ -419,39 +419,40 @@ bool ChromeContentBrowserClientExtensionsPart::
     return false;
 
   // 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.
+  // scripting by existing tabs) if one of the URLs corresponds to the Chrome
+  // Web Store hosted app, and the other does not.
   //
-  // 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
-  // RenderFrameHostManager.
+  // We don't force a BrowsingInstance swap in other cases (i.e., when opening
+  // a popup from one extension to a different extension, or to a non-extension
+  // URL) to preserve script connections and allow use cases like postMessage
+  // via window.opener. Those cases would still force a SiteInstance swap in
+  // RenderFrameHostManager.  This behavior is similar to how extension
+  // subframes on a web main frame are also placed in the same BrowsingInstance
+  // (by the content/ part of ShouldSwapBrowsingInstancesForNavigation); this
+  // check is just doing the same for top-level frames.  See
+  // https://crbug.com/590068.
   const Extension* current_extension =
       registry->enabled_extensions().GetExtensionOrAppByURL(current_url);
-  if (current_extension && current_extension->is_hosted_app() &&
-      current_extension->id() != kWebStoreAppId)
-    current_extension = NULL;
+  bool is_current_url_for_web_store =
+      current_extension && current_extension->id() == kWebStoreAppId;
 
   const Extension* new_extension =
       registry->enabled_extensions().GetExtensionOrAppByURL(new_url);
-  if (new_extension && new_extension->is_hosted_app() &&
-      new_extension->id() != kWebStoreAppId)
-    new_extension = NULL;
+  bool is_new_url_for_web_store =
+      new_extension && new_extension->id() == kWebStoreAppId;
 
-  // 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.
+  // First do a process check.  We should force a BrowsingInstance swap if we
+  // are going to Chrome Web Store, but the current process doesn't know about
+  // CWS, even if current_extension somehow corresponds to CWS.
   ProcessMap* process_map = ProcessMap::Get(site_instance->GetBrowserContext());
-  if (new_extension &&
-      site_instance->HasProcess() &&
-      !process_map->Contains(
-          new_extension->id(), site_instance->GetProcess()->GetID()))
+  if (is_new_url_for_web_store && site_instance->HasProcess() &&
+      !process_map->Contains(new_extension->id(),
+                             site_instance->GetProcess()->GetID()))
     return true;
 
-  // Otherwise, swap BrowsingInstances if current_extension and new_extension
-  // differ.
-  return current_extension != new_extension;
+  // Otherwise, swap BrowsingInstances when transitioning to/from Chrome Web
+  // Store.
+  return is_current_url_for_web_store != is_new_url_for_web_store;
 }
 
 // static