Refactoring: Simplify script loader factories for service/shared workers.

Pass the network factory instead of the URLLoaderFactoryGetter, as
they only need the network factory.

Change-Id: Ie421f779d18edec9217bc8f3a5fe4946b75719d6
Reviewed-on: https://chromium-review.googlesource.com/1058746
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558647}

content/browser/service_worker/service_worker_new_script_loader.cc

@@ -35,8 +35,8 @@ ServiceWorkerNewScriptLoader::ServiceWorkerNewScriptLoader(
     const network::ResourceRequest& original_request,
     network::mojom::URLLoaderClientPtr client,
     scoped_refptr<ServiceWorkerVersion> version,
-    scoped_refptr<URLLoaderFactoryGetter> loader_factory_getter,
-    network::mojom::URLLoaderFactoryPtr non_network_loader_factory,
+    scoped_refptr<network::SharedURLLoaderFactory> network_factory,
+    network::mojom::URLLoaderFactoryPtr non_network_factory,
     const net::MutableNetworkTrafficAnnotationTag& traffic_annotation)
     : request_url_(original_request.url),
       resource_type_(static_cast<ResourceType>(original_request.resource_type)),
@@ -46,7 +46,8 @@ ServiceWorkerNewScriptLoader::ServiceWorkerNewScriptLoader(
       network_watcher_(FROM_HERE,
                        mojo::SimpleWatcher::ArmingPolicy::MANUAL,
                        base::SequencedTaskRunnerHandle::Get()),
-      non_network_loader_factory_(std::move(non_network_loader_factory)),
+      network_factory_(std::move(network_factory)),
+      non_network_factory_(std::move(non_network_factory)),
       client_(std::move(client)),
       weak_factory_(this) {
   // ServiceWorkerNewScriptLoader is used for fetching the service worker main
@@ -113,13 +114,13 @@ ServiceWorkerNewScriptLoader::ServiceWorkerNewScriptLoader(
 
   network::mojom::URLLoaderClientPtr network_client;
   network_client_binding_.Bind(mojo::MakeRequest(&network_client));
-  if (non_network_loader_factory_) {
-    non_network_loader_factory_->CreateLoaderAndStart(
+  if (non_network_factory_) {
+    non_network_factory_->CreateLoaderAndStart(
         mojo::MakeRequest(&network_loader_), routing_id, request_id, options,
         *resource_request_.get(), std::move(network_client),
         traffic_annotation);
   } else {
-    loader_factory_getter->GetNetworkFactory()->CreateLoaderAndStart(
+    network_factory_->CreateLoaderAndStart(
         mojo::MakeRequest(&network_loader_), routing_id, request_id, options,
         *resource_request_.get(), std::move(network_client),
         traffic_annotation);

content/browser/service_worker/service_worker_new_script_loader.h

@@ -19,7 +19,6 @@ namespace content {
 
 class ServiceWorkerCacheWriter;
 class ServiceWorkerVersion;
-class URLLoaderFactoryGetter;
 struct HttpResponseInfoIOBuffer;
 
 // S13nServiceWorker:
@@ -45,7 +44,7 @@ struct HttpResponseInfoIOBuffer;
 // worker. If the script is identical, the load succeeds but no script is
 // written, and ServiceWorkerVersion is told to terminate startup.
 //
-// NOTE: To load a script, this class uses |non_network_loader_factory| when the
+// NOTE: To load a script, this class uses |non_network_factory| when the
 // URL has a non-http(s) scheme, e.g., a chrome-extension:// URL. Regardless,
 // that is still called a "network" request in comments and naming. "network" is
 // meant to distinguish from the load this URLLoader does for its client:
@@ -55,10 +54,9 @@ class CONTENT_EXPORT ServiceWorkerNewScriptLoader
     : public network::mojom::URLLoader,
       public network::mojom::URLLoaderClient {
  public:
-  // |loader_factory_getter| is used to get the network factory when the script
-  // URL has an http(s) scheme.
+  // |network_factory| is used when the script URL has an http(s) scheme.
   //
-  // |non_network_loader_factory| is non-null when the script URL has a
+  // |non_network_factory| is non-null when the script URL has a
   // non-http(s) scheme (e.g., a chrome-extension:// URL). It is used in that
   // case since the network factory can't be used.
   ServiceWorkerNewScriptLoader(
@@ -68,8 +66,8 @@ class CONTENT_EXPORT ServiceWorkerNewScriptLoader
       const network::ResourceRequest& original_request,
       network::mojom::URLLoaderClientPtr client,
       scoped_refptr<ServiceWorkerVersion> version,
-      scoped_refptr<URLLoaderFactoryGetter> loader_factory_getter,
-      network::mojom::URLLoaderFactoryPtr non_network_loader_factory,
+      scoped_refptr<network::SharedURLLoaderFactory> network_factory,
+      network::mojom::URLLoaderFactoryPtr non_network_factory,
       const net::MutableNetworkTrafficAnnotationTag& traffic_annotation);
   ~ServiceWorkerNewScriptLoader() override;
 
@@ -154,10 +152,11 @@ class CONTENT_EXPORT ServiceWorkerNewScriptLoader
   mojo::ScopedDataPipeConsumerHandle network_consumer_;
   mojo::SimpleWatcher network_watcher_;
   bool network_load_completed_ = false;
-  // |non_network_loader_factory_| is non-null when the script URL is
+  scoped_refptr<network::SharedURLLoaderFactory> network_factory_;
+  // |non_network_factory_| is non-null when the script URL is
   // non-http(s). It is used to make the "network" request because the usual
   // network factory can't be used in that case. See class comments.
-  network::mojom::URLLoaderFactoryPtr non_network_loader_factory_;
+  network::mojom::URLLoaderFactoryPtr non_network_factory_;
 
   // Used for responding with the fetched script to this loader's client.
   network::mojom::URLLoaderClientPtr client_;

content/browser/service_worker/service_worker_new_script_loader_unittest.cc

@@ -227,7 +227,7 @@ class ServiceWorkerNewScriptLoaderTest : public testing::Test {
     client_ = std::make_unique<network::TestURLLoaderClient>();
     loader_ = std::make_unique<ServiceWorkerNewScriptLoader>(
         routing_id, request_id, options, request, client_->CreateInterfacePtr(),
-        version_, helper_->url_loader_factory_getter(),
+        version_, helper_->url_loader_factory_getter()->GetNetworkFactory(),
         nullptr /* non_network_loader_factory */,
         net::MutableNetworkTrafficAnnotationTag(TRAFFIC_ANNOTATION_FOR_TESTS));
   }

content/browser/service_worker/service_worker_provider_host.cc

@@ -688,7 +688,8 @@ ServiceWorkerProviderHost::CompleteStartWorkerPreparation(
   if (ServiceWorkerUtils::IsServicificationEnabled()) {
     mojo::MakeStrongAssociatedBinding(
         std::make_unique<ServiceWorkerScriptLoaderFactory>(
-            context_, AsWeakPtr(), context_->loader_factory_getter(),
+            context_, AsWeakPtr(),
+            context_->loader_factory_getter()->GetNetworkFactory(),
             std::move(non_network_loader_factory)),
         mojo::MakeRequest(&script_loader_factory_ptr_info));
     provider_info->script_loader_factory_ptr_info =

content/browser/service_worker/service_worker_script_loader_factory.cc

@@ -10,22 +10,22 @@
 #include "content/browser/service_worker/service_worker_new_script_loader.h"
 #include "content/browser/service_worker/service_worker_provider_host.h"
 #include "content/browser/service_worker/service_worker_version.h"
-#include "content/browser/url_loader_factory_getter.h"
 #include "content/common/service_worker/service_worker_utils.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
 #include "services/network/public/cpp/resource_response.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
 
 namespace content {
 
 ServiceWorkerScriptLoaderFactory::ServiceWorkerScriptLoaderFactory(
     base::WeakPtr<ServiceWorkerContextCore> context,
     base::WeakPtr<ServiceWorkerProviderHost> provider_host,
-    scoped_refptr<URLLoaderFactoryGetter> loader_factory_getter,
-    network::mojom::URLLoaderFactoryPtr non_network_loader_factory)
+    scoped_refptr<network::SharedURLLoaderFactory> network_factory,
+    network::mojom::URLLoaderFactoryPtr non_network_factory)
     : context_(context),
       provider_host_(provider_host),
-      loader_factory_getter_(loader_factory_getter),
-      non_network_loader_factory_(std::move(non_network_loader_factory)) {
+      network_factory_(std::move(network_factory)),
+      non_network_factory_(std::move(non_network_factory)) {
   DCHECK(provider_host_->IsProviderForServiceWorker());
 }
 
@@ -40,20 +40,19 @@ void ServiceWorkerScriptLoaderFactory::CreateLoaderAndStart(
     network::mojom::URLLoaderClientPtr client,
     const net::MutableNetworkTrafficAnnotationTag& traffic_annotation) {
   DCHECK(ServiceWorkerUtils::IsServicificationEnabled());
-  DCHECK(loader_factory_getter_);
+  DCHECK(network_factory_);
   if (!ShouldHandleScriptRequest(resource_request)) {
     // If the request should not be handled (e.g., a fetch() request), just do a
     // passthrough load. This needs a relaying as we use different associated
     // message pipes.
     // TODO(kinuko): Record the reason like what we do with netlog in
     // ServiceWorkerContextRequestHandler.
-    if (!resource_request.url.SchemeIsHTTPOrHTTPS() &&
-        non_network_loader_factory_) {
-      non_network_loader_factory_->CreateLoaderAndStart(
+    if (!resource_request.url.SchemeIsHTTPOrHTTPS() && non_network_factory_) {
+      non_network_factory_->CreateLoaderAndStart(
           std::move(request), routing_id, request_id, options, resource_request,
           std::move(client), traffic_annotation);
     } else {
-      loader_factory_getter_->GetNetworkFactory()->CreateLoaderAndStart(
+      network_factory_->CreateLoaderAndStart(
           std::move(request), routing_id, request_id, options, resource_request,
           std::move(client), traffic_annotation);
     }
@@ -81,17 +80,14 @@ void ServiceWorkerScriptLoaderFactory::CreateLoaderAndStart(
   }
 
   // The common case: load the script and install it.
-  network::mojom::URLLoaderFactoryPtr cloned_non_network_loader_factory;
-  if (!resource_request.url.SchemeIsHTTPOrHTTPS() &&
-      non_network_loader_factory_) {
-    non_network_loader_factory_->Clone(
-        mojo::MakeRequest(&cloned_non_network_loader_factory));
-  }
+  network::mojom::URLLoaderFactoryPtr cloned_non_network_factory;
+  if (!resource_request.url.SchemeIsHTTPOrHTTPS() && non_network_factory_)
+    non_network_factory_->Clone(mojo::MakeRequest(&cloned_non_network_factory));
   mojo::MakeStrongBinding(
       std::make_unique<ServiceWorkerNewScriptLoader>(
           routing_id, request_id, options, resource_request, std::move(client),
-          provider_host_->running_hosted_version(), loader_factory_getter_,
-          std::move(cloned_non_network_loader_factory), traffic_annotation),
+          provider_host_->running_hosted_version(), network_factory_,
+          std::move(cloned_non_network_factory), traffic_annotation),
       std::move(request));
 }
 

content/browser/service_worker/service_worker_script_loader_factory.h

@@ -8,11 +8,14 @@
 #include "base/macros.h"
 #include "services/network/public/mojom/url_loader_factory.mojom.h"
 
+namespace network {
+class SharedURLLoaderFactory;
+}  // namespace network
+
 namespace content {
 
 class ServiceWorkerContextCore;
 class ServiceWorkerProviderHost;
-class URLLoaderFactoryGetter;
 
 // S13nServiceWorker:
 // Created per one running service worker for loading its scripts. This is kept
@@ -29,10 +32,10 @@ class URLLoaderFactoryGetter;
 class ServiceWorkerScriptLoaderFactory
     : public network::mojom::URLLoaderFactory {
  public:
-  // |loader_factory_getter| is used to get the network factory for
-  // loading the script.
+  // |network_factory| is used to load scripts when the service worker main
+  // script has an http(s) scheme.
   //
-  // |non_network_loader_factory| is non-null when the service worker main
+  // |non_network_factory| is non-null when the service worker main
   // script URL has a non-http(s) scheme (e.g., a chrome-extension:// URL).
   // It will be used to load the main script and any non-http(s) imported
   // script.
@@ -48,8 +51,8 @@ class ServiceWorkerScriptLoaderFactory
   ServiceWorkerScriptLoaderFactory(
       base::WeakPtr<ServiceWorkerContextCore> context,
       base::WeakPtr<ServiceWorkerProviderHost> provider_host,
-      scoped_refptr<URLLoaderFactoryGetter> loader_factory_getter,
-      network::mojom::URLLoaderFactoryPtr non_network_loader_factory);
+      scoped_refptr<network::SharedURLLoaderFactory> network_factory,
+      network::mojom::URLLoaderFactoryPtr non_network_factory);
   ~ServiceWorkerScriptLoaderFactory() override;
 
   // network::mojom::URLLoaderFactory:
@@ -69,8 +72,8 @@ class ServiceWorkerScriptLoaderFactory
 
   base::WeakPtr<ServiceWorkerContextCore> context_;
   base::WeakPtr<ServiceWorkerProviderHost> provider_host_;
-  scoped_refptr<URLLoaderFactoryGetter> loader_factory_getter_;
-  network::mojom::URLLoaderFactoryPtr non_network_loader_factory_;
+  scoped_refptr<network::SharedURLLoaderFactory> network_factory_;
+  network::mojom::URLLoaderFactoryPtr non_network_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ServiceWorkerScriptLoaderFactory);
 };

content/browser/shared_worker/shared_worker_script_loader.cc

@@ -6,10 +6,10 @@
 
 #include "content/browser/loader/navigation_loader_interceptor.h"
 #include "content/browser/service_worker/service_worker_provider_host.h"
-#include "content/browser/url_loader_factory_getter.h"
 #include "content/common/service_worker/service_worker_utils.h"
 #include "content/public/browser/resource_context.h"
 #include "net/url_request/redirect_util.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
 
 namespace content {
 
@@ -21,7 +21,7 @@ SharedWorkerScriptLoader::SharedWorkerScriptLoader(
     network::mojom::URLLoaderClientPtr client,
     base::WeakPtr<ServiceWorkerProviderHost> service_worker_provider_host,
     ResourceContext* resource_context,
-    scoped_refptr<URLLoaderFactoryGetter> loader_factory_getter,
+    scoped_refptr<network::SharedURLLoaderFactory> network_factory,
     const net::MutableNetworkTrafficAnnotationTag& traffic_annotation)
     : routing_id_(routing_id),
       request_id_(request_id),
@@ -30,7 +30,7 @@ SharedWorkerScriptLoader::SharedWorkerScriptLoader(
       client_(std::move(client)),
       service_worker_provider_host_(service_worker_provider_host),
       resource_context_(resource_context),
-      loader_factory_getter_(std::move(loader_factory_getter)),
+      network_factory_(std::move(network_factory)),
       traffic_annotation_(traffic_annotation),
       url_loader_client_binding_(this),
       weak_factory_(this) {
@@ -84,7 +84,7 @@ void SharedWorkerScriptLoader::MaybeStartLoader(
 void SharedWorkerScriptLoader::LoadFromNetwork() {
   network::mojom::URLLoaderClientPtr client;
   url_loader_client_binding_.Bind(mojo::MakeRequest(&client));
-  url_loader_factory_ = loader_factory_getter_->GetNetworkFactory();
+  url_loader_factory_ = network_factory_;
   url_loader_factory_->CreateLoaderAndStart(
       mojo::MakeRequest(&url_loader_), routing_id_, request_id_, options_,
       resource_request_, std::move(client), traffic_annotation_);

content/browser/shared_worker/shared_worker_script_loader.h

@@ -12,11 +12,14 @@
 #include "services/network/public/cpp/resource_request.h"
 #include "services/network/public/mojom/url_loader.mojom.h"
 
+namespace network {
+class SharedURLLoaderFactory;
+}  // namespace network
+
 namespace content {
 class NavigationLoaderInterceptor;
 class ResourceContext;
 class ServiceWorkerProviderHost;
-class URLLoaderFactoryGetter;
 
 // The URLLoader for loading a shared worker script. Only used for the main
 // script request.
@@ -40,7 +43,7 @@ class SharedWorkerScriptLoader : public network::mojom::URLLoader,
       network::mojom::URLLoaderClientPtr client,
       base::WeakPtr<ServiceWorkerProviderHost> service_worker_provider_host,
       ResourceContext* resource_context,
-      scoped_refptr<URLLoaderFactoryGetter> loader_factory_getter,
+      scoped_refptr<network::SharedURLLoaderFactory> network_factory,
       const net::MutableNetworkTrafficAnnotationTag& traffic_annotation);
   ~SharedWorkerScriptLoader() override;
 
@@ -86,7 +89,7 @@ class SharedWorkerScriptLoader : public network::mojom::URLLoader,
   network::mojom::URLLoaderClientPtr client_;
   base::WeakPtr<ServiceWorkerProviderHost> service_worker_provider_host_;
   ResourceContext* resource_context_;
-  scoped_refptr<URLLoaderFactoryGetter> loader_factory_getter_;
+  scoped_refptr<network::SharedURLLoaderFactory> network_factory_;
   net::MutableNetworkTrafficAnnotationTag traffic_annotation_;
 
   base::Optional<net::RedirectInfo> redirect_info_;

content/browser/shared_worker/shared_worker_script_loader_factory.cc

@@ -10,10 +10,10 @@
 #include "content/browser/service_worker/service_worker_provider_host.h"
 #include "content/browser/service_worker/service_worker_version.h"
 #include "content/browser/shared_worker/shared_worker_script_loader.h"
-#include "content/browser/url_loader_factory_getter.h"
 #include "content/common/service_worker/service_worker_utils.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
 #include "services/network/public/cpp/resource_response.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
 #include "third_party/blink/public/mojom/service_worker/service_worker_provider_type.mojom.h"
 
 namespace content {
@@ -22,10 +22,10 @@ SharedWorkerScriptLoaderFactory::SharedWorkerScriptLoaderFactory(
     ServiceWorkerContextWrapper* context,
     base::WeakPtr<ServiceWorkerProviderHost> service_worker_provider_host,
     ResourceContext* resource_context,
-    scoped_refptr<URLLoaderFactoryGetter> loader_factory_getter)
+    scoped_refptr<network::SharedURLLoaderFactory> network_factory)
     : service_worker_provider_host_(service_worker_provider_host),
       resource_context_(resource_context),
-      loader_factory_getter_(loader_factory_getter) {
+      network_factory_(std::move(network_factory)) {
   DCHECK(ServiceWorkerUtils::IsServicificationEnabled());
   DCHECK_EQ(service_worker_provider_host_->provider_type(),
             blink::mojom::ServiceWorkerProviderType::kForSharedWorker);
@@ -54,8 +54,8 @@ void SharedWorkerScriptLoaderFactory::CreateLoaderAndStart(
   mojo::MakeStrongBinding(
       std::make_unique<SharedWorkerScriptLoader>(
           routing_id, request_id, options, resource_request, std::move(client),
-          service_worker_provider_host_, resource_context_,
-          loader_factory_getter_, traffic_annotation),
+          service_worker_provider_host_, resource_context_, network_factory_,
+          traffic_annotation),
       std::move(request));
 }
 

content/browser/shared_worker/shared_worker_script_loader_factory.h

@@ -8,11 +8,14 @@
 #include "base/macros.h"
 #include "services/network/public/mojom/url_loader_factory.mojom.h"
 
+namespace network {
+class SharedURLLoaderFactory;
+}  // namespace network
+
 namespace content {
 
 class ServiceWorkerContextWrapper;
 class ServiceWorkerProviderHost;
-class URLLoaderFactoryGetter;
 class ResourceContext;
 
 // S13nServiceWorker:
@@ -32,7 +35,7 @@ class SharedWorkerScriptLoaderFactory
       ServiceWorkerContextWrapper* context,
       base::WeakPtr<ServiceWorkerProviderHost> provider_host,
       ResourceContext* resource_context,
-      scoped_refptr<URLLoaderFactoryGetter> loader_factory_getter);
+      scoped_refptr<network::SharedURLLoaderFactory> network_factory);
   ~SharedWorkerScriptLoaderFactory() override;
 
   // network::mojom::URLLoaderFactory:
@@ -49,7 +52,7 @@ class SharedWorkerScriptLoaderFactory
  private:
   base::WeakPtr<ServiceWorkerProviderHost> service_worker_provider_host_;
   ResourceContext* resource_context_ = nullptr;
-  scoped_refptr<URLLoaderFactoryGetter> loader_factory_getter_;
+  scoped_refptr<network::SharedURLLoaderFactory> network_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(SharedWorkerScriptLoaderFactory);
 };

content/browser/shared_worker/shared_worker_service_impl.cc

@@ -57,7 +57,7 @@ void CreateScriptLoaderOnIO(
   mojo::MakeStrongAssociatedBinding(
       std::make_unique<SharedWorkerScriptLoaderFactory>(
           context.get(), host->AsWeakPtr(), context->resource_context(),
-          std::move(loader_factory_getter)),
+          loader_factory_getter->GetNetworkFactory()),
       mojo::MakeRequest(&script_loader_factory));
 
   BrowserThread::PostTask(