service worker: Check installing registrations in ServiceWorkerRegistry (1 of 3)

Before this CL, FindRegistrationForClientUrl() worked like the below:
1. Initialize database if it's not via LazyInitialize().
   LazyInitialize() initializes |registered_origins_|.
2. Check if |registered_origins_| contains registration(s).
   If not, try to find registrations from installing ones.
3. If |registered_origins_| contains registration(s), try to
   find registrations from database.

This ordering doesn't fit well for the storage service migration
because step 2 depends on in-memory representation of registrations
but we want to move step 1 and 3 to the storage service. Keeping
this ordering will require exposing low-level methods like
LazyInitialize() to content/.

This CL changes the ordering as the blow so that we don't have to
expose low-level methods:
1. Initialize database if it's not via LazyInitialize().
2. Check |registered_origins_| if there is stored registration(s).
   If not exists, skip database lookup and go to step 4.
3. Try to find registrations from database.
4. If NOT_FOUND is returned from database, try to find registrations
   from installing ones.

The semantics changed slightly but it should be almost the same.

There will be two following CLs which do the same for
FindRegistrationForScope() and FindRegistrationForId{,Only}().

Bug: 1039200
Change-Id: I97a7821f9c534faae6ab4a638da6d731a4455a6f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1999974
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732293}

content/browser/service_worker/service_worker_registry.cc

@@ -5,12 +5,30 @@
 #include "content/browser/service_worker/service_worker_registry.h"
 
 #include "base/memory/ptr_util.h"
+#include "base/memory/weak_ptr.h"
+#include "base/trace_event/trace_event.h"
 #include "content/browser/service_worker/service_worker_registration.h"
 #include "content/browser/service_worker/service_worker_version.h"
 #include "content/common/service_worker/service_worker_utils.h"
 
 namespace content {
 
+namespace {
+
+void CompleteFindNow(scoped_refptr<ServiceWorkerRegistration> registration,
+                     blink::ServiceWorkerStatusCode status,
+                     ServiceWorkerRegistry::FindRegistrationCallback callback) {
+  if (registration && registration->is_deleted()) {
+    // It's past the point of no return and no longer findable.
+    std::move(callback).Run(blink::ServiceWorkerStatusCode::kErrorNotFound,
+                            nullptr);
+    return;
+  }
+  std::move(callback).Run(status, std::move(registration));
+}
+
+}  // namespace
+
 ServiceWorkerRegistry::ServiceWorkerRegistry(
     const base::FilePath& user_data_directory,
     ServiceWorkerContextCore* context,
@@ -36,7 +54,18 @@ ServiceWorkerRegistry::~ServiceWorkerRegistry() = default;
 void ServiceWorkerRegistry::FindRegistrationForClientUrl(
     const GURL& client_url,
     FindRegistrationCallback callback) {
-  storage()->FindRegistrationForClientUrl(client_url, std::move(callback));
+  // To connect this TRACE_EVENT with the callback, Time::Now() is used as a
+  // trace event id.
+  int64_t trace_event_id =
+      base::Time::Now().ToDeltaSinceWindowsEpoch().InMicroseconds();
+  TRACE_EVENT_ASYNC_BEGIN1(
+      "ServiceWorker", "ServiceWorkerRegistry::FindRegistrationForClientUrl",
+      trace_event_id, "URL", client_url.spec());
+  storage()->FindRegistrationForClientUrl(
+      client_url,
+      base::BindOnce(&ServiceWorkerRegistry::DidFindRegistrationForClientUrl,
+                     weak_factory_.GetWeakPtr(), client_url, trace_event_id,
+                     std::move(callback)));
 }
 
 void ServiceWorkerRegistry::FindRegistrationForScope(
@@ -135,4 +164,38 @@ ServiceWorkerRegistry::FindInstallingRegistrationForId(
   return found->second.get();
 }
 
+void ServiceWorkerRegistry::DidFindRegistrationForClientUrl(
+    const GURL& client_url,
+    int64_t trace_event_id,
+    FindRegistrationCallback callback,
+    blink::ServiceWorkerStatusCode status,
+    scoped_refptr<ServiceWorkerRegistration> registration) {
+  if (status == blink::ServiceWorkerStatusCode::kErrorNotFound) {
+    // Look for something currently being installed.
+    scoped_refptr<ServiceWorkerRegistration> installing_registration =
+        FindInstallingRegistrationForClientUrl(client_url);
+    if (installing_registration) {
+      blink::ServiceWorkerStatusCode installing_status =
+          installing_registration->is_deleted()
+              ? blink::ServiceWorkerStatusCode::kErrorNotFound
+              : blink::ServiceWorkerStatusCode::kOk;
+      TRACE_EVENT_ASYNC_END2(
+          "ServiceWorker",
+          "ServiceWorkerRegistry::FindRegistrationForClientUrl", trace_event_id,
+          "Status", blink::ServiceWorkerStatusToString(status), "Info",
+          (installing_status == blink::ServiceWorkerStatusCode::kOk)
+              ? "Installing registration is found"
+              : "Any registrations are not found");
+      CompleteFindNow(std::move(installing_registration), installing_status,
+                      std::move(callback));
+      return;
+    }
+  }
+
+  TRACE_EVENT_ASYNC_END1(
+      "ServiceWorker", "ServiceWorkerRegistry::FindRegistrationForClientUrl",
+      trace_event_id, "Status", blink::ServiceWorkerStatusToString(status));
+  std::move(callback).Run(status, std::move(registration));
+}
+
 }  // namespace content

content/browser/service_worker/service_worker_registry.h

@@ -105,11 +105,20 @@ class CONTENT_EXPORT ServiceWorkerRegistry {
   }
 
  private:
+  void DidFindRegistrationForClientUrl(
+      const GURL& client_url,
+      int64_t trace_event_id,
+      FindRegistrationCallback callback,
+      blink::ServiceWorkerStatusCode status,
+      scoped_refptr<ServiceWorkerRegistration> registration);
+
   std::unique_ptr<ServiceWorkerStorage> storage_;
 
   // For finding registrations being installed or uninstalled.
   RegistrationRefsById installing_registrations_;
   RegistrationRefsById uninstalling_registrations_;
+
+  base::WeakPtrFactory<ServiceWorkerRegistry> weak_factory_{this};
 };
 
 }  // namespace content

content/browser/service_worker/service_worker_storage.cc

@@ -44,6 +44,7 @@ void RunSoon(const base::Location& from_here, base::OnceClosure closure) {
   base::ThreadTaskRunnerHandle::Get()->PostTask(from_here, std::move(closure));
 }
 
+// TODO(crbug.com/1039200): Remove; this depends on ServiceWorkerRegistration.
 void CompleteFindNow(scoped_refptr<ServiceWorkerRegistration> registration,
                      blink::ServiceWorkerStatusCode status,
                      ServiceWorkerStorage::FindRegistrationCallback callback) {
@@ -148,9 +149,8 @@ void ServiceWorkerStorage::FindRegistrationForClientUrl(
   DCHECK(!client_url.has_ref());
   switch (state_) {
     case STORAGE_STATE_DISABLED:
-      CompleteFindNow(scoped_refptr<ServiceWorkerRegistration>(),
-                      blink::ServiceWorkerStatusCode::kErrorAbort,
-                      std::move(callback));
+      std::move(callback).Run(blink::ServiceWorkerStatusCode::kErrorAbort,
+                              nullptr);
       return;
     case STORAGE_STATE_INITIALIZING:  // Fall-through.
     case STORAGE_STATE_UNINITIALIZED:
@@ -166,40 +166,20 @@ void ServiceWorkerStorage::FindRegistrationForClientUrl(
       break;
   }
 
-  // See if there are any stored registrations for the origin.
+  // Bypass database lookup when there is no stored registration.
   if (!base::Contains(registered_origins_, client_url.GetOrigin())) {
-    // Look for something currently being installed.
-    scoped_refptr<ServiceWorkerRegistration> installing_registration =
-        registry_->FindInstallingRegistrationForClientUrl(client_url);
-    blink::ServiceWorkerStatusCode status =
-        installing_registration
-            ? blink::ServiceWorkerStatusCode::kOk
-            : blink::ServiceWorkerStatusCode::kErrorNotFound;
-    TRACE_EVENT_INSTANT2(
-        "ServiceWorker",
-        "ServiceWorkerStorage::FindRegistrationForClientUrl:CheckInstalling",
-        TRACE_EVENT_SCOPE_THREAD, "URL", client_url.spec(), "Status",
-        blink::ServiceWorkerStatusToString(status));
-    CompleteFindNow(std::move(installing_registration), status,
-                    std::move(callback));
+    std::move(callback).Run(blink::ServiceWorkerStatusCode::kErrorNotFound,
+                            nullptr);
     return;
   }
 
-  // To connect this TRACE_EVENT with the callback, Time is used for
-  // callback id.
-  int64_t callback_id =
-      base::Time::Now().ToDeltaSinceWindowsEpoch().InMicroseconds();
-  TRACE_EVENT_ASYNC_BEGIN1("ServiceWorker",
-                           "ServiceWorkerStorage::FindRegistrationForClientUrl",
-                           callback_id, "URL", client_url.spec());
   database_task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(
           &FindForClientUrlInDB, database_.get(),
           base::ThreadTaskRunnerHandle::Get(), client_url,
           base::BindOnce(&ServiceWorkerStorage::DidFindRegistrationForClientUrl,
-                         weak_factory_.GetWeakPtr(), client_url,
-                         std::move(callback), callback_id)));
+                         weak_factory_.GetWeakPtr(), std::move(callback))));
 }
 
 void ServiceWorkerStorage::FindRegistrationForScope(
@@ -1206,46 +1186,20 @@ void ServiceWorkerStorage::DidReadInitialData(
 }
 
 void ServiceWorkerStorage::DidFindRegistrationForClientUrl(
-    const GURL& client_url,
     FindRegistrationCallback callback,
-    int64_t callback_id,
     const ServiceWorkerDatabase::RegistrationData& data,
     const ResourceList& resources,
     ServiceWorkerDatabase::Status status) {
   if (status == ServiceWorkerDatabase::STATUS_OK) {
     ReturnFoundRegistration(std::move(callback), data, resources);
-    TRACE_EVENT_ASYNC_END1(
-        "ServiceWorker", "ServiceWorkerStorage::FindRegistrationForClientUrl",
-        callback_id, "Status", ServiceWorkerDatabase::StatusToString(status));
     return;
   }
 
-  if (status == ServiceWorkerDatabase::STATUS_ERROR_NOT_FOUND) {
-    // Look for something currently being installed.
-    scoped_refptr<ServiceWorkerRegistration> installing_registration =
-        registry_->FindInstallingRegistrationForClientUrl(client_url);
-    blink::ServiceWorkerStatusCode installing_status =
-        installing_registration
-            ? blink::ServiceWorkerStatusCode::kOk
-            : blink::ServiceWorkerStatusCode::kErrorNotFound;
-    std::move(callback).Run(installing_status,
-                            std::move(installing_registration));
-    TRACE_EVENT_ASYNC_END2(
-        "ServiceWorker", "ServiceWorkerStorage::FindRegistrationForClientUrl",
-        callback_id, "Status", ServiceWorkerDatabase::StatusToString(status),
-        "Info",
-        (installing_status == blink::ServiceWorkerStatusCode::kOk)
-            ? "Installing registration is found"
-            : "Any registrations are not found");
-    return;
-  }
+  if (status != ServiceWorkerDatabase::STATUS_ERROR_NOT_FOUND)
+    ScheduleDeleteAndStartOver();
 
-  ScheduleDeleteAndStartOver();
   std::move(callback).Run(DatabaseStatusToStatusCode(status),
                           scoped_refptr<ServiceWorkerRegistration>());
-  TRACE_EVENT_ASYNC_END1(
-      "ServiceWorker", "ServiceWorkerStorage::FindRegistrationForClientUrl",
-      callback_id, "Status", ServiceWorkerDatabase::StatusToString(status));
 }
 
 void ServiceWorkerStorage::DidFindRegistrationForScope(

content/browser/service_worker/service_worker_storage.h

@@ -386,9 +386,7 @@ class CONTENT_EXPORT ServiceWorkerStorage {
   void DidReadInitialData(std::unique_ptr<InitialData> data,
                           ServiceWorkerDatabase::Status status);
   void DidFindRegistrationForClientUrl(
-      const GURL& client_url,
       FindRegistrationCallback callback,
-      int64_t callback_id,
       const ServiceWorkerDatabase::RegistrationData& data,
       const ResourceList& resources,
       ServiceWorkerDatabase::Status status);