Avoid always hopping to IO for service worker for NavigationLoaderOnUI

I noticed the service worker interceptor blocks navigation on a
UI -> IO -> UI thread hop while looking up registrations in
MaybeCreateLoader. This CL keeps a map of the origins with
registrations on the UI thread, so we can skip this hop for origins
that have no service workers registered.

This will no longer be needed if service workers are migrated fully to
UI, so depending on what the timeline of that is this may not be
needed.

Bug: 824858
Change-Id: I2ce383ce4b8a370617c823d954376b9dd9f84a6e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1710848
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#679875}

content/browser/service_worker/service_worker_context_wrapper.cc

@@ -23,7 +23,9 @@
 #include "base/task/post_task.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "content/browser/blob_storage/chrome_blob_storage_context.h"
+#include "content/browser/loader/navigation_url_loader_impl.h"
 #include "content/browser/service_worker/embedded_worker_status.h"
+#include "content/browser/service_worker/service_worker_context_watcher.h"
 #include "content/browser/service_worker/service_worker_process_manager.h"
 #include "content/browser/service_worker/service_worker_quota_client.h"
 #include "content/browser/service_worker/service_worker_version.h"
@@ -206,6 +208,15 @@ ServiceWorkerContextWrapper::ServiceWorkerContextWrapper(
   // Add this object as an observer of the wrapped |context_core_|. This lets us
   // forward observer methods to observers outside of content.
   core_observer_list_->AddObserver(this);
+
+  if (NavigationURLLoaderImpl::IsNavigationLoaderOnUIEnabled()) {
+    watcher_ = base::MakeRefCounted<ServiceWorkerContextWatcher>(
+        this,
+        base::BindRepeating(&ServiceWorkerContextWrapper::OnRegistrationUpdated,
+                            base::Unretained(this)),
+        base::DoNothing(), base::DoNothing());
+    watcher_->Start();
+  }
 }
 
 void ServiceWorkerContextWrapper::Init(
@@ -240,6 +251,10 @@ void ServiceWorkerContextWrapper::Shutdown() {
 
   storage_partition_ = nullptr;
   process_manager_->Shutdown();
+  if (NavigationURLLoaderImpl::IsNavigationLoaderOnUIEnabled()) {
+    watcher_->Stop();
+    watcher_ = nullptr;
+  }
   base::PostTaskWithTraits(
       FROM_HERE, {BrowserThread::IO},
       base::BindOnce(&ServiceWorkerContextWrapper::ShutdownOnIO, this));
@@ -1527,4 +1542,33 @@ std::unique_ptr<blink::URLLoaderFactoryBundleInfo> ServiceWorkerContextWrapper::
   return factory_bundle;
 }
 
+bool ServiceWorkerContextWrapper::HasRegistrationForOrigin(
+    const GURL& origin) const {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK(NavigationURLLoaderImpl::IsNavigationLoaderOnUIEnabled());
+  return !registrations_initialized_ ||
+         registrations_for_origin_.find(origin) !=
+             registrations_for_origin_.end();
+}
+
+void ServiceWorkerContextWrapper::OnRegistrationUpdated(
+    const std::vector<ServiceWorkerRegistrationInfo>& registrations) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  // The first call will initialize stored registrations.
+  registrations_initialized_ = true;
+
+  for (const auto& registration : registrations) {
+    GURL origin = registration.scope.GetOrigin();
+    int64_t registration_id = registration.registration_id;
+    if (registration.delete_flag == ServiceWorkerRegistrationInfo::IS_DELETED) {
+      auto& registration_ids = registrations_for_origin_[origin];
+      registration_ids.erase(registration_id);
+      if (registration_ids.empty())
+        registrations_for_origin_.erase(origin);
+    } else {
+      registrations_for_origin_[origin].insert(registration_id);
+    }
+  }
+}
+
 }  // namespace content

content/browser/service_worker/service_worker_context_wrapper.h

@@ -43,6 +43,7 @@ class BrowserContext;
 class ChromeBlobStorageContext;
 class ResourceContext;
 class ServiceWorkerContextObserver;
+class ServiceWorkerContextWatcher;
 class StoragePartitionImpl;
 class URLLoaderFactoryGetter;
 
@@ -314,6 +315,9 @@ class CONTENT_EXPORT ServiceWorkerContextWrapper
   // DeleteAndStartOver fails.
   ServiceWorkerContextCore* context();
 
+  // Whether |origin| has any registrations. Must be called on UI thread.
+  bool HasRegistrationForOrigin(const GURL& origin) const;
+
  private:
   friend class BackgroundSyncManagerTest;
   friend class base::RefCountedThreadSafe<ServiceWorkerContextWrapper>;
@@ -442,6 +446,11 @@ class CONTENT_EXPORT ServiceWorkerContextWrapper
   CreateNonNetworkURLLoaderFactoryBundleInfoForUpdateCheck(
       BrowserContext* browser_context);
 
+  // Called when the stored registrations are loaded, and each time a new
+  // service worker is registered.
+  void OnRegistrationUpdated(
+      const std::vector<ServiceWorkerRegistrationInfo>& registrations);
+
   // Observers of |context_core_| which live within content's implementation
   // boundary. Shared with |context_core_|.
   using ServiceWorkerContextObserverList =
@@ -469,6 +478,15 @@ class CONTENT_EXPORT ServiceWorkerContextWrapper
   // OnVersionRunningStatusChanged events.
   base::flat_set<int64_t /* version_id */> running_service_workers_;
 
+  // Maps the origin to a set of registration ids for that origin. Must be
+  // accessed on UI thread.
+  // TODO(http://crbug.com/824858): This can be removed when service workers are
+  // fully converted to running on the UI thread.
+  base::flat_map<GURL, base::flat_set<int64_t>> registrations_for_origin_;
+  bool registrations_initialized_ = false;
+
+  scoped_refptr<ServiceWorkerContextWatcher> watcher_;
+
   DISALLOW_COPY_AND_ASSIGN(ServiceWorkerContextWrapper);
 };
 

content/browser/service_worker/service_worker_controllee_request_handler.cc

@@ -106,24 +106,15 @@ void ServiceWorkerControlleeRequestHandler::MaybeCreateLoader(
     ResourceContext* resource_context,
     LoaderCallback callback,
     FallbackCallback fallback_callback) {
-  ClearJob();
-
-  if (!provider_host_) {
+  // InitializeProvider() 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 (!InitializeProvider(tentative_resource_request)) {
     // We can't do anything other than to fall back to network.
     std::move(callback).Run({});
     return;
   }
 
-  // Update the provider host with this request, clearing old controller state
-  // if this is a redirect. It's important to update the host before falling
-  // back to network below, so service worker APIs still work even if the
-  // service worker is bypassed for request interception.
-  provider_host_->SetControllerRegistration(nullptr,
-                                            /*notify_controllerchange=*/false);
-  stripped_url_ = net::SimplifyUrlForRequest(tentative_resource_request.url);
-  provider_host_->UpdateUrls(stripped_url_,
-                             tentative_resource_request.site_for_cookies);
-
   // Fall back to network if we were instructed to bypass the service worker for
   // request interception, or if the context is gone so we have to bypass
   // anyway.
@@ -204,6 +195,24 @@ ServiceWorkerControlleeRequestHandler::MaybeCreateSubresourceLoaderParams() {
   return base::Optional<SubresourceLoaderParams>(std::move(params));
 }
 
+bool ServiceWorkerControlleeRequestHandler::InitializeProvider(
+    const network::ResourceRequest& tentative_resource_request) {
+  ClearJob();
+
+  if (!provider_host_) {
+    return false;
+  }
+
+  // Update the provider host with this request, clearing old controller state
+  // if this is a redirect.
+  provider_host_->SetControllerRegistration(nullptr,
+                                            /*notify_controllerchange=*/false);
+  stripped_url_ = net::SimplifyUrlForRequest(tentative_resource_request.url);
+  provider_host_->UpdateUrls(stripped_url_,
+                             tentative_resource_request.site_for_cookies);
+  return true;
+}
+
 void ServiceWorkerControlleeRequestHandler::ContinueWithRegistration(
     blink::ServiceWorkerStatusCode status,
     scoped_refptr<ServiceWorkerRegistration> registration) {

content/browser/service_worker/service_worker_controllee_request_handler.h

@@ -60,6 +60,9 @@ class CONTENT_EXPORT ServiceWorkerControlleeRequestHandler final
   base::Optional<SubresourceLoaderParams> MaybeCreateSubresourceLoaderParams()
       override;
 
+  // Does all initialization of |provider_host_| for a request.
+  bool InitializeProvider(const network::ResourceRequest& tentative_request);
+
   // Exposed for testing.
   ServiceWorkerNavigationLoader* loader() {
     return loader_wrapper_ ? loader_wrapper_->get() : nullptr;

content/browser/service_worker/service_worker_navigation_handle.cc

@@ -8,6 +8,7 @@
 
 #include "base/bind.h"
 #include "base/task/post_task.h"
+#include "content/browser/service_worker/service_worker_context_wrapper.h"
 #include "content/browser/service_worker/service_worker_navigation_handle_core.h"
 #include "content/common/service_worker/service_worker_utils.h"
 #include "content/public/browser/browser_task_traits.h"
@@ -16,7 +17,8 @@
 namespace content {
 
 ServiceWorkerNavigationHandle::ServiceWorkerNavigationHandle(
-    ServiceWorkerContextWrapper* context_wrapper) {
+    ServiceWorkerContextWrapper* context_wrapper)
+    : context_wrapper_(context_wrapper) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   core_ = new ServiceWorkerNavigationHandleCore(weak_factory_.GetWeakPtr(),
                                                 context_wrapper);

content/browser/service_worker/service_worker_navigation_handle.h

@@ -69,11 +69,16 @@ class ServiceWorkerNavigationHandle {
 
   ServiceWorkerNavigationHandleCore* core() const { return core_; }
 
+  const ServiceWorkerContextWrapper* context_wrapper() const {
+    return context_wrapper_.get();
+  }
+
  private:
   blink::mojom::ServiceWorkerProviderInfoForClientPtr provider_info_;
   // TODO(leonhsl): Use std::unique_ptr<ServiceWorkerNavigationHandleCore,
   // BrowserThread::DeleteOnIOThread> instead.
   ServiceWorkerNavigationHandleCore* core_;
+  scoped_refptr<ServiceWorkerContextWrapper> context_wrapper_;
   base::WeakPtrFactory<ServiceWorkerNavigationHandle> weak_factory_{this};
   DISALLOW_COPY_AND_ASSIGN(ServiceWorkerNavigationHandle);
 };

content/browser/service_worker/service_worker_navigation_loader_interceptor.cc

@@ -31,6 +31,7 @@ void LoaderCallbackWrapperOnIO(
     ServiceWorkerNavigationHandleCore* handle_core,
     base::WeakPtr<ServiceWorkerNavigationLoaderInterceptor> interceptor_on_ui,
     blink::mojom::ServiceWorkerProviderInfoForClientPtr provider_info,
+    NavigationLoaderInterceptor::LoaderCallback loader_callback,
     SingleRequestURLLoaderFactory::RequestHandler handler) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
@@ -45,18 +46,21 @@ void LoaderCallbackWrapperOnIO(
       base::BindOnce(
           &ServiceWorkerNavigationLoaderInterceptor::LoaderCallbackWrapper,
           interceptor_on_ui, std::move(provider_info),
-          std::move(subresource_loader_params), std::move(handler)));
+          std::move(subresource_loader_params), std::move(loader_callback),
+          std::move(handler)));
 }
 
 void FallbackCallbackWrapperOnIO(
     base::WeakPtr<ServiceWorkerNavigationLoaderInterceptor> interceptor_on_ui,
+    NavigationLoaderInterceptor::FallbackCallback fallback_callback,
     bool reset_subresource_loader_params) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   PostTaskWithTraits(
       FROM_HERE, {BrowserThread::UI},
       base::BindOnce(
           &ServiceWorkerNavigationLoaderInterceptor::FallbackCallbackWrapper,
-          interceptor_on_ui, reset_subresource_loader_params));
+          interceptor_on_ui, std::move(fallback_callback),
+          reset_subresource_loader_params));
 }
 
 void InvokeRequestHandlerOnIO(
@@ -79,7 +83,10 @@ void MaybeCreateLoaderOnIO(
     ResourceType resource_type,
     bool skip_service_worker,
     const network::ResourceRequest& tentative_resource_request,
-    BrowserContext* browser_context) {
+    BrowserContext* browser_context,
+    NavigationLoaderInterceptor::LoaderCallback loader_callback,
+    NavigationLoaderInterceptor::FallbackCallback fallback_callback,
+    bool initialize_provider_only) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   ServiceWorkerContextCore* context_core =
@@ -89,6 +96,7 @@ void MaybeCreateLoaderOnIO(
   if (!context_core || !resource_context) {
     LoaderCallbackWrapperOnIO(handle_core, std::move(interceptor_on_ui),
                               /*provider_info=*/nullptr,
+                              std::move(loader_callback),
                               /*handler=*/{});
     return;
   }
@@ -115,6 +123,18 @@ void MaybeCreateLoaderOnIO(
             skip_service_worker));
   }
 
+  // If |initialize_provider_only| is true, we have already determined there is
+  // no registered service worker on the UI thread, so just initialize the
+  // provider for this request.
+  if (initialize_provider_only) {
+    handle_core->interceptor()->InitializeProvider(tentative_resource_request);
+    LoaderCallbackWrapperOnIO(handle_core, interceptor_on_ui,
+                              std::move(provider_info),
+                              std::move(loader_callback),
+                              /*handler=*/{});
+    return;
+  }
+
   // Start the inner interceptor. We continue in LoaderCallbackWrapperOnIO().
   //
   // It's safe to bind the raw |handle_core| to the callback because it owns the
@@ -122,8 +142,9 @@ void MaybeCreateLoaderOnIO(
   handle_core->interceptor()->MaybeCreateLoader(
       tentative_resource_request, browser_context, resource_context,
       base::BindOnce(&LoaderCallbackWrapperOnIO, handle_core, interceptor_on_ui,
-                     std::move(provider_info)),
-      base::BindOnce(&FallbackCallbackWrapperOnIO, interceptor_on_ui));
+                     std::move(provider_info), std::move(loader_callback)),
+      base::BindOnce(&FallbackCallbackWrapperOnIO, interceptor_on_ui,
+                     std::move(fallback_callback)));
 }
 ///////////////////////////////////////////////////////////////////////////////
 
@@ -157,16 +178,26 @@ void ServiceWorkerNavigationLoaderInterceptor::MaybeCreateLoader(
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(!resource_context);
 
+  bool initialize_provider_only = false;
+  if (!handle_->context_wrapper()->HasRegistrationForOrigin(
+          tentative_resource_request.url.GetOrigin())) {
+    // We have no registrations, so it's safe to continue the request now
+    // without hopping to IO.
+    std::move(loader_callback).Run({});
+    loader_callback =
+        base::BindOnce([](SingleRequestURLLoaderFactory::RequestHandler) {});
+    initialize_provider_only = true;
+  }
+
   // Start the inner interceptor on the IO thread. It will call back to
   // LoaderCallbackWrapper() on the UI thread.
-  loader_callback_ = std::move(loader_callback);
-  fallback_callback_ = std::move(fallback_callback);
   base::PostTaskWithTraits(
       FROM_HERE, {BrowserThread::IO},
       base::BindOnce(&MaybeCreateLoaderOnIO, GetWeakPtr(), handle_->core(),
                      are_ancestors_secure_, frame_tree_node_id_, resource_type_,
                      skip_service_worker_, tentative_resource_request,
-                     browser_context));
+                     browser_context, std::move(loader_callback),
+                     std::move(fallback_callback), initialize_provider_only));
 }
 
 base::Optional<SubresourceLoaderParams>
@@ -178,6 +209,7 @@ ServiceWorkerNavigationLoaderInterceptor::MaybeCreateSubresourceLoaderParams() {
 void ServiceWorkerNavigationLoaderInterceptor::LoaderCallbackWrapper(
     blink::mojom::ServiceWorkerProviderInfoForClientPtr provider_info,
     base::Optional<SubresourceLoaderParams> subresource_loader_params,
+    LoaderCallback loader_callback,
     SingleRequestURLLoaderFactory::RequestHandler handler_on_io) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
@@ -189,23 +221,24 @@ void ServiceWorkerNavigationLoaderInterceptor::LoaderCallbackWrapper(
   subresource_loader_params_ = std::move(subresource_loader_params);
 
   if (!handler_on_io) {
-    std::move(loader_callback_).Run({});
+    std::move(loader_callback).Run({});
     return;
   }
 
   // The inner IO thread interceptor wants to handle the request. However,
   // |handler_on_io| expects to run on the IO thread. Give our own wrapper to
   // the loader callback.
-  std::move(loader_callback_)
+  std::move(loader_callback)
       .Run(base::BindOnce(
           &ServiceWorkerNavigationLoaderInterceptor::RequestHandlerWrapper,
           GetWeakPtr(), std::move(handler_on_io)));
 }
 
 void ServiceWorkerNavigationLoaderInterceptor::FallbackCallbackWrapper(
+    FallbackCallback fallback_callback,
     bool reset_subresource_loader_params) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  std::move(fallback_callback_).Run(reset_subresource_loader_params);
+  std::move(fallback_callback).Run(reset_subresource_loader_params);
 }
 
 base::WeakPtr<ServiceWorkerNavigationLoaderInterceptor>

content/browser/service_worker/service_worker_navigation_loader_interceptor.h

@@ -56,8 +56,10 @@ class ServiceWorkerNavigationLoaderInterceptor final
   void LoaderCallbackWrapper(
       blink::mojom::ServiceWorkerProviderInfoForClientPtr provider_info,
       base::Optional<SubresourceLoaderParams> subresource_loader_params,
+      LoaderCallback loader_callback,
       SingleRequestURLLoaderFactory::RequestHandler handler_on_io);
-  void FallbackCallbackWrapper(bool reset_subresource_loader_params);
+  void FallbackCallbackWrapper(FallbackCallback fallback_callback,
+                               bool reset_subresource_loader_params);
 
   base::WeakPtr<ServiceWorkerNavigationLoaderInterceptor> GetWeakPtr();
 
@@ -77,9 +79,6 @@ class ServiceWorkerNavigationLoaderInterceptor final
   const ResourceType resource_type_;
   const bool skip_service_worker_;
 
-  LoaderCallback loader_callback_;
-  FallbackCallback fallback_callback_;
-
   base::Optional<SubresourceLoaderParams> subresource_loader_params_;
 
   base::WeakPtrFactory<ServiceWorkerNavigationLoaderInterceptor> weak_factory_{