Remove MockRenderProcessHost::OverrideURLLoaderFactory().

This function was confusing because there is also SetNetworkFactory(),
which overrode it. The only callsites were to give a fake factory in
order to prevent the test from crashing with a null factory. This CL
instead makes all MockRenderProcesHosts have a default factory, and
removes the callsites.

Bug: 1001435
Change-Id: Ib0fd1a3ab8ba7987a1c0b5aac8c5eba9311f7878
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1981237Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#727343}

content/browser/service_worker/embedded_worker_test_helper.cc

@@ -61,18 +61,6 @@ EmbeddedWorkerTestHelper::EmbeddedWorkerTestHelper(
       blink::mojom::EmbeddedWorkerInstanceClient::Name_,
       base::BindRepeating(&EmbeddedWorkerTestHelper::OnInstanceClientRequest,
                           base::Unretained(this)));
-
-  // Set a basic network URL loader factory so tests don't crash. Tests that
-  // want to customize further should use URLLoaderInterceptor which will
-  // override this.
-  // TODO(falken): Just make all MockRenderProcessHosts create and own
-  // their own url loader factory.
-  default_network_loader_factory_ =
-      std::make_unique<FakeNetworkURLLoaderFactory>();
-  render_process_host_->OverrideURLLoaderFactory(
-      default_network_loader_factory_.get());
-  new_render_process_host_->OverrideURLLoaderFactory(
-      default_network_loader_factory_.get());
 }
 
 void EmbeddedWorkerTestHelper::AddPendingInstanceClient(

content/browser/service_worker/embedded_worker_test_helper.h

@@ -23,7 +23,6 @@
 
 namespace content {
 
-class FakeNetworkURLLoaderFactory;
 class FakeServiceWorker;
 class MockRenderProcessHost;
 class ServiceWorkerContextCore;
@@ -182,7 +181,6 @@ class EmbeddedWorkerTestHelper {
   int new_mock_render_process_id_;
 
   scoped_refptr<URLLoaderFactoryGetter> url_loader_factory_getter_;
-  std::unique_ptr<FakeNetworkURLLoaderFactory> default_network_loader_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(EmbeddedWorkerTestHelper);
 };

content/browser/worker_host/shared_worker_host_unittest.cc

@@ -44,15 +44,10 @@ class SharedWorkerHostTest : public testing::Test {
   }
 
   SharedWorkerHostTest()
-      : default_mock_url_loader_factory_(
-            std::make_unique<NotImplementedNetworkURLLoaderFactory>()),
-        mock_render_process_host_(&browser_context_),
+      : mock_render_process_host_(&browser_context_),
         service_(nullptr /* storage_partition */,
                  nullptr /* service_worker_context */,
-                 nullptr /* appcache_service */) {
-    mock_render_process_host_.OverrideURLLoaderFactory(
-        default_mock_url_loader_factory_.get());
-  }
+                 nullptr /* appcache_service */) {}
 
   base::WeakPtr<SharedWorkerHost> CreateHost() {
     GURL url("http://www.example.com/w.js");
@@ -144,8 +139,6 @@ class SharedWorkerHostTest : public testing::Test {
  protected:
   BrowserTaskEnvironment task_environment_;
   TestBrowserContext browser_context_;
-  std::unique_ptr<network::mojom::URLLoaderFactory>
-      default_mock_url_loader_factory_;
   MockRenderProcessHost mock_render_process_host_;
   std::unique_ptr<EmbeddedWorkerTestHelper> helper_;
 

content/public/test/mock_render_process_host.cc

@@ -35,7 +35,7 @@
 #include "content/public/browser/render_widget_host_iterator.h"
 #include "content/public/browser/site_instance.h"
 #include "content/public/browser/storage_partition.h"
-#include "content/test/not_implemented_network_url_loader_factory.h"
+#include "content/test/fake_network_url_loader_factory.h"
 #include "media/media_buildflags.h"
 #include "services/network/public/mojom/network_context.mojom.h"
 #include "services/network/public/mojom/url_loader_factory.mojom.h"
@@ -67,7 +67,7 @@ MockRenderProcessHost::MockRenderProcessHost(BrowserContext* browser_context)
       is_unused_(true),
       keep_alive_ref_count_(0),
       foreground_service_worker_count_(0),
-      url_loader_factory_(nullptr) {
+      url_loader_factory_(std::make_unique<FakeNetworkURLLoaderFactory>()) {
   // Child process security operations can't be unit tested unless we add
   // ourselves as an existing child process.
   ChildProcessSecurityPolicyImpl::GetInstance()->Add(GetID(), browser_context);
@@ -500,14 +500,7 @@ void MockRenderProcessHost::OverrideRendererInterfaceForTesting(
   renderer_interface_ = std::move(renderer_interface);
 }
 
-void MockRenderProcessHost::OverrideURLLoaderFactory(
-    network::mojom::URLLoaderFactory* factory) {
-  url_loader_factory_ = factory;
-}
-
-MockRenderProcessHostFactory::MockRenderProcessHostFactory()
-    : default_mock_url_loader_factory_(
-          std::make_unique<NotImplementedNetworkURLLoaderFactory>()) {}
+MockRenderProcessHostFactory::MockRenderProcessHostFactory() = default;
 
 MockRenderProcessHostFactory::~MockRenderProcessHostFactory() {
   // Detach this object from MockRenderProcesses to prevent them from calling
@@ -521,7 +514,6 @@ RenderProcessHost* MockRenderProcessHostFactory::CreateRenderProcessHost(
     SiteInstance* site_instance) {
   std::unique_ptr<MockRenderProcessHost> host =
       std::make_unique<MockRenderProcessHost>(browser_context);
-  host->OverrideURLLoaderFactory(default_mock_url_loader_factory_.get());
   processes_.push_back(std::move(host));
   processes_.back()->SetFactory(this);
   return processes_.back().get();

content/public/test/mock_render_process_host.h

@@ -247,10 +247,6 @@ class MockRenderProcessHost : public RenderProcessHost {
       std::unique_ptr<mojo::AssociatedRemote<mojom::Renderer>>
           renderer_interface);
 
-  // SetNetworkFactory() wins over this.
-  // TODO(falken): Remove this method or call it SetURLLoaderFactory().
-  void OverrideURLLoaderFactory(network::mojom::URLLoaderFactory* factory);
-
   bool is_renderer_locked_to_site() const {
     return is_renderer_locked_to_site_;
   }
@@ -283,7 +279,7 @@ class MockRenderProcessHost : public RenderProcessHost {
   std::unique_ptr<mojo::AssociatedRemote<mojom::Renderer>> renderer_interface_;
   std::map<std::string, InterfaceBinder> binder_overrides_;
   bool is_renderer_locked_to_site_ = false;
-  network::mojom::URLLoaderFactory* url_loader_factory_;
+  std::unique_ptr<network::mojom::URLLoaderFactory> url_loader_factory_;
   mojo::PendingReceiver<blink::mojom::CacheStorage> cache_storage_receiver_;
   mojo::PendingReceiver<blink::mojom::IDBFactory> idb_factory_receiver_;
   base::WeakPtrFactory<MockRenderProcessHost> weak_ptr_factory_{this};