Network service: avoid requesting a new URLLoaderFactory for each NavigationURLLoaderNetworkService.

content/browser/loader/navigation_url_loader_network_service.cc

 // Copyright 2017 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/browser/loader/navigation_url_loader_network_service.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/lazy_instance.h"
 #include "base/memory/ptr_util.h"
@@ skipping 12 matching lines @@
 #include "content/public/common/referrer.h"
 #include "content/public/common/service_manager_connection.h"
 #include "content/public/common/service_names.mojom.h"
 #include "net/base/load_flags.h"
 #include "net/url_request/url_request_context.h"
 #include "services/service_manager/public/cpp/connector.h"
 
 namespace content {
 
 namespace {
+
 static base::LazyInstance<mojom::URLLoaderFactoryPtr>::Leaky
     g_url_loader_factory = LAZY_INSTANCE_INITIALIZER;
-}
 
 // This function is called on the IO thread for POST/PUT requests for
 // attaching blob information to the request body.
 void HandleRequestsWithBody(
     std::unique_ptr<ResourceRequest> request,
     ResourceContext* resource_context,
     base::WeakPtr<NavigationURLLoaderNetworkService> url_loader) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(request->request_body.get());
 
   AttachRequestBodyBlobDataHandles(request->request_body.get(),
                                     resource_context);
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(&NavigationURLLoaderNetworkService::StartURLRequest,
                   url_loader, base::Passed(&request)));
 }
 
+}  // namespace
+
 NavigationURLLoaderNetworkService::NavigationURLLoaderNetworkService(
     ResourceContext* resource_context,
     StoragePartition* storage_partition,
     std::unique_ptr<NavigationRequestInfo> request_info,
     std::unique_ptr<NavigationUIData> navigation_ui_data,
     ServiceWorkerNavigationHandle* service_worker_handle,
     AppCacheNavigationHandle* appcache_handle,
     NavigationURLLoaderDelegate* delegate)
     : delegate_(delegate), binding_(this), weak_factory_(this) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   TRACE_EVENT_ASYNC_BEGIN_WITH_TIMESTAMP1(
       "navigation", "Navigation timeToResponseStarted", this,
       request_info->common_params.navigation_start, "FrameTreeNode id",
       request_info->frame_tree_node_id);
 
-  // TODO(scottmg): Maybe some of this setup should be done only once, instead
-  // of every time.
-  if (g_url_loader_factory.Get().get()) {
-    url_loader_factory_ = std::move(g_url_loader_factory.Get());
-  } else {
-    ServiceManagerConnection::GetForProcess()->GetConnector()->BindInterface(
-        mojom::kNetworkServiceName, &url_loader_factory_);
-  }
-
   // TODO(scottmg): Port over stuff from RDHI::BeginNavigationRequest() here.
   auto new_request = base::MakeUnique<ResourceRequest>();
 
   new_request->method = request_info->common_params.method;
   new_request->url = request_info->common_params.url;
   new_request->first_party_for_cookies = request_info->first_party_for_cookies;
   new_request->priority = net::HIGHEST;
 
   // The code below to set fields like request_initiator, referrer, etc has
   // been copied from ResourceDispatcherHostImpl. We did not refactor the
@@ skipping 110 matching lines @@
   }
 }
 
 void NavigationURLLoaderNetworkService::StartURLRequest(
     std::unique_ptr<ResourceRequest> request) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   mojom::URLLoaderClientPtr url_loader_client_ptr_to_pass;
   binding_.Bind(&url_loader_client_ptr_to_pass);
 
-  url_loader_factory_->CreateLoaderAndStart(
+  GetURLLoaderFactory()->CreateLoaderAndStart(
       mojo::MakeRequest(&url_loader_associated_ptr_), 0 /* routing_id? */,
       0 /* request_id? */, mojom::kURLLoadOptionSendSSLInfo, *request,
       std::move(url_loader_client_ptr_to_pass));
 }
 
+mojom::URLLoaderFactory*

[scottmg, 2017/05/02 18:00:56]

nit; maybe make this return a & instead of a pointer, just so it's clear you'll
always get one (since it's a little confusing now that we're creating/nulling
the factory out)

[yzshen1, 2017/05/02 20:20:14]

Done.

+NavigationURLLoaderNetworkService::GetURLLoaderFactory() {
+  // TODO(yzshen): We will need the ability to customize the factory per frame
+  // e.g., for appacache or service worker.

[scottmg, 2017/05/02 18:00:56]

"appcache"

[yzshen1, 2017/05/02 20:20:14]

Done.

+  if (!g_url_loader_factory.Get()) {
+    ServiceManagerConnection::GetForProcess()->GetConnector()->BindInterface(
+        mojom::kNetworkServiceName, &g_url_loader_factory.Get());
+  }
+
+  return g_url_loader_factory.Get().get();
+}
+
 }  // namespace content