Get rid of URLLoaderFactory in browser-side case

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/memory/ptr_util.h"
 #include "base/trace_event/trace_event.h"
@@ skipping 41 matching lines @@
   if (!frame_tree_node)
     return nullptr;
 
   return WebContentsImpl::FromFrameTreeNode(frame_tree_node);
 }
 
 }  // namespace
 
 // Kept around during the lifetime of the navigation request, and is
 // responsible for dispatching a ResourceRequest to the appropriate
-// URLLoaderFactory.  In order to get the right URLLoaderFactory it
-// builds a vector of URLLoaderRequestHandler's and successively calls
-// MaybeCreateLoaderFactory on each until the request is successfully
-// handled. The same sequence may be performed multiple times when
-// redirects happen.
+// URLLoader.  In order to get the right URLLoader it builds a vector
+// of URLLoaderRequestHandler's and successively calls MaybeCreateLoader
+// on each until the request is successfully handled. The same sequence
+// may be performed multiple times when redirects happen.
 class NavigationURLLoaderNetworkService::URLLoaderRequestController {
  public:
   URLLoaderRequestController(
       std::unique_ptr<ResourceRequest> resource_request,
       ResourceContext* resource_context,
       scoped_refptr<URLLoaderFactoryGetter> url_loader_factory_getter)
       : resource_request_(std::move(resource_request)),
         resource_context_(resource_context),
         url_loader_factory_getter_(url_loader_factory_getter) {}
 
@@ skipping 59 matching lines @@
 
     Restart(std::move(url_loader_request), std::move(url_loader_client_ptr_));
   }
 
   // This could be called multiple times.
   void Restart(mojom::URLLoaderAssociatedRequest url_loader_request,
                mojom::URLLoaderClientPtr url_loader_client_ptr) {
     url_loader_request_ = std::move(url_loader_request);
     url_loader_client_ptr_ = std::move(url_loader_client_ptr);
     handler_index_ = 0;
-    MaybeStartLoader(nullptr);
+    MaybeStartLoader(StartLoaderCallback());
   }
 
-  void MaybeStartLoader(mojom::URLLoaderFactory* factory) {
+  void MaybeStartLoader(StartLoaderCallback start_loader_callback) {
     DCHECK(url_loader_client_ptr_.is_bound());
 
-    if (factory) {
-      factory->CreateLoaderAndStart(
-          std::move(url_loader_request_), 0 /* routing_id? */,
-          0 /* request_id? */, mojom::kURLLoadOptionSendSSLInfo,
-          *resource_request_, std::move(url_loader_client_ptr_));
+    if (start_loader_callback) {
+      std::move(start_loader_callback)
+          .Run(std::move(url_loader_request_),

[yzshen1, 2017/06/07 23:28:25]

I noticed that this still uses associated URLLoader. Because it is not passed
over a message pipe (i.e., a non-associated interface or an already-associated
interface), it is in a pending-association state -- it doesn't know on which
message pipe to send messages. In this state, calling the associated URLLoader
interface will crash.

We will need to use something like mojo::MakeIsolatedRequest() to create a
dedicated message pipe for this associated interface. One annoying thing is that
the corresponding URLLoaderAssociatedPtr belongs to
NavigationURLLoaderNetworkService and in use now. So mojo::MakeIsolatedRequest()
doesn't fit.

I should be able to fix this with my browser-side SB check CL. Alternatively,
the problem will go away if we switch to non-associated URLLoader.

+               std::move(url_loader_client_ptr_));
       return;
     }
 
     if (handler_index_ < handlers_.size()) {
-      handlers_[handler_index_++]->MaybeCreateLoaderFactory(
+      handlers_[handler_index_++]->MaybeCreateLoader(
           *resource_request_, resource_context_,
           base::BindOnce(&URLLoaderRequestController::MaybeStartLoader,
                          base::Unretained(this)));
       return;
     }
 
+    mojom::URLLoaderFactory* factory = nullptr;
     DCHECK_EQ(handlers_.size(), handler_index_);
     if (resource_request_->url.SchemeIs(url::kBlobScheme)) {
       factory = url_loader_factory_getter_->GetBlobFactory()->get();
     } else {
       factory = url_loader_factory_getter_->GetNetworkFactory()->get();
     }
-    MaybeStartLoader(factory);
+    factory->CreateLoaderAndStart(
+        std::move(url_loader_request_), 0 /* routing_id? */,
+        0 /* request_id? */, mojom::kURLLoadOptionSendSSLInfo,
+        *resource_request_, std::move(url_loader_client_ptr_));
   }
 
  private:
   std::vector<std::unique_ptr<URLLoaderRequestHandler>> handlers_;
   size_t handler_index_ = 0;
 
   std::unique_ptr<ResourceRequest> resource_request_;
   ResourceContext* resource_context_;
 
   scoped_refptr<URLLoaderFactoryGetter> url_loader_factory_getter_;
@@ skipping 167 matching lines @@
     TRACE_EVENT_ASYNC_END2("navigation", "Navigation timeToResponseStarted",
                            this, "&NavigationURLLoaderNetworkService", this,
                            "success", false);
 
     delegate_->OnRequestFailed(completion_status.exists_in_cache,
                                completion_status.error_code);
   }
 }
 
 }  // namespace content