Get rid of URLLoaderFactory in browser-side case

content/browser/loader/url_loader_request_handler.h

Index: content/browser/loader/url_loader_request_handler.h
diff --git a/content/browser/loader/url_loader_request_handler.h b/content/browser/loader/url_loader_request_handler.h
index f72cc2a88e5d2cd45ef1d7ef6356387276b4cd1f..c214890fce1eed19e89bd76fb78598e72e629175 100644
--- a/content/browser/loader/url_loader_request_handler.h
+++ b/content/browser/loader/url_loader_request_handler.h
@@ -14,8 +14,7 @@ namespace content {
 class ResourceContext;
 struct ResourceRequest;
 
-using LoaderFactoryCallback =
-    base::OnceCallback<void(mojom::URLLoaderFactory*)>;
+using URLLoaderCallback = base::OnceCallback<void(mojom::URLLoader*)>;
 
 // An instance of this class is a per-request object and kept around during
 // the lifetime of a request (including multiple redirect legs) on IO thread.
@@ -24,21 +23,16 @@ class CONTENT_EXPORT URLLoaderRequestHandler {
   URLLoaderRequestHandler() = default;
   virtual ~URLLoaderRequestHandler() = default;
 
-  // Calls |callback| with non-null factory if this handler can handle
-  // the request, calls it with nullptr otherwise.
+  // Calls |callback| with a non-null URLLoader that is not started yet
+  // if this handler can handle the request, calls it with nullptr otherwise.
   // Some implementation notes:
-  // 1) The returned pointer needs to be valid only until a single
-  // CreateLoaderAndStart call is made, and it is okay to do CHECK(false) for
-  // any subsequent calls.
-  // 2) The implementor is not supposed to set up and return URLLoaderFactory
-  // until it finds out that the handler is really going to handle the
-  // request. (For example ServiceWorker's request handler would not need to
-  // call the callback until it gets response from SW, and it may still
-  // call the callback with nullptr if it turns out that it needs to fallback
-  // to the network.)
-  virtual void MaybeCreateLoaderFactory(const ResourceRequest& resource_request,
-                                        ResourceContext* resource_context,
-                                        LoaderFactoryCallback callback) = 0;
+  // 1) The caller will immediately call Start() method on the returned ptr
+  //    if it is non-null.
+  // 2) The implementor is not supposed to set up and return URLLoader until
+  //    it finds out that the handler is really going to handle the request.
+  virtual void MaybeCreateLoader(const ResourceRequest& resource_request,

[jam, 2017/06/05 19:39:24]

I'm probably missing something, but why can't this interface always return null
or a started URLLoader?

[michaeln, 2017/06/05 21:52:39]

There is a use case for a non-started loaders.

Looking forward to appcache redirect and response fallback handling, an appcache
fallback should only be used if certain conditions arise. Upon return from this
function, we know if there's a fallback resource or not. We could return a
loader that has not be started and a bit that indicates its a fallback. The
caller would only start the fallback loader if it was needed.

[jam, 2017/06/06 00:07:45]

This could also be implemented by using a callback that creates a started
loader? i.e. i'm just wondering if we absolutely need this new interface in the
mojo interface or if we can avoid it.

[jam, 2017/06/06 00:55:40]

(I'm guessing I'm missing the obvious reason here, or else Kinuko would have
just done this).

If we do need a factory, can it be a synchronous C++ factory as we briefly
brainstormed in-person? That gets rid of the pain points of using
mojom::URLLoaderFactory (no extra hops, no need to worry about associated pipes
that we can't forward, no need to worry about the Sync call).

+                                 ResourceContext* resource_context,
+                                 URLLoaderCallback callback) = 0;
 };
 
 }  // namespace content