Fix for URLLoaderInterceptor with StoragePartition

Ensure we recreate the SharedURLLoaderFactory used by StoragePartition
whenever SetGetURLLoaderFactoryForBrowserProcessCallbackForTesting() is
called. This makes the URLLoaderInterceptor work properly with the
SharedURLLoaderFactory returned by
StoragePartition::GetURLLoaderFactoryForBrowserProcess().

Bug: 857250
Change-Id: Id913ba93322dc62c2c03d8acd4c31b53fc8d436e
Reviewed-on: https://chromium-review.googlesource.com/1117822
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570995}

content/browser/storage_partition_impl.cc

@@ -1235,9 +1235,12 @@ void StoragePartitionImpl::GetQuotaSettings(
 
 network::mojom::URLLoaderFactory*
 StoragePartitionImpl::GetURLLoaderFactoryForBrowserProcessInternal() {
-  // Create the URLLoaderFactory as needed.
+  // Create the URLLoaderFactory as needed, but make sure not to reuse a
+  // previously created one if the test override has changed.
   if (url_loader_factory_for_browser_process_ &&
-      !url_loader_factory_for_browser_process_.encountered_error()) {
+      !url_loader_factory_for_browser_process_.encountered_error() &&
+      is_test_url_loader_factory_for_browser_process_ !=
+          g_url_loader_factory_callback_for_test.Get().is_null()) {
     return url_loader_factory_for_browser_process_.get();
   }
 
@@ -1251,6 +1254,7 @@ StoragePartitionImpl::GetURLLoaderFactoryForBrowserProcessInternal() {
         browser_context(), nullptr, false /* is_navigation */, &request);
     GetNetworkContext()->CreateURLLoaderFactory(std::move(request),
                                                 std::move(params));
+    is_test_url_loader_factory_for_browser_process_ = false;
     return url_loader_factory_for_browser_process_.get();
   }
 
@@ -1260,6 +1264,7 @@ StoragePartitionImpl::GetURLLoaderFactoryForBrowserProcessInternal() {
   url_loader_factory_for_browser_process_ =
       g_url_loader_factory_callback_for_test.Get().Run(
           std::move(original_factory));
+  is_test_url_loader_factory_for_browser_process_ = true;
   return url_loader_factory_for_browser_process_.get();
 }
 

content/browser/storage_partition_impl.h

@@ -331,7 +331,8 @@ class CONTENT_EXPORT StoragePartitionImpl
   // StoragePartition::GetURLLoaderFactoryForBrowserProcess() for
   // more details
   network::mojom::URLLoaderFactoryPtr url_loader_factory_for_browser_process_;
-  ::network::mojom::CookieManagerPtr cookie_manager_for_browser_process_;
+  bool is_test_url_loader_factory_for_browser_process_ = false;
+  network::mojom::CookieManagerPtr cookie_manager_for_browser_process_;
 
   // When the network service is disabled, a NetworkContext is created on the IO
   // thread that wraps access to the URLRequestContext.

content/browser/storage_partition_impl_browsertest.cc

@@ -6,20 +6,25 @@
 
 #include <string>
 
+#include "base/test/bind_test_util.h"
 #include "base/test/scoped_feature_list.h"
 #include "build/build_config.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/content_browser_test.h"
+#include "content/public/test/simple_url_loader_test_helper.h"
+#include "content/public/test/url_loader_interceptor.h"
 #include "content/shell/browser/shell.h"
 #include "content/shell/browser/shell_browser_context.h"
 #include "content/test/storage_partition_test_utils.h"
 #include "mojo/public/cpp/bindings/interface_request.h"
 #include "net/http/http_response_headers.h"
+#include "net/http/http_status_code.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
 #include "services/network/public/cpp/features.h"
 #include "services/network/public/cpp/resource_response_info.h"
+#include "services/network/public/cpp/simple_url_loader.h"
 #include "services/network/public/mojom/network_service.mojom.h"
 #include "services/network/public/mojom/url_loader.mojom.h"
 #include "services/network/public/mojom/url_loader_factory.mojom.h"
@@ -29,6 +34,8 @@
 
 namespace content {
 
+namespace {
+
 enum class NetworkServiceState {
   kDisabled,
   kEnabled,
@@ -54,6 +61,39 @@ class StoragePartititionImplBrowsertest
   base::test::ScopedFeatureList feature_list_;
 };
 
+// Creates a SimpleURLLoader and starts it to download |url|. Blocks until the
+// load is complete.
+std::unique_ptr<network::SimpleURLLoader> DownloadUrl(
+    const GURL& url,
+    StoragePartition* partition) {
+  auto request = std::make_unique<network::ResourceRequest>();
+  request->url = url;
+  std::unique_ptr<network::SimpleURLLoader> url_loader =
+      network::SimpleURLLoader::Create(std::move(request),
+                                       NO_TRAFFIC_ANNOTATION_YET);
+  SimpleURLLoaderTestHelper url_loader_helper;
+  url_loader->DownloadToString(
+      partition->GetURLLoaderFactoryForBrowserProcess().get(),
+      url_loader_helper.GetCallback(),
+      /*max_body_size=*/1024 * 1024);
+  url_loader_helper.WaitForCallback();
+  return url_loader;
+}
+
+void CheckSimpleURLLoaderState(network::SimpleURLLoader* url_loader,
+                               int net_error,
+                               net::HttpStatusCode http_status_code) {
+  EXPECT_EQ(net_error, url_loader->NetError());
+  if (net_error != net::OK)
+    return;
+  ASSERT_TRUE(url_loader->ResponseInfo());
+  ASSERT_TRUE(url_loader->ResponseInfo()->headers);
+  EXPECT_EQ(http_status_code,
+            url_loader->ResponseInfo()->headers->response_code());
+}
+
+}  // namespace
+
 // Make sure that the NetworkContext returned by a StoragePartition works, both
 // with the network service enabled and with it disabled, when one is created
 // that wraps the URLRequestContext created by the BrowserContext.
@@ -158,6 +198,54 @@ IN_PROC_BROWSER_TEST_P(StoragePartititionImplBrowsertest,
             factory_owner->LoadBasicRequestOnIOThread(GetTestURL()));
 }
 
+// Checks that the network::URLLoaderIntercpetor works as expected with the
+// SharedURLLoaderFactory returned by StoragePartititionImpl.
+IN_PROC_BROWSER_TEST_P(StoragePartititionImplBrowsertest,
+                       URLLoaderInterceptor) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  const GURL kEchoUrl(embedded_test_server()->GetURL("/echo"));
+
+  base::ScopedAllowBlockingForTesting allow_blocking;
+  std::unique_ptr<ShellBrowserContext> browser_context =
+      std::make_unique<ShellBrowserContext>(true, nullptr);
+  auto* partition =
+      BrowserContext::GetDefaultStoragePartition(browser_context.get());
+
+  // Run a request the first time without the interceptor set, as the
+  // StoragePartitionImpl lazily creates the factory and we want to make sure
+  // it will create a new one once the interceptor is set (and not simply reuse
+  // the cached one).
+  {
+    std::unique_ptr<network::SimpleURLLoader> url_loader =
+        DownloadUrl(kEchoUrl, partition);
+    CheckSimpleURLLoaderState(url_loader.get(), net::OK, net::HTTP_OK);
+  }
+
+  // Use a URLLoaderInterceptor to simulate an error.
+  {
+    URLLoaderInterceptor interceptor(base::BindLambdaForTesting(
+        [&](URLLoaderInterceptor::RequestParams* params) -> bool {
+          if (params->url_request.url != kEchoUrl)
+            return false;
+          params->client->OnComplete(
+              network::URLLoaderCompletionStatus(net::ERR_NOT_IMPLEMENTED));
+          return true;
+        }));
+    std::unique_ptr<network::SimpleURLLoader> url_loader =
+        DownloadUrl(kEchoUrl, partition);
+    CheckSimpleURLLoaderState(url_loader.get(), net::ERR_NOT_IMPLEMENTED,
+                              net::HTTP_OK);
+  }
+
+  // Run one more time without the interceptor, we should be back to the
+  // original behavior.
+  {
+    std::unique_ptr<network::SimpleURLLoader> url_loader =
+        DownloadUrl(kEchoUrl, partition);
+    CheckSimpleURLLoaderState(url_loader.get(), net::OK, net::HTTP_OK);
+  }
+}
+
 // NetworkServiceState::kEnabled currently DCHECKs on Android, as Android isn't
 // expected to create extra processes.
 #if defined(OS_ANDROID)