Introduce ServiceWorkerStorage::CreateNewResponseWriter()

This method puts resource id assignment and response writer creation
together so that call sites don't have to assign a resource id
explicitly for new resources. See the linked bug for the context of
this change.

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

content/browser/service_worker/service_worker_job_unittest.cc

@@ -1122,14 +1122,10 @@ void OnIOComplete(int* rv_out, int rv) {
   *rv_out = rv;
 }
 
-void WriteResponse(ServiceWorkerStorage* storage,
-                   int64_t id,
+void WriteResponse(ServiceWorkerResponseWriter* writer,
                    const std::string& headers,
                    IOBuffer* body,
                    int length) {
-  std::unique_ptr<ServiceWorkerResponseWriter> writer =
-      storage->CreateResponseWriter(id);
-
   std::unique_ptr<net::HttpResponseInfo> info =
       std::make_unique<net::HttpResponseInfo>();
   info->request_time = base::Time::Now();
@@ -1150,14 +1146,13 @@ void WriteResponse(ServiceWorkerStorage* storage,
   EXPECT_EQ(length, rv);
 }
 
-void WriteStringResponse(ServiceWorkerStorage* storage,
-                         int64_t id,
+void WriteStringResponse(ServiceWorkerResponseWriter* writer,
                          const std::string& body) {
   scoped_refptr<IOBuffer> body_buffer =
       base::MakeRefCounted<WrappedIOBuffer>(body.data());
   const char kHttpHeaders[] = "HTTP/1.0 200 HONKYDORY\0\0";
   std::string headers(kHttpHeaders, base::size(kHttpHeaders));
-  WriteResponse(storage, id, headers, body_buffer.get(), body.length());
+  WriteResponse(writer, headers, body_buffer.get(), body.length());
 }
 
 class UpdateJobTestHelper : public EmbeddedWorkerTestHelper,
@@ -1258,17 +1253,19 @@ class UpdateJobTestHelper : public EmbeddedWorkerTestHelper,
     bool is_update = registration->active_version() &&
                      version != registration->active_version();
 
-    int64_t resource_id = storage()->NewResourceId();
+    std::unique_ptr<ServiceWorkerResponseWriter> writer =
+        CreateNewResponseWriterSync(storage());
+    int64_t resource_id = writer->response_id();
     version->script_cache_map()->NotifyStartedCaching(script, resource_id);
     if (!is_update) {
       // Spoof caching the script for the initial version.
-      WriteStringResponse(storage(), resource_id, kBody);
+      WriteStringResponse(writer.get(), kBody);
       version->script_cache_map()->NotifyFinishedCaching(
           script, sizeof(kBody) / sizeof(char), net::OK, std::string());
     } else {
       EXPECT_NE(NoChangeOrigin(), script.GetOrigin());
       // The script must be changed.
-      WriteStringResponse(storage(), resource_id, kNewBody);
+      WriteStringResponse(writer.get(), kNewBody);
       version->script_cache_map()->NotifyFinishedCaching(
           script, sizeof(kNewBody) / sizeof(char), net::OK, std::string());
     }
@@ -1711,8 +1708,10 @@ TEST_F(ServiceWorkerUpdateJobTest, Update_ScriptUrlChanged) {
 
   // Make sure the storage has the data of the current waiting version.
   const int64_t resource_id = 2;
+  std::unique_ptr<ServiceWorkerResponseWriter> writer =
+      storage()->CreateResponseWriter(resource_id);
   version->script_cache_map()->NotifyStartedCaching(new_script, resource_id);
-  WriteStringResponse(update_helper_->storage(), resource_id, kBody);
+  WriteStringResponse(writer.get(), kBody);
   version->script_cache_map()->NotifyFinishedCaching(
       new_script, sizeof(kBody) / sizeof(char), net::OK, std::string());
 

content/browser/service_worker/service_worker_storage.cc

@@ -491,6 +491,16 @@ ServiceWorkerStorage::CreateResponseMetadataWriter(int64_t resource_id) {
       resource_id, disk_cache()->GetWeakPtr()));
 }
 
+void ServiceWorkerStorage::CreateNewResponseWriter(
+    ResponseWriterCreationCallback callback) {
+  int64_t resource_id = NewResourceId();
+  if (resource_id == ServiceWorkerConsts::kInvalidServiceWorkerResourceId) {
+    std::move(callback).Run(resource_id, nullptr);
+  } else {
+    std::move(callback).Run(resource_id, CreateResponseWriter(resource_id));
+  }
+}
+
 void ServiceWorkerStorage::StoreUncommittedResourceId(
     int64_t resource_id,
     DatabaseStatusCallback callback) {

content/browser/service_worker/service_worker_storage.h

@@ -90,6 +90,10 @@ class CONTENT_EXPORT ServiceWorkerStorage {
       int64_t deleted_version_id,
       const std::vector<int64_t>& newly_purgeable_resources)>;
 
+  using ResponseWriterCreationCallback = base::OnceCallback<void(
+      int64_t resource_id,
+      std::unique_ptr<ServiceWorkerResponseWriter> response_writer)>;
+
   using DatabaseStatusCallback =
       base::OnceCallback<void(ServiceWorkerDatabase::Status status)>;
   using GetUserDataInDBCallback =
@@ -192,6 +196,14 @@ class CONTENT_EXPORT ServiceWorkerStorage {
   std::unique_ptr<ServiceWorkerResponseMetadataWriter>
   CreateResponseMetadataWriter(int64_t resource_id);
 
+  // Assigns a new resource ID and creates a response writer associated with
+  // the resource ID. If ID allocation fails, kInvalidServiceWorkerResourceId
+  // and null writer are returned.
+  // NOTE: Currently this method is synchronous but intentionally uses async
+  // style because ServiceWorkerStorage will be accessed via mojo calls soon.
+  // See crbug.com/1046335 for details.
+  void CreateNewResponseWriter(ResponseWriterCreationCallback callback);
+
   // Adds |resource_id| to the set of resources that are in the disk cache
   // but not yet stored with a registration.
   void StoreUncommittedResourceId(int64_t resource_id,

content/browser/service_worker/service_worker_test_utils.cc

@@ -430,6 +430,21 @@ ServiceWorkerDatabase::ResourceRecord WriteToDiskCacheAsync(
       std::move(callback));
 }
 
+std::unique_ptr<ServiceWorkerResponseWriter> CreateNewResponseWriterSync(
+    ServiceWorkerStorage* storage) {
+  base::RunLoop run_loop;
+  std::unique_ptr<ServiceWorkerResponseWriter> writer;
+  storage->CreateNewResponseWriter(base::BindLambdaForTesting(
+      [&](int64_t /*resource_id*/,
+          std::unique_ptr<ServiceWorkerResponseWriter> new_writer) {
+        DCHECK(new_writer);
+        writer = std::move(new_writer);
+        run_loop.Quit();
+      }));
+  run_loop.Run();
+  return writer;
+}
+
 MockServiceWorkerResponseReader::MockServiceWorkerResponseReader()
     : ServiceWorkerResponseReader(/* resource_id=*/0, /*disk_cache=*/nullptr) {}
 

content/browser/service_worker/service_worker_test_utils.h

@@ -172,6 +172,11 @@ ServiceWorkerDatabase::ResourceRecord WriteToDiskCacheAsync(
     const std::string& meta_data,
     base::OnceClosure callback);
 
+// Calls ServiceWorkerStorage::CreateNewResponseWriter() and returns the
+// created writer synchronously.
+std::unique_ptr<ServiceWorkerResponseWriter> CreateNewResponseWriterSync(
+    ServiceWorkerStorage* storage);
+
 // A test implementation of ServiceWorkerResponseReader.
 //
 // This class exposes the ability to expect reads (see ExpectRead*() below).