Fix URLLoaderInterceptor not intercepting non-network schemes for navigations...

Fix URLLoaderInterceptor not intercepting non-network schemes for navigations when the network service is enabled.

Bug: b/120908346
Change-Id: Ib1cbc7be93a417e662c9dafcd337637fab1483e3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1566021
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#650496}

content/browser/loader/navigation_url_loader_impl.cc

@@ -916,6 +916,15 @@ class NavigationURLLoaderImpl::URLLoaderRequestController
             base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
                 non_network_factory.get());
       }
+
+      if (g_loader_factory_interceptor.Get()) {
+        network::mojom::URLLoaderFactoryPtr factory_ptr;
+        auto request = mojo::MakeRequest(&factory_ptr);
+        g_loader_factory_interceptor.Get().Run(&request);
+        factory->Clone(std::move(request));
+        factory = base::MakeRefCounted<network::WrapperSharedURLLoaderFactory>(
+            std::move(factory_ptr));
+      }
     } else {
       default_loader_used_ = true;
 

content/browser/loader/navigation_url_loader_impl.h

@@ -83,9 +83,10 @@ class CONTENT_EXPORT NavigationURLLoaderImpl : public NavigationURLLoader {
   static void SetBeginNavigationInterceptorForTesting(
       const BeginNavigationInterceptor& interceptor);
 
-  // Intercepts loading of frame requests when network service is enabled and a
-  // network::mojom::TrustedURLLoaderHeaderClient is being used. This must be
-  // called on the UI thread or before threads start.
+  // Intercepts loading of frame requests when network service is enabled and
+  // either a network::mojom::TrustedURLLoaderHeaderClient is being used or for
+  // schemes not handled by network service (e.g. files). This must be called on
+  // the UI thread or before threads start.
   using URLLoaderFactoryInterceptor = base::RepeatingCallback<void(
       network::mojom::URLLoaderFactoryRequest* request)>;
   static void SetURLLoaderFactoryInterceptorForTesting(

content/public/test/url_loader_interceptor.cc

@@ -140,7 +140,8 @@ class URLLoaderInterceptor::IOState
   }
 
   // Callback on IO thread whenever NavigationURLLoaderImpl needs a
-  // URLLoaderFactory with a network::mojom::TrustedURLLoaderHeaderClient.
+  // URLLoaderFactory with a network::mojom::TrustedURLLoaderHeaderClient or
+  // for a non-network-service scheme.
   void InterceptNavigationRequestCallback(
       network::mojom::URLLoaderFactoryRequest* request) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));

content/test/url_loader_interceptor_test.cc

@@ -19,6 +19,7 @@
 #include "content/shell/browser/shell.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
 #include "mojo/public/cpp/system/data_pipe_utils.h"
+#include "net/base/filename_util.h"
 #include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
 #include "services/network/public/cpp/features.h"
 #include "services/network/test/test_url_loader_client.h"
@@ -68,21 +69,43 @@ IN_PROC_BROWSER_TEST_F(URLLoaderInterceptorTest, MonitorFrame) {
 }
 
 IN_PROC_BROWSER_TEST_F(URLLoaderInterceptorTest, InterceptFrame) {
+  bool seen = false;
   GURL url = GetPageURL();
   URLLoaderInterceptor interceptor(base::BindLambdaForTesting(
       [&](URLLoaderInterceptor::RequestParams* params) {
         EXPECT_EQ(params->url_request.url, url);
         EXPECT_EQ(params->process_id, 0);
+        seen = true;
         network::URLLoaderCompletionStatus status;
         status.error_code = net::ERR_FAILED;
         params->client->OnComplete(status);
         return true;
       }));
   EXPECT_FALSE(NavigateToURL(shell(), GetPageURL()));
+  EXPECT_TRUE(seen);
+}
+
+IN_PROC_BROWSER_TEST_F(URLLoaderInterceptorTest, InterceptFrameWithFileScheme) {
+  bool seen = false;
+  base::FilePath path = GetTestFilePath(nullptr, "empty.html");
+  GURL url = net::FilePathToFileURL(path);
+  URLLoaderInterceptor interceptor(base::BindLambdaForTesting(
+      [&](URLLoaderInterceptor::RequestParams* params) {
+        EXPECT_EQ(params->url_request.url, url);
+        EXPECT_EQ(params->process_id, 0);
+        seen = true;
+        network::URLLoaderCompletionStatus status;
+        status.error_code = net::ERR_FAILED;
+        params->client->OnComplete(status);
+        return true;
+      }));
+  EXPECT_FALSE(NavigateToURL(shell(), url));
+  EXPECT_TRUE(seen);
 }
 
 IN_PROC_BROWSER_TEST_F(URLLoaderInterceptorTest,
                        AsynchronousInitializationInterceptFrame) {
+  bool seen = false;
   GURL url = GetPageURL();
   base::RunLoop run_loop;
   URLLoaderInterceptor interceptor(
@@ -90,6 +113,7 @@ IN_PROC_BROWSER_TEST_F(URLLoaderInterceptorTest,
           [&](URLLoaderInterceptor::RequestParams* params) {
             EXPECT_EQ(params->url_request.url, url);
             EXPECT_EQ(params->process_id, 0);
+            seen = true;
             network::URLLoaderCompletionStatus status;
             status.error_code = net::ERR_FAILED;
             params->client->OnComplete(status);
@@ -98,18 +122,21 @@ IN_PROC_BROWSER_TEST_F(URLLoaderInterceptorTest,
       run_loop.QuitClosure());
   run_loop.Run();
   EXPECT_FALSE(NavigateToURL(shell(), GetPageURL()));
+  EXPECT_TRUE(seen);
 }
 
 IN_PROC_BROWSER_TEST_F(URLLoaderInterceptorTest,
                        AsynchronousDestructionIsAppliedImmediately) {
   const GURL url = GetPageURL();
   {
+    bool seen = false;
     base::RunLoop run_loop;
     URLLoaderInterceptor interceptor(
         base::BindLambdaForTesting(
             [&](URLLoaderInterceptor::RequestParams* params) {
               EXPECT_EQ(params->url_request.url, url);
               EXPECT_EQ(params->process_id, 0);
+              seen = true;
               network::URLLoaderCompletionStatus status;
               status.error_code = net::ERR_FAILED;
               params->client->OnComplete(status);
@@ -119,6 +146,7 @@ IN_PROC_BROWSER_TEST_F(URLLoaderInterceptorTest,
     run_loop.Run();
 
     ASSERT_FALSE(NavigateToURL(shell(), url));
+    EXPECT_TRUE(seen);
   }
   EXPECT_TRUE(NavigateToURL(shell(), url));
 }
@@ -157,17 +185,20 @@ IN_PROC_BROWSER_TEST_F(URLLoaderInterceptorTest,
   content::ContentBrowserClient* old_browser_client =
       content::SetBrowserClientForTesting(&browser_client);
 
+  bool seen = false;
   GURL url = GetPageURL();
   URLLoaderInterceptor interceptor(base::BindLambdaForTesting(
       [&](URLLoaderInterceptor::RequestParams* params) {
         EXPECT_EQ(params->url_request.url, url);
         EXPECT_EQ(params->process_id, 0);
+        seen = true;
         network::URLLoaderCompletionStatus status;
         status.error_code = net::ERR_FAILED;
         params->client->OnComplete(status);
         return true;
       }));
   EXPECT_FALSE(NavigateToURL(shell(), GetPageURL()));
+  EXPECT_TRUE(seen);
 
   SetBrowserClientForTesting(old_browser_client);
 }
@@ -190,10 +221,12 @@ IN_PROC_BROWSER_TEST_F(URLLoaderInterceptorTest, MonitorSubresource) {
 }
 
 IN_PROC_BROWSER_TEST_F(URLLoaderInterceptorTest, InterceptSubresource) {
+  bool seen = false;
   GURL url = GetImageURL();
   URLLoaderInterceptor interceptor(base::BindLambdaForTesting(
       [&](URLLoaderInterceptor::RequestParams* params) {
         if (params->url_request.url == url) {
+          seen = true;
           network::URLLoaderCompletionStatus status;
           status.error_code = net::ERR_FAILED;
           params->client->OnComplete(status);
@@ -203,9 +236,11 @@ IN_PROC_BROWSER_TEST_F(URLLoaderInterceptorTest, InterceptSubresource) {
       }));
   Test();
   EXPECT_FALSE(DidImageLoad());
+  EXPECT_TRUE(seen);
 }
 
 IN_PROC_BROWSER_TEST_F(URLLoaderInterceptorTest, InterceptBrowser) {
+  bool seen = false;
   GURL url = GetImageURL();
   network::mojom::URLLoaderPtr loader;
   network::TestURLLoaderClient client;
@@ -214,6 +249,7 @@ IN_PROC_BROWSER_TEST_F(URLLoaderInterceptorTest, InterceptBrowser) {
 
   URLLoaderInterceptor interceptor(base::BindLambdaForTesting(
       [&](URLLoaderInterceptor::RequestParams* params) {
+        seen = true;
         EXPECT_EQ(params->url_request.url, url);
         network::URLLoaderCompletionStatus status;
         status.error_code = net::ERR_FAILED;
@@ -229,6 +265,7 @@ IN_PROC_BROWSER_TEST_F(URLLoaderInterceptorTest, InterceptBrowser) {
       net::MutableNetworkTrafficAnnotationTag(TRAFFIC_ANNOTATION_FOR_TESTS));
   client.RunUntilComplete();
   EXPECT_EQ(net::ERR_FAILED, client.completion_status().error_code);
+  EXPECT_TRUE(seen);
 }
 
 IN_PROC_BROWSER_TEST_F(URLLoaderInterceptorTest, WriteResponse) {