Explicitly pass |is_for_isolated_world| to URLLoaderFactoryManager.

Before this CL, extensions::URLLoaderFactoryManager's
OverrideURLLoaderFactoryParams method would not be explicitly told
whether the factory will be used by a content script.  Instead, the
method would try to infer this, by checking if the |process| where the
factory will be used is an extension process.

The inference above will give incorrect results in case of content
scripts being injected into (other) extensions - in this case the
|process|-based inference will incorrectly conclude that the factory is
*not* for a content script.

After this CL, an explicit |is_for_isolated_world| parameter is passed
to the OverrideURLLoaderFactoryParams method.

Bug: 1025303
Change-Id: If947796f6017ddbe2dd6968254cb8b9ea7f34c94
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1920004
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#722673}

chrome/browser/chrome_content_browser_client.cc

@@ -1631,12 +1631,13 @@ bool ChromeContentBrowserClient::
 }
 
 void ChromeContentBrowserClient::OverrideURLLoaderFactoryParams(
-    content::RenderProcessHost* process,
+    content::BrowserContext* browser_context,
     const url::Origin& origin,
+    bool is_for_isolated_world,
     network::mojom::URLLoaderFactoryParams* factory_params) {
 #if BUILDFLAG(ENABLE_EXTENSIONS)
   ChromeContentBrowserClientExtensionsPart::OverrideURLLoaderFactoryParams(
-      process, origin, factory_params);
+      browser_context, origin, is_for_isolated_world, factory_params);
 #endif
 }
 

chrome/browser/chrome_content_browser_client.h

@@ -158,8 +158,9 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
       base::StringPiece scheme,
       bool is_embedded_origin_secure) override;
   void OverrideURLLoaderFactoryParams(
-      content::RenderProcessHost* process,
+      content::BrowserContext* browser_context,
       const url::Origin& origin,
+      bool is_for_isolated_world,
       network::mojom::URLLoaderFactoryParams* factory_params) override;
   void GetAdditionalWebUISchemes(
       std::vector<std::string>* additional_schemes) override;

chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc

@@ -625,11 +625,12 @@ ChromeContentBrowserClientExtensionsPart::GetVpnServiceProxy(
 
 // static
 void ChromeContentBrowserClientExtensionsPart::OverrideURLLoaderFactoryParams(
-    content::RenderProcessHost* process,
+    content::BrowserContext* browser_context,
     const url::Origin& origin,
+    bool is_for_isolated_world,
     network::mojom::URLLoaderFactoryParams* factory_params) {
-  URLLoaderFactoryManager::OverrideURLLoaderFactoryParams(process, origin,
-                                                          factory_params);
+  URLLoaderFactoryManager::OverrideURLLoaderFactoryParams(
+      browser_context, origin, is_for_isolated_world, factory_params);
 }
 
 // static

chrome/browser/extensions/chrome_content_browser_client_extensions_part.h

@@ -88,8 +88,9 @@ class ChromeContentBrowserClientExtensionsPart
       content::BrowserContext* browser_context);
 
   static void OverrideURLLoaderFactoryParams(
-      content::RenderProcessHost* process,
+      content::BrowserContext* browser_context,
       const url::Origin& origin,
+      bool is_for_isolated_world,
       network::mojom::URLLoaderFactoryParams* factory_params);
 
   static bool IsBuiltinComponent(content::BrowserContext* browser_context,

content/browser/url_loader_factory_params_helper.cc

@@ -39,7 +39,8 @@ network::mojom::URLLoaderFactoryParamsPtr CreateParams(
     const base::Optional<base::UnguessableToken>& top_frame_token,
     const base::Optional<net::NetworkIsolationKey>& network_isolation_key,
     network::mojom::CrossOriginEmbedderPolicy cross_origin_embedder_policy,
-    bool allow_universal_access_from_file_urls) {
+    bool allow_universal_access_from_file_urls,
+    bool is_for_isolated_world) {
   DCHECK(process);
 
   // "chrome-guest://..." is never used as a main or isolated world origin.
@@ -76,8 +77,9 @@ network::mojom::URLLoaderFactoryParamsPtr CreateParams(
     params->is_corb_enabled = true;
   }
 
-  GetContentClient()->browser()->OverrideURLLoaderFactoryParams(process, origin,
-                                                                params.get());
+  GetContentClient()->browser()->OverrideURLLoaderFactoryParams(
+      process->GetBrowserContext(), origin, is_for_isolated_world,
+      params.get());
 
   return params;
 }
@@ -98,7 +100,8 @@ network::mojom::URLLoaderFactoryParamsPtr URLLoaderFactoryParamsHelper::Create(
                       false,         // is_trusted
                       top_frame_token, network_isolation_key,
                       cross_origin_embedder_policy,
-                      preferences.allow_universal_access_from_file_urls);
+                      preferences.allow_universal_access_from_file_urls,
+                      false);  // is_for_isolated_world
 }
 
 // static
@@ -117,7 +120,8 @@ URLLoaderFactoryParamsHelper::CreateForIsolatedWorld(
                       false,                  // is_trusted
                       top_frame_token, network_isolation_key,
                       cross_origin_embedder_policy,
-                      preferences.allow_universal_access_from_file_urls);
+                      preferences.allow_universal_access_from_file_urls,
+                      true);  // is_for_isolated_world
 }
 
 network::mojom::URLLoaderFactoryParamsPtr
@@ -134,7 +138,8 @@ URLLoaderFactoryParamsHelper::CreateForPrefetch(
                       top_frame_token,
                       base::nullopt,  // network_isolation_key
                       cross_origin_embedder_policy,
-                      preferences.allow_universal_access_from_file_urls);
+                      preferences.allow_universal_access_from_file_urls,
+                      false);  // is_for_isolated_world
 }
 
 // static
@@ -153,7 +158,8 @@ URLLoaderFactoryParamsHelper::CreateForWorker(
                       base::nullopt,  // top_frame_token
                       network_isolation_key,
                       network::mojom::CrossOriginEmbedderPolicy::kNone,
-                      false);  // allow_universal_access_from_file_urls
+                      false,   // allow_universal_access_from_file_urls
+                      false);  // is_for_isolated_world
 }
 
 // static
@@ -186,7 +192,8 @@ URLLoaderFactoryParamsHelper::CreateForRendererProcess(
       false,                        // is_trusted
       top_frame_token, network_isolation_key,
       network::mojom::CrossOriginEmbedderPolicy::kNone,
-      false);  // allow_universal_access_from_file_urls
+      false,   // allow_universal_access_from_file_urls
+      false);  // is_for_isolated_world
 }
 
 }  // namespace content

content/public/browser/content_browser_client.cc

@@ -147,8 +147,9 @@ bool ContentBrowserClient::ShouldIgnoreSameSiteCookieRestrictionsWhenTopLevel(
 }
 
 void ContentBrowserClient::OverrideURLLoaderFactoryParams(
-    RenderProcessHost* process,
+    BrowserContext* browser_context,
     const url::Origin& origin,
+    bool is_for_isolated_world,
     network::mojom::URLLoaderFactoryParams* factory_params) {}
 
 void ContentBrowserClient::GetAdditionalViewSourceSchemes(

content/public/browser/content_browser_client.h

@@ -379,8 +379,9 @@ class CONTENT_EXPORT ContentBrowserClient {
   //   In this case |origin| is the origin of the isolated world and the
   //   |request_initiator_site_lock| is the origin committed in the frame.
   virtual void OverrideURLLoaderFactoryParams(
-      RenderProcessHost* process,
+      BrowserContext* browser_context,
       const url::Origin& origin,
+      bool is_for_isolated_world,
       network::mojom::URLLoaderFactoryParams* factory_params);
 
   // Returns a list additional WebUI schemes, if any.  These additional schemes

extensions/browser/url_loader_factory_manager.cc

@@ -506,10 +506,10 @@ void URLLoaderFactoryManager::WillExecuteCode(content::RenderFrameHost* frame,
 
 // static
 void URLLoaderFactoryManager::OverrideURLLoaderFactoryParams(
-    content::RenderProcessHost* process,
+    content::BrowserContext* browser_context,
     const url::Origin& origin,
+    bool is_for_isolated_world,
     network::mojom::URLLoaderFactoryParams* factory_params) {
-  content::BrowserContext* browser_context = process->GetBrowserContext();
   const ExtensionRegistry* registry = ExtensionRegistry::Get(browser_context);
   DCHECK(registry);  // CreateFactory shouldn't happen during shutdown.
 
@@ -534,14 +534,10 @@ void URLLoaderFactoryManager::OverrideURLLoaderFactoryParams(
     return;
   }
 
-  // Figure out if the factory is needed for content scripts VS extension
-  // renderer.
-  FactoryUser factory_user = FactoryUser::kContentScript;
-  ProcessMap* process_map = ProcessMap::Get(browser_context);
-  if (process_map->Contains(extension->id(), process->GetID()))
-    factory_user = FactoryUser::kExtensionProcess;
-
   // Don't change |factory_params| unless required.
+  FactoryUser factory_user = is_for_isolated_world
+                                 ? FactoryUser::kContentScript
+                                 : FactoryUser::kExtensionProcess;
   if (!IsSpecialURLLoaderFactoryRequired(*extension, factory_user))
     return;
 

extensions/browser/url_loader_factory_manager.h

@@ -14,8 +14,8 @@
 #include "url/gurl.h"
 
 namespace content {
+class BrowserContext;
 class RenderFrameHost;
-class RenderProcessHost;
 }  // namespace content
 
 namespace url {
@@ -72,7 +72,7 @@ class URLLoaderFactoryManager {
   //   extensions), but some extensions might need to set extension-specific
   //   security properties in the URLLoaderFactory used by content scripts.
   // The method recognizes the intended consumer based on |origin| ("web" vs
-  // other cases) and |process| ("extension" vs "content script").
+  // other cases) and |is_for_isolated_world| ("extension" vs "content script").
   //
   // The following examples might help understand the difference between
   // |origin| and other properties of a factory and/or network request:
@@ -92,8 +92,9 @@ class URLLoaderFactoryManager {
   //    - is_corb_enabled          |  secure     |  ext-based  | ext-based if
   //    - ..._access_patterns      |    default  |             |   allowlisted
   static void OverrideURLLoaderFactoryParams(
-      content::RenderProcessHost* process,
+      content::BrowserContext* browser_context,
       const url::Origin& origin,
+      bool is_for_isolated_world,
       network::mojom::URLLoaderFactoryParams* factory_params);
 
   static void AddExtensionToAllowlistForTesting(const Extension& extension);

extensions/shell/browser/shell_content_browser_client.cc

@@ -349,11 +349,12 @@ bool ShellContentBrowserClient::HandleExternalProtocol(
 }
 
 void ShellContentBrowserClient::OverrideURLLoaderFactoryParams(
-    content::RenderProcessHost* process,
+    content::BrowserContext* browser_context,
     const url::Origin& origin,
+    bool is_for_isolated_world,
     network::mojom::URLLoaderFactoryParams* factory_params) {
-  URLLoaderFactoryManager::OverrideURLLoaderFactoryParams(process, origin,
-                                                          factory_params);
+  URLLoaderFactoryManager::OverrideURLLoaderFactoryParams(
+      browser_context, origin, is_for_isolated_world, factory_params);
 }
 
 std::string ShellContentBrowserClient::GetUserAgent() {

extensions/shell/browser/shell_content_browser_client.h

@@ -107,8 +107,9 @@ class ShellContentBrowserClient : public content::ContentBrowserClient {
       mojo::PendingRemote<network::mojom::URLLoaderFactory>* out_factory)
       override;
   void OverrideURLLoaderFactoryParams(
-      content::RenderProcessHost* process,
+      content::BrowserContext* browser_context,
       const url::Origin& origin,
+      bool is_for_isolated_world,
       network::mojom::URLLoaderFactoryParams* factory_params) override;
   std::string GetUserAgent() override;