Reland: URLRequestJob: change ReadRawData contract

This CL is to reland crrev.com/1410643007 which was
reverted in crrev.com/1437523002.

Previously, the interface for URLRequestJob::ReadRawData was as follows:

  bool ReadRawData(IOBuffer*, int, int*)

Subclasses were expected to signal completion of the ReadRawData call by
calling NotifyDone, SetStatus, or maybe one of the other Notify* functions
on URLRequestJob, most of which do internal housekeeping and also drive the
URLRequest's state machine. This made it difficult to reason about the
URLRequestJob's state machine and needlessly complicated most of URLRequestJob.

The new interface is as follows:

  int ReadRawData(IOBuffer*, int)

Subclasses are required to either:

a) Return ERR_IO_PENDING, and call ReadRawDataComplete when the read completes in any way, or
b) Return a count of bytes read >= 0, indicating synchronous success, or
c) Return another error code < 0, indicating synchronous failure.

This substantially narrows the interface between URLRequestJob and its
subclasses and moves the logic for the URLRequest state machine largely into
URLRequestJob.

Also, the signature of URLRequestJob::ReadFilteredData and some other internal
URLRequestJob helpers changes to propagate detailed error codes instead of
coercing all errors to FAILED.

TBR=michaeln@chromium.org,mtomasz@chromium.org,jianli@chromium.org,zork@chromium.org

BUG=474859
BUG=329902

Review URL: https://codereview.chromium.org/1439953006

Cr-Commit-Position: refs/heads/master@{#360327}

15 files changed, 274 insertions, 351 deletions

diff --git a/content/browser/android/url_request_content_job.cc b/content/browser/android/url_request_content_job.cc
index 6e661a2..1bcbf210 100644
--- a/content/browser/android/url_request_content_job.cc
+++ b/content/browser/android/url_request_content_job.cc
@@ -11,7 +11,6 @@
 #include "base/task_runner.h"
 #include "net/base/file_stream.h"
 #include "net/base/io_buffer.h"
-#include "net/base/net_errors.h"
 #include "net/http/http_util.h"
 #include "net/url_request/url_request_error_job.h"
 #include "url/gurl.h"
@@ -34,6 +33,7 @@ URLRequestContentJob::URLRequestContentJob(
       content_path_(content_path),
       stream_(new net::FileStream(content_task_runner)),
       content_task_runner_(content_task_runner),
+      range_parse_result_(net::OK),
       remaining_bytes_(0),
       io_pending_(false),
       weak_ptr_factory_(this) {}
@@ -56,43 +56,28 @@ void URLRequestContentJob::Kill() {
   net::URLRequestJob::Kill();
 }
 
-bool URLRequestContentJob::ReadRawData(net::IOBuffer* dest,
-                                       int dest_size,
-                                       int* bytes_read) {
+int URLRequestContentJob::ReadRawData(net::IOBuffer* dest, int dest_size) {
   DCHECK_GT(dest_size, 0);
-  DCHECK(bytes_read);
   DCHECK_GE(remaining_bytes_, 0);
 
   if (remaining_bytes_ < dest_size)
-    dest_size = static_cast<int>(remaining_bytes_);
+    dest_size = remaining_bytes_;
 
   // If we should copy zero bytes because |remaining_bytes_| is zero, short
   // circuit here.
-  if (!dest_size) {
-    *bytes_read = 0;
-    return true;
-  }
-
-  int rv =
-      stream_->Read(dest, dest_size, base::Bind(&URLRequestContentJob::DidRead,
-                                                weak_ptr_factory_.GetWeakPtr(),
-                                                make_scoped_refptr(dest)));
-  if (rv >= 0) {
-    // Data is immediately available.
-    *bytes_read = rv;
-    remaining_bytes_ -= rv;
-    DCHECK_GE(remaining_bytes_, 0);
-    return true;
-  }
+  if (!dest_size)
+    return 0;
 
-  // Otherwise, a read error occured.  We may just need to wait...
+  int rv = stream_->Read(dest, dest_size,
+                         base::Bind(&URLRequestContentJob::DidRead,
+                                    weak_ptr_factory_.GetWeakPtr()));
   if (rv == net::ERR_IO_PENDING) {
     io_pending_ = true;
-    SetStatus(net::URLRequestStatus(net::URLRequestStatus::IO_PENDING, 0));
-  } else {
-    NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED, rv));
+  } else if (rv > 0) {
+    remaining_bytes_ -= rv;
   }
-  return false;
+  DCHECK_GE(remaining_bytes_, 0);
+  return rv;
 }
 
 bool URLRequestContentJob::IsRedirectResponse(GURL* location,
@@ -115,15 +100,16 @@ void URLRequestContentJob::SetExtraRequestHeaders(
   if (!headers.GetHeader(net::HttpRequestHeaders::kRange, &range_header))
     return;
 
-  // We only care about "Range" header here.
+  // Currently this job only cares about the Range header. Note that validation
+  // is deferred to DidOpen(), because NotifyStartError is not legal to call
+  // since the job has not started.
   std::vector<net::HttpByteRange> ranges;
   if (net::HttpUtil::ParseRangeHeader(range_header, &ranges)) {
     if (ranges.size() == 1) {
       byte_range_ = ranges[0];
     } else {
       // We don't support multiple range requests.
-      NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED,
-                                       net::ERR_REQUEST_RANGE_NOT_SATISFIABLE));
+      range_parse_result_ = net::ERR_REQUEST_RANGE_NOT_SATISFIABLE;
     }
   }
 }
@@ -160,13 +146,20 @@ void URLRequestContentJob::DidFetchMetaInfo(const ContentMetaInfo* meta_info) {
 
 void URLRequestContentJob::DidOpen(int result) {
   if (result != net::OK) {
-    NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED, result));
+    NotifyStartError(
+        net::URLRequestStatus(net::URLRequestStatus::FAILED, result));
+    return;
+  }
+
+  if (range_parse_result_ != net::OK) {
+    NotifyStartError(net::URLRequestStatus(net::URLRequestStatus::FAILED,
+                                           range_parse_result_));
     return;
   }
 
   if (!byte_range_.ComputeBounds(meta_info_.content_size)) {
-    NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED,
-                                     net::ERR_REQUEST_RANGE_NOT_SATISFIABLE));
+    NotifyStartError(net::URLRequestStatus(
+        net::URLRequestStatus::FAILED, net::ERR_REQUEST_RANGE_NOT_SATISFIABLE));
     return;
   }
 
@@ -193,8 +186,8 @@ void URLRequestContentJob::DidOpen(int result) {
 
 void URLRequestContentJob::DidSeek(int64 result) {
   if (result != byte_range_.first_byte_position()) {
-    NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED,
-                                     net::ERR_REQUEST_RANGE_NOT_SATISFIABLE));
+    NotifyStartError(net::URLRequestStatus(
+        net::URLRequestStatus::FAILED, net::ERR_REQUEST_RANGE_NOT_SATISFIABLE));
     return;
   }
 
@@ -202,24 +195,16 @@ void URLRequestContentJob::DidSeek(int64 result) {
   NotifyHeadersComplete();
 }
 
-void URLRequestContentJob::DidRead(scoped_refptr<net::IOBuffer> buf,
-                                   int result) {
-  if (result > 0) {
-    SetStatus(net::URLRequestStatus());  // Clear the IO_PENDING status
-    remaining_bytes_ -= result;
-    DCHECK_GE(remaining_bytes_, 0);
-  }
-
+void URLRequestContentJob::DidRead(int result) {
   DCHECK(io_pending_);
   io_pending_ = false;
 
-  if (result == 0) {
-    NotifyDone(net::URLRequestStatus());
-  } else if (result < 0) {
-    NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED, result));
+  if (result > 0) {
+    remaining_bytes_ -= result;
+    DCHECK_GE(remaining_bytes_, 0);
   }
 
-  NotifyReadComplete(result);
+  ReadRawDataComplete(result);
 }
 
 }  // namespace content
diff --git a/content/browser/android/url_request_content_job.h b/content/browser/android/url_request_content_job.h
index 5d3d933..dad9fbf 100644
--- a/content/browser/android/url_request_content_job.h
+++ b/content/browser/android/url_request_content_job.h
@@ -12,6 +12,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "content/common/content_export.h"
+#include "net/base/net_errors.h"
 #include "net/http/http_byte_range.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_job.h"
@@ -42,7 +43,7 @@ class CONTENT_EXPORT URLRequestContentJob : public net::URLRequestJob {
   // net::URLRequestJob:
   void Start() override;
   void Kill() override;
-  bool ReadRawData(net::IOBuffer* buf, int buf_size, int* bytes_read) override;
+  int ReadRawData(net::IOBuffer* buf, int buf_size) override;
   bool IsRedirectResponse(GURL* location, int* http_status_code) override;
   bool GetMimeType(std::string* mime_type) const override;
   void SetExtraRequestHeaders(const net::HttpRequestHeaders& headers) override;
@@ -79,7 +80,7 @@ class CONTENT_EXPORT URLRequestContentJob : public net::URLRequestJob {
   void DidSeek(int64 result);
 
   // Callback after data is asynchronously read from the content URI into |buf|.
-  void DidRead(scoped_refptr<net::IOBuffer> buf, int result);
+  void DidRead(int result);
 
   // The full path of the content URI.
   base::FilePath content_path_;
@@ -89,6 +90,7 @@ class CONTENT_EXPORT URLRequestContentJob : public net::URLRequestJob {
   const scoped_refptr<base::TaskRunner> content_task_runner_;
 
   net::HttpByteRange byte_range_;
+  net::Error range_parse_result_;
   int64 remaining_bytes_;
 
   bool io_pending_;
diff --git a/content/browser/appcache/appcache_url_request_job.cc b/content/browser/appcache/appcache_url_request_job.cc
index 82997f7..de924a1 100644
--- a/content/browser/appcache/appcache_url_request_job.cc
+++ b/content/browser/appcache/appcache_url_request_job.cc
@@ -341,7 +341,6 @@ void AppCacheURLRequestJob::SetupRangeResponse() {
 void AppCacheURLRequestJob::OnReadComplete(int result) {
   DCHECK(is_delivering_appcache_response());
   if (result == 0) {
-    NotifyDone(net::URLRequestStatus());
     AppCacheHistograms::CountResponseRetrieval(
         true, is_main_resource_, manifest_url_.GetOrigin());
   } else if (result < 0) {
@@ -349,13 +348,10 @@ void AppCacheURLRequestJob::OnReadComplete(int result) {
       storage_->service()->CheckAppCacheResponse(manifest_url_, cache_id_,
                                                  entry_.response_id());
     }
-    NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED, result));
     AppCacheHistograms::CountResponseRetrieval(
         false, is_main_resource_, manifest_url_.GetOrigin());
-  } else {
-    SetStatus(net::URLRequestStatus());  // Clear the IO_PENDING status
   }
-  NotifyReadComplete(result);
+  ReadRawDataComplete(result);
 }
 
 // net::URLRequestJob overrides ------------------------------------------------
@@ -424,18 +420,14 @@ int AppCacheURLRequestJob::GetResponseCode() const {
   return http_info()->headers->response_code();
 }
 
-bool AppCacheURLRequestJob::ReadRawData(net::IOBuffer* buf,
-                                        int buf_size,
-                                        int* bytes_read) {
+int AppCacheURLRequestJob::ReadRawData(net::IOBuffer* buf, int buf_size) {
   DCHECK(is_delivering_appcache_response());
   DCHECK_NE(buf_size, 0);
-  DCHECK(bytes_read);
   DCHECK(!reader_->IsReadPending());
   reader_->ReadData(buf, buf_size,
                     base::Bind(&AppCacheURLRequestJob::OnReadComplete,
                                base::Unretained(this)));
-  SetStatus(net::URLRequestStatus(net::URLRequestStatus::IO_PENDING, 0));
-  return false;
+  return net::ERR_IO_PENDING;
 }
 
 void AppCacheURLRequestJob::SetExtraRequestHeaders(
diff --git a/content/browser/appcache/appcache_url_request_job.h b/content/browser/appcache/appcache_url_request_job.h
index 95a3e18..8f51e7b 100644
--- a/content/browser/appcache/appcache_url_request_job.h
+++ b/content/browser/appcache/appcache_url_request_job.h
@@ -148,7 +148,7 @@ class CONTENT_EXPORT AppCacheURLRequestJob
   net::LoadState GetLoadState() const override;
   bool GetCharset(std::string* charset) override;
   void GetResponseInfo(net::HttpResponseInfo* info) override;
-  bool ReadRawData(net::IOBuffer* buf, int buf_size, int* bytes_read) override;
+  int ReadRawData(net::IOBuffer* buf, int buf_size) override;
 
   // Sets extra request headers for Job types that support request headers.
   // This is how we get informed of range-requests.
diff --git a/content/browser/fileapi/file_writer_delegate_unittest.cc b/content/browser/fileapi/file_writer_delegate_unittest.cc
index c9bcfbb..9eb73b1 100644
--- a/content/browser/fileapi/file_writer_delegate_unittest.cc
+++ b/content/browser/fileapi/file_writer_delegate_unittest.cc
@@ -184,17 +184,15 @@ class FileWriterDelegateTestJob : public net::URLRequestJob {
         base::Bind(&FileWriterDelegateTestJob::NotifyHeadersComplete, this));
   }
 
-  bool ReadRawData(net::IOBuffer* buf, int buf_size, int* bytes_read) override {
+  int ReadRawData(net::IOBuffer* buf, int buf_size) override {
     if (remaining_bytes_ < buf_size)
-      buf_size = static_cast<int>(remaining_bytes_);
+      buf_size = remaining_bytes_;
 
     for (int i = 0; i < buf_size; ++i)
       buf->data()[i] = content_[cursor_++];
     remaining_bytes_ -= buf_size;
 
-    SetStatus(net::URLRequestStatus());
-    *bytes_read = buf_size;
-    return true;
+    return buf_size;
   }
 
   int GetResponseCode() const override { return 200; }
diff --git a/content/browser/net/view_http_cache_job_factory.cc b/content/browser/net/view_http_cache_job_factory.cc
index 823ced9..22b408e 100644
--- a/content/browser/net/view_http_cache_job_factory.cc
+++ b/content/browser/net/view_http_cache_job_factory.cc
@@ -45,15 +45,8 @@ class ViewHttpCacheJob : public net::URLRequestJob {
   bool GetCharset(std::string* charset) override {
     return core_->GetCharset(charset);
   }
-  bool ReadRawData(net::IOBuffer* buf,
-                   int buf_size,
-                   int* out_bytes_read) override {
-    size_t bytes_read;
-    if (!core_->ReadRawData(buf, base::checked_cast<size_t>(buf_size),
-                            &bytes_read))
-      return false;
-    *out_bytes_read = base::checked_cast<int>(bytes_read);
-    return true;
+  int ReadRawData(net::IOBuffer* buf, int buf_size) override {
+    return core_->ReadRawData(buf, buf_size);
   }
 
  private:
@@ -73,7 +66,7 @@ class ViewHttpCacheJob : public net::URLRequestJob {
 
     bool GetMimeType(std::string* mime_type) const;
     bool GetCharset(std::string* charset);
-    bool ReadRawData(net::IOBuffer* buf, size_t buf_size, size_t* bytes_read);
+    int ReadRawData(net::IOBuffer* buf, int buf_size);
 
    private:
     friend class base::RefCounted<Core>;
@@ -172,18 +165,14 @@ bool ViewHttpCacheJob::Core::GetCharset(std::string* charset) {
   return true;
 }
 
-bool ViewHttpCacheJob::Core::ReadRawData(net::IOBuffer* buf,
-                                         size_t buf_size,
-                                         size_t* bytes_read) {
-  DCHECK(bytes_read);
+int ViewHttpCacheJob::Core::ReadRawData(net::IOBuffer* buf, int buf_size) {
   DCHECK_LE(data_offset_, data_.size());
-  size_t remaining = data_.size() - data_offset_;
+  int remaining = base::checked_cast<int>(data_.size() - data_offset_);
   if (buf_size > remaining)
     buf_size = remaining;
   memcpy(buf->data(), data_.data() + data_offset_, buf_size);
   data_offset_ += buf_size;
-  *bytes_read = buf_size;
-  return true;
+  return buf_size;
 }
 
 void ViewHttpCacheJob::Core::OnIOComplete(int result) {
diff --git a/content/browser/service_worker/service_worker_read_from_cache_job.cc b/content/browser/service_worker/service_worker_read_from_cache_job.cc
index 9357991..6aa92af8 100644
--- a/content/browser/service_worker/service_worker_read_from_cache_job.cc
+++ b/content/browser/service_worker/service_worker_read_from_cache_job.cc
@@ -41,26 +41,9 @@ ServiceWorkerReadFromCacheJob::~ServiceWorkerReadFromCacheJob() {
 }
 
 void ServiceWorkerReadFromCacheJob::Start() {
-  TRACE_EVENT_ASYNC_BEGIN1("ServiceWorker",
-                           "ServiceWorkerReadFromCacheJob::ReadInfo", this,
-                           "URL", request_->url().spec());
-  if (!context_) {
-    NotifyStartError(
-        net::URLRequestStatus(net::URLRequestStatus::FAILED, net::ERR_FAILED));
-    return;
-  }
-
-  // Create a response reader and start reading the headers,
-  // we'll continue when thats done.
-  if (is_main_script())
-    version_->embedded_worker()->OnScriptReadStarted();
-  reader_ = context_->storage()->CreateResponseReader(resource_id_);
-  http_info_io_buffer_ = new HttpResponseInfoIOBuffer;
-  reader_->ReadInfo(
-      http_info_io_buffer_.get(),
-      base::Bind(&ServiceWorkerReadFromCacheJob::OnReadInfoComplete,
-                 weak_factory_.GetWeakPtr()));
-  SetStatus(net::URLRequestStatus(net::URLRequestStatus::IO_PENDING, 0));
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::Bind(&ServiceWorkerReadFromCacheJob::StartAsync,
+                            weak_factory_.GetWeakPtr()));
 }
 
 void ServiceWorkerReadFromCacheJob::Kill() {
@@ -122,11 +105,9 @@ void ServiceWorkerReadFromCacheJob::SetExtraRequestHeaders(
     range_requested_ = ranges[0];
 }
 
-bool ServiceWorkerReadFromCacheJob::ReadRawData(net::IOBuffer* buf,
-                                                int buf_size,
-                                                int* bytes_read) {
+int ServiceWorkerReadFromCacheJob::ReadRawData(net::IOBuffer* buf,
+                                               int buf_size) {
   DCHECK_NE(buf_size, 0);
-  DCHECK(bytes_read);
   DCHECK(!reader_->IsReadPending());
   TRACE_EVENT_ASYNC_BEGIN1("ServiceWorker",
                            "ServiceWorkerReadFromCacheJob::ReadRawData",
@@ -135,8 +116,31 @@ bool ServiceWorkerReadFromCacheJob::ReadRawData(net::IOBuffer* buf,
   reader_->ReadData(buf, buf_size,
                     base::Bind(&ServiceWorkerReadFromCacheJob::OnReadComplete,
                                weak_factory_.GetWeakPtr()));
+  return net::ERR_IO_PENDING;
+}
+
+void ServiceWorkerReadFromCacheJob::StartAsync() {
+  TRACE_EVENT_ASYNC_BEGIN1("ServiceWorker",
+                           "ServiceWorkerReadFromCacheJob::ReadInfo", this,
+                           "URL", request_->url().spec());
+  if (!context_) {
+    // NotifyStartError is not safe to call synchronously in Start.
+    NotifyStartError(
+        net::URLRequestStatus(net::URLRequestStatus::FAILED, net::ERR_FAILED));
+    return;
+  }
+
+  // Create a response reader and start reading the headers,
+  // we'll continue when thats done.
+  if (is_main_script())
+    version_->embedded_worker()->OnScriptReadStarted();
+  reader_ = context_->storage()->CreateResponseReader(resource_id_);
+  http_info_io_buffer_ = new HttpResponseInfoIOBuffer;
+  reader_->ReadInfo(
+      http_info_io_buffer_.get(),
+      base::Bind(&ServiceWorkerReadFromCacheJob::OnReadInfoComplete,
+                 weak_factory_.GetWeakPtr()));
   SetStatus(net::URLRequestStatus(net::URLRequestStatus::IO_PENDING, 0));
-  return false;
 }
 
 const net::HttpResponseInfo* ServiceWorkerReadFromCacheJob::http_info() const {
@@ -154,6 +158,8 @@ void ServiceWorkerReadFromCacheJob::OnReadInfoComplete(int result) {
     ServiceWorkerMetrics::CountReadResponseResult(
         ServiceWorkerMetrics::READ_HEADERS_ERROR);
     Done(net::URLRequestStatus(net::URLRequestStatus::FAILED, result));
+    NotifyStartError(
+        net::URLRequestStatus(net::URLRequestStatus::FAILED, result));
     return;
   }
   DCHECK_GE(result, 0);
@@ -207,23 +213,22 @@ void ServiceWorkerReadFromCacheJob::Done(const net::URLRequestStatus& status) {
   }
   if (is_main_script())
     version_->embedded_worker()->OnScriptReadFinished();
-  NotifyDone(status);
 }
 
 void ServiceWorkerReadFromCacheJob::OnReadComplete(int result) {
   ServiceWorkerMetrics::ReadResponseResult check_result;
-  if (result == 0) {
+
+  if (result >= 0) {
     check_result = ServiceWorkerMetrics::READ_OK;
-    Done(net::URLRequestStatus());
-  } else if (result < 0) {
+    if (result == 0)
+      Done(net::URLRequestStatus());
+  } else {
     check_result = ServiceWorkerMetrics::READ_DATA_ERROR;
     Done(net::URLRequestStatus(net::URLRequestStatus::FAILED, result));
-  } else {
-    check_result = ServiceWorkerMetrics::READ_OK;
-    SetStatus(net::URLRequestStatus());  // Clear the IO_PENDING status
   }
+
   ServiceWorkerMetrics::CountReadResponseResult(check_result);
-  NotifyReadComplete(result);
+  ReadRawDataComplete(result);
   TRACE_EVENT_ASYNC_END1("ServiceWorker",
                          "ServiceWorkerReadFromCacheJob::ReadRawData",
                          this,
diff --git a/content/browser/service_worker/service_worker_read_from_cache_job.h b/content/browser/service_worker/service_worker_read_from_cache_job.h
index fccde7a..a62893d 100644
--- a/content/browser/service_worker/service_worker_read_from_cache_job.h
+++ b/content/browser/service_worker/service_worker_read_from_cache_job.h
@@ -51,13 +51,14 @@ class CONTENT_EXPORT ServiceWorkerReadFromCacheJob
   void GetResponseInfo(net::HttpResponseInfo* info) override;
   int GetResponseCode() const override;
   void SetExtraRequestHeaders(const net::HttpRequestHeaders& headers) override;
-  bool ReadRawData(net::IOBuffer* buf, int buf_size, int* bytes_read) override;
+  int ReadRawData(net::IOBuffer* buf, int buf_size) override;
 
   // Reader completion callbacks.
   void OnReadInfoComplete(int result);
   void OnReadComplete(int result);
 
   // Helpers
+  void StartAsync();
   const net::HttpResponseInfo* http_info() const;
   bool is_range_request() const { return range_requested_.IsValid(); }
   void SetupRangeResponse(int response_data_size);
diff --git a/content/browser/service_worker/service_worker_url_request_job.cc b/content/browser/service_worker/service_worker_url_request_job.cc
index 15c85a1..143e354 100644
--- a/content/browser/service_worker/service_worker_url_request_job.cc
+++ b/content/browser/service_worker/service_worker_url_request_job.cc
@@ -203,50 +203,44 @@ void ServiceWorkerURLRequestJob::SetExtraRequestHeaders(
     byte_range_ = ranges[0];
 }
 
-bool ServiceWorkerURLRequestJob::ReadRawData(net::IOBuffer* buf,
-                                             int buf_size,
-                                             int* bytes_read) {
+int ServiceWorkerURLRequestJob::ReadRawData(net::IOBuffer* buf, int buf_size) {
   DCHECK(buf);
   DCHECK_GE(buf_size, 0);
-  DCHECK(bytes_read);
   DCHECK(waiting_stream_url_.is_empty());
+
+  int bytes_read = 0;
+
   if (stream_.get()) {
-    switch (stream_->ReadRawData(buf, buf_size, bytes_read)) {
+    switch (stream_->ReadRawData(buf, buf_size, &bytes_read)) {
       case Stream::STREAM_HAS_DATA:
-        DCHECK_GT(*bytes_read, 0);
-        return true;
+        DCHECK_GT(bytes_read, 0);
+        return bytes_read;
       case Stream::STREAM_COMPLETE:
-        DCHECK(!*bytes_read);
+        DCHECK_EQ(0, bytes_read);
         RecordResult(ServiceWorkerMetrics::REQUEST_JOB_STREAM_RESPONSE);
-        return true;
+        return 0;
       case Stream::STREAM_EMPTY:
         stream_pending_buffer_ = buf;
         stream_pending_buffer_size_ = buf_size;
-        SetStatus(net::URLRequestStatus(net::URLRequestStatus::IO_PENDING, 0));
-        return false;
+        return net::ERR_IO_PENDING;
       case Stream::STREAM_ABORTED:
         // Handle this as connection reset.
         RecordResult(ServiceWorkerMetrics::REQUEST_JOB_ERROR_STREAM_ABORTED);
-        NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED,
-                                         net::ERR_CONNECTION_RESET));
-        return false;
+        return net::ERR_CONNECTION_RESET;
     }
     NOTREACHED();
-    return false;
+    return net::ERR_FAILED;
   }
 
-  if (!blob_request_) {
-    *bytes_read = 0;
-    return true;
-  }
-  blob_request_->Read(buf, buf_size, bytes_read);
+  if (!blob_request_)
+    return 0;
+  blob_request_->Read(buf, buf_size, &bytes_read);
   net::URLRequestStatus status = blob_request_->status();
-  SetStatus(status);
-  if (status.is_io_pending())
-    return false;
-  if (status.is_success() && *bytes_read == 0)
+  if (status.status() != net::URLRequestStatus::SUCCESS)
+    return status.error();
+  if (bytes_read == 0)
     RecordResult(ServiceWorkerMetrics::REQUEST_JOB_BLOB_RESPONSE);
-  return status.is_success();
+  return bytes_read;
 }
 
 // TODO(falken): Refactor Blob and Stream specific handling to separate classes.
@@ -292,29 +286,18 @@ void ServiceWorkerURLRequestJob::OnResponseStarted(net::URLRequest* request) {
 
 void ServiceWorkerURLRequestJob::OnReadCompleted(net::URLRequest* request,
                                                  int bytes_read) {
-  SetStatus(request->status());
   if (!request->status().is_success()) {
     RecordResult(ServiceWorkerMetrics::REQUEST_JOB_ERROR_BLOB_READ);
-    NotifyDone(request->status());
-    return;
-  }
-
-  if (bytes_read == 0) {
-    // Protect because NotifyReadComplete() can destroy |this|.
-    scoped_refptr<ServiceWorkerURLRequestJob> protect(this);
+  } else if (bytes_read == 0) {
     RecordResult(ServiceWorkerMetrics::REQUEST_JOB_BLOB_RESPONSE);
-    NotifyReadComplete(bytes_read);
-    NotifyDone(request->status());
-    return;
   }
-  NotifyReadComplete(bytes_read);
+  net::URLRequestStatus status = request->status();
+  ReadRawDataComplete(status.is_success() ? bytes_read : status.error());
 }
 
 // Overrides for Stream reading -----------------------------------------------
 
 void ServiceWorkerURLRequestJob::OnDataAvailable(Stream* stream) {
-  // Clear the IO_PENDING status.
-  SetStatus(net::URLRequestStatus());
   // Do nothing if stream_pending_buffer_ is empty, i.e. there's no ReadRawData
   // operation waiting for IO completion.
   if (!stream_pending_buffer_.get())
@@ -323,15 +306,15 @@ void ServiceWorkerURLRequestJob::OnDataAvailable(Stream* stream) {
   // stream_pending_buffer_ is set to the IOBuffer instance provided to
   // ReadRawData() by URLRequestJob.
 
-  int bytes_read = 0;
+  int result = 0;
   switch (stream_->ReadRawData(stream_pending_buffer_.get(),
-                               stream_pending_buffer_size_, &bytes_read)) {
+                               stream_pending_buffer_size_, &result)) {
     case Stream::STREAM_HAS_DATA:
-      DCHECK_GT(bytes_read, 0);
+      DCHECK_GT(result, 0);
       break;
     case Stream::STREAM_COMPLETE:
       // Calling NotifyReadComplete with 0 signals completion.
-      DCHECK(!bytes_read);
+      DCHECK(!result);
       RecordResult(ServiceWorkerMetrics::REQUEST_JOB_STREAM_RESPONSE);
       break;
     case Stream::STREAM_EMPTY:
@@ -339,9 +322,8 @@ void ServiceWorkerURLRequestJob::OnDataAvailable(Stream* stream) {
       break;
     case Stream::STREAM_ABORTED:
       // Handle this as connection reset.
+      result = net::ERR_CONNECTION_RESET;
       RecordResult(ServiceWorkerMetrics::REQUEST_JOB_ERROR_STREAM_ABORTED);
-      NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED,
-                                       net::ERR_CONNECTION_RESET));
       break;
   }
 
@@ -349,7 +331,7 @@ void ServiceWorkerURLRequestJob::OnDataAvailable(Stream* stream) {
   // safe for the observer to read.
   stream_pending_buffer_ = nullptr;
   stream_pending_buffer_size_ = 0;
-  NotifyReadComplete(bytes_read);
+  ReadRawDataComplete(result);
 }
 
 void ServiceWorkerURLRequestJob::OnStreamRegistered(Stream* stream) {
@@ -645,7 +627,7 @@ void ServiceWorkerURLRequestJob::DidDispatchFetchEvent(
   // error.
   if (response.status_code == 0) {
     RecordStatusZeroResponseError(response.error);
-    NotifyDone(
+    NotifyStartError(
         net::URLRequestStatus(net::URLRequestStatus::FAILED, net::ERR_FAILED));
     return;
   }
diff --git a/content/browser/service_worker/service_worker_url_request_job.h b/content/browser/service_worker/service_worker_url_request_job.h
index 4815c0b..2a50c94 100644
--- a/content/browser/service_worker/service_worker_url_request_job.h
+++ b/content/browser/service_worker/service_worker_url_request_job.h
@@ -87,7 +87,7 @@ class CONTENT_EXPORT ServiceWorkerURLRequestJob
   void GetLoadTimingInfo(net::LoadTimingInfo* load_timing_info) const override;
   int GetResponseCode() const override;
   void SetExtraRequestHeaders(const net::HttpRequestHeaders& headers) override;
-  bool ReadRawData(net::IOBuffer* buf, int buf_size, int* bytes_read) override;
+  int ReadRawData(net::IOBuffer* buf, int buf_size) override;
 
   // net::URLRequest::Delegate overrides that read the blob from the
   // ServiceWorkerFetchResponse.
diff --git a/content/browser/service_worker/service_worker_write_to_cache_job.cc b/content/browser/service_worker/service_worker_write_to_cache_job.cc
index 24bcd0f..ba605c0 100644
--- a/content/browser/service_worker/service_worker_write_to_cache_job.cc
+++ b/content/browser/service_worker/service_worker_write_to_cache_job.cc
@@ -7,6 +7,7 @@
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/strings/stringprintf.h"
+#include "base/thread_task_runner_handle.h"
 #include "base/trace_event/trace_event.h"
 #include "content/browser/service_worker/service_worker_cache_writer.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
@@ -47,7 +48,7 @@ const char kServiceWorkerAllowed[] = "Service-Worker-Allowed";
 // developers.
 // TODO(falken): Redesign this class so we don't have to fail at the network
 // stack layer just to cancel the update.
-const int kIdenticalScriptError = net::ERR_FILE_EXISTS;
+const net::Error kIdenticalScriptError = net::ERR_FILE_EXISTS;
 
 }  // namespace
 
@@ -79,11 +80,18 @@ ServiceWorkerWriteToCacheJob::~ServiceWorkerWriteToCacheJob() {
 }
 
 void ServiceWorkerWriteToCacheJob::Start() {
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::Bind(&ServiceWorkerWriteToCacheJob::StartAsync,
+                            weak_factory_.GetWeakPtr()));
+}
+
+void ServiceWorkerWriteToCacheJob::StartAsync() {
   TRACE_EVENT_ASYNC_BEGIN1("ServiceWorker",
                            "ServiceWorkerWriteToCacheJob::ExecutingJob",
                            this,
                            "URL", request_->url().spec());
   if (!context_) {
+    // NotifyStartError is not safe to call synchronously in Start().
     NotifyStartError(
         net::URLRequestStatus(net::URLRequestStatus::FAILED, net::ERR_FAILED));
     return;
@@ -108,8 +116,9 @@ void ServiceWorkerWriteToCacheJob::Kill() {
   has_been_killed_ = true;
   net_request_.reset();
   if (did_notify_started_) {
-    NotifyFinishedCaching(net::URLRequestStatus::FromError(net::ERR_ABORTED),
-                          kKilledError);
+    net::Error error = NotifyFinishedCaching(
+        net::URLRequestStatus::FromError(net::ERR_ABORTED), kKilledError);
+    DCHECK_EQ(net::ERR_ABORTED, error);
   }
   writer_.reset();
   context_.reset();
@@ -156,40 +165,20 @@ void ServiceWorkerWriteToCacheJob::SetExtraRequestHeaders(
   net_request_->SetExtraRequestHeaders(headers);
 }
 
-bool ServiceWorkerWriteToCacheJob::ReadRawData(net::IOBuffer* buf,
-                                               int buf_size,
-                                               int* bytes_read) {
-  net::URLRequestStatus status = ReadNetData(buf, buf_size, bytes_read);
-  SetStatus(status);
+int ServiceWorkerWriteToCacheJob::ReadRawData(net::IOBuffer* buf,
+                                              int buf_size) {
+  int bytes_read = 0;
+  net::URLRequestStatus status = ReadNetData(buf, buf_size, &bytes_read);
   if (status.is_io_pending())
-    return false;
-
-  if (!status.is_success()) {
-    NotifyDoneHelper(status, kFetchScriptError);
-    return false;
-  }
-
-  HandleNetData(*bytes_read);
-  status = GetStatus();
-
-  // Synchronous EOFs that do not replace the incumbent entry are considered
-  // failures. Since normally the URLRequestJob's status would be set by
-  // ReadNetData or HandleNetData, this code has to manually fix up the status
-  // to match the failure this function is about to return.
-  if (status.status() == net::URLRequestStatus::SUCCESS && *bytes_read == 0 &&
-      !cache_writer_->did_replace()) {
-    status = net::URLRequestStatus::FromError(kIdenticalScriptError);
-  }
+    return net::ERR_IO_PENDING;
 
   if (!status.is_success()) {
-    NotifyDoneHelper(status, "");
-    return false;
+    net::Error error = NotifyFinishedCaching(status, kFetchScriptError);
+    DCHECK_EQ(status.error(), error);
+    return error;
   }
 
-  // Since URLRequestStatus::is_success() means "SUCCESS or IO_PENDING", but the
-  // contract of this function is "return true for synchronous successes only",
-  // it is important to test against SUCCESS explicitly here.
-  return status.status() == net::URLRequestStatus::SUCCESS;
+  return HandleNetData(bytes_read);
 }
 
 const net::HttpResponseInfo* ServiceWorkerWriteToCacheJob::http_info() const {
@@ -244,9 +233,9 @@ void ServiceWorkerWriteToCacheJob::OnReceivedRedirect(
   TRACE_EVENT0("ServiceWorker",
                "ServiceWorkerWriteToCacheJob::OnReceivedRedirect");
   // Script resources can't redirect.
-  NotifyDoneHelper(net::URLRequestStatus(net::URLRequestStatus::FAILED,
-                                         net::ERR_UNSAFE_REDIRECT),
-                   kRedirectError);
+  NotifyStartErrorHelper(net::URLRequestStatus(net::URLRequestStatus::FAILED,
+                                               net::ERR_UNSAFE_REDIRECT),
+                         kRedirectError);
 }
 
 void ServiceWorkerWriteToCacheJob::OnAuthRequired(
@@ -256,7 +245,7 @@ void ServiceWorkerWriteToCacheJob::OnAuthRequired(
   TRACE_EVENT0("ServiceWorker",
                "ServiceWorkerWriteToCacheJob::OnAuthRequired");
   // TODO(michaeln): Pass this thru to our jobs client.
-  NotifyDoneHelper(
+  NotifyStartErrorHelper(
       net::URLRequestStatus(net::URLRequestStatus::FAILED, net::ERR_FAILED),
       kClientAuthenticationError);
 }
@@ -269,7 +258,7 @@ void ServiceWorkerWriteToCacheJob::OnCertificateRequested(
                "ServiceWorkerWriteToCacheJob::OnCertificateRequested");
   // TODO(michaeln): Pass this thru to our jobs client.
   // see NotifyCertificateRequested.
-  NotifyDoneHelper(
+  NotifyStartErrorHelper(
       net::URLRequestStatus(net::URLRequestStatus::FAILED, net::ERR_FAILED),
       kClientAuthenticationError);
 }
@@ -283,9 +272,9 @@ void ServiceWorkerWriteToCacheJob::OnSSLCertificateError(
                "ServiceWorkerWriteToCacheJob::OnSSLCertificateError");
   // TODO(michaeln): Pass this thru to our jobs client,
   // see NotifySSLCertificateError.
-  NotifyDoneHelper(net::URLRequestStatus(net::URLRequestStatus::FAILED,
-                                         net::ERR_INSECURE_RESPONSE),
-                   kSSLError);
+  NotifyStartErrorHelper(net::URLRequestStatus(net::URLRequestStatus::FAILED,
+                                               net::ERR_INSECURE_RESPONSE),
+                         kSSLError);
 }
 
 void ServiceWorkerWriteToCacheJob::OnBeforeNetworkStart(
@@ -301,15 +290,15 @@ void ServiceWorkerWriteToCacheJob::OnResponseStarted(
     net::URLRequest* request) {
   DCHECK_EQ(net_request_.get(), request);
   if (!request->status().is_success()) {
-    NotifyDoneHelper(request->status(), kFetchScriptError);
+    NotifyStartErrorHelper(request->status(), kFetchScriptError);
     return;
   }
   if (request->GetResponseCode() / 100 != 2) {
     std::string error_message =
         base::StringPrintf(kBadHTTPResponseError, request->GetResponseCode());
-    NotifyDoneHelper(net::URLRequestStatus(net::URLRequestStatus::FAILED,
-                                           net::ERR_INVALID_RESPONSE),
-                     error_message);
+    NotifyStartErrorHelper(net::URLRequestStatus(net::URLRequestStatus::FAILED,
+                                                 net::ERR_INVALID_RESPONSE),
+                           error_message);
     // TODO(michaeln): Instead of error'ing immediately, send the net
     // response to our consumer, just don't cache it?
     return;
@@ -320,9 +309,10 @@ void ServiceWorkerWriteToCacheJob::OnResponseStarted(
     const net::HttpNetworkSession::Params* session_params =
         request->context()->GetNetworkSessionParams();
     if (!session_params || !session_params->ignore_certificate_errors) {
-      NotifyDoneHelper(net::URLRequestStatus(net::URLRequestStatus::FAILED,
-                                             net::ERR_INSECURE_RESPONSE),
-                       kSSLError);
+      NotifyStartErrorHelper(
+          net::URLRequestStatus(net::URLRequestStatus::FAILED,
+                                net::ERR_INSECURE_RESPONSE),
+          kSSLError);
       return;
     }
   }
@@ -337,9 +327,10 @@ void ServiceWorkerWriteToCacheJob::OnResponseStarted(
           mime_type.empty()
               ? kNoMIMEError
               : base::StringPrintf(kBadMIMEError, mime_type.c_str());
-      NotifyDoneHelper(net::URLRequestStatus(net::URLRequestStatus::FAILED,
-                                             net::ERR_INSECURE_RESPONSE),
-                       error_message);
+      NotifyStartErrorHelper(
+          net::URLRequestStatus(net::URLRequestStatus::FAILED,
+                                net::ERR_INSECURE_RESPONSE),
+          error_message);
       return;
     }
 
@@ -375,52 +366,31 @@ void ServiceWorkerWriteToCacheJob::OnWriteHeadersComplete(net::Error error) {
   NotifyHeadersComplete();
 }
 
-void ServiceWorkerWriteToCacheJob::HandleNetData(int bytes_read) {
-  io_buffer_bytes_ = bytes_read;
-  net::Error error = cache_writer_->MaybeWriteData(
-      io_buffer_.get(), bytes_read,
-      base::Bind(&ServiceWorkerWriteToCacheJob::OnWriteDataComplete,
-                 weak_factory_.GetWeakPtr()));
-  SetStatus(net::URLRequestStatus::FromError(error));
-
-  // In case of ERR_IO_PENDING, this logic is done in OnWriteDataComplete.
-  if (error != net::ERR_IO_PENDING && bytes_read == 0) {
-    NotifyFinishedCaching(net::URLRequestStatus::FromError(error),
-                          std::string());
-  }
-}
-
 void ServiceWorkerWriteToCacheJob::OnWriteDataComplete(net::Error error) {
   SetStatus(net::URLRequestStatus::FromError(error));
   DCHECK_NE(net::ERR_IO_PENDING, error);
-  if (io_buffer_bytes_ == 0) {
-    NotifyDoneHelper(net::URLRequestStatus::FromError(error), std::string());
-  }
-  NotifyReadComplete(error == net::OK ? io_buffer_bytes_ : error);
+  if (io_buffer_bytes_ == 0)
+    error = NotifyFinishedCaching(net::URLRequestStatus::FromError(error), "");
+  ReadRawDataComplete(error == net::OK ? io_buffer_bytes_ : error);
 }
 
 void ServiceWorkerWriteToCacheJob::OnReadCompleted(net::URLRequest* request,
                                                    int bytes_read) {
   DCHECK_EQ(net_request_.get(), request);
+
+  int result;
   if (bytes_read < 0) {
     DCHECK(!request->status().is_success());
-    NotifyDoneHelper(request->status(), kFetchScriptError);
-    return;
-  }
-  HandleNetData(bytes_read);
-  // HandleNetData can cause status of this job to change. If the status changes
-  // to IO_PENDING, that means HandleNetData has pending IO, and
-  // NotifyReadComplete will be called later by the appropriate callback.
-  if (!GetStatus().is_io_pending()) {
-    int result = GetStatus().status() == net::URLRequestStatus::SUCCESS
-                     ? bytes_read
-                     : GetStatus().error();
-    // If bytes_read is 0, HandleNetData synchronously completed and this job is
-    // at EOF.
-    if (bytes_read == 0)
-      NotifyDoneHelper(GetStatus(), std::string());
-    NotifyReadComplete(result);
+    result = NotifyFinishedCaching(request->status(), kFetchScriptError);
+  } else {
+    result = HandleNetData(bytes_read);
   }
+
+  // ReadRawDataComplete will be called in OnWriteDataComplete, so return early.
+  if (result == net::ERR_IO_PENDING)
+    return;
+
+  ReadRawDataComplete(result);
 }
 
 bool ServiceWorkerWriteToCacheJob::CheckPathRestriction(
@@ -434,43 +404,57 @@ bool ServiceWorkerWriteToCacheJob::CheckPathRestriction(
   if (!ServiceWorkerUtils::IsPathRestrictionSatisfied(
           version_->scope(), url_,
           has_header ? &service_worker_allowed : nullptr, &error_message)) {
-    NotifyDoneHelper(net::URLRequestStatus(net::URLRequestStatus::FAILED,
-                                           net::ERR_INSECURE_RESPONSE),
-                     error_message);
+    NotifyStartErrorHelper(net::URLRequestStatus(net::URLRequestStatus::FAILED,
+                                                 net::ERR_INSECURE_RESPONSE),
+                           error_message);
     return false;
   }
   return true;
 }
 
-void ServiceWorkerWriteToCacheJob::NotifyDoneHelper(
+int ServiceWorkerWriteToCacheJob::HandleNetData(int bytes_read) {
+  io_buffer_bytes_ = bytes_read;
+  net::Error error = cache_writer_->MaybeWriteData(
+      io_buffer_.get(), bytes_read,
+      base::Bind(&ServiceWorkerWriteToCacheJob::OnWriteDataComplete,
+                 weak_factory_.GetWeakPtr()));
+
+  // In case of ERR_IO_PENDING, this logic is done in OnWriteDataComplete.
+  if (error != net::ERR_IO_PENDING && bytes_read == 0) {
+    error = NotifyFinishedCaching(net::URLRequestStatus::FromError(error),
+                                  std::string());
+  }
+  return error == net::OK ? bytes_read : error;
+}
+
+void ServiceWorkerWriteToCacheJob::NotifyStartErrorHelper(
     const net::URLRequestStatus& status,
     const std::string& status_message) {
   DCHECK(!status.is_io_pending());
 
-  // Note that NotifyFinishedCaching has logic in it to detect the special case
-  // mentioned below as well.
-  NotifyFinishedCaching(status, status_message);
+  net::Error error = NotifyFinishedCaching(status, status_message);
+  // The special case mentioned in NotifyFinishedCaching about script being
+  // identical does not apply here, since the entire body needs to be read
+  // before this is relevant.
+  DCHECK_EQ(status.error(), error);
 
   net::URLRequestStatus reported_status = status;
   std::string reported_status_message = status_message;
 
-  // A strange special case: requests that successfully fetch the entire
-  // ServiceWorker and write it back, but which did not replace the incumbent
-  // script because the new script was identical, are considered to have failed.
-  if (status.is_success() && !cache_writer_->did_replace()) {
-    reported_status = net::URLRequestStatus::FromError(kIdenticalScriptError);
-    reported_status_message = "";
-  }
-
   SetStatus(reported_status);
-  NotifyDone(reported_status);
+  NotifyStartError(reported_status);
 }
 
-void ServiceWorkerWriteToCacheJob::NotifyFinishedCaching(
+net::Error ServiceWorkerWriteToCacheJob::NotifyFinishedCaching(
     net::URLRequestStatus status,
     const std::string& status_message) {
+  net::Error result = static_cast<net::Error>(status.error());
   if (did_notify_finished_)
-    return;
+    return result;
+
+  int size = -1;
+  if (status.is_success())
+    size = cache_writer_->bytes_written();
 
   // If all the calls to MaybeWriteHeaders/MaybeWriteData succeeded, but the
   // incumbent entry wasn't actually replaced because the new entry was
@@ -478,17 +462,18 @@ void ServiceWorkerWriteToCacheJob::NotifyFinishedCaching(
   // exists.
   if (status.status() == net::URLRequestStatus::SUCCESS &&
       !cache_writer_->did_replace()) {
-    status = net::URLRequestStatus::FromError(kIdenticalScriptError);
+    result = kIdenticalScriptError;
+    status = net::URLRequestStatus::FromError(result);
     version_->SetStartWorkerStatusCode(SERVICE_WORKER_ERROR_EXISTS);
+    version_->script_cache_map()->NotifyFinishedCaching(url_, size, status,
+                                                        std::string());
+  } else {
+    version_->script_cache_map()->NotifyFinishedCaching(url_, size, status,
+                                                        status_message);
   }
 
-  int size = -1;
-  if (status.is_success())
-    size = cache_writer_->bytes_written();
-
-  version_->script_cache_map()->NotifyFinishedCaching(url_, size, status,
-                                                      status_message);
   did_notify_finished_ = true;
+  return result;
 }
 
 scoped_ptr<ServiceWorkerResponseReader>
diff --git a/content/browser/service_worker/service_worker_write_to_cache_job.h b/content/browser/service_worker/service_worker_write_to_cache_job.h
index 9227239..e614912 100644
--- a/content/browser/service_worker/service_worker_write_to_cache_job.h
+++ b/content/browser/service_worker/service_worker_write_to_cache_job.h
@@ -64,6 +64,7 @@ class CONTENT_EXPORT ServiceWorkerWriteToCacheJob
 
   // net::URLRequestJob overrides
   void Start() override;
+  void StartAsync();
   void Kill() override;
   net::LoadState GetLoadState() const override;
   bool GetCharset(std::string* charset) override;
@@ -71,7 +72,7 @@ class CONTENT_EXPORT ServiceWorkerWriteToCacheJob
   void GetResponseInfo(net::HttpResponseInfo* info) override;
   int GetResponseCode() const override;
   void SetExtraRequestHeaders(const net::HttpRequestHeaders& headers) override;
-  bool ReadRawData(net::IOBuffer* buf, int buf_size, int* bytes_read) override;
+  int ReadRawData(net::IOBuffer* buf, int buf_size) override;
 
   const net::HttpResponseInfo* http_info() const;
 
@@ -109,18 +110,22 @@ class CONTENT_EXPORT ServiceWorkerWriteToCacheJob
   bool CheckPathRestriction(net::URLRequest* request);
 
   // Writes network data back to the script cache if needed, and notifies the
-  // script cache of fetch completion at EOF. This function might need to do
-  // asynchronous IO; if so, it signals this through setting the URLRequestJob's
-  // status to IO_PENDING. After this function returns, if the URLRequestJob
-  // isn't IO_PENDING, all of the data in |io_buffer_| has been written back to
-  // the script cache if necessary.
-  void HandleNetData(int bytes_read);
-
-  void NotifyDoneHelper(const net::URLRequestStatus& status,
-                        const std::string& status_message);
-
-  void NotifyFinishedCaching(net::URLRequestStatus status,
-                             const std::string& status_message);
+  // script cache of fetch completion at EOF. This function returns
+  // net::IO_PENDING if the IO is to be completed asynchronously, returns a
+  // negative number that represents a corresponding net error code (other than
+  // net::IO_PENDING) if an error occurred, or returns a non-negative number
+  // that represents the number of network bytes read. If the return value is
+  // non-negative, all of the data in |io_buffer_| has been written back to the
+  // script cache if necessary.
+  int HandleNetData(int bytes_read);
+
+  void NotifyStartErrorHelper(const net::URLRequestStatus& status,
+                              const std::string& status_message);
+
+  // Returns an error code that is passed in through |status| or a new one if an
+  // additional error is found.
+  net::Error NotifyFinishedCaching(net::URLRequestStatus status,
+                                   const std::string& status_message);
 
   scoped_ptr<ServiceWorkerResponseReader> CreateCacheResponseReader();
   scoped_ptr<ServiceWorkerResponseWriter> CreateCacheResponseWriter();
@@ -140,6 +145,7 @@ class CONTENT_EXPORT ServiceWorkerWriteToCacheJob
   bool has_been_killed_;
   bool did_notify_started_;
   bool did_notify_finished_;
+
   base::WeakPtrFactory<ServiceWorkerWriteToCacheJob> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ServiceWorkerWriteToCacheJob);
diff --git a/content/browser/streams/stream_url_request_job.cc b/content/browser/streams/stream_url_request_job.cc
index 0392d10..e26fe35 100644
--- a/content/browser/streams/stream_url_request_job.cc
+++ b/content/browser/streams/stream_url_request_job.cc
@@ -40,8 +40,6 @@ StreamURLRequestJob::~StreamURLRequestJob() {
 }
 
 void StreamURLRequestJob::OnDataAvailable(Stream* stream) {
-  // Clear the IO_PENDING status.
-  SetStatus(net::URLRequestStatus());
   // Do nothing if pending_buffer_ is empty, i.e. there's no ReadRawData()
   // operation waiting for IO completion.
   if (!pending_buffer_.get())
@@ -50,24 +48,22 @@ void StreamURLRequestJob::OnDataAvailable(Stream* stream) {
   // pending_buffer_ is set to the IOBuffer instance provided to ReadRawData()
   // by URLRequestJob.
 
-  int bytes_read;
+  int result = 0;
   switch (stream_->ReadRawData(pending_buffer_.get(), pending_buffer_size_,
-                               &bytes_read)) {
+                               &result)) {
     case Stream::STREAM_HAS_DATA:
-      DCHECK_GT(bytes_read, 0);
+      DCHECK_GT(result, 0);
       break;
     case Stream::STREAM_COMPLETE:
-      // Ensure this. Calling NotifyReadComplete call with 0 signals
-      // completion.
-      bytes_read = 0;
+      // Ensure ReadRawData gives net::OK.
+      DCHECK_EQ(net::OK, result);
       break;
     case Stream::STREAM_EMPTY:
       NOTREACHED();
       break;
     case Stream::STREAM_ABORTED:
       // Handle this as connection reset.
-      NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED,
-                                       net::ERR_CONNECTION_RESET));
+      result = net::ERR_CONNECTION_RESET;
       break;
   }
 
@@ -76,8 +72,9 @@ void StreamURLRequestJob::OnDataAvailable(Stream* stream) {
   pending_buffer_ = NULL;
   pending_buffer_size_ = 0;
 
-  total_bytes_read_ += bytes_read;
-  NotifyReadComplete(bytes_read);
+  if (result > 0)
+    total_bytes_read_ += result;
+  ReadRawDataComplete(result);
 }
 
 // net::URLRequestJob methods.
@@ -94,43 +91,40 @@ void StreamURLRequestJob::Kill() {
   ClearStream();
 }
 
-bool StreamURLRequestJob::ReadRawData(net::IOBuffer* buf,
-                                      int buf_size,
-                                      int* bytes_read) {
+int StreamURLRequestJob::ReadRawData(net::IOBuffer* buf, int buf_size) {
+  // TODO(ellyjones): This is not right. The old code returned true here, but
+  // ReadRawData's old contract was to return true only for synchronous
+  // successes, which had the effect of treating all errors as synchronous EOFs.
+  // See https://crbug.com/508957
   if (request_failed_)
-    return true;
+    return 0;
 
   DCHECK(buf);
-  DCHECK(bytes_read);
   int to_read = buf_size;
   if (max_range_ && to_read) {
     if (to_read + total_bytes_read_ > max_range_)
       to_read = max_range_ - total_bytes_read_;
 
-    if (to_read <= 0) {
-      *bytes_read = 0;
-      return true;
-    }
+    if (to_read == 0)
+      return 0;
   }
 
-  switch (stream_->ReadRawData(buf, to_read, bytes_read)) {
+  int bytes_read = 0;
+  switch (stream_->ReadRawData(buf, to_read, &bytes_read)) {
     case Stream::STREAM_HAS_DATA:
     case Stream::STREAM_COMPLETE:
-      total_bytes_read_ += *bytes_read;
-      return true;
+      total_bytes_read_ += bytes_read;
+      return bytes_read;
     case Stream::STREAM_EMPTY:
       pending_buffer_ = buf;
       pending_buffer_size_ = to_read;
-      SetStatus(net::URLRequestStatus(net::URLRequestStatus::IO_PENDING, 0));
-      return false;
+      return net::ERR_IO_PENDING;
     case Stream::STREAM_ABORTED:
       // Handle this as connection reset.
-      NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED,
-                                       net::ERR_CONNECTION_RESET));
-      return false;
+      return net::ERR_CONNECTION_RESET;
   }
   NOTREACHED();
-  return false;
+  return net::ERR_FAILED;
 }
 
 bool StreamURLRequestJob::GetMimeType(std::string* mime_type) const {
@@ -189,13 +183,8 @@ void StreamURLRequestJob::DidStart() {
 void StreamURLRequestJob::NotifyFailure(int error_code) {
   request_failed_ = true;
 
-  // If we already return the headers on success, we can't change the headers
-  // now. Instead, we just error out.
-  if (headers_set_) {
-    NotifyDone(
-        net::URLRequestStatus(net::URLRequestStatus::FAILED, error_code));
-    return;
-  }
+  // This method can only be called before headers are set.
+  DCHECK(!headers_set_);
 
   // TODO(zork): Share these with BlobURLRequestJob.
   net::HttpStatusCode status_code = net::HTTP_INTERNAL_SERVER_ERROR;
diff --git a/content/browser/streams/stream_url_request_job.h b/content/browser/streams/stream_url_request_job.h
index 05c9551..f22311d 100644
--- a/content/browser/streams/stream_url_request_job.h
+++ b/content/browser/streams/stream_url_request_job.h
@@ -29,7 +29,7 @@ class CONTENT_EXPORT StreamURLRequestJob
   // net::URLRequestJob methods.
   void Start() override;
   void Kill() override;
-  bool ReadRawData(net::IOBuffer* buf, int buf_size, int* bytes_read) override;
+  int ReadRawData(net::IOBuffer* buf, int buf_size) override;
   bool GetMimeType(std::string* mime_type) const override;
   void GetResponseInfo(net::HttpResponseInfo* info) override;
   int GetResponseCode() const override;
diff --git a/content/browser/webui/url_data_manager_backend.cc b/content/browser/webui/url_data_manager_backend.cc
index 44302df..485aff6 100644
--- a/content/browser/webui/url_data_manager_backend.cc
+++ b/content/browser/webui/url_data_manager_backend.cc
@@ -119,7 +119,7 @@ class URLRequestChromeJob : public net::URLRequestJob {
   // net::URLRequestJob implementation.
   void Start() override;
   void Kill() override;
-  bool ReadRawData(net::IOBuffer* buf, int buf_size, int* bytes_read) override;
+  int ReadRawData(net::IOBuffer* buf, int buf_size) override;
   bool GetMimeType(std::string* mime_type) const override;
   int GetResponseCode() const override;
   void GetResponseInfo(net::HttpResponseInfo* info) override;
@@ -190,8 +190,9 @@ class URLRequestChromeJob : public net::URLRequestJob {
   bool RequiresUnsafeEval() const;
 
   // Do the actual copy from data_ (the data we're serving) into |buf|.
-  // Separate from ReadRawData so we can handle async I/O.
-  void CompleteRead(net::IOBuffer* buf, int buf_size, int* bytes_read);
+  // Separate from ReadRawData so we can handle async I/O. Returns the number of
+  // bytes read.
+  int CompleteRead(net::IOBuffer* buf, int buf_size);
 
   // The actual data we're serving.  NULL until it's been fetched.
   scoped_refptr<base::RefCountedMemory> data_;
@@ -336,22 +337,16 @@ void URLRequestChromeJob::MimeTypeAvailable(const std::string& mime_type) {
 void URLRequestChromeJob::DataAvailable(base::RefCountedMemory* bytes) {
   TRACE_EVENT_ASYNC_END0("browser", "DataManager:Request", this);
   if (bytes) {
-    // The request completed, and we have all the data.
-    // Clear any IO pending status.
-    SetStatus(net::URLRequestStatus());
-
     data_ = bytes;
-    int bytes_read;
     if (pending_buf_.get()) {
       CHECK(pending_buf_->data());
-      CompleteRead(pending_buf_.get(), pending_buf_size_, &bytes_read);
+      int result = CompleteRead(pending_buf_.get(), pending_buf_size_);
       pending_buf_ = NULL;
-      NotifyReadComplete(bytes_read);
+      ReadRawDataComplete(result);
     }
   } else {
     // The request failed.
-    NotifyDone(
-        net::URLRequestStatus(net::URLRequestStatus::FAILED, net::ERR_FAILED));
+    ReadRawDataComplete(net::ERR_FAILED);
   }
 }
 
@@ -359,27 +354,21 @@ base::WeakPtr<URLRequestChromeJob> URLRequestChromeJob::AsWeakPtr() {
   return weak_factory_.GetWeakPtr();
 }
 
-bool URLRequestChromeJob::ReadRawData(net::IOBuffer* buf,
-                                      int buf_size,
-                                      int* bytes_read) {
+int URLRequestChromeJob::ReadRawData(net::IOBuffer* buf, int buf_size) {
   if (!data_.get()) {
-    SetStatus(net::URLRequestStatus(net::URLRequestStatus::IO_PENDING, 0));
     DCHECK(!pending_buf_.get());
     CHECK(buf->data());
     pending_buf_ = buf;
     pending_buf_size_ = buf_size;
-    return false;  // Tell the caller we're still waiting for data.
+    return net::ERR_IO_PENDING;
   }
 
   // Otherwise, the data is available.
-  CompleteRead(buf, buf_size, bytes_read);
-  return true;
+  return CompleteRead(buf, buf_size);
 }
 
-void URLRequestChromeJob::CompleteRead(net::IOBuffer* buf,
-                                       int buf_size,
-                                       int* bytes_read) {
-  int remaining = static_cast<int>(data_->size()) - data_offset_;
+int URLRequestChromeJob::CompleteRead(net::IOBuffer* buf, int buf_size) {
+  int remaining = data_->size() - data_offset_;
   if (buf_size > remaining)
     buf_size = remaining;
   if (buf_size > 0) {
@@ -391,7 +380,7 @@ void URLRequestChromeJob::CompleteRead(net::IOBuffer* buf,
     memcpy(buf->data(), data_->front() + data_offset_, buf_size);
     data_offset_ += buf_size;
   }
-  *bytes_read = buf_size;
+  return buf_size;
 }
 
 void URLRequestChromeJob::CheckStoragePartitionMatches(