service worker: Prefer RunLoop over RunUntilIdle for storage unittest.

The test was using RunUntilIdle() to run the tasks posted to the IO
thread as well as the database thread. It's better to use explicit
RunLoops so it's clear what tasks are being waited on. This also helps
remove the requirement in tests to set the database thread to the UI/IO
thread[1], which will help unblock creating StoragePartitionImpl for tests,
which will help unblock ServiceWorkerOnUI.

As part of that direction, the CL also changes the test to use the
database task runner explicitly when needed.

However many other tests also use RunUntilIdle() like the background
sync tests and depend on it to pump the database tasks, so I might
not complete this work fully.

[1] EmbeddedWorkerTestHelper's constructor creates
ServiceWorkerContextWrapper and does:

  scoped_refptr<base::SequencedTaskRunner> database_task_runner =
        base::ThreadTaskRunnerHandle::Get();
  wrapper_->InitOnCoreThread(..., std::move(database_task_runner), ...

Bug: 1000141
Change-Id: I61efe2925a33b2b1afd62c407caee4c373476ce4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1782458Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#693024}

content/browser/service_worker/service_worker_register_job.cc

@@ -821,8 +821,13 @@ void ServiceWorkerRegisterJob::BumpLastUpdateCheckTimeIfNeeded() {
       registration()->last_update_check().is_null()) {
     registration()->set_last_update_check(base::Time::Now());
 
-    if (registration()->newest_installed_version())
-      context_->storage()->UpdateLastUpdateCheckTime(registration());
+    if (registration()->newest_installed_version()) {
+      context_->storage()->UpdateLastUpdateCheckTime(
+          registration(),
+          base::BindOnce([](blink::ServiceWorkerStatusCode status) {
+            // Ignore errors; bumping the update check time is just best-effort.
+          }));
+    }
   }
 }
 

content/browser/service_worker/service_worker_storage.cc

@@ -500,21 +500,30 @@ void ServiceWorkerStorage::UpdateToActiveState(
 }
 
 void ServiceWorkerStorage::UpdateLastUpdateCheckTime(
-    ServiceWorkerRegistration* registration) {
+    ServiceWorkerRegistration* registration,
+    StatusCallback callback) {
   DCHECK(registration);
   DCHECK(state_ == STORAGE_STATE_INITIALIZED ||
          state_ == STORAGE_STATE_DISABLED)
       << state_;
-  if (IsDisabled())
+  if (IsDisabled()) {
+    RunSoon(FROM_HERE,
+            base::BindOnce(std::move(callback),
+                           blink::ServiceWorkerStatusCode::kErrorAbort));
     return;
+  }
 
-  database_task_runner_->PostTask(
-      FROM_HERE,
+  base::PostTaskAndReplyWithResult(
+      database_task_runner_.get(), FROM_HERE,
+      base::BindOnce(&ServiceWorkerDatabase::UpdateLastCheckTime,
+                     base::Unretained(database_.get()), registration->id(),
+                     registration->scope().GetOrigin(),
+                     registration->last_update_check()),
       base::BindOnce(
-          base::IgnoreResult(&ServiceWorkerDatabase::UpdateLastCheckTime),
-          base::Unretained(database_.get()), registration->id(),
-          registration->scope().GetOrigin(),
-          registration->last_update_check()));
+          [](StatusCallback callback, ServiceWorkerDatabase::Status status) {
+            std::move(callback).Run(DatabaseStatusToStatusCode(status));
+          },
+          std::move(callback)));
 }
 
 void ServiceWorkerStorage::UpdateNavigationPreloadEnabled(
@@ -1185,6 +1194,11 @@ void ServiceWorkerStorage::LazyInitializeForTest() {
   loop.Run();
 }
 
+void ServiceWorkerStorage::SetPurgingCompleteCallbackForTest(
+    base::OnceClosure callback) {
+  purging_complete_callback_for_test_ = std::move(callback);
+}
+
 void ServiceWorkerStorage::LazyInitialize(base::OnceClosure callback) {
   DCHECK(state_ == STORAGE_STATE_UNINITIALIZED ||
          state_ == STORAGE_STATE_INITIALIZING)
@@ -1766,8 +1780,13 @@ void ServiceWorkerStorage::StartPurgingResources(
 }
 
 void ServiceWorkerStorage::ContinuePurgingResources() {
-  if (purgeable_resource_ids_.empty() || is_purge_pending_)
+  if (is_purge_pending_)
     return;
+  if (purgeable_resource_ids_.empty()) {
+    if (purging_complete_callback_for_test_)
+      std::move(purging_complete_callback_for_test_).Run();
+    return;
+  }
 
   // Do one at a time until we're done, use RunSoon to avoid recursion when
   // DoomEntry returns immediately.

content/browser/service_worker/service_worker_storage.h

@@ -153,7 +153,8 @@ class CONTENT_EXPORT ServiceWorkerStorage
 
   // Updates the stored time to match the value of
   // registration->last_update_check().
-  void UpdateLastUpdateCheckTime(ServiceWorkerRegistration* registration);
+  void UpdateLastUpdateCheckTime(ServiceWorkerRegistration* registration,
+                                 StatusCallback callback);
 
   // Updates the specified registration's navigation preload state in storage.
   // The caller is responsible for mutating the live registration's state.
@@ -289,6 +290,8 @@ class CONTENT_EXPORT ServiceWorkerStorage
 
   void LazyInitializeForTest();
 
+  void SetPurgingCompleteCallbackForTest(base::OnceClosure callback);
+
  private:
   friend class service_worker_storage_unittest::ServiceWorkerStorageTest;
   friend class service_worker_storage_unittest::
@@ -614,6 +617,7 @@ class CONTENT_EXPORT ServiceWorkerStorage
   base::circular_deque<int64_t> purgeable_resource_ids_;
   bool is_purge_pending_;
   bool has_checked_for_stale_resources_;
+  base::OnceClosure purging_complete_callback_for_test_;
 
   base::WeakPtrFactory<ServiceWorkerStorage> weak_factory_{this};