ServiceWorkerWriteToCacheJob: refactor

content/browser/service_worker/service_worker_cache_writer.cc

Index: content/browser/service_worker/service_worker_cache_writer.cc
diff --git a/content/browser/service_worker/service_worker_cache_writer.cc b/content/browser/service_worker/service_worker_cache_writer.cc
new file mode 100644
index 0000000000000000000000000000000000000000..b8c6cfcaf9a9279d49aea29f8b1c16ec8fd12a59
--- /dev/null
+++ b/content/browser/service_worker/service_worker_cache_writer.cc
@@ -0,0 +1,496 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/browser/service_worker/service_worker_cache_writer.h"
+
+#include <algorithm>
+#include <string>
+
+#include "content/browser/appcache/appcache_response.h"
+#include "content/browser/service_worker/service_worker_disk_cache.h"
+#include "content/browser/service_worker/service_worker_storage.h"
+
+namespace {
+
+const size_t kCopyBufferSize = 16 * 1024;
+
+// Shim class used to turn always-async functions into async-or-result
+// functions. See the comments below near ReadInfoHelper.
+class AsyncOnlyCompletionCallbackAdaptor
+    : public base::RefCounted<AsyncOnlyCompletionCallbackAdaptor> {
+ public:
+  explicit AsyncOnlyCompletionCallbackAdaptor(
+      const net::CompletionCallback& callback)
+      : async_(false), result_(net::ERR_IO_PENDING), callback_(callback) {}
+
+  void set_async(bool async) { async_ = async; }
+  bool async() { return async_; }
+  int result() { return result_; }
+
+  void WrappedCallback(int result) {
+    result_ = result;
+    if (async_)
+      callback_.Run(result);
+  }
+
+ private:
+  friend class base::RefCounted<AsyncOnlyCompletionCallbackAdaptor>;
+  virtual ~AsyncOnlyCompletionCallbackAdaptor() {}
+
+  bool async_;
+  int result_;
+  net::CompletionCallback callback_;
+};
+
+}  // namespace
+
+namespace content {
+
+int ServiceWorkerCacheWriter::DoLoop(int status) {
+  do {
+    switch (state_) {
+      case STATE_START:
+        status = DoStart(status);
+        break;
+      case STATE_READ_HEADERS_FOR_COMPARE:
+        status = DoReadHeadersForCompare(status);
+        break;
+      case STATE_READ_HEADERS_FOR_COMPARE_DONE:
+        status = DoReadHeadersForCompareDone(status);
+        break;
+      case STATE_READ_DATA_FOR_COMPARE:
+        status = DoReadDataForCompare(status);
+        break;
+      case STATE_READ_DATA_FOR_COMPARE_DONE:
+        status = DoReadDataForCompareDone(status);
+        break;
+      case STATE_READ_HEADERS_FOR_COPY:
+        status = DoReadHeadersForCopy(status);
+        break;
+      case STATE_READ_HEADERS_FOR_COPY_DONE:
+        status = DoReadHeadersForCopyDone(status);
+        break;
+      case STATE_READ_DATA_FOR_COPY:
+        status = DoReadDataForCopy(status);
+        break;
+      case STATE_READ_DATA_FOR_COPY_DONE:
+        status = DoReadDataForCopyDone(status);
+        break;
+      case STATE_WRITE_HEADERS_FOR_PASSTHROUGH:
+        status = DoWriteHeadersForPassthrough(status);
+        break;
+      case STATE_WRITE_HEADERS_FOR_PASSTHROUGH_DONE:
+        status = DoWriteHeadersForPassthroughDone(status);
+        break;
+      case STATE_WRITE_DATA_FOR_PASSTHROUGH:
+        status = DoWriteDataForPassthrough(status);
+        break;
+      case STATE_WRITE_DATA_FOR_PASSTHROUGH_DONE:
+        status = DoWriteDataForPassthroughDone(status);
+        break;
+      case STATE_WRITE_HEADERS_FOR_COPY:
+        status = DoWriteHeadersForCopy(status);
+        break;
+      case STATE_WRITE_HEADERS_FOR_COPY_DONE:
+        status = DoWriteHeadersForCopyDone(status);
+        break;
+      case STATE_WRITE_DATA_FOR_COPY:
+        status = DoWriteDataForCopy(status);
+        break;
+      case STATE_WRITE_DATA_FOR_COPY_DONE:
+        status = DoWriteDataForCopyDone(status);
+        break;
+      case STATE_DONE:
+        status = DoDone(status);
+        break;
+      default:

[Randy Smith (Not in Mondays), 2015/09/16 18:57:28]

Under what circumstances should this happen?  Should we have a NOTREACHED()?  (A
la HttpStreamParser.)

[Elly Fong-Jones, 2015/09/16 19:25:57]

Done.

+        state_ = STATE_DONE;
+        break;
+    }
+  } while (status >= net::OK && state_ != STATE_DONE);

[Randy Smith (Not in Mondays), 2015/09/16 18:57:28]

In other code I've read, I think of "STATE_NONE" as meaning "state machine ready
to be re-driven if needed" and STATE_DONE as meaning "all tapped out, not good
for anything ever again".  This idiom doesn't strike me as key to the DoLoop(),
and it's a bit contradictory to the earlier comment (which was basically saying
that the DoLoop() mechanism shouldn't keep state after an external method
invocation had completed), so I'm not committed to the standard pattern.  But I
would like a comment on the enum about STATE_DONE if you want to use it this
way, just so people don't assume it means *done* done.

[Elly Fong-Jones, 2015/09/16 19:25:57]

Done.

+  io_pending_ = (status == net::ERR_IO_PENDING);
+  return status;
+}
+
+ServiceWorkerCacheWriter::ServiceWorkerCacheWriter(
+    const ResponseReaderCreator& reader_creator,
+    const ResponseWriterCreator& writer_creator)
+    : state_(STATE_START),
+      io_pending_(false),
+      comparing_(false),
+      did_replace_(false),
+      reader_creator_(reader_creator),
+      writer_creator_(writer_creator),
+      weak_factory_(this) {}
+
+ServiceWorkerCacheWriter::~ServiceWorkerCacheWriter() {}
+
+net::Error ServiceWorkerCacheWriter::MaybeWriteHeaders(
+    HttpResponseInfoIOBuffer* headers,
+    const OnWriteCompleteCallback& callback) {
+  DCHECK(!io_pending_);
+
+  headers_to_write_ = headers;
+  pending_callback_ = callback;
+  DCHECK_EQ(state_, STATE_START);
+  int result = DoLoop(net::OK);
+
+  // Synchronous errors always go to ERR_IO_PENDING.

[Randy Smith (Not in Mondays), 2015/09/16 18:57:28]

I don't understand this comment?

[Elly Fong-Jones, 2015/09/16 19:25:57]

That's because it makes absolutely no sense. Fixed.

+  if (result != net::ERR_IO_PENDING)
+    DCHECK_EQ(state_, STATE_DONE);
+
+  // ERR_IO_PENDING has to have one of the "done" states as the next state.

[Randy Smith (Not in Mondays), 2015/09/16 18:57:28]

nit, suggestion: Make clear you're not talking about STATE_DONE here?

[Elly Fong-Jones, 2015/09/16 19:25:57]

Done.

+  if (result == net::ERR_IO_PENDING) {

[Randy Smith (Not in Mondays), 2015/09/16 18:57:28]

nit, random thought (i.e. not even a suggestion, feel free to ignore): If you
include the ERR_IO_PENDING conditional in the DCHECK, any error messages/crashes
from it will make it clear the failure (returned IO_PENDING without waiting on
IO).  Applies elsewhere as well (if you decide to take it).

[Elly Fong-Jones, 2015/09/16 19:25:57]

I think it's already clear from getting this message in the first place? Maybe I
have misunderstood.

[Randy Smith (Not in Mondays), 2015/09/16 20:00:48]

It's targeting the folks who have a vague idea of how this code works before
they actually look at the lines of code; the "Aha, I see what's going on" might
happen a bit earlier.  But it's really not important; that's a small subset of
the potential audience.

+    DCHECK(state_ == STATE_READ_HEADERS_FOR_COMPARE_DONE ||
+           state_ == STATE_WRITE_HEADERS_FOR_COPY_DONE ||
+           state_ == STATE_WRITE_HEADERS_FOR_PASSTHROUGH_DONE)
+        << "Unexpected state: " << state_;
+    io_pending_ = true;
+  }
+
+  return result >= 0 ? net::OK : static_cast<net::Error>(result);
+}
+
+net::Error ServiceWorkerCacheWriter::MaybeWriteData(
+    net::IOBuffer* buf,
+    size_t buf_size,
+    const OnWriteCompleteCallback& callback) {
+  DCHECK(!io_pending_);
+
+  data_to_write_ = buf;
+  len_to_write_ = buf_size;
+  pending_callback_ = callback;
+
+  if (comparing_)
+    state_ = STATE_READ_DATA_FOR_COMPARE;
+  else
+    state_ = STATE_WRITE_DATA_FOR_PASSTHROUGH;
+
+  int result = DoLoop(net::OK);
+
+  // Synchronous completions are always STATE_DONE.
+  if (result != net::ERR_IO_PENDING)
+    DCHECK_EQ(state_, STATE_DONE);
+
+  // Asynchronous completion means the state machine must be waiting in one of
+  // the Done states for an IO operation to complete:
+  if (result == net::ERR_IO_PENDING) {
+    // Note that STATE_READ_HEADERS_FOR_COMPARE_DONE is excluded because the
+    // headers are compared in MaybeWriteHeaders, not here, and
+    // STATE_WRITE_HEADERS_FOR_PASSTHROUGH_DONE is excluded because that write
+    // is done by MaybeWriteHeaders.
+    DCHECK(state_ == STATE_READ_DATA_FOR_COMPARE_DONE ||
+           state_ == STATE_READ_HEADERS_FOR_COPY_DONE ||
+           state_ == STATE_READ_DATA_FOR_COPY_DONE ||
+           state_ == STATE_WRITE_HEADERS_FOR_COPY_DONE ||
+           state_ == STATE_WRITE_DATA_FOR_COPY_DONE ||
+           state_ == STATE_WRITE_DATA_FOR_PASSTHROUGH_DONE)
+        << "Unexpected state: " << state_;
+  }
+
+  return result >= 0 ? net::OK : static_cast<net::Error>(result);
+}
+
+int ServiceWorkerCacheWriter::DoStart(int result) {
+  bytes_written_ = 0;
+  compare_reader_ = reader_creator_.Run();
+  if (compare_reader_.get()) {
+    state_ = STATE_READ_HEADERS_FOR_COMPARE;
+    comparing_ = true;
+  } else {
+    // No existing reader, just write the headers back directly.
+    state_ = STATE_WRITE_HEADERS_FOR_PASSTHROUGH;
+    comparing_ = false;
+  }
+  return net::OK;
+}
+
+int ServiceWorkerCacheWriter::DoReadHeadersForCompare(int result) {
+  DCHECK(headers_to_write_);
+
+  headers_to_read_ = new HttpResponseInfoIOBuffer;
+  state_ = STATE_READ_HEADERS_FOR_COMPARE_DONE;
+  return ReadInfoHelper(compare_reader_, headers_to_read_.get());
+}
+
+int ServiceWorkerCacheWriter::DoReadHeadersForCompareDone(int result) {
+  if (result < 0) {
+    state_ = STATE_DONE;
+    return result;
+  }
+  cached_length_ = headers_to_read_->response_data_size;
+  bytes_compared_ = 0;
+  state_ = STATE_DONE;
+  return net::OK;
+}
+
+int ServiceWorkerCacheWriter::DoReadDataForCompare(int result) {
+  DCHECK(data_to_write_);
+
+  data_to_read_ = new net::IOBuffer(len_to_write_);
+  len_to_read_ = len_to_write_;
+  state_ = STATE_READ_DATA_FOR_COMPARE_DONE;
+  compare_offset_ = 0;
+  // If this was an EOF, don't issue a read.
+  if (len_to_write_ > 0)
+    result = ReadDataHelper(compare_reader_, data_to_read_.get(), len_to_read_);
+  return result;
+}
+
+int ServiceWorkerCacheWriter::DoReadDataForCompareDone(int result) {
+  DCHECK(data_to_read_);
+  DCHECK(data_to_write_);
+  DCHECK_EQ(len_to_read_, len_to_write_);
+  DCHECK_LE(result + compare_offset_, static_cast<size_t>(len_to_write_));
+
+  if (result < 0) {
+    state_ = STATE_DONE;
+    return result;
+  }
+
+  // Premature EOF while reading the service worker script cache data to
+  // compare. Fail the comparison.
+  if (result == 0 && len_to_write_ != 0) {
+    comparing_ = false;
+    state_ = STATE_READ_HEADERS_FOR_COPY;
+    return net::OK;
+  }
+
+  // Compare the data from the ServiceWorker script cache to the data from the
+  // network.
+  if (memcmp(data_to_read_->data(), data_to_write_->data() + compare_offset_,
+             result)) {
+    // Data mismatched. This method already validated that all the bytes through
+    // |bytes_compared_| were identical, so copy the first |bytes_compared_|
+    // over, then start writing network data back after the changed point.
+    comparing_ = false;
+    state_ = STATE_READ_HEADERS_FOR_COPY;
+    return net::OK;
+  }
+
+  compare_offset_ += result;
+
+  // This is a little bit tricky. It is possible that not enough data was read
+  // to finish comparing the entire block of data from the network (which is
+  // kept in len_to_write_), so this method may need to issue another read and
+  // return to this state.
+  //
+  // Compare isn't complete yet. Issue another read for the remaining data. Note
+  // that this reuses the same IOBuffer.
+  if (compare_offset_ < static_cast<size_t>(len_to_read_)) {
+    state_ = STATE_READ_DATA_FOR_COMPARE_DONE;
+    return ReadDataHelper(compare_reader_, data_to_read_.get(),
+                          len_to_read_ - compare_offset_);
+  }
+
+  // Cached entry is longer than the network entry but the prefix matches. Copy
+  // just the prefix.
+  if (len_to_read_ == 0 && bytes_compared_ + compare_offset_ < cached_length_) {
+    comparing_ = false;
+    state_ = STATE_READ_HEADERS_FOR_COPY;
+    return net::OK;
+  }
+
+  // bytes_compared_ only gets incremented when a full block is compared, to
+  // avoid having to use only parts of the buffered network data.
+  bytes_compared_ += result;
+  state_ = STATE_DONE;
+  return net::OK;
+}
+
+int ServiceWorkerCacheWriter::DoReadHeadersForCopy(int result) {
+  bytes_copied_ = 0;
+  copy_reader_ = reader_creator_.Run();
+  headers_to_read_ = new HttpResponseInfoIOBuffer;
+  data_to_copy_ = new net::IOBuffer(kCopyBufferSize);
+  state_ = STATE_READ_HEADERS_FOR_COPY_DONE;
+  return ReadInfoHelper(copy_reader_, headers_to_read_.get());
+}
+
+int ServiceWorkerCacheWriter::DoReadHeadersForCopyDone(int result) {
+  if (result < 0) {
+    state_ = STATE_DONE;
+    return result;
+  }
+  state_ = STATE_WRITE_HEADERS_FOR_COPY;
+  return net::OK;
+}
+
+// Write the just-read headers back to the cache.
+// Note that this method must create |writer_|, since the only paths to this
+// state never create a writer.
+// Also note that this *discards* the read headers and replaces them with the
+// net headers.
+int ServiceWorkerCacheWriter::DoWriteHeadersForCopy(int result) {
+  DCHECK(!writer_);
+  writer_ = writer_creator_.Run();
+  state_ = STATE_WRITE_HEADERS_FOR_COPY_DONE;
+  return WriteInfoHelper(writer_, headers_to_write_.get());
+}
+
+int ServiceWorkerCacheWriter::DoWriteHeadersForCopyDone(int result) {
+  if (result < 0) {
+    state_ = STATE_DONE;
+    return result;
+  }
+  state_ = STATE_READ_DATA_FOR_COPY;
+  return net::OK;
+}
+
+int ServiceWorkerCacheWriter::DoReadDataForCopy(int result) {
+  size_t to_read = std::min(kCopyBufferSize, bytes_compared_ - bytes_copied_);
+  // At this point, all compared bytes have been read. Currently
+  // |data_to_write_| and |len_to_write_| hold the chunk of network input that
+  // caused the comparison failure, so those need to be written back and this
+  // object needs to go into passthrough mode.
+  if (to_read == 0) {
+    state_ = STATE_WRITE_DATA_FOR_PASSTHROUGH;
+    return net::OK;
+  }
+  state_ = STATE_READ_DATA_FOR_COPY_DONE;
+  return ReadDataHelper(copy_reader_, data_to_copy_.get(), to_read);
+}
+
+int ServiceWorkerCacheWriter::DoReadDataForCopyDone(int result) {
+  if (result < 0) {
+    state_ = STATE_DONE;
+    return result;
+  }
+  state_ = STATE_WRITE_DATA_FOR_COPY;
+  return result;
+}
+
+int ServiceWorkerCacheWriter::DoWriteDataForCopy(int result) {
+  state_ = STATE_WRITE_DATA_FOR_COPY_DONE;
+  DCHECK_GT(result, 0);
+  return WriteDataHelper(writer_, data_to_copy_.get(), result);
+}
+
+int ServiceWorkerCacheWriter::DoWriteDataForCopyDone(int result) {
+  if (result < 0) {
+    state_ = STATE_DONE;
+    return result;
+  }
+  bytes_written_ += result;
+  bytes_copied_ += result;
+  state_ = STATE_READ_DATA_FOR_COPY;
+  return result;
+}
+
+int ServiceWorkerCacheWriter::DoWriteHeadersForPassthrough(int result) {
+  writer_ = writer_creator_.Run();
+  state_ = STATE_WRITE_HEADERS_FOR_PASSTHROUGH_DONE;
+  return WriteInfoHelper(writer_, headers_to_write_.get());
+}
+
+int ServiceWorkerCacheWriter::DoWriteHeadersForPassthroughDone(int result) {
+  state_ = STATE_DONE;
+  return net::OK;
+}
+
+int ServiceWorkerCacheWriter::DoWriteDataForPassthrough(int result) {
+  state_ = STATE_WRITE_DATA_FOR_PASSTHROUGH_DONE;
+  if (len_to_write_ > 0)
+    result = WriteDataHelper(writer_, data_to_write_.get(), len_to_write_);
+  return result;
+}
+
+int ServiceWorkerCacheWriter::DoWriteDataForPassthroughDone(int result) {
+  if (result < 0) {
+    state_ = STATE_DONE;
+    return result;
+  }
+  bytes_written_ += result;
+  state_ = STATE_DONE;
+  return net::OK;
+}
+
+int ServiceWorkerCacheWriter::DoDone(int result) {
+  state_ = STATE_DONE;
+  return net::OK;
+}
+
+// These helpers adapt the AppCache "always use the callback" pattern to the
+// //net "only use the callback for async" pattern using
+// AsyncCompletionCallbackAdaptor.
+//
+// Specifically, these methods return result codes directly for synchronous
+// completions, and only run their callback (which is AsyncDoLoop) for
+// asynchronous completions.
+
+int ServiceWorkerCacheWriter::ReadInfoHelper(
+    const scoped_ptr<ServiceWorkerResponseReader>& reader,
+    HttpResponseInfoIOBuffer* buf) {
+  net::CompletionCallback run_callback = base::Bind(
+      &ServiceWorkerCacheWriter::AsyncDoLoop, weak_factory_.GetWeakPtr());
+  scoped_refptr<AsyncOnlyCompletionCallbackAdaptor> adaptor(
+      new AsyncOnlyCompletionCallbackAdaptor(run_callback));
+  reader->ReadInfo(
+      buf, base::Bind(&AsyncOnlyCompletionCallbackAdaptor::WrappedCallback,
+                      adaptor));
+  adaptor->set_async(true);
+  return adaptor->result();
+}
+
+int ServiceWorkerCacheWriter::ReadDataHelper(
+    const scoped_ptr<ServiceWorkerResponseReader>& reader,
+    net::IOBuffer* buf,
+    int buf_len) {
+  net::CompletionCallback run_callback = base::Bind(
+      &ServiceWorkerCacheWriter::AsyncDoLoop, weak_factory_.GetWeakPtr());
+  scoped_refptr<AsyncOnlyCompletionCallbackAdaptor> adaptor(
+      new AsyncOnlyCompletionCallbackAdaptor(run_callback));
+  reader->ReadData(
+      buf, buf_len,
+      base::Bind(&AsyncOnlyCompletionCallbackAdaptor::WrappedCallback,
+                 adaptor));
+  adaptor->set_async(true);
+  return adaptor->result();
+}
+
+int ServiceWorkerCacheWriter::WriteInfoHelper(
+    const scoped_ptr<ServiceWorkerResponseWriter>& writer,
+    HttpResponseInfoIOBuffer* buf) {
+  did_replace_ = true;
+  net::CompletionCallback run_callback = base::Bind(
+      &ServiceWorkerCacheWriter::AsyncDoLoop, weak_factory_.GetWeakPtr());
+  scoped_refptr<AsyncOnlyCompletionCallbackAdaptor> adaptor(
+      new AsyncOnlyCompletionCallbackAdaptor(run_callback));
+  writer->WriteInfo(
+      buf, base::Bind(&AsyncOnlyCompletionCallbackAdaptor::WrappedCallback,
+                      adaptor));
+  adaptor->set_async(true);
+  return adaptor->result();
+}
+
+int ServiceWorkerCacheWriter::WriteDataHelper(
+    const scoped_ptr<ServiceWorkerResponseWriter>& writer,
+    net::IOBuffer* buf,
+    int buf_len) {
+  net::CompletionCallback run_callback = base::Bind(
+      &ServiceWorkerCacheWriter::AsyncDoLoop, weak_factory_.GetWeakPtr());
+  scoped_refptr<AsyncOnlyCompletionCallbackAdaptor> adaptor(
+      new AsyncOnlyCompletionCallbackAdaptor(run_callback));
+  writer->WriteData(
+      buf, buf_len,
+      base::Bind(&AsyncOnlyCompletionCallbackAdaptor::WrappedCallback,
+                 adaptor));
+  adaptor->set_async(true);
+  return adaptor->result();
+}
+
+void ServiceWorkerCacheWriter::AsyncDoLoop(int result) {
+  result = DoLoop(result);
+  // If the result is ERR_IO_PENDING, the pending callback will be run by a
+  // later invocation of AsyncDoLoop.
+  if (result != net::ERR_IO_PENDING) {
+    OnWriteCompleteCallback callback = pending_callback_;
+    pending_callback_.Reset();
+    net::Error error = result >= 0 ? net::OK : static_cast<net::Error>(result);
+    io_pending_ = false;
+    callback.Run(error);
+  }
+}
+
+}  // namespace content