Use NotImplementedNetworkURLLoaderFactory in MockRenderProcessHostFactory

After crrev.com/c/1272419, all unittests which trigers navigation
requires MockRenderProcessHost::url_loader_factory_ when
ServiceWorkerServicification is enabled.
This is causing many crashes in content_unittests.

To fix this, this CL introduces NotImplementedNetworkURLLoaderFactory and use
it as default of MockRenderProcessHost's url_loader_factory_.

Bug: 895187
Change-Id: I8119845cf73d0a97297efc473db87b84021cf71d
Reviewed-on: https://chromium-review.googlesource.com/c/1280072Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599816}

content/browser/shared_worker/shared_worker_host_unittest.cc

@@ -21,6 +21,7 @@
 #include "content/public/test/test_browser_context.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "content/public/test/test_utils.h"
+#include "content/test/not_implemented_network_url_loader_factory.h"
 #include "mojo/public/cpp/bindings/binding.h"
 #include "mojo/public/cpp/bindings/strong_associated_binding.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
@@ -35,49 +36,10 @@ using blink::MessagePortChannel;
 
 namespace content {
 
-namespace {
-
-// A mock URLLoaderFactory which just fails to create a loader. This is
-// sufficient because the tests don't exercise script loading.
-class NotImplementedNetworkURLLoaderFactory final
-    : public network::mojom::URLLoaderFactory {
- public:
-  NotImplementedNetworkURLLoaderFactory() = default;
-
-  // network::mojom::URLLoaderFactory implementation.
-  void CreateLoaderAndStart(network::mojom::URLLoaderRequest request,
-                            int32_t routing_id,
-                            int32_t request_id,
-                            uint32_t options,
-                            const network::ResourceRequest& url_request,
-                            network::mojom::URLLoaderClientPtr client,
-                            const net::MutableNetworkTrafficAnnotationTag&
-                                traffic_annotation) override {
-    network::URLLoaderCompletionStatus status;
-    status.error_code = net::ERR_NOT_IMPLEMENTED;
-    client->OnComplete(status);
-  }
-
-  void Clone(network::mojom::URLLoaderFactoryRequest request) override {
-    bindings_.AddBinding(this, std::move(request));
-  }
-
- private:
-  mojo::BindingSet<network::mojom::URLLoaderFactory> bindings_;
-
-  DISALLOW_COPY_AND_ASSIGN(NotImplementedNetworkURLLoaderFactory);
-};
-
-}  // namespace
-
 class SharedWorkerHostTest : public testing::Test {
  public:
   void SetUp() override {
     helper_.reset(new EmbeddedWorkerTestHelper(base::FilePath()));
-    mock_url_loader_factory_ =
-        std::make_unique<NotImplementedNetworkURLLoaderFactory>();
-    mock_render_process_host_.OverrideURLLoaderFactory(
-        mock_url_loader_factory_.get());
   }
 
   SharedWorkerHostTest()
@@ -179,9 +141,6 @@ class SharedWorkerHostTest : public testing::Test {
  protected:
   TestBrowserThreadBundle test_browser_thread_bundle_;
   TestBrowserContext browser_context_;
-  // This URLLoaderFactory is used in MockRenderProcessHost.
-  std::unique_ptr<NotImplementedNetworkURLLoaderFactory>
-      mock_url_loader_factory_;
   MockRenderProcessHost mock_render_process_host_;
   std::unique_ptr<EmbeddedWorkerTestHelper> helper_;
 

content/browser/shared_worker/shared_worker_service_impl_unittest.cc

@@ -17,6 +17,7 @@
 #include "content/public/test/mock_render_process_host.h"
 #include "content/public/test/test_browser_context.h"
 #include "content/public/test/test_utils.h"
+#include "content/test/not_implemented_network_url_loader_factory.h"
 #include "content/test/test_render_frame_host.h"
 #include "content/test/test_render_view_host.h"
 #include "content/test/test_web_contents.h"
@@ -30,42 +31,6 @@ using blink::MessagePortChannel;
 
 namespace content {
 
-namespace {
-
-// A mock URLLoaderFactory which just fails to create a loader. This is
-// sufficient because the tests don't exercise script loading. Used when
-// S13nServiceWorker is enabled.
-class NotImplementedNetworkURLLoaderFactory final
-    : public network::mojom::URLLoaderFactory {
- public:
-  NotImplementedNetworkURLLoaderFactory() = default;
-
-  // network::mojom::URLLoaderFactory implementation.
-  void CreateLoaderAndStart(network::mojom::URLLoaderRequest request,
-                            int32_t routing_id,
-                            int32_t request_id,
-                            uint32_t options,
-                            const network::ResourceRequest& url_request,
-                            network::mojom::URLLoaderClientPtr client,
-                            const net::MutableNetworkTrafficAnnotationTag&
-                                traffic_annotation) override {
-    network::URLLoaderCompletionStatus status;
-    status.error_code = net::ERR_NOT_IMPLEMENTED;
-    client->OnComplete(status);
-  }
-
-  void Clone(network::mojom::URLLoaderFactoryRequest request) override {
-    bindings_.AddBinding(this, std::move(request));
-  }
-
- private:
-  mojo::BindingSet<network::mojom::URLLoaderFactory> bindings_;
-
-  DISALLOW_COPY_AND_ASSIGN(NotImplementedNetworkURLLoaderFactory);
-};
-
-}  // namespace
-
 class SharedWorkerServiceImplTest : public RenderViewHostImplTestHarness {
  public:
   mojom::SharedWorkerConnectorPtr MakeSharedWorkerConnector(

content/public/test/mock_render_process_host.cc

@@ -34,8 +34,10 @@
 #include "content/public/browser/storage_partition.h"
 #include "content/public/common/service_manager_connection.h"
 #include "content/public/common/service_names.mojom.h"
+#include "content/test/not_implemented_network_url_loader_factory.h"
 #include "media/media_buildflags.h"
 #include "mojo/public/cpp/bindings/associated_interface_ptr.h"
+#include "services/network/public/mojom/url_loader_factory.mojom.h"
 #include "services/resource_coordinator/public/mojom/coordination_unit.mojom.h"
 
 namespace content {
@@ -489,7 +491,9 @@ void MockRenderProcessHost::OverrideURLLoaderFactory(
   url_loader_factory_ = factory;
 }
 
-MockRenderProcessHostFactory::MockRenderProcessHostFactory() {}
+MockRenderProcessHostFactory::MockRenderProcessHostFactory()
+    : default_mock_url_loader_factory_(
+          std::make_unique<NotImplementedNetworkURLLoaderFactory>()) {}
 
 MockRenderProcessHostFactory::~MockRenderProcessHostFactory() {
   // Detach this object from MockRenderProcesses to prevent them from calling
@@ -501,8 +505,10 @@ MockRenderProcessHostFactory::~MockRenderProcessHostFactory() {
 RenderProcessHost* MockRenderProcessHostFactory::CreateRenderProcessHost(
     BrowserContext* browser_context,
     SiteInstance* site_instance) const {
-  processes_.push_back(
-      std::make_unique<MockRenderProcessHost>(browser_context));
+  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

@@ -34,6 +34,14 @@
 #include "content/public/browser/android/child_process_importance.h"
 #endif
 
+namespace network {
+namespace mojom {
+
+class URLLoaderFactory;
+
+}  // namespace mojom
+}  // namespace network
+
 namespace content {
 
 class MockRenderProcessHostFactory;
@@ -248,6 +256,10 @@ class MockRenderProcessHostFactory : public RenderProcessHostFactory {
   // the destructor and prevent them from being leaked.
   mutable std::vector<std::unique_ptr<MockRenderProcessHost>> processes_;
 
+  // A mock URLLoaderFactory which just fails to create a loader.
+  std::unique_ptr<network::mojom::URLLoaderFactory>
+      default_mock_url_loader_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(MockRenderProcessHostFactory);
 };
 

content/test/BUILD.gn

@@ -249,6 +249,8 @@ jumbo_static_library("test_support") {
     "mock_widget_input_handler.h",
     "net/url_request_abort_on_end_job.cc",
     "net/url_request_abort_on_end_job.h",
+    "not_implemented_network_url_loader_factory.cc",
+    "not_implemented_network_url_loader_factory.h",
     "ppapi_unittest.cc",
     "ppapi_unittest.h",
     "storage_partition_test_utils.cc",

content/test/not_implemented_network_url_loader_factory.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/test/not_implemented_network_url_loader_factory.h"
+
+namespace content {
+
+NotImplementedNetworkURLLoaderFactory::NotImplementedNetworkURLLoaderFactory() =
+    default;
+
+NotImplementedNetworkURLLoaderFactory::
+    ~NotImplementedNetworkURLLoaderFactory() = default;
+
+void NotImplementedNetworkURLLoaderFactory::CreateLoaderAndStart(
+    network::mojom::URLLoaderRequest request,
+    int32_t routing_id,
+    int32_t request_id,
+    uint32_t options,
+    const network::ResourceRequest& url_request,
+    network::mojom::URLLoaderClientPtr client,
+    const net::MutableNetworkTrafficAnnotationTag& traffic_annotation) {
+  network::URLLoaderCompletionStatus status;
+  status.error_code = net::ERR_NOT_IMPLEMENTED;
+  client->OnComplete(status);
+}
+
+void NotImplementedNetworkURLLoaderFactory::Clone(
+    network::mojom::URLLoaderFactoryRequest request) {
+  bindings_.AddBinding(this, std::move(request));
+}
+
+}  // namespace content

content/test/not_implemented_network_url_loader_factory.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CONTENT_TEST_NOT_IMPLEMENTED_NETWORK_URL_LOADER_FACTORY_H_
+#define CONTENT_TEST_NOT_IMPLEMENTED_NETWORK_URL_LOADER_FACTORY_H_
+
+#include "mojo/public/cpp/bindings/binding_set.h"
+#include "services/network/public/mojom/url_loader_factory.mojom.h"
+
+namespace content {
+
+// A mock URLLoaderFactory which just fails to create a loader.
+class NotImplementedNetworkURLLoaderFactory final
+    : public network::mojom::URLLoaderFactory {
+ public:
+  NotImplementedNetworkURLLoaderFactory();
+  ~NotImplementedNetworkURLLoaderFactory() override;
+
+  // network::mojom::URLLoaderFactory implementation.
+  void CreateLoaderAndStart(network::mojom::URLLoaderRequest request,
+                            int32_t routing_id,
+                            int32_t request_id,
+                            uint32_t options,
+                            const network::ResourceRequest& url_request,
+                            network::mojom::URLLoaderClientPtr client,
+                            const net::MutableNetworkTrafficAnnotationTag&
+                                traffic_annotation) override;
+
+  void Clone(network::mojom::URLLoaderFactoryRequest request) override;
+
+ private:
+  mojo::BindingSet<network::mojom::URLLoaderFactory> bindings_;
+
+  DISALLOW_COPY_AND_ASSIGN(NotImplementedNetworkURLLoaderFactory);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_TEST_NOT_IMPLEMENTED_NETWORK_URL_LOADER_FACTORY_H_