Fill in the Network Isolation Key for request from extension

Some extension browser tests are failing when split cache is enabled, because
the request is not considered as cacheable when the Network Isolation Key is
missing.

This CL fills in the Network Isolation Key for the factory used by request
from extensions. This factory is used for xhr and fetch requests from the
extension’s background script.


Bug: 1003882
Change-Id: If5ff7f159dbccb490b1f8ec5d5f98e08301ba1bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1811477
Commit-Queue: Yao Xiao <yaoxia@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Shivani Sharma <shivanisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#700012}

chrome/browser/chrome_content_browser_client.cc

@@ -1608,11 +1608,13 @@ ChromeContentBrowserClient::CreateURLLoaderFactoryForNetworkRequests(
     network::mojom::NetworkContext* network_context,
     mojo::PendingRemote<network::mojom::TrustedURLLoaderHeaderClient>*
         header_client,
-    const url::Origin& request_initiator) {
+    const url::Origin& request_initiator,
+    const base::Optional<net::NetworkIsolationKey>& network_isolation_key) {
 #if BUILDFLAG(ENABLE_EXTENSIONS)
   return ChromeContentBrowserClientExtensionsPart::
-      CreateURLLoaderFactoryForNetworkRequests(
-          process, network_context, header_client, request_initiator);
+      CreateURLLoaderFactoryForNetworkRequests(process, network_context,
+                                               header_client, request_initiator,
+                                               network_isolation_key);
 #else
   return network::mojom::URLLoaderFactoryPtrInfo();
 #endif

chrome/browser/chrome_content_browser_client.h

@@ -157,7 +157,9 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
       network::mojom::NetworkContext* network_context,
       mojo::PendingRemote<network::mojom::TrustedURLLoaderHeaderClient>*
           header_client,
-      const url::Origin& request_initiator) override;
+      const url::Origin& request_initiator,
+      const base::Optional<net::NetworkIsolationKey>& network_isolation_key)
+      override;
   void GetAdditionalWebUISchemes(
       std::vector<std::string>* additional_schemes) override;
   void GetAdditionalViewSourceSchemes(

chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc

@@ -652,9 +652,11 @@ ChromeContentBrowserClientExtensionsPart::
         network::mojom::NetworkContext* network_context,
         mojo::PendingRemote<network::mojom::TrustedURLLoaderHeaderClient>*
             header_client,
-        const url::Origin& request_initiator) {
+        const url::Origin& request_initiator,
+        const base::Optional<net::NetworkIsolationKey>& network_isolation_key) {
   return URLLoaderFactoryManager::CreateFactory(
-      process, network_context, header_client, request_initiator);
+      process, network_context, header_client, request_initiator,
+      network_isolation_key);
 }
 
 // static

chrome/browser/extensions/chrome_content_browser_client_extensions_part.h

@@ -99,7 +99,8 @@ class ChromeContentBrowserClientExtensionsPart
       network::mojom::NetworkContext* network_context,
       mojo::PendingRemote<network::mojom::TrustedURLLoaderHeaderClient>*
           header_client,
-      const url::Origin& request_initiator);
+      const url::Origin& request_initiator,
+      const base::Optional<net::NetworkIsolationKey>& network_isolation_key);
 
   static bool IsBuiltinComponent(content::BrowserContext* browser_context,
                                  const url::Origin& origin);

chrome/browser/extensions/extension_url_loader_throttle_browsertest.cc

@@ -15,6 +15,7 @@
 #include "extensions/common/switches.h"
 #include "extensions/test/result_catcher.h"
 #include "net/base/backoff_entry.h"
+#include "net/base/features.h"
 #include "net/base/url_util.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
@@ -98,19 +99,26 @@ class ExtensionURLLoaderThrottleBrowserTest : public ExtensionBrowserTest {
   const Extension* extension_;
 };
 
-class ExtensionURLLoaderThrottleCommandLineBrowserTest
-    : public ExtensionURLLoaderThrottleBrowserTest {
+class ExtensionURLLoaderThrottleWithSplitCacheBrowserTest
+    : public ExtensionURLLoaderThrottleBrowserTest,
+      public ::testing::WithParamInterface<bool> {
  public:
-  void SetUpCommandLine(base::CommandLine* command_line) override {
-    ExtensionURLLoaderThrottleBrowserTest::SetUpCommandLine(command_line);
-    command_line->AppendSwitch(
-        extensions::switches::kDisableExtensionsHttpThrottling);
+  void SetUp() override {
+    bool split_cache_by_network_isolation_key = GetParam();
+    feature_list_.InitWithFeatureState(
+        net::features::kSplitCacheByNetworkIsolationKey,
+        split_cache_by_network_isolation_key);
+    ExtensionURLLoaderThrottleBrowserTest::SetUp();
   }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
 };
 
 // Tests that if the same URL is requested repeatedly by an extension, it will
 // eventually be throttled.
-IN_PROC_BROWSER_TEST_F(ExtensionURLLoaderThrottleBrowserTest, ThrottleRequest) {
+IN_PROC_BROWSER_TEST_P(ExtensionURLLoaderThrottleWithSplitCacheBrowserTest,
+                       ThrottleRequest) {
   embedded_test_server()->RegisterRequestHandler(
       base::BindRepeating(&HandleRequest, false, false));
   ASSERT_TRUE(embedded_test_server()->Start());
@@ -123,7 +131,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionURLLoaderThrottleBrowserTest, ThrottleRequest) {
 
 // Tests that if the same URL is repeatedly requested by an extension, and the
 // response is served from the cache, it will not be throttled.
-IN_PROC_BROWSER_TEST_F(ExtensionURLLoaderThrottleBrowserTest,
+IN_PROC_BROWSER_TEST_P(ExtensionURLLoaderThrottleWithSplitCacheBrowserTest,
                        DoNotThrottleCachedResponse) {
   embedded_test_server()->RegisterRequestHandler(
       base::BindRepeating(&HandleRequest, false, true));
@@ -136,7 +144,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionURLLoaderThrottleBrowserTest,
 }
 
 // Tests that the redirected request is also being throttled.
-IN_PROC_BROWSER_TEST_F(ExtensionURLLoaderThrottleBrowserTest,
+IN_PROC_BROWSER_TEST_P(ExtensionURLLoaderThrottleWithSplitCacheBrowserTest,
                        ThrottleRequest_Redirect) {
   embedded_test_server()->RegisterRequestHandler(
       base::BindRepeating(&HandleRequest, false, false));
@@ -160,7 +168,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionURLLoaderThrottleBrowserTest,
 
 // Tests that if both redirect (302) and non-redirect (503) responses are
 // served from cache, the extension throttle does not throttle the request.
-IN_PROC_BROWSER_TEST_F(ExtensionURLLoaderThrottleBrowserTest,
+IN_PROC_BROWSER_TEST_P(ExtensionURLLoaderThrottleWithSplitCacheBrowserTest,
                        DoNotThrottleCachedResponse_Redirect) {
   embedded_test_server()->RegisterRequestHandler(
       base::BindRepeating(&HandleRequest, true, true));
@@ -175,7 +183,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionURLLoaderThrottleBrowserTest,
 // Tests that if the redirect (302) is served from cache, but the non-redirect
 // (503) is not, the extension throttle throttles the requests for the second
 // url.
-IN_PROC_BROWSER_TEST_F(ExtensionURLLoaderThrottleBrowserTest,
+IN_PROC_BROWSER_TEST_P(ExtensionURLLoaderThrottleWithSplitCacheBrowserTest,
                        ThrottleRequest_RedirectCached) {
   embedded_test_server()->RegisterRequestHandler(
       base::BindRepeating(&HandleRequest, true, false));
@@ -197,7 +205,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionURLLoaderThrottleBrowserTest,
 // Tests that if the redirect (302) is not served from cache, but the
 // non-redirect (503) is, the extension throttle only throttles requests to the
 // redirect URL.
-IN_PROC_BROWSER_TEST_F(ExtensionURLLoaderThrottleBrowserTest,
+IN_PROC_BROWSER_TEST_P(ExtensionURLLoaderThrottleWithSplitCacheBrowserTest,
                        DoNotThrottleCachedResponse_NonRedirectCached) {
   embedded_test_server()->RegisterRequestHandler(
       base::BindRepeating(&HandleRequest, false, true));
@@ -216,6 +224,21 @@ IN_PROC_BROWSER_TEST_F(ExtensionURLLoaderThrottleBrowserTest,
               ""));
 }
 
+INSTANTIATE_TEST_SUITE_P(
+    /* no prefix */,
+    ExtensionURLLoaderThrottleWithSplitCacheBrowserTest,
+    testing::Bool());
+
+class ExtensionURLLoaderThrottleCommandLineBrowserTest
+    : public ExtensionURLLoaderThrottleBrowserTest {
+ public:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    ExtensionURLLoaderThrottleBrowserTest::SetUpCommandLine(command_line);
+    command_line->AppendSwitch(
+        extensions::switches::kDisableExtensionsHttpThrottling);
+  }
+};
+
 // Tests that if switches::kDisableExtensionsHttpThrottling is set on the
 // command line, throttling is disabled.
 IN_PROC_BROWSER_TEST_F(ExtensionURLLoaderThrottleCommandLineBrowserTest,

content/browser/renderer_host/render_process_host_impl.cc

@@ -2467,7 +2467,7 @@ void RenderProcessHostImpl::CreateURLLoaderFactoryInternal(
     const base::Optional<url::Origin>& origin,
     network::mojom::CrossOriginEmbedderPolicy embedder_policy,
     const WebPreferences* preferences,
-    base::Optional<net::NetworkIsolationKey> network_isolation_key,
+    const base::Optional<net::NetworkIsolationKey>& network_isolation_key,
     mojo::PendingRemote<network::mojom::TrustedURLLoaderHeaderClient>
         header_client,
     network::mojom::URLLoaderFactoryRequest request,
@@ -2485,7 +2485,8 @@ void RenderProcessHostImpl::CreateURLLoaderFactoryInternal(
   if (origin.has_value()) {
     embedder_provided_factory =
         GetContentClient()->browser()->CreateURLLoaderFactoryForNetworkRequests(
-            this, network_context, &header_client, origin.value());
+            this, network_context, &header_client, origin.value(),
+            network_isolation_key);
   }
   if (embedder_provided_factory) {
     mojo::FuseInterface(std::move(request),

content/browser/renderer_host/render_process_host_impl.h

@@ -757,7 +757,7 @@ class CONTENT_EXPORT RenderProcessHostImpl
       const base::Optional<url::Origin>& origin,
       network::mojom::CrossOriginEmbedderPolicy embedder_policy,
       const WebPreferences* preferences,
-      base::Optional<net::NetworkIsolationKey> network_isolation_key,
+      const base::Optional<net::NetworkIsolationKey>& network_isolation_key,
       mojo::PendingRemote<network::mojom::TrustedURLLoaderHeaderClient>
           header_client,
       network::mojom::URLLoaderFactoryRequest request,

content/public/browser/content_browser_client.cc

@@ -140,7 +140,8 @@ ContentBrowserClient::CreateURLLoaderFactoryForNetworkRequests(
     network::mojom::NetworkContext* network_context,
     mojo::PendingRemote<network::mojom::TrustedURLLoaderHeaderClient>*
         header_client,
-    const url::Origin& request_initiator) {
+    const url::Origin& request_initiator,
+    const base::Optional<net::NetworkIsolationKey>& network_isolation_key) {
   return network::mojom::URLLoaderFactoryPtrInfo();
 }
 

content/public/browser/content_browser_client.h

@@ -356,7 +356,8 @@ class CONTENT_EXPORT ContentBrowserClient {
       network::mojom::NetworkContext* network_context,
       mojo::PendingRemote<network::mojom::TrustedURLLoaderHeaderClient>*
           header_client,
-      const url::Origin& request_initiator);
+      const url::Origin& request_initiator,
+      const base::Optional<net::NetworkIsolationKey>& network_isolation_key);
 
   // Returns a list additional WebUI schemes, if any.  These additional schemes
   // act as aliases to the chrome: scheme.  The additional schemes may or may

extensions/browser/url_loader_factory_manager.cc

@@ -312,11 +312,14 @@ network::mojom::URLLoaderFactoryPtrInfo CreateURLLoaderFactory(
     network::mojom::NetworkContext* network_context,
     mojo::PendingRemote<network::mojom::TrustedURLLoaderHeaderClient>*
         header_client,
-    const Extension& extension) {
+    const Extension& extension,
+    const base::Optional<net::NetworkIsolationKey>& network_isolation_key) {
   // Compute relaxed CORB config to be used by |extension|.
   network::mojom::URLLoaderFactoryParamsPtr params =
       network::mojom::URLLoaderFactoryParams::New();
 
+  params->network_isolation_key = network_isolation_key;
+
   // Setup factory bound allow list that overwrites per-profile common list
   // to allow tab specific permissions only for this newly created factory.
   params->factory_bound_allow_patterns = CreateCorsOriginAccessAllowList(
@@ -518,7 +521,8 @@ network::mojom::URLLoaderFactoryPtrInfo URLLoaderFactoryManager::CreateFactory(
     network::mojom::NetworkContext* network_context,
     mojo::PendingRemote<network::mojom::TrustedURLLoaderHeaderClient>*
         header_client,
-    const url::Origin& initiator_origin) {
+    const url::Origin& initiator_origin,
+    const base::Optional<net::NetworkIsolationKey>& network_isolation_key) {
   content::BrowserContext* browser_context = process->GetBrowserContext();
   const ExtensionRegistry* registry = ExtensionRegistry::Get(browser_context);
   DCHECK(registry);  // CreateFactory shouldn't happen during shutdown.
@@ -555,7 +559,7 @@ network::mojom::URLLoaderFactoryPtrInfo URLLoaderFactoryManager::CreateFactory(
   if (!IsSpecialURLLoaderFactoryRequired(*extension, factory_user))
     return network::mojom::URLLoaderFactoryPtrInfo();
   return CreateURLLoaderFactory(process, network_context, header_client,
-                                *extension);
+                                *extension, network_isolation_key);
 }
 
 // static

extensions/browser/url_loader_factory_manager.h

@@ -67,7 +67,8 @@ class URLLoaderFactoryManager {
       network::mojom::NetworkContext* network_context,
       mojo::PendingRemote<network::mojom::TrustedURLLoaderHeaderClient>*
           header_client,
-      const url::Origin& initiator_origin);
+      const url::Origin& initiator_origin,
+      const base::Optional<net::NetworkIsolationKey>& network_isolation_key);
 
   static void AddExtensionToAllowlistForTesting(const Extension& extension);
   static void RemoveExtensionFromAllowlistForTesting(

extensions/shell/browser/shell_content_browser_client.cc

@@ -341,9 +341,11 @@ ShellContentBrowserClient::CreateURLLoaderFactoryForNetworkRequests(
     network::mojom::NetworkContext* network_context,
     mojo::PendingRemote<network::mojom::TrustedURLLoaderHeaderClient>*
         header_client,
-    const url::Origin& request_initiator) {
+    const url::Origin& request_initiator,
+    const base::Optional<net::NetworkIsolationKey>& network_isolation_key) {
   return URLLoaderFactoryManager::CreateFactory(
-      process, network_context, header_client, request_initiator);
+      process, network_context, header_client, request_initiator,
+      network_isolation_key);
 }
 
 std::string ShellContentBrowserClient::GetUserAgent() {

extensions/shell/browser/shell_content_browser_client.h

@@ -101,7 +101,9 @@ class ShellContentBrowserClient : public content::ContentBrowserClient {
       network::mojom::NetworkContext* network_context,
       mojo::PendingRemote<network::mojom::TrustedURLLoaderHeaderClient>*
           header_client,
-      const url::Origin& request_initiator) override;
+      const url::Origin& request_initiator,
+      const base::Optional<net::NetworkIsolationKey>& network_isolation_key)
+      override;
   std::string GetUserAgent() override;
 
  protected: