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..c62a5606a39218eb80f328283a5d4af830fcf99a
--- /dev/null
+++ b/content/browser/service_worker/service_worker_cache_writer.cc
@@ -0,0 +1,498 @@
+// 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"
+

[falken, 2015/09/01 14:43:52]

need <algorithm> for std::min (according to git-cl lint)

[Elly Fong-Jones, 2015/09/09 13:36:08]

Done.

+#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;
+
+enum {

[Randy Smith (Not in Mondays), 2015/09/01 20:35:42]

Include some comments to indicate what the state machine looks like?  You can
just break the enums out into blocks that are linear, then annotate with the
branches between the linear aspects.  (I have a GraphViz file for the state
machine if you want it.)

It'd also be really useful to document the general flow of the state machine,
something like "Read headers and data from the cache to compare with incoming
values.  If this results in a miscompare, re-read the headers from the cache and
write them back out [? Why is this necessary?], then copy out the network data
that's been written to this class, then go into passthrough mode copying
anything written to this class."

[Elly Fong-Jones, 2015/09/09 13:36:08]

Done.

+  STATE_START,
+  STATE_READ_HEADERS_FOR_COMPARE,
+  STATE_READ_HEADERS_FOR_COMPARE_DONE,
+  STATE_READ_DATA_FOR_COMPARE,
+  STATE_READ_DATA_FOR_COMPARE_DONE,
+  STATE_READ_HEADERS_FOR_COPY,
+  STATE_READ_HEADERS_FOR_COPY_DONE,
+  STATE_READ_DATA_FOR_COPY,
+  STATE_READ_DATA_FOR_COPY_DONE,
+  STATE_WRITE_HEADERS_FOR_PASSTHROUGH,
+  STATE_WRITE_HEADERS_FOR_PASSTHROUGH_DONE,
+  STATE_WRITE_DATA_FOR_PASSTHROUGH,
+  STATE_WRITE_DATA_FOR_PASSTHROUGH_DONE,
+  STATE_WRITE_HEADERS_FOR_COPY,
+  STATE_WRITE_HEADERS_FOR_COPY_DONE,
+  STATE_WRITE_DATA_FOR_COPY,
+  STATE_WRITE_DATA_FOR_COPY_DONE,
+  STATE_DONE,
+};
+
+// 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:
+  AsyncOnlyCompletionCallbackAdaptor(const net::CompletionCallback& callback)

[falken, 2015/09/01 14:43:52]

nit: explicit

[Elly Fong-Jones, 2015/09/09 13:36:08]

Done.

+      : 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) {
+  bool pause = false;
+  do {
+    int next_state = -1;
+    switch (state_) {
+      case STATE_START:
+        status = Start(&next_state, &pause, status);

[Randy Smith (Not in Mondays), 2015/09/01 20:35:42]

The standard DoLoop idiom is for the function corresponding to STATE_<STATE> to
be named Do<State>.  From a selfish POV, it makes it easier for a reviewer to
figure out what the sub-functions of DoLoop are :-}.

[Elly Fong-Jones, 2015/09/09 13:36:08]

Done.

+        break;
+      case STATE_READ_HEADERS_FOR_COMPARE:
+        status = ReadHeadersForCompare(&next_state, &pause, status);
+        break;
+      case STATE_READ_HEADERS_FOR_COMPARE_DONE:
+        status = ReadHeadersForCompareDone(&next_state, &pause, status);
+        break;
+      case STATE_READ_DATA_FOR_COMPARE:
+        status = ReadDataForCompare(&next_state, &pause, status);
+        break;
+      case STATE_READ_DATA_FOR_COMPARE_DONE:
+        status = ReadDataForCompareDone(&next_state, &pause, status);
+        break;
+      case STATE_READ_HEADERS_FOR_COPY:
+        status = ReadHeadersForCopy(&next_state, &pause, status);
+        break;
+      case STATE_READ_HEADERS_FOR_COPY_DONE:
+        status = ReadHeadersForCopyDone(&next_state, &pause, status);
+        break;
+      case STATE_READ_DATA_FOR_COPY:
+        status = ReadDataForCopy(&next_state, &pause, status);
+        break;
+      case STATE_READ_DATA_FOR_COPY_DONE:
+        status = ReadDataForCopyDone(&next_state, &pause, status);
+        break;
+      case STATE_WRITE_HEADERS_FOR_PASSTHROUGH:
+        status = WriteHeadersForPassthrough(&next_state, &pause, status);
+        break;
+      case STATE_WRITE_HEADERS_FOR_PASSTHROUGH_DONE:
+        status = WriteHeadersForPassthroughDone(&next_state, &pause, status);
+        break;
+      case STATE_WRITE_DATA_FOR_PASSTHROUGH:
+        status = WriteDataForPassthrough(&next_state, &pause, status);
+        break;
+      case STATE_WRITE_DATA_FOR_PASSTHROUGH_DONE:
+        status = WriteDataForPassthroughDone(&next_state, &pause, status);
+        break;
+      case STATE_WRITE_HEADERS_FOR_COPY:
+        status = WriteHeadersForCopy(&next_state, &pause, status);
+        break;
+      case STATE_WRITE_HEADERS_FOR_COPY_DONE:
+        status = WriteHeadersForCopyDone(&next_state, &pause, status);
+        break;
+      case STATE_WRITE_DATA_FOR_COPY:
+        status = WriteDataForCopy(&next_state, &pause, status);
+        break;
+      case STATE_WRITE_DATA_FOR_COPY_DONE:
+        status = WriteDataForCopyDone(&next_state, &pause, status);
+        break;
+      case STATE_DONE:
+        status = Done(&next_state, &pause, status);
+        break;
+      default:
+        next_state = STATE_DONE;
+        break;
+    }
+    state_ = next_state;
+  } while (status >= net::OK && state_ != STATE_DONE && !pause);

[Randy Smith (Not in Mondays), 2015/09/01 20:35:42]

Hmmm.  |pause| is not the standard DoLoop() idiom (at least, I've never seen it
before--if you can give me pointers to examples that would calm me down fairly
quickly), and I'm a bit reluctant to see it added here.  DoLoop() is already
complicated enough without having variations on it in different places (yeah,
I'm sure there already are, but we want to minimize that for maintainability).  

As I understand it, this idiomlet is here because you want to leave the state
machine in a particular state on return and let it be driven from that state by
a new consumer call.  Most other places I've seen, specific consumer calls imply
specific states (i.e. you set the state in the call and then call DoLoop()); the
reason you can't do that here (maybe with some DCHECKs to make sure that
appropriate things have already happened) is that you might be in either compare
or passthrough mode; there aren't any other states that you'll want to pause in
for consumer call driving.  So that could be taken care of by a single boolean
in the class, allowing us to keep the DoLoop idiom consistent.

(Side note: The strong impression I get doing the DoLoop doc CL is that DoLoop
is *not* intended as a general state machine implementation.  Instead, it's
intended to handle code which would ideally, from a maintainability perspective,
be written in a single threaded blocking fashion, possibly with branches. 
Everywhere the code needs to pause for async IO return, it has a state
transition and returns ERR_IO_PENDING.  "Returning" (either synchronously or via
callback) from consumer initiated operations implies that the operation is
completed (there's no way to block for more consumer input).  Presuming this
interpretation of mine is accurate, I'm very wary of adding the "pause" flag,
which isn't a very big code change, and *is* a very big philosophy change. 
Those changes are the worst kind.)

[Elly Fong-Jones, 2015/09/09 13:36:08]

Are you proposing deleting the pause variable and the concept of pause
altogether? The reason it exists right now is that the state machine needs to
"succeed" multiple times, and so there has to be a way to differentiate
"success, and keep going" from "success, and return that result". The only other
option I could think of was to store the knowledge of which states pause in
DoLoop itself, but one of them (ReadDataForCompareDone) only pauses under
somewhat complicated circumstances (basically, it only pauses if it has compared
the entire block of input data already), so I don't think I can do that.

I can move the pause variable to be a member of the class I guess, but I don't
like that it makes this class's DoLoop *look* similar to other uses of DoLoop
while having different semantics. I think that things with different behavior
should look different.
I agree; the usual way DoLoop is used is when there is a single "result", which
is returned to the caller via a callback or something when the machine hits
DONE. This use of it has multiple results that are generated in sequence, hence
the pause idiom. I don't know how to escape this without moving away from using
DoLoop.

+  return status;
+}
+
+ServiceWorkerCacheWriter::ServiceWorkerCacheWriter(
+    const ResponseReaderCreator& reader_creator,
+    const ResponseWriterCreator& writer_creator)
+    : state_(STATE_START),
+      reader_creator_(reader_creator),
+      writer_creator_(writer_creator),
+      weak_factory_(this) {}
+
+ServiceWorkerCacheWriter::~ServiceWorkerCacheWriter() {}
+
+net::Error ServiceWorkerCacheWriter::MaybeWriteHeaders(
+    HttpResponseInfoIOBuffer* headers,
+    const OnWriteCompleteCallback& callback) {
+  headers_to_write_ = headers;
+  pending_callback_ = callback;
+  DCHECK_EQ(STATE_START, state_);
+  int result = DoLoop(net::OK);
+  return result >= 0 ? net::OK : static_cast<net::Error>(result);
+}
+
+net::Error ServiceWorkerCacheWriter::MaybeWriteData(
+    net::IOBuffer* buf,
+    size_t buf_size,
+    const OnWriteCompleteCallback& callback) {
+  data_to_write_ = buf;
+  len_to_write_ = buf_size;
+  pending_callback_ = callback;
+  int result = DoLoop(net::OK);
+  return result >= 0 ? net::OK : static_cast<net::Error>(result);
+}
+
+int ServiceWorkerCacheWriter::Start(int* next_state, bool* pause, int result) {
+  bytes_written_ = 0;
+  compare_reader_ = reader_creator_.Run();
+  if (compare_reader_.get())
+    *next_state = STATE_READ_HEADERS_FOR_COMPARE;
+  else
+    // No existing reader, just write the headers back directly.
+    *next_state = STATE_WRITE_HEADERS_FOR_PASSTHROUGH;

[falken, 2015/09/01 14:43:52]

nit: I prefer braces for multi-line bodies

[Randy Smith (Not in Mondays), 2015/09/01 20:35:42]

I think this is required by the style guide.

[Elly Fong-Jones, 2015/09/09 13:36:08]

Done.

[Elly Fong-Jones, 2015/09/09 13:36:08]

Acknowledged.

+  return net::OK;
+}
+
+int ServiceWorkerCacheWriter::ReadHeadersForCompare(int* next_state,
+                                                    bool* pause,
+                                                    int result) {
+  DCHECK(headers_to_write_);
+
+  headers_to_read_ = new HttpResponseInfoIOBuffer;
+  *next_state = STATE_READ_HEADERS_FOR_COMPARE_DONE;
+  return ReadInfoHelper(compare_reader_, headers_to_read_.get());
+}
+
+int ServiceWorkerCacheWriter::ReadHeadersForCompareDone(int* next_state,

[Randy Smith (Not in Mondays), 2015/09/01 20:35:42]

I like this idiom better than the normal DoLoop() idiom, but I still think
consistency is better than proliferation. so I think you should change it to
having a class internal next_state_ variable.

[Elly Fong-Jones, 2015/09/09 13:36:08]

Repeating my concern from above: since this DoLoop has different behavior from
the ones elsewhere in the network stack, I think it should look different as
well.

+                                                        bool* pause,
+                                                        int result) {
+  if (result < 0) {
+    *next_state = STATE_DONE;
+    return static_cast<int>(result);

[Randy Smith (Not in Mondays), 2015/09/01 20:35:42]

Why the static cast?  It was and is an int.

[Elly Fong-Jones, 2015/09/09 13:36:08]

Leftover from earlier. Done and removed elsewhere.

+  }
+  cached_length_ = headers_to_read_->response_data_size;
+  net_length_ = headers_to_write_->response_data_size;
+  bytes_compared_ = 0;
+  *pause = true;
+  *next_state = STATE_READ_DATA_FOR_COMPARE;
+  return net::OK;
+}
+
+int ServiceWorkerCacheWriter::ReadDataForCompare(int* next_state,
+                                                 bool* pause,
+                                                 int result) {
+  DCHECK(data_to_write_);
+
+  data_to_read_ = new net::IOBuffer(len_to_write_);
+  len_to_read_ = len_to_write_;
+  *next_state = STATE_READ_DATA_FOR_COMPARE_DONE;
+  compare_offset_ = 0;
+  return ReadDataHelper(compare_reader_, data_to_read_.get(), len_to_read_);
+}
+
+int ServiceWorkerCacheWriter::ReadDataForCompareDone(int* next_state,
+                                                     bool* pause,
+                                                     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) {
+    *next_state = STATE_DONE;
+    return result;
+  }
+
+  // Premature EOF while reading the service worker script cache data to
+  // compare. Fail the comparison.
+  if (result == 0) {
+    *next_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.
+    //
+    // Note that the state machine does NOT get paused here, since there is
+    // still further work to do before this object is ready to handle another
+    // WriteData call.
+    *next_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_)) {
+    *next_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 (bytes_compared_ + compare_offset_ >= net_length_ &&
+      net_length_ < cached_length_) {
+    *next_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;
+  *next_state = STATE_READ_DATA_FOR_COMPARE;
+  *pause = true;
+  return net::OK;
+}
+
+int ServiceWorkerCacheWriter::WriteHeadersForPassthrough(int* next_state,
+                                                         bool* pause,
+                                                         int result) {
+  writer_ = writer_creator_.Run();
+  *next_state = STATE_WRITE_HEADERS_FOR_PASSTHROUGH_DONE;
+  return WriteInfoHelper(writer_, headers_to_write_.get());
+}
+
+int ServiceWorkerCacheWriter::WriteHeadersForPassthroughDone(int* next_state,
+                                                             bool* pause,
+                                                             int result) {
+  *next_state = STATE_WRITE_DATA_FOR_PASSTHROUGH;
+  *pause = true;
+  return net::OK;
+}
+
+int ServiceWorkerCacheWriter::WriteDataForPassthrough(int* next_state,
+                                                      bool* pause,
+                                                      int result) {
+  *next_state = STATE_WRITE_DATA_FOR_PASSTHROUGH_DONE;
+  return WriteDataHelper(writer_, data_to_write_.get(), len_to_write_);
+}
+
+int ServiceWorkerCacheWriter::WriteDataForPassthroughDone(int* next_state,
+                                                          bool* pause,
+                                                          int result) {
+  if (result < 0) {
+    *next_state = STATE_DONE;
+    return static_cast<int>(result);
+  }
+  bytes_written_ += result;
+  *next_state = STATE_WRITE_DATA_FOR_PASSTHROUGH;
+  *pause = true;
+  return net::OK;
+}
+
+int ServiceWorkerCacheWriter::ReadHeadersForCopy(int* next_state,
+                                                 bool* pause,
+                                                 int result) {
+  bytes_copied_ = 0;
+  copy_reader_ = reader_creator_.Run();
+  headers_to_read_ = new HttpResponseInfoIOBuffer;
+  data_to_copy_ = new net::IOBuffer(kCopyBufferSize);
+  *next_state = STATE_READ_HEADERS_FOR_COPY_DONE;
+  return ReadInfoHelper(copy_reader_, headers_to_read_.get());
+}
+
+int ServiceWorkerCacheWriter::ReadHeadersForCopyDone(int* next_state,
+                                                     bool* pause,
+                                                     int result) {
+  if (result < 0) {
+    *next_state = STATE_DONE;
+    return static_cast<int>(result);
+  }
+  *next_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::WriteHeadersForCopy(int* next_state,
+                                                  bool* pause,
+                                                  int result) {
+  DCHECK(!writer_);
+  writer_ = writer_creator_.Run();
+  *next_state = STATE_WRITE_HEADERS_FOR_COPY_DONE;
+  return WriteInfoHelper(writer_, headers_to_write_.get());
+}
+
+int ServiceWorkerCacheWriter::WriteHeadersForCopyDone(int* next_state,
+                                                      bool* pause,
+                                                      int result) {
+  if (result < 0) {
+    *next_state = STATE_DONE;
+    return static_cast<int>(result);
+  }
+  *next_state = STATE_READ_DATA_FOR_COPY;
+  return net::OK;
+}
+
+int ServiceWorkerCacheWriter::ReadDataForCopy(int* next_state,
+                                              bool* pause,
+                                              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) {
+    *next_state = STATE_WRITE_DATA_FOR_PASSTHROUGH;
+    return net::OK;
+  }
+  *next_state = STATE_READ_DATA_FOR_COPY_DONE;
+  return ReadDataHelper(copy_reader_, data_to_copy_.get(), to_read);
+}
+
+int ServiceWorkerCacheWriter::ReadDataForCopyDone(int* next_state,
+                                                  bool* pause,
+                                                  int result) {
+  if (result < 0) {
+    *next_state = STATE_DONE;
+    return result;
+  }
+  *next_state = STATE_WRITE_DATA_FOR_COPY;
+  return result;
+}
+
+int ServiceWorkerCacheWriter::WriteDataForCopy(int* next_state,
+                                               bool* pause,
+                                               int result) {
+  *next_state = STATE_WRITE_DATA_FOR_COPY_DONE;
+  DCHECK_GT(result, 0);
+  return WriteDataHelper(writer_, data_to_copy_.get(), result);
+}
+
+int ServiceWorkerCacheWriter::WriteDataForCopyDone(int* next_state,
+                                                   bool* pause,
+                                                   int result) {
+  if (result < 0) {
+    *next_state = STATE_DONE;
+    return result;
+  }
+  bytes_written_ += result;
+  bytes_copied_ += result;
+  *next_state = STATE_READ_DATA_FOR_COPY;
+  return result;
+}
+
+int ServiceWorkerCacheWriter::Done(int* next_state, bool* pause, int result) {
+  *next_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) {
+  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);
+    callback.Run(error);
+  }
+}
+
+}  // namespace content