Remove direct use of old service worker response writers

This CL replaces old writers with new mojo-based writers in tests.

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

content/browser/service_worker/service_worker_job_unittest.cc

@@ -13,6 +13,7 @@
 #include "base/optional.h"
 #include "base/run_loop.h"
 #include "base/stl_util.h"
+#include "base/test/bind_test_util.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/test/test_simple_task_runner.h"
@@ -1171,18 +1172,11 @@ const char kHeaders[] =
 const char kBody[] = "/* old body */";
 const char kNewBody[] = "/* new body */";
 
-void RunNestedUntilIdle() {
-  base::RunLoop(base::RunLoop::Type::kNestableTasksAllowed).RunUntilIdle();
-}
-
-void OnIOComplete(int* rv_out, int rv) {
-  *rv_out = rv;
-}
-
-void WriteResponse(ServiceWorkerResponseWriter* writer,
-                   const std::string& headers,
-                   IOBuffer* body,
-                   int length) {
+void WriteResponse(
+    mojo::Remote<storage::mojom::ServiceWorkerResourceWriter>& writer,
+    const std::string& headers,
+    mojo_base::BigBuffer body) {
+  int length = body.size();
   auto response_head = network::mojom::URLResponseHead::New();
   response_head->request_time = base::Time::Now();
   response_head->response_time = base::Time::Now();
@@ -1190,24 +1184,37 @@ void WriteResponse(ServiceWorkerResponseWriter* writer,
   response_head->content_length = length;
 
   int rv = -1234;
-  writer->WriteResponseHead(*response_head, length,
-                            base::BindOnce(&OnIOComplete, &rv));
-  RunNestedUntilIdle();
-  EXPECT_LT(0, rv);
+  {
+    base::RunLoop loop(base::RunLoop::Type::kNestableTasksAllowed);
+    writer->WriteResponseHead(std::move(response_head),
+                              base::BindLambdaForTesting([&](int result) {
+                                rv = result;
+                                loop.Quit();
+                              }));
+    loop.Run();
+    EXPECT_LT(0, rv);
+  }
 
   rv = -1234;
-  writer->WriteData(body, length, base::BindOnce(&OnIOComplete, &rv));
-  RunNestedUntilIdle();
-  EXPECT_EQ(length, rv);
+  {
+    base::RunLoop loop(base::RunLoop::Type::kNestableTasksAllowed);
+    writer->WriteData(std::move(body),
+                      base::BindLambdaForTesting([&](int result) {
+                        rv = result;
+                        loop.Quit();
+                      }));
+    loop.Run();
+    EXPECT_EQ(length, rv);
+  }
 }
 
-void WriteStringResponse(ServiceWorkerResponseWriter* writer,
-                         const std::string& body) {
-  scoped_refptr<IOBuffer> body_buffer =
-      base::MakeRefCounted<WrappedIOBuffer>(body.data());
+void WriteStringResponse(
+    mojo::Remote<storage::mojom::ServiceWorkerResourceWriter>& writer,
+    const std::string& body) {
+  mojo_base::BigBuffer body_buffer(base::as_bytes(base::make_span(body)));
   const char kHttpHeaders[] = "HTTP/1.0 200 HONKYDORY\0\0";
   std::string headers(kHttpHeaders, base::size(kHttpHeaders));
-  WriteResponse(writer, headers, body_buffer.get(), body.length());
+  WriteResponse(writer, headers, std::move(body_buffer));
 }
 
 class UpdateJobTestHelper : public EmbeddedWorkerTestHelper,
@@ -1322,6 +1329,14 @@ class UpdateJobTestHelper : public EmbeddedWorkerTestHelper,
   // EmbeddedWorkerTestHelper overrides:
   void PopulateScriptCacheMap(int64_t version_id,
                               base::OnceClosure callback) override {
+    context()->GetStorageControl()->GetNewResourceId(base::BindOnce(
+        &UpdateJobTestHelper::DidGetNewResourceIdForScriptCache,
+        weak_factory_.GetWeakPtr(), version_id, std::move(callback)));
+  }
+
+  void DidGetNewResourceIdForScriptCache(int64_t version_id,
+                                         base::OnceClosure callback,
+                                         int64_t resource_id) {
     ServiceWorkerVersion* version = context()->GetLiveVersion(version_id);
     ASSERT_TRUE(version);
     ServiceWorkerRegistration* registration =
@@ -1331,19 +1346,19 @@ class UpdateJobTestHelper : public EmbeddedWorkerTestHelper,
     bool is_update = registration->active_version() &&
                      version != registration->active_version();
 
-    std::unique_ptr<ServiceWorkerResponseWriter> writer =
-        CreateNewResponseWriterSync(storage());
-    int64_t resource_id = writer->response_id();
+    mojo::Remote<storage::mojom::ServiceWorkerResourceWriter> writer;
+    context()->GetStorageControl()->CreateResourceWriter(
+        resource_id, writer.BindNewPipeAndPassReceiver());
     version->script_cache_map()->NotifyStartedCaching(script, resource_id);
     if (!is_update) {
       // Spoof caching the script for the initial version.
-      WriteStringResponse(writer.get(), kBody);
+      WriteStringResponse(writer, kBody);
       version->script_cache_map()->NotifyFinishedCaching(
           script, base::size(kBody), net::OK, std::string());
     } else {
       EXPECT_NE(GURL(kNoChangeOrigin), script.GetOrigin());
       // The script must be changed.
-      WriteStringResponse(writer.get(), kNewBody);
+      WriteStringResponse(writer, kNewBody);
       version->script_cache_map()->NotifyFinishedCaching(
           script, base::size(kNewBody), net::OK, std::string());
     }
@@ -1750,10 +1765,11 @@ 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);
+  mojo::Remote<storage::mojom::ServiceWorkerResourceWriter> writer;
+  context()->GetStorageControl()->CreateResourceWriter(
+      resource_id, writer.BindNewPipeAndPassReceiver());
   version->script_cache_map()->NotifyStartedCaching(new_script, resource_id);
-  WriteStringResponse(writer.get(), kBody);
+  WriteStringResponse(writer, kBody);
   version->script_cache_map()->NotifyFinishedCaching(
       new_script, base::size(kBody), net::OK, std::string());
 

content/browser/service_worker/service_worker_new_script_loader_unittest.cc

@@ -218,7 +218,7 @@ class ServiceWorkerNewScriptLoaderTest : public testing::Test {
     int routing_id = 0;
     int request_id = 10;
     uint32_t options = 0;
-    int64_t resource_id = GetNewResourceIdSync(context()->storage());
+    int64_t resource_id = GetNewResourceIdSync(context()->GetStorageControl());
 
     network::ResourceRequest request;
     request.url = url;

content/browser/service_worker/service_worker_storage_unittest.cc

@@ -1304,8 +1304,8 @@ class ServiceWorkerResourceStorageTest : public ServiceWorkerStorageTest {
     script_ = GURL("http://www.test.not/script.js");
     import_ = GURL("http://www.test.not/import.js");
     document_url_ = GURL("http://www.test.not/scope/document.html");
-    resource_id1_ = GetNewResourceIdSync(storage());
-    resource_id2_ = GetNewResourceIdSync(storage());
+    resource_id1_ = GetNewResourceIdSync(storage_control());
+    resource_id2_ = GetNewResourceIdSync(storage_control());
     resource_id1_size_ = 239193;
     resource_id2_size_ = 59923;
 
@@ -1380,7 +1380,7 @@ TEST_F(ServiceWorkerResourceStorageTest,
        WriteMetadataWithServiceWorkerResponseMetadataWriter) {
   const char kMetadata1[] = "Test metadata";
   const char kMetadata2[] = "small";
-  int64_t new_resource_id_ = GetNewResourceIdSync(storage());
+  int64_t new_resource_id_ = GetNewResourceIdSync(storage_control());
   // Writing metadata to nonexistent resoirce ID must fail.
   EXPECT_GE(0, WriteResponseMetadata(storage_control(), new_resource_id_,
                                      kMetadata1));
@@ -1544,7 +1544,7 @@ TEST_F(ServiceWorkerResourceStorageDiskTest, CleanupOnRestart) {
   EXPECT_TRUE(VerifyBasicResponse(storage_control(), resource_id2_, true));
 
   // Also add an uncommitted resource.
-  int64_t kStaleUncommittedResourceId = GetNewResourceIdSync(storage());
+  int64_t kStaleUncommittedResourceId = GetNewResourceIdSync(storage_control());
   registry()->StoreUncommittedResourceId(kStaleUncommittedResourceId,
                                          registration_->scope());
   verify_ids = GetUncommittedResourceIdsFromDB();
@@ -1561,7 +1561,7 @@ TEST_F(ServiceWorkerResourceStorageDiskTest, CleanupOnRestart) {
   // Store a new uncommitted resource. This triggers stale resource cleanup.
   base::RunLoop loop;
   storage()->SetPurgingCompleteCallbackForTest(loop.QuitClosure());
-  int64_t kNewResourceId = GetNewResourceIdSync(storage());
+  int64_t kNewResourceId = GetNewResourceIdSync(storage_control());
   WriteBasicResponse(storage_control(), kNewResourceId);
   registry()->StoreUncommittedResourceId(kNewResourceId,
                                          registration_->scope());

content/browser/service_worker/service_worker_test_utils.cc

@@ -518,15 +518,8 @@ void WriteToDiskCacheAsync(
                                    std::move(writer), std::move(callback)));
 }
 
-std::unique_ptr<ServiceWorkerResponseWriter> CreateNewResponseWriterSync(
-    ServiceWorkerStorage* storage) {
-  base::RunLoop run_loop;
-  std::unique_ptr<ServiceWorkerResponseWriter> writer;
-  int64_t resource_id = GetNewResourceIdSync(storage);
-  return storage->CreateResponseWriter(resource_id);
-}
-
-int64_t GetNewResourceIdSync(ServiceWorkerStorage* storage) {
+int64_t GetNewResourceIdSync(
+    mojo::Remote<storage::mojom::ServiceWorkerStorageControl>& storage) {
   base::RunLoop run_loop;
   int64_t resource_id;
   storage->GetNewResourceId(

content/browser/service_worker/service_worker_test_utils.h

@@ -201,13 +201,9 @@ void WriteToDiskCacheAsync(
     const std::string& meta_data,
     WriteToDiskCacheCallback callback);
 
-// Calls ServiceWorkerStorage::CreateNewResponseWriter() and returns the
-// created writer synchronously.
-std::unique_ptr<ServiceWorkerResponseWriter> CreateNewResponseWriterSync(
-    ServiceWorkerStorage* storage);
-
-// Calls ServiceWorkerStorage::GetNewResourceId() synchronously.
-int64_t GetNewResourceIdSync(ServiceWorkerStorage* storage);
+// Calls ServiceWorkerStorageControl::GetNewResourceId() synchronously.
+int64_t GetNewResourceIdSync(
+    mojo::Remote<storage::mojom::ServiceWorkerStorageControl>& storage);
 
 // A test implementation of ServiceWorkerResourceReader.
 //