Use mojofied resource reader in ServiceWorkerCacheWriter

This CL starts to use storage::mojom::ServiceWorkerResourceReader in
ServiceWorkerCacheWriter.
Previously we used ServiceWorkerResponseReader, whose interface is to
pass an IO buffer and get the response asynchronously. However, the new
interface is using Mojo's data pipe to read the body, whose interface is
to ask to read and get the buffer. DataPipeReader is introduced as a
convenient class to encapsulate the difference.

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

content/browser/service_worker/service_worker_cache_writer.h

@@ -13,7 +13,9 @@
 
 #include "base/callback.h"
 #include "base/memory/weak_ptr.h"
+#include "components/services/storage/public/mojom/service_worker_storage_control.mojom.h"
 #include "content/common/content_export.h"
+#include "mojo/public/cpp/bindings/remote.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 #include "services/network/public/mojom/url_response_head.mojom.h"
@@ -35,6 +37,12 @@ class ServiceWorkerResponseWriter;
 //
 // This class's behavior is modelled as a state machine; see the DoLoop function
 // for comments about this.
+//
+// Note that currently we have two types of interfaces to create an instance of
+// ServiceWorkerCacheWriter: storage service and non storage service.
+// After storage service is shipped, we use Mojo connection to read and write
+// the resource.
+// See https://crbug.com/1055677 for more info.
 class CONTENT_EXPORT ServiceWorkerCacheWriter {
  public:
   using OnWriteCompleteCallback = base::OnceCallback<void(net::Error)>;
@@ -63,9 +71,14 @@ class CONTENT_EXPORT ServiceWorkerCacheWriter {
 
   // Create a cache writer instance that copies a script already in storage. The
   // script is read by |copy_reader|.
+  // Non-storage service:
   static std::unique_ptr<ServiceWorkerCacheWriter> CreateForCopy(
       std::unique_ptr<ServiceWorkerResponseReader> copy_reader,
       std::unique_ptr<ServiceWorkerResponseWriter> writer);
+  // Storage service:
+  static std::unique_ptr<ServiceWorkerCacheWriter> CreateForCopy(
+      mojo::Remote<storage::mojom::ServiceWorkerResourceReader> copy_reader,
+      std::unique_ptr<ServiceWorkerResponseWriter> writer);
 
   // Create a cache writer instance that unconditionally write back data
   // supplied to |MaybeWriteHeaders| and |MaybeWriteData| to storage.
@@ -82,11 +95,18 @@ class CONTENT_EXPORT ServiceWorkerCacheWriter {
   // resumed later. If |pause_when_not_identical| is false, and the data is
   // different, it would be written to storage directly. |copy_reader| is used
   // for copying identical data blocks during writing.
+  // Non-storage service:
   static std::unique_ptr<ServiceWorkerCacheWriter> CreateForComparison(
       std::unique_ptr<ServiceWorkerResponseReader> compare_reader,
       std::unique_ptr<ServiceWorkerResponseReader> copy_reader,
       std::unique_ptr<ServiceWorkerResponseWriter> writer,
       bool pause_when_not_identical);
+  // Storage service:
+  static std::unique_ptr<ServiceWorkerCacheWriter> CreateForComparison(
+      mojo::Remote<storage::mojom::ServiceWorkerResourceReader> compare_reader,
+      mojo::Remote<storage::mojom::ServiceWorkerResourceReader> copy_reader,
+      std::unique_ptr<ServiceWorkerResponseWriter> writer,
+      bool pause_when_not_identical);
 
   ~ServiceWorkerCacheWriter();
 
@@ -140,6 +160,7 @@ class CONTENT_EXPORT ServiceWorkerCacheWriter {
 
  private:
   class ReadResponseHeadCallbackAdapter;
+  class DataPipeReader;
 
   friend class ServiceWorkerUpdateCheckTestUtils;
 
@@ -200,11 +221,18 @@ class CONTENT_EXPORT ServiceWorkerCacheWriter {
     STATE_DONE,
   };
 
+  // Non-storage service:
   ServiceWorkerCacheWriter(
       std::unique_ptr<ServiceWorkerResponseReader> compare_reader,
       std::unique_ptr<ServiceWorkerResponseReader> copy_reader,
       std::unique_ptr<ServiceWorkerResponseWriter> writer,
       bool pause_when_not_identical);
+  // Storage service:
+  ServiceWorkerCacheWriter(
+      mojo::Remote<storage::mojom::ServiceWorkerResourceReader> compare_reader,
+      mojo::Remote<storage::mojom::ServiceWorkerResourceReader> copy_reader,
+      std::unique_ptr<ServiceWorkerResponseWriter> writer,
+      bool pause_when_not_identical);
 
   // Drives this class's state machine. This function steps the state machine
   // until one of:
@@ -244,11 +272,20 @@ class CONTENT_EXPORT ServiceWorkerCacheWriter {
   //   a) Return ERR_IO_PENDING, and schedule a callback to run the state
   //      machine's Run() later, or
   //   b) Return some other value and do not schedule a callback.
+  // Non-storage service:
   int ReadResponseHead(
       const std::unique_ptr<ServiceWorkerResponseReader>& reader);
   int ReadDataHelper(const std::unique_ptr<ServiceWorkerResponseReader>& reader,
                      net::IOBuffer* buf,
                      int buf_len);
+  // Storage service:
+  // These are always case a) above.
+  int ReadResponseHead(storage::mojom::ServiceWorkerResourceReader* reader);
+  int ReadDataHelper(storage::mojom::ServiceWorkerResourceReader* reader,
+                     std::unique_ptr<DataPipeReader>& data_pipe_reader,
+                     net::IOBuffer* buf,
+                     int buf_len);
+
   // If no write observer is set through set_write_observer(),
   // WriteResponseHead() operates the same as
   // WriteResponseHeadToResponseWriter() and WriteData() operates the same as
@@ -318,8 +355,15 @@ class CONTENT_EXPORT ServiceWorkerCacheWriter {
 
   WriteObserver* write_observer_ = nullptr;
 
-  std::unique_ptr<ServiceWorkerResponseReader> compare_reader_;
-  std::unique_ptr<ServiceWorkerResponseReader> copy_reader_;
+  // Non-storage service:
+  std::unique_ptr<ServiceWorkerResponseReader> legacy_compare_reader_;
+  std::unique_ptr<ServiceWorkerResponseReader> legacy_copy_reader_;
+  // Storage service:
+  mojo::Remote<storage::mojom::ServiceWorkerResourceReader> compare_reader_;
+  std::unique_ptr<DataPipeReader> compare_data_pipe_reader_;
+  mojo::Remote<storage::mojom::ServiceWorkerResourceReader> copy_reader_;
+  std::unique_ptr<DataPipeReader> copy_data_pipe_reader_;
+
   std::unique_ptr<ServiceWorkerResponseWriter> writer_;
   base::WeakPtrFactory<ServiceWorkerCacheWriter> weak_factory_{this};
 };

content/browser/service_worker/service_worker_test_utils.cc

@@ -34,6 +34,8 @@
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/bindings/self_owned_receiver.h"
+#include "mojo/public/cpp/system/data_pipe.h"
+#include "mojo/public/cpp/system/data_pipe_utils.h"
 #include "net/base/completion_once_callback.h"
 #include "net/base/io_buffer.h"
 #include "net/base/test_completion_callback.h"
@@ -544,6 +546,14 @@ MockServiceWorkerResponseReader::MockServiceWorkerResponseReader()
 
 MockServiceWorkerResponseReader::~MockServiceWorkerResponseReader() {}
 
+mojo::PendingRemote<storage::mojom::ServiceWorkerResourceReader>
+MockServiceWorkerResponseReader::BindNewPipeAndPassRemote(
+    base::OnceClosure disconnect_handler) {
+  auto remote = receiver_.BindNewPipeAndPassRemote();
+  receiver_.set_disconnect_handler(std::move(disconnect_handler));
+  return remote;
+}
+
 void MockServiceWorkerResponseReader::ReadInfo(
     HttpResponseInfoIOBuffer* info_buf,
     net::CompletionOnceCallback callback) {
@@ -561,14 +571,8 @@ void MockServiceWorkerResponseReader::ReadInfo(
   DCHECK(!expected_reads_.empty());
   ExpectedRead expected = expected_reads_.front();
   EXPECT_TRUE(expected.info);
-  if (expected.async) {
-    pending_info_ = info_buf;
-    pending_callback_ = std::move(callback);
-  } else {
-    expected_reads_.pop();
-    info_buf->response_data_size = expected.len;
-    std::move(callback).Run(expected.result);
-  }
+  pending_info_ = info_buf;
+  pending_callback_ = std::move(callback);
 }
 
 void MockServiceWorkerResponseReader::ReadData(
@@ -579,69 +583,105 @@ void MockServiceWorkerResponseReader::ReadData(
   ExpectedRead expected = expected_reads_.front();
   EXPECT_FALSE(expected.info);
   EXPECT_LE(static_cast<int>(expected.len), buf_len);
-  if (expected.async) {
-    pending_callback_ = std::move(callback);
-    pending_buffer_ = buf;
-    pending_buffer_len_ = static_cast<size_t>(buf_len);
-  } else {
-    expected_reads_.pop();
-    if (expected.len > 0) {
-      size_t to_read = std::min(static_cast<size_t>(buf_len), expected.len);
-      memcpy(buf->data(), expected.data, to_read);
-    }
-    std::move(callback).Run(expected.result);
-  }
+  pending_callback_ = std::move(callback);
+  pending_buffer_ = buf;
+  pending_buffer_len_ = static_cast<size_t>(buf_len);
 }
 
-void MockServiceWorkerResponseReader::ExpectReadInfo(size_t len,
-                                                     bool async,
-                                                     int result) {
-  expected_reads_.push(ExpectedRead(len, async, result));
+void MockServiceWorkerResponseReader::ReadResponseHead(
+    storage::mojom::ServiceWorkerResourceReader::ReadResponseHeadCallback
+        callback) {
+  pending_read_response_head_callback_ = std::move(callback);
 }
 
-void MockServiceWorkerResponseReader::ExpectReadInfoOk(size_t len, bool async) {
-  expected_reads_.push(ExpectedRead(len, async, len));
+void MockServiceWorkerResponseReader::ReadData(int64_t,
+                                               ReadDataCallback callback) {
+  DCHECK(!body_.is_valid());
+  mojo::ScopedDataPipeConsumerHandle consumer;
+  MojoCreateDataPipeOptions options;
+  options.struct_size = sizeof(MojoCreateDataPipeOptions);
+  options.flags = MOJO_CREATE_DATA_PIPE_FLAG_NONE;
+  options.element_num_bytes = 1;
+  options.capacity_num_bytes = expected_max_data_bytes_;
+  mojo::CreateDataPipe(&options, &body_, &consumer);
+  std::move(callback).Run(std::move(std::move(consumer)));
+}
+
+void MockServiceWorkerResponseReader::ExpectReadInfo(size_t len, int result) {
+  expected_reads_.push(ExpectedRead(len, result));
+}
+
+void MockServiceWorkerResponseReader::ExpectReadInfoOk(size_t len) {
+  expected_reads_.push(ExpectedRead(len, len));
 }
 
 void MockServiceWorkerResponseReader::ExpectReadData(const char* data,
                                                      size_t len,
-                                                     bool async,
                                                      int result) {
-  expected_reads_.push(ExpectedRead(data, len, async, result));
+  expected_max_data_bytes_ = std::max(expected_max_data_bytes_, len);
+  expected_reads_.push(ExpectedRead(data, len, result));
 }
 
-void MockServiceWorkerResponseReader::ExpectReadDataOk(const std::string& data,
-                                                       bool async) {
-  expected_reads_.push(
-      ExpectedRead(data.data(), data.size(), async, data.size()));
+void MockServiceWorkerResponseReader::ExpectReadDataOk(
+    const std::string& data) {
+  expected_reads_.push(ExpectedRead(data.data(), data.size(), data.size()));
 }
 
 void MockServiceWorkerResponseReader::ExpectReadOk(
     const std::vector<std::string>& stored_data,
-    const size_t bytes_stored,
-    const bool async) {
-  ExpectReadInfoOk(bytes_stored, async);
+    const size_t bytes_stored) {
+  ExpectReadInfoOk(bytes_stored);
   for (const auto& data : stored_data)
-    ExpectReadDataOk(data, async);
+    ExpectReadDataOk(data);
 }
 
 void MockServiceWorkerResponseReader::CompletePendingRead() {
   DCHECK(!expected_reads_.empty());
   ExpectedRead expected = expected_reads_.front();
   expected_reads_.pop();
-  EXPECT_TRUE(expected.async);
+
+  // Legacy API calls.
+  // TODO(https://crbug.com/1055677): Remove this when ReadInfo() and legacy
+  // ReadData() are removed.
+  if (pending_callback_) {
+    if (expected.info) {
+      pending_info_->response_data_size = expected.len;
+      std::move(pending_callback_).Run(expected.result);
+    } else {
+      if (expected.len > 0) {
+        size_t to_read =
+            std::min(static_cast<size_t>(pending_buffer_len_), expected.len);
+        memcpy(pending_buffer_->data(), expected.data, to_read);
+      }
+      std::move(pending_callback_).Run(expected.result);
+    }
+    return;
+  }
+
+  // Make sure that all messages are received at this point.
+  receiver_.FlushForTesting();
+
   if (expected.info) {
-    pending_info_->response_data_size = expected.len;
+    DCHECK(pending_read_response_head_callback_);
+    auto response_head = network::mojom::URLResponseHead::New();
+    response_head->headers =
+        base::MakeRefCounted<net::HttpResponseHeaders>("HTTP/1.0 200 OK\0\0");
+    response_head->content_length = expected.len;
+    std::move(pending_read_response_head_callback_)
+        .Run(expected.result, std::move(response_head),
+             /*metadata=*/base::nullopt);
   } else {
-    size_t to_read = std::min(pending_buffer_len_, expected.len);
-    if (to_read > 0)
-      memcpy(pending_buffer_->data(), expected.data, to_read);
+    if (expected.len == 0) {
+      body_.reset();
+    } else {
+      DCHECK(body_.is_valid());
+      EXPECT_TRUE(mojo::BlockingCopyFromString(
+          std::string(expected.data, expected.len), body_));
+    }
   }
-  pending_info_ = nullptr;
-  pending_buffer_ = nullptr;
-  net::CompletionOnceCallback callback = std::move(pending_callback_);
-  pending_callback_.Reset();
-  std::move(callback).Run(expected.result);
+
+  // Wait until the body is received by the user of the response reader.
+  base::RunLoop().RunUntilIdle();
 }
 
 MockServiceWorkerResponseWriter::MockServiceWorkerResponseWriter()

content/browser/service_worker/service_worker_test_utils.h

@@ -12,6 +12,7 @@
 #include "base/command_line.h"
 #include "base/memory/weak_ptr.h"
 #include "base/task/post_task.h"
+#include "components/services/storage/public/mojom/service_worker_storage_control.mojom.h"
 #include "content/browser/service_worker/service_worker_cache_writer.h"
 #include "content/browser/service_worker/service_worker_database.h"
 #include "content/browser/service_worker/service_worker_disk_cache.h"
@@ -225,18 +226,29 @@ int64_t GetNewResourceIdSync(ServiceWorkerStorage* storage);
 // must be completed by the test using CompletePendingRead().
 // These is a convenience method AllExpectedReadsDone() which returns whether
 // there are any expected reads that have not yet happened.
-class MockServiceWorkerResponseReader : public ServiceWorkerResponseReader {
+class MockServiceWorkerResponseReader
+    : public ServiceWorkerResponseReader,
+      public storage::mojom::ServiceWorkerResourceReader {
  public:
   MockServiceWorkerResponseReader();
   ~MockServiceWorkerResponseReader() override;
 
-  // ServiceWorkerResponseReader overrides
+  mojo::PendingRemote<storage::mojom::ServiceWorkerResourceReader>
+  BindNewPipeAndPassRemote(base::OnceClosure disconnect_handler);
+
+  // ServiceWorkerResponseReader overrides:
   void ReadInfo(HttpResponseInfoIOBuffer* info_buf,
                 net::CompletionOnceCallback callback) override;
   void ReadData(net::IOBuffer* buf,
                 int buf_len,
                 net::CompletionOnceCallback callback) override;
 
+  // storage::mojom::ServiceWorkerResourceReader overrides:
+  void ReadResponseHead(
+      storage::mojom::ServiceWorkerResourceReader::ReadResponseHeadCallback
+          callback) override;
+  void ReadData(int64_t, ReadDataCallback callback) override;
+
   // Test helpers. ExpectReadInfo() and ExpectReadData() give precise control
   // over both the data to be written and the result to return.
   // ExpectReadInfoOk() and ExpectReadDataOk() are convenience functions for
@@ -244,20 +256,20 @@ class MockServiceWorkerResponseReader : public ServiceWorkerResponseReader {
 
   // Expect a call to ReadInfo() on this reader. For these functions, |len| will
   // be used as |response_data_size|, not as the length of this particular read.
-  void ExpectReadInfo(size_t len, bool async, int result);
-  void ExpectReadInfoOk(size_t len, bool async);
+  // TODO(https://crbug.com/1055677): Rename this to ExpectReadResponseHead().
+  void ExpectReadInfo(size_t len, int result);
+  void ExpectReadInfoOk(size_t len);
 
   // Expect a call to ReadData() on this reader. For these functions, |len| is
   // the length of the data to be written back; in ExpectReadDataOk(), |len| is
   // implicitly the length of |data|.
-  void ExpectReadData(const char* data, size_t len, bool async, int result);
-  void ExpectReadDataOk(const std::string& data, bool async);
+  void ExpectReadData(const char* data, size_t len, int result);
+  void ExpectReadDataOk(const std::string& data);
 
   // Convenient method for calling ExpectReadInfoOk() with the length being
   // |bytes_stored|, and ExpectReadDataOk() for each element of |stored_data|.
   void ExpectReadOk(const std::vector<std::string>& stored_data,
-                    const size_t bytes_stored,
-                    const bool async);
+                    const size_t bytes_stored);
 
   // Complete a pending async read. It is an error to call this function without
   // a pending async read (ie, a previous call to ReadInfo() or ReadData()
@@ -269,23 +281,31 @@ class MockServiceWorkerResponseReader : public ServiceWorkerResponseReader {
 
  private:
   struct ExpectedRead {
-    ExpectedRead(size_t len, bool async, int result)
-        : data(nullptr), len(len), info(true), async(async), result(result) {}
-    ExpectedRead(const char* data, size_t len, bool async, int result)
-        : data(data), len(len), info(false), async(async), result(result) {}
+    ExpectedRead(size_t len, int result)
+        : data(nullptr), len(len), info(true), result(result) {}
+    ExpectedRead(const char* data, size_t len, int result)
+        : data(data), len(len), info(false), result(result) {}
     const char* data;
     size_t len;
     bool info;
-    bool async;
     int result;
   };
 
   base::queue<ExpectedRead> expected_reads_;
+  size_t expected_max_data_bytes_ = 0;
+
   scoped_refptr<net::IOBuffer> pending_buffer_;
   size_t pending_buffer_len_;
   scoped_refptr<HttpResponseInfoIOBuffer> pending_info_;
   net::CompletionOnceCallback pending_callback_;
 
+  mojo::Receiver<storage::mojom::ServiceWorkerResourceReader> receiver_{this};
+  storage::mojom::ServiceWorkerResourceReader::ReadResponseHeadCallback
+      pending_read_response_head_callback_;
+  storage::mojom::ServiceWorkerResourceReader::ReadDataCallback
+      pending_read_data_callback_;
+  mojo::ScopedDataPipeProducerHandle body_;
+
   DISALLOW_COPY_AND_ASSIGN(MockServiceWorkerResponseReader);
 };