Support retry for ServiceWorkerRegistry::StoreRegistration()

Adds a specialized InflightCall sub class for the method because
some parameters of ServiceWorkerStorageControl::StoreRegistration()
are move-only and we need to clone them for each method call.

Bug: 1133143
Change-Id: I5842f32a7b2d88786d6a04a7eae06e18e5c2d1a0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2550962
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#831225}

content/browser/service_worker/service_worker_registry.cc

@@ -107,6 +107,39 @@ class ServiceWorkerRegistry::InflightCallWithInvoker
   Invoker invoker_;
 };
 
+class ServiceWorkerRegistry::InflightCallStoreRegistration
+    : public ServiceWorkerRegistry::InflightCall {
+ public:
+  InflightCallStoreRegistration(
+      storage::mojom::ServiceWorkerRegistrationDataPtr data,
+      std::vector<storage::mojom::ServiceWorkerResourceRecordPtr> resources,
+      base::RepeatingCallback<
+          void(storage::mojom::ServiceWorkerDatabaseStatus status)> callback)
+      : data_(std::move(data)),
+        resources_(std::move(resources)),
+        callback_(std::move(callback)) {}
+  ~InflightCallStoreRegistration() override = default;
+
+  void Run(ServiceWorkerRegistry* registry) override {
+    DCHECK(registry);
+    DCHECK(registry->GetRemoteStorageControl().is_connected());
+    std::vector<storage::mojom::ServiceWorkerResourceRecordPtr>
+        passed_resources;
+    for (const auto& resource : resources_)
+      passed_resources.push_back(resource.Clone());
+
+    registry->GetRemoteStorageControl()->StoreRegistration(
+        data_.Clone(), std::move(passed_resources), callback_);
+  }
+
+ private:
+  storage::mojom::ServiceWorkerRegistrationDataPtr data_;
+  std::vector<storage::mojom::ServiceWorkerResourceRecordPtr> resources_;
+  base::RepeatingCallback<void(
+      storage::mojom::ServiceWorkerDatabaseStatus status)>
+      callback_;
+};
+
 // A helper class that runs on the IO thread to observe storage policy updates.
 class ServiceWorkerRegistry::StoragePolicyObserver
     : public storage::SpecialStoragePolicy::Observer {
@@ -376,12 +409,14 @@ void ServiceWorkerRegistry::StoreRegistration(
   }
   data->resources_total_size_bytes = resources_total_size_bytes;
 
-  GetRemoteStorageControl()->StoreRegistration(
+  uint64_t call_id = GetNextCallId();
+  auto call = std::make_unique<InflightCallStoreRegistration>(
       std::move(data), std::move(resources),
-      base::BindOnce(&ServiceWorkerRegistry::DidStoreRegistration,
-                     weak_factory_.GetWeakPtr(), registration->id(),
-                     resources_total_size_bytes, registration->scope(),
-                     std::move(callback)));
+      base::BindRepeating(&ServiceWorkerRegistry::DidStoreRegistration,
+                          weak_factory_.GetWeakPtr(), registration->id(),
+                          resources_total_size_bytes, registration->scope(),
+                          base::Passed(&callback), call_id));
+  StartRemoteCall(call_id, std::move(call));
 }
 
 void ServiceWorkerRegistry::DeleteRegistration(
@@ -1087,8 +1122,10 @@ void ServiceWorkerRegistry::DidStoreRegistration(
     uint64_t stored_resources_total_size_bytes,
     const GURL& stored_scope,
     StatusCallback callback,
+    uint64_t call_id,
     storage::mojom::ServiceWorkerDatabaseStatus database_status) {
   DCHECK_CURRENTLY_ON(ServiceWorkerContext::GetCoreThreadId());
+  FinishRemoteCall(call_id);
   blink::ServiceWorkerStatusCode status =
       DatabaseStatusToStatusCode(database_status);
 

content/browser/service_worker/service_worker_registry.h

@@ -314,6 +314,7 @@ class CONTENT_EXPORT ServiceWorkerRegistry {
       uint64_t stored_resources_total_size_bytes,
       const GURL& stored_scope,
       StatusCallback callback,
+      uint64_t call_id,
       storage::mojom::ServiceWorkerDatabaseStatus database_status);
   void DidDeleteRegistration(
       int64_t registration_id,
@@ -384,6 +385,10 @@ class CONTENT_EXPORT ServiceWorkerRegistry {
   // represent a mojo remote call of which parameters are copyable.
   class InflightCallWithInvoker;
 
+  // An InflightCall implementation for StoreRegistration(). This specialization
+  // is needed to hold move-only parameters and clone them for retry.
+  class InflightCallStoreRegistration;
+
   uint64_t GetNextCallId();
   void StartRemoteCall(uint64_t call_id, std::unique_ptr<InflightCall> call);
   void FinishRemoteCall(uint64_t call_id);

content/browser/service_worker/service_worker_registry_unittest.cc

@@ -1271,6 +1271,51 @@ TEST_F(ServiceWorkerRegistryTest, RemoteStorageDisconnection) {
   ASSERT_EQ(result.status, blink::ServiceWorkerStatusCode::kOk);
 }
 
+// Tests that inflight remote calls are retried after the remote storage is
+// restarted.
+TEST_F(ServiceWorkerRegistryTest, RetryInflightCalls) {
+  const GURL kScope1("http://www.example.com/scope/");
+  const GURL kScriptUrl1("http://www.example.com/script.js");
+  const auto kOrigin1(url::Origin::Create(kScope1));
+  scoped_refptr<ServiceWorkerRegistration> registration1 =
+      CreateServiceWorkerRegistrationAndVersion(context(), kScope1, kScriptUrl1,
+                                                /*resource_id=*/1);
+
+  const GURL kScope2("http://www.example.com/scope/foo");
+  const GURL kScriptUrl2("http://www.example.com/fooscript.js");
+  const auto kOrigin2(url::Origin::Create(kScope2));
+  scoped_refptr<ServiceWorkerRegistration> registration2 =
+      CreateServiceWorkerRegistrationAndVersion(context(), kScope2, kScriptUrl2,
+                                                /*resource_id=*/2);
+
+  {
+    registry()->SimulateStorageRestartForTesting();
+
+    base::RunLoop loop1;
+    registry()->StoreRegistration(
+        registration1.get(), registration1->waiting_version(),
+        base::BindLambdaForTesting([&](blink::ServiceWorkerStatusCode status) {
+          EXPECT_EQ(status, blink::ServiceWorkerStatusCode::kOk);
+          loop1.Quit();
+        }));
+
+    registry()->SimulateStorageRestartForTesting();
+
+    base::RunLoop loop2;
+    registry()->StoreRegistration(
+        registration2.get(), registration2->waiting_version(),
+        base::BindLambdaForTesting([&](blink::ServiceWorkerStatusCode status) {
+          EXPECT_EQ(status, blink::ServiceWorkerStatusCode::kOk);
+          loop2.Quit();
+        }));
+
+    registry()->SimulateStorageRestartForTesting();
+
+    loop1.Run();
+    loop2.Run();
+  }
+}
+
 class ServiceWorkerRegistryOriginTrialsTest : public ServiceWorkerRegistryTest {
  public:
   ServiceWorkerRegistryOriginTrialsTest() {