Revert "service worker: Simplify main resource "handler" & "interceptor" (1/3)"

This reverts commit 6be1b3d6.

Reason for revert: Suspected cause of AutofillPrivateApiTest.GetCountryList browser_test failure.
https://ci.chromium.org/p/chromium/builders/ci/Mac10.11%20Tests/59127

Original change's description:
> service worker: Simplify main resource  "handler" & "interceptor" (1/3)
>
> ServiceWorkerMainResourceLoaderInteceptor doesn't need to delegate
> to ServiceWorkerControlleeRequestHandler to create subresource
> params. It can access the ServiceWorkerProviderHost directly.
>
> This simplifies calling the loader callback.
>
> Bug: 1138155
> Change-Id: Ibf31c379a755c9c94471c707633a582bf7cba85d
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2602965
> Commit-Queue: Matt Falkenhagen <falken@chromium.org>
> Reviewed-by: Asami Doi <asamidoi@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#840506}

TBR=falken@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com,asamidoi@chromium.org

Change-Id: I05ae0bac856d4de7570d7e03980557d8a9cb2b6a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1138155
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2612719Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#840542}

content/browser/service_worker/service_worker_controllee_request_handler.cc

@@ -18,6 +18,7 @@
 #include "content/browser/service_worker/service_worker_context_wrapper.h"
 #include "content/browser/service_worker/service_worker_main_resource_loader.h"
 #include "content/browser/service_worker/service_worker_metrics.h"
+#include "content/browser/service_worker/service_worker_object_host.h"
 #include "content/browser/service_worker/service_worker_registration.h"
 #include "content/common/service_worker/service_worker_utils.h"
 #include "content/public/browser/allow_service_worker_result.h"
@@ -173,6 +174,49 @@ void ServiceWorkerControlleeRequestHandler::MaybeCreateLoader(
           weak_factory_.GetWeakPtr()));
 }
 
+base::Optional<SubresourceLoaderParams>
+ServiceWorkerControlleeRequestHandler::MaybeCreateSubresourceLoaderParams() {
+  // ContinueWithRegistration() for the request didn't find a matching service
+  // worker for this request, and
+  // ServiceWorkerContainerHost::SetControllerRegistration() was not called.
+  if (!container_host_ || !container_host_->controller())
+    return base::nullopt;
+
+  // Otherwise let's send the controller service worker information along
+  // with the navigation commit.
+  SubresourceLoaderParams params;
+  auto controller_info = blink::mojom::ControllerServiceWorkerInfo::New();
+  controller_info->mode = container_host_->GetControllerMode();
+  // Note that |controller_info->remote_controller| is null if the controller
+  // has no fetch event handler. In that case the renderer frame won't get the
+  // controller pointer upon the navigation commit, and subresource loading will
+  // not be intercepted. (It might get intercepted later if the controller
+  // changes due to skipWaiting() so SetController is sent.)
+  mojo::Remote<blink::mojom::ControllerServiceWorker> remote =
+      container_host_->GetRemoteControllerServiceWorker();
+  if (remote.is_bound()) {
+    controller_info->remote_controller = remote.Unbind();
+  }
+
+  controller_info->client_id = container_host_->client_uuid();
+  if (container_host_->fetch_request_window_id()) {
+    controller_info->fetch_request_window_id =
+        base::make_optional(container_host_->fetch_request_window_id());
+  }
+  base::WeakPtr<ServiceWorkerObjectHost> object_host =
+      container_host_->GetOrCreateServiceWorkerObjectHost(
+          container_host_->controller());
+  if (object_host) {
+    params.controller_service_worker_object_host = object_host;
+    controller_info->object_info = object_host->CreateIncompleteObjectInfo();
+  }
+  for (const auto feature : container_host_->controller()->used_features()) {
+    controller_info->used_features.push_back(feature);
+  }
+  params.controller_service_worker_info = std::move(controller_info);
+  return base::Optional<SubresourceLoaderParams>(std::move(params));
+}
+
 bool ServiceWorkerControlleeRequestHandler::InitializeContainerHost(
     const network::ResourceRequest& tentative_resource_request) {
   ClearJob();

content/browser/service_worker/service_worker_controllee_request_handler.h

@@ -59,6 +59,10 @@ class CONTENT_EXPORT ServiceWorkerControlleeRequestHandler final {
       BrowserContext* browser_context,
       ServiceWorkerLoaderCallback callback,
       NavigationLoaderInterceptor::FallbackCallback fallback_callback);
+  // Returns params with the ControllerServiceWorkerInfoPtr if we have found
+  // a matching controller service worker for the |request| that is given
+  // to MaybeCreateLoader(). Otherwise this returns base::nullopt.
+  base::Optional<SubresourceLoaderParams> MaybeCreateSubresourceLoaderParams();
 
   // Does all initialization of |container_host_| for a request.
   bool InitializeContainerHost(

content/browser/service_worker/service_worker_main_resource_loader_interceptor.cc

@@ -13,12 +13,10 @@
 #include "build/chromeos_buildflags.h"
 #include "content/browser/renderer_host/frame_tree_node.h"
 #include "content/browser/renderer_host/navigation_request_info.h"
-#include "content/browser/service_worker/service_worker_container_host.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
 #include "content/browser/service_worker/service_worker_context_wrapper.h"
 #include "content/browser/service_worker/service_worker_controllee_request_handler.h"
 #include "content/browser/service_worker/service_worker_main_resource_handle.h"
-#include "content/browser/service_worker/service_worker_object_host.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/origin_util.h"
 #include "content/public/common/url_constants.h"
@@ -217,48 +215,8 @@ void ServiceWorkerMainResourceLoaderInterceptor::MaybeCreateLoader(
 base::Optional<SubresourceLoaderParams>
 ServiceWorkerMainResourceLoaderInterceptor::
     MaybeCreateSubresourceLoaderParams() {
-  base::WeakPtr<ServiceWorkerContainerHost> container_host =
-      handle_->container_host();
-
-  // We didn't find a matching service worker for this request, and
-  // ServiceWorkerContainerHost::SetControllerRegistration() was not called.
-  if (!container_host || !container_host->controller()) {
-    return base::nullopt;
-  }
-
-  // Otherwise let's send the controller service worker information along
-  // with the navigation commit.
-  SubresourceLoaderParams params;
-  auto controller_info = blink::mojom::ControllerServiceWorkerInfo::New();
-  controller_info->mode = container_host->GetControllerMode();
-  // Note that |controller_info->remote_controller| is null if the controller
-  // has no fetch event handler. In that case the renderer frame won't get the
-  // controller pointer upon the navigation commit, and subresource loading will
-  // not be intercepted. (It might get intercepted later if the controller
-  // changes due to skipWaiting() so SetController is sent.)
-  mojo::Remote<blink::mojom::ControllerServiceWorker> remote =
-      container_host->GetRemoteControllerServiceWorker();
-  if (remote.is_bound()) {
-    controller_info->remote_controller = remote.Unbind();
-  }
-
-  controller_info->client_id = container_host->client_uuid();
-  if (container_host->fetch_request_window_id()) {
-    controller_info->fetch_request_window_id =
-        base::make_optional(container_host->fetch_request_window_id());
-  }
-  base::WeakPtr<ServiceWorkerObjectHost> object_host =
-      container_host->GetOrCreateServiceWorkerObjectHost(
-          container_host->controller());
-  if (object_host) {
-    params.controller_service_worker_object_host = object_host;
-    controller_info->object_info = object_host->CreateIncompleteObjectInfo();
-  }
-  for (const auto feature : container_host->controller()->used_features()) {
-    controller_info->used_features.push_back(feature);
-  }
-  params.controller_service_worker_info = std::move(controller_info);
-  return base::Optional<SubresourceLoaderParams>(std::move(params));
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  return std::move(subresource_loader_params_);
 }
 
 void ServiceWorkerMainResourceLoaderInterceptor::LoaderCallbackWrapper(
@@ -266,6 +224,21 @@ void ServiceWorkerMainResourceLoaderInterceptor::LoaderCallbackWrapper(
     SingleRequestURLLoaderFactory::RequestHandler handler) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
+  // For worker main script requests, |handle_| can be destroyed during
+  // interception. The initiator of this interceptor (i.e., WorkerScriptLoader)
+  // will handle the case.
+  // For navigation requests, this case should not happen because it's
+  // guaranteed that this interceptor is destroyed before |handle_|.
+  if (!handle_) {
+    std::move(loader_callback).Run({});
+    return;
+  }
+
+  if (handle_->interceptor()) {
+    subresource_loader_params_ =
+        handle_->interceptor()->MaybeCreateSubresourceLoaderParams();
+  }
+
   if (!handler) {
     std::move(loader_callback).Run({});
     return;

content/browser/service_worker/service_worker_main_resource_loader_interceptor.h

@@ -132,6 +132,8 @@ class CONTENT_EXPORT ServiceWorkerMainResourceLoaderInterceptor final
   const int process_id_;
   const base::Optional<DedicatedOrSharedWorkerToken> worker_token_;
 
+  base::Optional<SubresourceLoaderParams> subresource_loader_params_;
+
   base::WeakPtrFactory<ServiceWorkerMainResourceLoaderInterceptor>
       weak_factory_{this};