service worker: Allow LazyInitialize on resource reader/writer creation

This CL moves MakeSelfOwneReceiver() for resource reader/writer from
ServiceWorkerStorageControlImpl to ServiceWorkerStorage. This move
allows us to call LazyInitialize() if the internal storage hasn't been
initialized yet. This behavior simplifies service worker storage service
recovery steps because it would eliminate a need for force
initialization after restart.

As a side effect we need to update helper functions in
service_worker_storage_unittest.cc

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

content/browser/service_worker/service_worker_storage.cc

@@ -21,6 +21,7 @@
 #include "base/trace_event/trace_event.h"
 #include "components/services/storage/public/cpp/constants.h"
 #include "content/browser/service_worker/service_worker_disk_cache.h"
+#include "mojo/public/cpp/bindings/self_owned_receiver.h"
 #include "net/base/completion_once_callback.h"
 #include "net/base/net_errors.h"
 #include "storage/browser/quota/quota_manager_proxy.h"
@@ -578,22 +579,74 @@ void ServiceWorkerStorage::PerformStorageCleanup(base::OnceClosure callback) {
       std::move(callback));
 }
 
-std::unique_ptr<ServiceWorkerResourceReaderImpl>
-ServiceWorkerStorage::CreateResourceReader(int64_t resource_id) {
-  return std::make_unique<ServiceWorkerResourceReaderImpl>(
-      resource_id, disk_cache()->GetWeakPtr());
+void ServiceWorkerStorage::CreateResourceReader(
+    int64_t resource_id,
+    mojo::PendingReceiver<storage::mojom::ServiceWorkerResourceReader>
+        receiver) {
+  DCHECK_NE(resource_id, blink::mojom::kInvalidServiceWorkerResourceId);
+  switch (state_) {
+    case STORAGE_STATE_DISABLED:
+      return;
+    case STORAGE_STATE_INITIALIZING:
+    case STORAGE_STATE_UNINITIALIZED:
+      LazyInitialize(base::BindOnce(&ServiceWorkerStorage::CreateResourceReader,
+                                    weak_factory_.GetWeakPtr(), resource_id,
+                                    std::move(receiver)));
+      return;
+    case STORAGE_STATE_INITIALIZED:
+      break;
+  }
+
+  mojo::MakeSelfOwnedReceiver(std::make_unique<ServiceWorkerResourceReaderImpl>(
+                                  resource_id, disk_cache()->GetWeakPtr()),
+                              std::move(receiver));
 }
 
-std::unique_ptr<ServiceWorkerResourceWriterImpl>
-ServiceWorkerStorage::CreateResourceWriter(int64_t resource_id) {
-  return std::make_unique<ServiceWorkerResourceWriterImpl>(
-      resource_id, disk_cache()->GetWeakPtr());
+void ServiceWorkerStorage::CreateResourceWriter(
+    int64_t resource_id,
+    mojo::PendingReceiver<storage::mojom::ServiceWorkerResourceWriter>
+        receiver) {
+  DCHECK_NE(resource_id, blink::mojom::kInvalidServiceWorkerResourceId);
+  switch (state_) {
+    case STORAGE_STATE_DISABLED:
+      return;
+    case STORAGE_STATE_INITIALIZING:
+    case STORAGE_STATE_UNINITIALIZED:
+      LazyInitialize(base::BindOnce(&ServiceWorkerStorage::CreateResourceWriter,
+                                    weak_factory_.GetWeakPtr(), resource_id,
+                                    std::move(receiver)));
+      return;
+    case STORAGE_STATE_INITIALIZED:
+      break;
+  }
+
+  mojo::MakeSelfOwnedReceiver(std::make_unique<ServiceWorkerResourceWriterImpl>(
+                                  resource_id, disk_cache()->GetWeakPtr()),
+                              std::move(receiver));
 }
 
-std::unique_ptr<ServiceWorkerResourceMetadataWriterImpl>
-ServiceWorkerStorage::CreateResourceMetadataWriter(int64_t resource_id) {
-  return std::make_unique<ServiceWorkerResourceMetadataWriterImpl>(
-      resource_id, disk_cache()->GetWeakPtr());
+void ServiceWorkerStorage::CreateResourceMetadataWriter(
+    int64_t resource_id,
+    mojo::PendingReceiver<storage::mojom::ServiceWorkerResourceMetadataWriter>
+        receiver) {
+  DCHECK_NE(resource_id, blink::mojom::kInvalidServiceWorkerResourceId);
+  switch (state_) {
+    case STORAGE_STATE_DISABLED:
+      return;
+    case STORAGE_STATE_INITIALIZING:
+    case STORAGE_STATE_UNINITIALIZED:
+      LazyInitialize(base::BindOnce(
+          &ServiceWorkerStorage::CreateResourceMetadataWriter,
+          weak_factory_.GetWeakPtr(), resource_id, std::move(receiver)));
+      return;
+    case STORAGE_STATE_INITIALIZED:
+      break;
+  }
+
+  mojo::MakeSelfOwnedReceiver(
+      std::make_unique<ServiceWorkerResourceMetadataWriterImpl>(
+          resource_id, disk_cache()->GetWeakPtr()),
+      std::move(receiver));
 }
 
 void ServiceWorkerStorage::StoreUncommittedResourceId(

content/browser/service_worker/service_worker_storage.h

@@ -179,12 +179,18 @@ class CONTENT_EXPORT ServiceWorkerStorage {
 
   // Creates a resource accessor. Never returns nullptr but an accessor may be
   // associated with the disabled disk cache if the storage is disabled.
-  std::unique_ptr<ServiceWorkerResourceReaderImpl> CreateResourceReader(
-      int64_t resource_id);
-  std::unique_ptr<ServiceWorkerResourceWriterImpl> CreateResourceWriter(
-      int64_t resource_id);
-  std::unique_ptr<ServiceWorkerResourceMetadataWriterImpl>
-  CreateResourceMetadataWriter(int64_t resource_id);
+  void CreateResourceReader(
+      int64_t resource_id,
+      mojo::PendingReceiver<storage::mojom::ServiceWorkerResourceReader>
+          receiver);
+  void CreateResourceWriter(
+      int64_t resource_id,
+      mojo::PendingReceiver<storage::mojom::ServiceWorkerResourceWriter>
+          receiver);
+  void CreateResourceMetadataWriter(
+      int64_t resource_id,
+      mojo::PendingReceiver<storage::mojom::ServiceWorkerResourceMetadataWriter>
+          receiver);
 
   // Adds |resource_id| to the set of resources that are in the disk cache
   // but not yet stored with a registration.

content/browser/service_worker/service_worker_storage_control_impl.cc

@@ -7,7 +7,6 @@
 #include "content/browser/service_worker/service_worker_resource_ops.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/bindings/receiver_set.h"
-#include "mojo/public/cpp/bindings/self_owned_receiver.h"
 
 namespace content {
 
@@ -249,26 +248,20 @@ void ServiceWorkerStorageControlImpl::GetNewResourceId(
 void ServiceWorkerStorageControlImpl::CreateResourceReader(
     int64_t resource_id,
     mojo::PendingReceiver<storage::mojom::ServiceWorkerResourceReader> reader) {
-  DCHECK_NE(resource_id, blink::mojom::kInvalidServiceWorkerResourceId);
-  mojo::MakeSelfOwnedReceiver(storage_->CreateResourceReader(resource_id),
-                              std::move(reader));
+  storage_->CreateResourceReader(resource_id, std::move(reader));
 }
 
 void ServiceWorkerStorageControlImpl::CreateResourceWriter(
     int64_t resource_id,
     mojo::PendingReceiver<storage::mojom::ServiceWorkerResourceWriter> writer) {
-  DCHECK_NE(resource_id, blink::mojom::kInvalidServiceWorkerResourceId);
-  mojo::MakeSelfOwnedReceiver(storage_->CreateResourceWriter(resource_id),
-                              std::move(writer));
+  storage_->CreateResourceWriter(resource_id, std::move(writer));
 }
 
 void ServiceWorkerStorageControlImpl::CreateResourceMetadataWriter(
     int64_t resource_id,
     mojo::PendingReceiver<storage::mojom::ServiceWorkerResourceMetadataWriter>
         writer) {
-  DCHECK_NE(resource_id, blink::mojom::kInvalidServiceWorkerResourceId);
-  mojo::MakeSelfOwnedReceiver(
-      storage_->CreateResourceMetadataWriter(resource_id), std::move(writer));
+  storage_->CreateResourceMetadataWriter(resource_id, std::move(writer));
 }
 
 void ServiceWorkerStorageControlImpl::StoreUncommittedResourceId(

content/browser/service_worker/service_worker_storage_unittest.cc

@@ -20,6 +20,7 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
 #include "ipc/ipc_message.h"
+#include "mojo/public/cpp/bindings/remote.h"
 #include "net/base/net_errors.h"
 #include "net/disk_cache/disk_cache.h"
 #include "net/http/http_response_headers.h"
@@ -79,88 +80,6 @@ void DatabaseStatusCallback(
   std::move(quit_closure).Run();
 }
 
-ReadResponseHeadResult ReadResponseHead(ServiceWorkerStorage* storage,
-                                        int64_t id) {
-  std::unique_ptr<ServiceWorkerResourceReaderImpl> reader =
-      storage->CreateResourceReader(id);
-
-  ReadResponseHeadResult out;
-  base::RunLoop loop;
-  reader->ReadResponseHead(base::BindLambdaForTesting(
-      [&](int result, network::mojom::URLResponseHeadPtr response_head,
-          base::Optional<mojo_base::BigBuffer> metadata) {
-        out.result = result;
-        out.response_head = std::move(response_head);
-        out.metadata = std::move(metadata);
-        loop.Quit();
-      }));
-  loop.Run();
-  return out;
-}
-
-int WriteBasicResponse(ServiceWorkerStorage* storage, int64_t id) {
-  const std::string kHttpHeaders =
-      "HTTP/1.0 200 HONKYDORY\0Content-Length: 5\0\0";
-  const std::string kHttpBody = "Hello";
-
-  std::string headers(kHttpHeaders, base::size(kHttpHeaders));
-  mojo_base::BigBuffer body(
-      base::as_bytes(base::make_span(kHttpBody.data(), kHttpBody.length())));
-
-  std::unique_ptr<ServiceWorkerResourceWriterImpl> writer =
-      storage->CreateResourceWriter(id);
-
-  int rv = 0;
-  {
-    auto response_head = network::mojom::URLResponseHead::New();
-    response_head->request_time = base::Time::Now();
-    response_head->response_time = base::Time::Now();
-    response_head->headers = new net::HttpResponseHeaders(headers);
-    response_head->content_length = body.size();
-
-    base::RunLoop loop;
-    writer->WriteResponseHead(std::move(response_head),
-                              base::BindLambdaForTesting([&](int result) {
-                                rv = result;
-                                loop.Quit();
-                              }));
-    loop.Run();
-    if (rv < 0)
-      return rv;
-  }
-
-  {
-    base::RunLoop loop;
-    writer->WriteData(std::move(body),
-                      base::BindLambdaForTesting([&](int result) {
-                        rv = result;
-                        loop.Quit();
-                      }));
-    loop.Run();
-  }
-
-  return rv;
-}
-
-int WriteResponseMetadata(ServiceWorkerStorage* storage,
-                          int64_t id,
-                          const std::string& metadata) {
-  mojo_base::BigBuffer buffer(
-      base::as_bytes(base::make_span(metadata.data(), metadata.length())));
-
-  std::unique_ptr<ServiceWorkerResourceMetadataWriterImpl> metadata_writer =
-      storage->CreateResourceMetadataWriter(id);
-  int rv = 0;
-  base::RunLoop loop;
-  metadata_writer->WriteMetadata(std::move(buffer),
-                                 base::BindLambdaForTesting([&](int result) {
-                                   rv = result;
-                                   loop.Quit();
-                                 }));
-  loop.Run();
-  return rv;
-}
-
 class ServiceWorkerStorageTest : public testing::Test {
  public:
   ServiceWorkerStorageTest() = default;
@@ -490,6 +409,101 @@ class ServiceWorkerStorageTest : public testing::Test {
     return result;
   }
 
+  ReadResponseHeadResult ReadResponseHead(int64_t id) {
+    ReadResponseHeadResult out;
+    base::RunLoop loop;
+
+    mojo::Remote<storage::mojom::ServiceWorkerResourceReader> reader;
+    storage()->CreateResourceReader(id, reader.BindNewPipeAndPassReceiver());
+    reader.set_disconnect_handler(base::BindLambdaForTesting([&]() {
+      out.result = net::ERR_CACHE_MISS;
+      loop.Quit();
+    }));
+
+    reader->ReadResponseHead(base::BindLambdaForTesting(
+        [&](int result, network::mojom::URLResponseHeadPtr response_head,
+            base::Optional<mojo_base::BigBuffer> metadata) {
+          out.result = result;
+          out.response_head = std::move(response_head);
+          out.metadata = std::move(metadata);
+          loop.Quit();
+        }));
+    loop.Run();
+    return out;
+  }
+
+  int WriteBasicResponse(int64_t id) {
+    const std::string kHttpHeaders =
+        "HTTP/1.0 200 HONKYDORY\0Content-Length: 5\0\0";
+    const std::string kHttpBody = "Hello";
+
+    std::string headers(kHttpHeaders, base::size(kHttpHeaders));
+    mojo_base::BigBuffer body(
+        base::as_bytes(base::make_span(kHttpBody.data(), kHttpBody.length())));
+
+    mojo::Remote<storage::mojom::ServiceWorkerResourceWriter> writer;
+    storage()->CreateResourceWriter(id, writer.BindNewPipeAndPassReceiver());
+
+    int rv = 0;
+    {
+      auto response_head = network::mojom::URLResponseHead::New();
+      response_head->request_time = base::Time::Now();
+      response_head->response_time = base::Time::Now();
+      response_head->headers = new net::HttpResponseHeaders(headers);
+      response_head->content_length = body.size();
+
+      base::RunLoop loop;
+      writer.set_disconnect_handler(base::BindLambdaForTesting([&]() {
+        rv = net::ERR_FAILED;
+        loop.Quit();
+      }));
+      writer->WriteResponseHead(std::move(response_head),
+                                base::BindLambdaForTesting([&](int result) {
+                                  rv = result;
+                                  loop.Quit();
+                                }));
+      loop.Run();
+      if (rv < 0)
+        return rv;
+    }
+
+    {
+      base::RunLoop loop;
+      writer->WriteData(std::move(body),
+                        base::BindLambdaForTesting([&](int result) {
+                          rv = result;
+                          loop.Quit();
+                        }));
+      loop.Run();
+    }
+
+    return rv;
+  }
+
+  int WriteResponseMetadata(int64_t id, const std::string& metadata) {
+    mojo_base::BigBuffer buffer(
+        base::as_bytes(base::make_span(metadata.data(), metadata.length())));
+
+    mojo::Remote<storage::mojom::ServiceWorkerResourceMetadataWriter>
+        metadata_writer;
+    storage()->CreateResourceMetadataWriter(
+        id, metadata_writer.BindNewPipeAndPassReceiver());
+
+    int rv = 0;
+    base::RunLoop loop;
+    metadata_writer.set_disconnect_handler(base::BindLambdaForTesting([&]() {
+      rv = net::ERR_FAILED;
+      loop.Quit();
+    }));
+    metadata_writer->WriteMetadata(std::move(buffer),
+                                   base::BindLambdaForTesting([&](int result) {
+                                     rv = result;
+                                     loop.Quit();
+                                   }));
+    loop.Run();
+    return rv;
+  }
+
   ServiceWorkerDatabase::Status StoreRegistrationData(
       storage::mojom::ServiceWorkerRegistrationDataPtr registration_data,
       std::vector<ResourceRecord> resources) {
@@ -559,11 +573,10 @@ TEST_F(ServiceWorkerStorageTest, DisabledStorage) {
 
   // Response reader and writer created by the disabled storage should fail to
   // access the disk cache.
-  ReadResponseHeadResult out = ReadResponseHead(storage(), kResourceId);
+  ReadResponseHeadResult out = ReadResponseHead(kResourceId);
   EXPECT_EQ(out.result, net::ERR_CACHE_MISS);
-  EXPECT_EQ(WriteBasicResponse(storage(), kResourceId), net::ERR_FAILED);
-  EXPECT_EQ(WriteResponseMetadata(storage(), kResourceId, "foo"),
-            net::ERR_FAILED);
+  EXPECT_EQ(WriteBasicResponse(kResourceId), net::ERR_FAILED);
+  EXPECT_EQ(WriteResponseMetadata(kResourceId, "foo"), net::ERR_FAILED);
 
   const std::string kUserDataKey = "key";
   std::vector<std::string> user_data_out;
@@ -829,7 +842,7 @@ class ServiceWorkerStorageDiskTest : public ServiceWorkerStorageTest {
 
     ASSERT_EQ(StoreRegistrationData(std::move(data), std::move(resources)),
               ServiceWorkerDatabase::Status::kOk);
-    WriteBasicResponse(storage(), 1);
+    WriteBasicResponse(1);
   }
 };