Avoid using ContentBrowserClient::IsIllegalOrigin in ResourceDispatcherHost.

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 8adeb96f87829a4b0395e697e12325fb4b7ac245..490d10cd02fb3017fc4972468478100febd4d539 100644
--- a/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc
+++ b/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc
@@ -27,6 +27,7 @@
 #include "content/public/browser/browser_url_handler.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
+#include "content/public/browser/resource_dispatcher_host.h"
 #include "content/public/browser/site_instance.h"
 #include "content/public/browser/vpn_service_proxy.h"
 #include "content/public/browser/web_contents.h"
@@ -272,52 +273,6 @@ bool ChromeContentBrowserClientExtensionsPart::CanCommitURL(
   return true;
 }
 
-bool ChromeContentBrowserClientExtensionsPart::IsIllegalOrigin(
-    content::ResourceContext* resource_context,
-    int child_process_id,
-    const GURL& origin) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
-  // Consider non-extension URLs safe; they will be checked elsewhere.
-  if (!origin.SchemeIs(kExtensionScheme))
-    return false;
-
-  // If there is no extension installed for the URL, it couldn't have committed.
-  // (If the extension was recently uninstalled, the tab would have closed.)
-  ProfileIOData* io_data = ProfileIOData::FromResourceContext(resource_context);
-  InfoMap* extension_info_map = io_data->GetExtensionInfoMap();
-  const Extension* extension =
-      extension_info_map->extensions().GetExtensionOrAppByURL(origin);
-  if (!extension)
-    return true;

[Charlie Reis, 2016/08/02 20:41:42]

Sanity check: We'll still deny this case because the extension's origin won't
have been registered, right?

[ananta, 2016/08/02 22:28:27]

The URL won't be found in the RDH map and hence will be treated as illegal. I
updated the RDH IsIllegalOrigin function comments to clarify this.

-
-  // Check for platform app origins.  These can only be committed by the app
-  // itself, or by one if its guests if there are accessible_resources.

[Charlie Reis, 2016/08/02 20:41:42]

Please don't lose all the comments in this method.  :)  I wanted it to be much
more strict, but we had to allow certain cases that are worth documenting.

[ananta, 2016/08/02 22:28:27]

Done.

-  const ProcessMap& process_map = extension_info_map->process_map();
-  if (extension->is_platform_app() &&
-      !process_map.Contains(extension->id(), child_process_id)) {
-    // This is a platform app origin not in the app's own process.  If there are
-    // no accessible resources, this is illegal.
-    if (!extension->GetManifestData(manifest_keys::kWebviewAccessibleResources))
-      return true;
-
-    // If there are accessible resources, the origin is only legal if the given
-    // process is a guest of the app.
-    std::string owner_extension_id;
-    int owner_process_id;
-    WebViewRendererState::GetInstance()->GetOwnerInfo(
-        child_process_id, &owner_process_id, &owner_extension_id);
-    const Extension* owner_extension =
-        extension_info_map->extensions().GetByID(owner_extension_id);
-    return !owner_extension || owner_extension != extension;
-  }
-
-  // With only the origin and not the full URL, we don't have enough information
-  // to validate hosted apps or web_accessible_resources in normal extensions.
-  // Assume they're legal.
-  return false;
-}
-
 // static
 bool ChromeContentBrowserClientExtensionsPart::IsSuitableHost(
     Profile* profile,
@@ -553,9 +508,12 @@ void ChromeContentBrowserClientExtensionsPart::SiteInstanceGotProcess(
 
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
-      base::Bind(&InfoMap::RegisterExtensionProcess,
-                 ExtensionSystem::Get(context)->info_map(), extension->id(),
-                 site_instance->GetProcess()->GetID(), site_instance->GetId()));
+      base::Bind(&ChromeContentBrowserClientExtensionsPart::
+                     RegisterExtensionProcessHelper,
+                 base::RetainedRef(ExtensionSystem::Get(context)->info_map()),
+                 base::RetainedRef(extension),
+                 site_instance->GetProcess()->GetID(), site_instance->GetId(),
+                 context->GetResourceContext()));
 }
 
 void ChromeContentBrowserClientExtensionsPart::SiteInstanceDeleting(
@@ -577,9 +535,12 @@ void ChromeContentBrowserClientExtensionsPart::SiteInstanceDeleting(
 
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
-      base::Bind(&InfoMap::UnregisterExtensionProcess,
-                 ExtensionSystem::Get(context)->info_map(), extension->id(),
-                 site_instance->GetProcess()->GetID(), site_instance->GetId()));
+      base::Bind(&ChromeContentBrowserClientExtensionsPart::
+                     UnregisterExtensionProcessHelper,
+                 base::RetainedRef(ExtensionSystem::Get(context)->info_map()),
+                 base::RetainedRef(extension),
+                 site_instance->GetProcess()->GetID(), site_instance->GetId(),
+                 context->GetResourceContext()));
 }
 
 void ChromeContentBrowserClientExtensionsPart::OverrideWebkitPrefs(
@@ -662,4 +623,44 @@ void ChromeContentBrowserClientExtensionsPart::
   }
 }
 
+// static
+void ChromeContentBrowserClientExtensionsPart::RegisterExtensionProcessHelper(
+    InfoMap* info_map,
+    const Extension* extension,
+    int process_id,
+    int site_instance_id,
+    const content::ResourceContext* context) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  info_map->RegisterExtensionProcess(extension->id(), process_id,
+                                     site_instance_id);
+
+  if (extension->is_platform_app()) {

[Charlie Reis, 2016/08/02 20:41:42]

This would be a good place for some of the previous comments.

[ananta, 2016/08/02 22:28:27]

Done.

+    content::ResourceDispatcherHost::Get()->AddProcessForOrigin(
+        context,
+        Extension::GetBaseURLFromExtensionId(extension->id()).spec(),
+        process_id);
+  }
+}
+
+// static
+void ChromeContentBrowserClientExtensionsPart::UnregisterExtensionProcessHelper(
+    InfoMap* info_map,
+    const Extension* extension,
+    int process_id,
+    int site_instance_id,
+    const content::ResourceContext* context) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  info_map->UnregisterExtensionProcess(extension->id(), process_id,
+                                       site_instance_id);
+
+  if (extension->is_platform_app()) {
+    content::ResourceDispatcherHost::Get()->RemoveProcessForOrigin(
+        context,
+        Extension::GetBaseURLFromExtensionId(extension->id()).spec(),
+        process_id);
+  }
+}
+
 }  // namespace extensions