Reland "service worker: Simplify main resource "handler" & "interceptor" (2/3)"

This is a reland of 28610054

It was reverted as a suspected cause of
AutofillPrivateApiTest.GetCountryList browser_test failure.
https://ci.chromium.org/p/chromium/builders/ci/Mac10.11%20Tests/59127
But that test has been flaky from before the CL and is now disabled,
see https://crbug.com/1155072 and https://crbug.com/1162474.

Original change's description:
> service worker: Simplify main resource "handler" & "interceptor" (2/3)
>
> The "handler" can call the loader callback directly.
>
> Bug: 1138155
> Change-Id: I618991ca62f83d1336405de7f824fcf43c893ac0
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2596592
> Commit-Queue: Matt Falkenhagen <falken@chromium.org>
> Reviewed-by: Asami Doi <asamidoi@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#840521}

Bug: 1138155
Change-Id: I53b8b4734bcc4abf0ee53fc8e30552bb2a1abb51
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2616639Reviewed-by: Asami Doi <asamidoi@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#841325}

content/browser/service_worker/service_worker_controllee_request_handler.cc

@@ -116,14 +116,14 @@ void ServiceWorkerControlleeRequestHandler::MaybeScheduleUpdate() {
 void ServiceWorkerControlleeRequestHandler::MaybeCreateLoader(
     const network::ResourceRequest& tentative_resource_request,
     BrowserContext* browser_context,
-    ServiceWorkerLoaderCallback callback,
+    NavigationLoaderInterceptor::LoaderCallback loader_callback,
     NavigationLoaderInterceptor::FallbackCallback fallback_callback) {
   // InitializeContainerHost() will update the host. This is important to do
   // before falling back to network below, so service worker APIs still work
   // even if the service worker is bypassed for request interception.
   if (!InitializeContainerHost(tentative_resource_request)) {
     // We can't do anything other than to fall back to network.
-    std::move(callback).Run({});
+    std::move(loader_callback).Run({});
     return;
   }
 
@@ -131,7 +131,7 @@ void ServiceWorkerControlleeRequestHandler::MaybeCreateLoader(
   // request interception, or if the context is gone so we have to bypass
   // anyway.
   if (skip_service_worker_ || !context_) {
-    std::move(callback).Run({});
+    std::move(loader_callback).Run({});
     return;
   }
 
@@ -145,7 +145,7 @@ void ServiceWorkerControlleeRequestHandler::MaybeCreateLoader(
   // headers between now and when the request handler passed to
   // |loader_callback_| is invoked.
   if (ShouldFallbackToLoadOfflinePage(tentative_resource_request.headers)) {
-    std::move(callback).Run({});
+    std::move(loader_callback).Run({});
     return;
   }
 #endif  // BUILDFLAG(ENABLE_OFFLINE_PAGES)
@@ -160,7 +160,7 @@ void ServiceWorkerControlleeRequestHandler::MaybeCreateLoader(
       TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT, "URL",
       tentative_resource_request.url.spec());
 
-  loader_callback_ = std::move(callback);
+  loader_callback_ = std::move(loader_callback);
   fallback_callback_ = std::move(fallback_callback);
   registration_lookup_start_time_ = base::TimeTicks::Now();
   browser_context_ = browser_context;
@@ -426,8 +426,9 @@ void ServiceWorkerControlleeRequestHandler::ContinueWithActivatedVersion(
       TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT, "Info",
       "Forwarded to the ServiceWorker");
   std::move(loader_callback_)
-      .Run(base::BindOnce(&ServiceWorkerMainResourceLoader::StartRequest,
-                          loader_wrapper_->get()->AsWeakPtr()));
+      .Run(base::MakeRefCounted<SingleRequestURLLoaderFactory>(
+          base::BindOnce(&ServiceWorkerMainResourceLoader::StartRequest,
+                         loader_wrapper_->get()->AsWeakPtr())));
 }
 
 void ServiceWorkerControlleeRequestHandler::DidUpdateRegistration(

content/browser/service_worker/service_worker_controllee_request_handler.h

@@ -52,12 +52,10 @@ class CONTENT_EXPORT ServiceWorkerControlleeRequestHandler final {
   // This could get called multiple times during the lifetime in redirect
   // cases. (In fallback-to-network cases we basically forward the request
   // to the request to the next request handler)
-  using ServiceWorkerLoaderCallback =
-      base::OnceCallback<void(SingleRequestURLLoaderFactory::RequestHandler)>;
   void MaybeCreateLoader(
       const network::ResourceRequest& tentative_request,
       BrowserContext* browser_context,
-      ServiceWorkerLoaderCallback callback,
+      NavigationLoaderInterceptor::LoaderCallback loader_callback,
       NavigationLoaderInterceptor::FallbackCallback fallback_callback);
 
   // Does all initialization of |container_host_| for a request.
@@ -113,7 +111,7 @@ class CONTENT_EXPORT ServiceWorkerControlleeRequestHandler final {
   bool force_update_started_;
   base::TimeTicks registration_lookup_start_time_;
 
-  ServiceWorkerLoaderCallback loader_callback_;
+  NavigationLoaderInterceptor::LoaderCallback loader_callback_;
   NavigationLoaderInterceptor::FallbackCallback fallback_callback_;
 
   ServiceWorkerAccessedCallback service_worker_accessed_callback_;

content/browser/service_worker/service_worker_main_resource_loader_interceptor.cc

@@ -124,8 +124,7 @@ void ServiceWorkerMainResourceLoaderInterceptor::MaybeCreateLoader(
   ServiceWorkerContextCore* context_core =
       handle_->context_wrapper()->context();
   if (!context_core || !browser_context) {
-    LoaderCallbackWrapper(std::move(loader_callback),
-                          /*handler=*/{});
+    std::move(loader_callback).Run(/*handler=*/{});
     return;
   }
 
@@ -198,19 +197,15 @@ void ServiceWorkerMainResourceLoaderInterceptor::MaybeCreateLoader(
     // ControllerServiceWorkerInfoPtr and ServiceWorkerObjectHost from the
     // subresource loader params which is created by the interceptor.
     if (inherit_container_host_only) {
-      LoaderCallbackWrapper(std::move(loader_callback),
-                            /*handler=*/{});
+      std::move(loader_callback).Run(/*handler=*/{});
       return;
     }
   }
 
-  // Start the inner interceptor. We continue in
-  // LoaderCallbackWrapper() or the fallback callback is called.
+  // Start the inner interceptor. It will invoke the loader callback
+  // or fallback callback.
   handle_->interceptor()->MaybeCreateLoader(
-      tentative_resource_request, browser_context,
-      base::BindOnce(
-          &ServiceWorkerMainResourceLoaderInterceptor::LoaderCallbackWrapper,
-          GetWeakPtr(), std::move(loader_callback)),
+      tentative_resource_request, browser_context, std::move(loader_callback),
       std::move(fallback_callback));
 }
 
@@ -261,28 +256,6 @@ ServiceWorkerMainResourceLoaderInterceptor::
   return base::Optional<SubresourceLoaderParams>(std::move(params));
 }
 
-void ServiceWorkerMainResourceLoaderInterceptor::LoaderCallbackWrapper(
-    LoaderCallback loader_callback,
-    SingleRequestURLLoaderFactory::RequestHandler handler) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-
-  if (!handler) {
-    std::move(loader_callback).Run({});
-    return;
-  }
-
-  // The inner interceptor wants to handle the request.
-  std::move(loader_callback)
-      .Run(base::MakeRefCounted<SingleRequestURLLoaderFactory>(
-          std::move(handler)));
-}
-
-base::WeakPtr<ServiceWorkerMainResourceLoaderInterceptor>
-ServiceWorkerMainResourceLoaderInterceptor::GetWeakPtr() {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  return weak_factory_.GetWeakPtr();
-}
-
 ServiceWorkerMainResourceLoaderInterceptor::
     ServiceWorkerMainResourceLoaderInterceptor(
         base::WeakPtr<ServiceWorkerMainResourceHandle> handle,

content/browser/service_worker/service_worker_main_resource_loader_interceptor.h

@@ -72,12 +72,6 @@ class CONTENT_EXPORT ServiceWorkerMainResourceLoaderInterceptor final
   base::Optional<SubresourceLoaderParams> MaybeCreateSubresourceLoaderParams()
       override;
 
-  void LoaderCallbackWrapper(
-      LoaderCallback loader_callback,
-      SingleRequestURLLoaderFactory::RequestHandler handler);
-
-  base::WeakPtr<ServiceWorkerMainResourceLoaderInterceptor> GetWeakPtr();
-
  private:
   friend class ServiceWorkerMainResourceLoaderInterceptorTest;
 
@@ -132,9 +126,6 @@ class CONTENT_EXPORT ServiceWorkerMainResourceLoaderInterceptor final
   const int process_id_;
   const base::Optional<DedicatedOrSharedWorkerToken> worker_token_;
 
-  base::WeakPtrFactory<ServiceWorkerMainResourceLoaderInterceptor>
-      weak_factory_{this};
-
   DISALLOW_COPY_AND_ASSIGN(ServiceWorkerMainResourceLoaderInterceptor);
 };