NetS13nSW: Get URLLoaderFactory from ResourceMessageFilter

When NetworkService is off, URLLoader should not be used when the network
request is coming from the renderer. This CL is to use URLLoaderFactoryImpl,
which internally routes requests to ResourceDispatcherHost, for
ServiceWorkerScriptLoaderFactory.

Bug: 846237
Change-Id: Ie656f55c22bcd27a7564cdb68bc448ca24e7176e
Reviewed-on: https://chromium-review.googlesource.com/1075926Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Commit-Queue: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565058}

content/browser/loader/cross_site_document_blocking_browsertest.cc

@@ -870,6 +870,11 @@ class CrossSiteDocumentBlockingServiceWorkerTest : public ContentBrowserTest {
 IN_PROC_BROWSER_TEST_F(CrossSiteDocumentBlockingServiceWorkerTest, NoNetwork) {
   SetUpServiceWorker();
 
+  // Make sure that the histograms generated by a service worker registration
+  // have been recorded.
+  if (base::FeatureList::IsEnabled(network::features::kNetworkService))
+    FetchHistogramsFromChildProcesses();
+
   base::HistogramTester histograms;
   std::string response;
   std::string script = R"(
@@ -899,6 +904,11 @@ IN_PROC_BROWSER_TEST_F(CrossSiteDocumentBlockingServiceWorkerTest,
                        NetworkToServiceWorkerResponse) {
   SetUpServiceWorker();
 
+  // Make sure that the histograms generated by a service worker registration
+  // have been recorded.
+  if (base::FeatureList::IsEnabled(network::features::kNetworkService))
+    FetchHistogramsFromChildProcesses();
+
   // Build a script for XHR-ing a cross-origin, nosniff HTML document.
   GURL cross_origin_url =
       GetURLOnCrossOriginServer("/site_isolation/nosniff.txt");

content/browser/loader/resource_dispatcher_host_impl.cc

@@ -1772,8 +1772,6 @@ void ResourceDispatcherHostImpl::OnRequestResourceWithMojo(
     network::mojom::URLLoaderRequest mojo_request,
     network::mojom::URLLoaderClientPtr url_loader_client,
     const net::NetworkTrafficAnnotationTag& traffic_annotation) {
-  DCHECK_EQ(network::mojom::kURLLoadOptionNone,
-            options & ~network::mojom::kURLLoadOptionSynchronous);
   if (!url_loader_client) {
     VLOG(1) << "Killed renderer for null client";
     bad_message::ReceivedBadMessage(requester_info->filter(),

content/browser/renderer_host/render_process_host_impl.cc

@@ -144,6 +144,7 @@
 #include "content/common/resource_messages.h"
 #include "content/common/service_manager/child_connection.h"
 #include "content/common/service_manager/service_manager_connection_impl.h"
+#include "content/common/service_worker/service_worker_utils.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/content_browser_client.h"
@@ -2113,20 +2114,6 @@ void RenderProcessHostImpl::CreateRendererHost(
   renderer_host_binding_.Bind(std::move(request));
 }
 
-void RenderProcessHostImpl::CreateURLLoaderFactory(
-    network::mojom::URLLoaderFactoryRequest request) {
-  if (!base::FeatureList::IsEnabled(network::features::kNetworkService)) {
-    NOTREACHED();
-    return;
-  }
-  network::mojom::URLLoaderFactoryParamsPtr params =
-      network::mojom::URLLoaderFactoryParams::New();
-  params->process_id = id_;
-  SiteIsolationPolicy::PopulateURLLoaderFactoryParamsPtrForCORB(params.get());
-  storage_partition_impl_->GetNetworkContext()->CreateURLLoaderFactory(
-      std::move(request), std::move(params));
-}
-
 int RenderProcessHostImpl::GetNextRoutingID() {
   return widget_helper_->GetNextRoutingID();
 }
@@ -2270,6 +2257,23 @@ RenderProcessHostImpl::GetProcessResourceCoordinator() {
   return process_resource_coordinator_.get();
 }
 
+void RenderProcessHostImpl::CreateURLLoaderFactory(
+    network::mojom::URLLoaderFactoryRequest request) {
+  if (!base::FeatureList::IsEnabled(network::features::kNetworkService)) {
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::BindOnce(&ResourceMessageFilter::Clone, resource_message_filter_,
+                       std::move(request)));
+    return;
+  }
+  network::mojom::URLLoaderFactoryParamsPtr params =
+      network::mojom::URLLoaderFactoryParams::New();
+  params->process_id = id_;
+  SiteIsolationPolicy::PopulateURLLoaderFactoryParamsPtrForCORB(params.get());
+  storage_partition_impl_->GetNetworkContext()->CreateURLLoaderFactory(
+      std::move(request), std::move(params));
+}
+
 void RenderProcessHostImpl::SetIsNeverSuitableForReuse() {
   is_never_suitable_for_reuse_ = true;
 }

content/browser/renderer_host/render_process_host_impl.h

@@ -206,6 +206,8 @@ class CONTENT_EXPORT RenderProcessHostImpl
   mojom::Renderer* GetRendererInterface() override;
   resource_coordinator::ProcessResourceCoordinator*
   GetProcessResourceCoordinator() override;
+  void CreateURLLoaderFactory(
+      network::mojom::URLLoaderFactoryRequest request) override;
 
   void SetIsNeverSuitableForReuse() override;
   bool MayReuseHost() override;
@@ -484,7 +486,6 @@ class CONTENT_EXPORT RenderProcessHostImpl
   void CreateStoragePartitionService(
       mojom::StoragePartitionServiceRequest request);
   void CreateRendererHost(mojom::RendererHostAssociatedRequest request);
-  void CreateURLLoaderFactory(network::mojom::URLLoaderFactoryRequest request);
 
   // Control message handlers.
   void OnUserMetricsRecordAction(const std::string& action);

content/browser/service_worker/embedded_worker_instance.cc

@@ -12,6 +12,7 @@
 #include "base/trace_event/trace_event.h"
 #include "content/browser/bad_message.h"
 #include "content/browser/devtools/service_worker_devtools_manager.h"
+#include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/browser/service_worker/embedded_worker_registry.h"
 #include "content/browser/service_worker/embedded_worker_status.h"
 #include "content/browser/service_worker/service_worker_content_settings_proxy_impl.h"
@@ -25,7 +26,6 @@
 #include "content/common/url_schemes.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
-#include "content/public/browser/render_process_host.h"
 #include "content/public/common/child_process_host.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/content_switches.h"
@@ -136,6 +136,19 @@ void SetupOnUIThread(base::WeakPtr<ServiceWorkerProcessManager> process_manager,
         std::move(request));
   }
 
+  // S13nServiceWorker:
+  // Create a default loader for network fallback.
+  // The factory from RPH::CreateURLLoaderFactory() doesn't support
+  // reconnection to the network service after a crash, but it's probably OK
+  // since it's used for a single service worker startup until installation
+  // finishes (with the exception of https://crbug.com/719052).
+  if (ServiceWorkerUtils::IsServicificationEnabled()) {
+    network::mojom::URLLoaderFactoryPtrInfo default_factory_info;
+    rph->CreateURLLoaderFactory(mojo::MakeRequest(&default_factory_info));
+    factory_bundle = std::make_unique<URLLoaderFactoryBundleInfo>();
+    factory_bundle->default_factory_info() = std::move(default_factory_info);
+  }
+
   // S13nServiceWorker:
   // Create the loader factories for non-http(s) URLs, for example
   // chrome-extension:// URLs. For performance, only do this step when the main
@@ -149,7 +162,6 @@ void SetupOnUIThread(base::WeakPtr<ServiceWorkerProcessManager> process_manager,
         ->RegisterNonNetworkSubresourceURLLoaderFactories(
             rph->GetID(), MSG_ROUTING_NONE, &factories);
 
-    factory_bundle = std::make_unique<URLLoaderFactoryBundleInfo>();
     for (auto& pair : factories) {
       const std::string& scheme = pair.first;
       std::unique_ptr<network::mojom::URLLoaderFactory> factory =
@@ -481,23 +493,9 @@ class EmbeddedWorkerInstance::StartTask {
     // S13nServiceWorker: Build the URLLoaderFactory for loading new scripts.
     scoped_refptr<network::SharedURLLoaderFactory> factory_for_new_scripts;
     if (ServiceWorkerUtils::IsServicificationEnabled()) {
-      if (factory_bundle) {
-        network::mojom::URLLoaderFactoryPtr network_factory_ptr;
-        // The factory from CloneNetworkFactory() doesn't support reconnection
-        // to the network service after a crash, but it's probably OK since it's
-        // used for a single service worker startup until installation finishes
-        // (with the exception of https://crbug.com/719052).
-        instance_->context_->loader_factory_getter()->CloneNetworkFactory(
-            mojo::MakeRequest(&network_factory_ptr));
-        scoped_refptr<URLLoaderFactoryBundle> factory =
-            base::MakeRefCounted<URLLoaderFactoryBundle>(
-                std::move(factory_bundle));
-        factory->SetDefaultFactory(std::move(network_factory_ptr));
-        factory_for_new_scripts = std::move(factory);
-      } else {
-        factory_for_new_scripts =
-            instance_->context_->loader_factory_getter()->GetNetworkFactory();
-      }
+      DCHECK(factory_bundle);
+      factory_for_new_scripts = base::MakeRefCounted<URLLoaderFactoryBundle>(
+          std::move(factory_bundle));
     }
 
     instance_->SendStartWorker(std::move(params),

content/browser/service_worker/service_worker_navigation_loader_unittest.cc

@@ -15,6 +15,7 @@
 #include "content/common/service_worker/service_worker_event_dispatcher.mojom.h"
 #include "content/common/service_worker/service_worker_utils.h"
 #include "content/common/single_request_url_loader_factory.h"
+#include "content/public/test/mock_render_process_host.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
 #include "mojo/public/cpp/system/data_pipe_utils.h"
@@ -210,6 +211,8 @@ class Helper : public EmbeddedWorkerTestHelper {
             base::MakeRefCounted<URLLoaderFactoryGetter>()) {
     url_loader_factory_getter()->SetNetworkFactoryForTesting(
         &mock_url_loader_factory_);
+    mock_render_process_host()->OverrideURLLoaderFactory(
+        &mock_url_loader_factory_);
   }
   ~Helper() override = default;
 

content/public/browser/render_process_host.h

@@ -22,6 +22,7 @@
 #include "ipc/ipc_channel_proxy.h"
 #include "ipc/ipc_sender.h"
 #include "media/media_buildflags.h"
+#include "services/network/public/mojom/url_loader_factory.mojom.h"
 #include "ui/gfx/native_widget_types.h"
 
 #if defined(OS_ANDROID)
@@ -385,6 +386,16 @@ class CONTENT_EXPORT RenderProcessHost : public IPC::Sender,
   virtual resource_coordinator::ProcessResourceCoordinator*
   GetProcessResourceCoordinator() = 0;
 
+  // Create an URLLoaderFactory for this process.
+  // When NetworkService is enabled, |request| will be bound with a new
+  // URLLoaderFactory created from the storage partition's Network Context. Note
+  // that the URLLoaderFactory returned by this method does NOT support
+  // auto-reconnect after a crash of Network Service.
+  // When NetworkService is not enabled, |request| will be bound with a
+  // URLLoaderFactory which routes requests to ResourceDispatcherHost.
+  virtual void CreateURLLoaderFactory(
+      network::mojom::URLLoaderFactoryRequest request) = 0;
+
   // Whether this process is locked out from ever being reused for sites other
   // than the ones it currently has.
   virtual void SetIsNeverSuitableForReuse() = 0;

content/public/test/mock_render_process_host.cc

@@ -392,6 +392,11 @@ MockRenderProcessHost::GetProcessResourceCoordinator() {
   return process_resource_coordinator_.get();
 }
 
+void MockRenderProcessHost::CreateURLLoaderFactory(
+    network::mojom::URLLoaderFactoryRequest request) {
+  url_loader_factory_->Clone(std::move(request));
+}
+
 void MockRenderProcessHost::SetIsNeverSuitableForReuse() {
   is_never_suitable_for_reuse_ = true;
 }
@@ -464,6 +469,11 @@ void MockRenderProcessHost::OverrideRendererInterfaceForTesting(
   renderer_interface_ = std::move(renderer_interface);
 }
 
+void MockRenderProcessHost::OverrideURLLoaderFactory(
+    network::mojom::URLLoaderFactory* factory) {
+  url_loader_factory_ = factory;
+}
+
 MockRenderProcessHostFactory::MockRenderProcessHostFactory() {}
 
 MockRenderProcessHostFactory::~MockRenderProcessHostFactory() {

content/public/test/mock_render_process_host.h

@@ -134,6 +134,8 @@ class MockRenderProcessHost : public RenderProcessHost {
   mojom::Renderer* GetRendererInterface() override;
   resource_coordinator::ProcessResourceCoordinator*
   GetProcessResourceCoordinator() override;
+  void CreateURLLoaderFactory(
+      network::mojom::URLLoaderFactoryRequest request) override;
 
   void SetIsNeverSuitableForReuse() override;
   bool MayReuseHost() override;
@@ -175,6 +177,8 @@ class MockRenderProcessHost : public RenderProcessHost {
       std::unique_ptr<mojo::AssociatedInterfacePtr<mojom::Renderer>>
           renderer_interface);
 
+  void OverrideURLLoaderFactory(network::mojom::URLLoaderFactory* factory);
+
   bool is_renderer_locked_to_site() const {
     return is_renderer_locked_to_site_;
   }
@@ -207,6 +211,7 @@ class MockRenderProcessHost : public RenderProcessHost {
       process_resource_coordinator_;
   service_manager::Identity child_identity_;
   bool is_renderer_locked_to_site_ = false;
+  network::mojom::URLLoaderFactory* url_loader_factory_;
   base::WeakPtrFactory<MockRenderProcessHost> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(MockRenderProcessHost);

third_party/WebKit/LayoutTests/TestExpectations

@@ -1858,7 +1858,17 @@ crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/xmlhttpreques
 
 crbug.com/771118 virtual/service-worker-servicification/external/wpt/service-workers/service-worker/mime-sniffing.https.html [ Failure ]
 
-crbug.com/846237 virtual/service-worker-servicification/external/wpt/service-workers/service-worker/import-scripts-updated-flag.https.html [ Crash ]
+crbug.com/848606 virtual/service-worker-servicification/external/wpt/service-workers/service-worker/claim-affect-other-registration.https.html [ Timeout ]
+crbug.com/848606 virtual/service-worker-servicification/external/wpt/service-workers/service-worker/claim-not-using-registration.https.html [ Timeout ]
+crbug.com/848606 virtual/service-worker-servicification/external/wpt/service-workers/service-worker/claim-using-registration.https.html [ Timeout ]
+crbug.com/848606 virtual/service-worker-servicification/external/wpt/service-workers/service-worker/register-default-scope.https.html [ Timeout ]
+crbug.com/848606 virtual/service-worker-servicification/external/wpt/service-workers/service-worker/registration-mime-types.https.html [ Timeout ]
+crbug.com/848606 virtual/service-worker-servicification/external/wpt/service-workers/service-worker/service-worker-csp-connect.https.html [ Timeout ]
+crbug.com/848606 virtual/service-worker-servicification/external/wpt/service-workers/service-worker/skip-waiting-installed.https.html [ Timeout ]
+crbug.com/848606 virtual/service-worker-servicification/external/wpt/service-workers/service-worker/skip-waiting-using-registration.https.html [ Timeout ]
+crbug.com/848606 virtual/service-worker-servicification/external/wpt/service-workers/service-worker/skip-waiting.https.html [ Timeout ]
+crbug.com/848606 virtual/service-worker-servicification/external/wpt/service-workers/service-worker/update-result.https.html [ Timeout ]
+crbug.com/848606 virtual/service-worker-servicification/http/tests/serviceworker/csp-fetch-from-installed-service-worker-connect.html [ Timeout ]
 
 crbug.com/492664 [ Linux ] external/wpt/css/css-writing-modes/box-offsets-rel-pos-vlr-005.xht [ Failure ]
 crbug.com/492664 [ Linux ] external/wpt/css/css-writing-modes/box-offsets-rel-pos-vrl-004.xht [ Failure ]
@@ -3915,7 +3925,6 @@ crbug.com/716320 virtual/sharedarraybuffer/external/wpt/html/infrastructure/safe
 crbug.com/688486 external/wpt/service-workers/service-worker/fetch-request-resources.https.html [ Failure Pass ]
 crbug.com/688486 virtual/service-worker-servicification/external/wpt/service-workers/service-worker/fetch-request-resources.https.html [ Failure Pass ]
 crbug.com/831509 external/wpt/service-workers/service-worker/skip-waiting-installed.https.html [ Failure Pass ]
-crbug.com/831509 virtual/service-worker-servicification/external/wpt/service-workers/service-worker/skip-waiting-installed.https.html [ Failure Pass ]
 crbug.com/831509 virtual/navigation-mojo-response/external/wpt/service-workers/service-worker/skip-waiting-installed.https.html [ Failure Pass ]
 
 # Sheriff failures 2017-02-21

third_party/WebKit/LayoutTests/virtual/service-worker-servicification/http/tests/serviceworker/register-error-messages-expected.txt

+This is a testharness.js-based test.
+PASS Registering same scope as the script directory without the last slash
+PASS Registration scope outside the script directory
+PASS Registration scope outside domain
+PASS Registering script outside domain
+PASS Registering non-existent script
+PASS Registering invalid chunked encoding script
+FAIL Registering script with no MIME type assert_equals: expected "Failed to register a ServiceWorker: The script does not have a MIME type." but got "Failed to register a ServiceWorker: The script has an unsupported MIME type ('text/plain')."
+PASS Registering script with bad MIME type
+PASS Registering redirected script
+PASS Registering script including parse error
+PASS Registering script including undefined error
+PASS Registering script including uncaught exception
+PASS Registering script importing malformed script
+PASS Registering script importing non-existent script
+PASS Script URL including URL-encoded slash
+Harness: the test ran to completion.
+