Revert "Reland: URLRequestJob: change ReadRawData contract"

This reverts commit d77911ac82186d65a8f11555a7a6b1678c769ba2.

The previous CL caused a top crasher on Canary.
Reverting this CL now since fixing the crash isn't
straightforward.

TBR=michaeln@chromium.org,mnaganov@chromium.org,skyostil@chromium.org,eugenebut@chromium.org,davidben@chromium.org,falken@chromium.org,mtomasz@chromium.org, sky@chromium.org,jianli@chromium.org,zork@chromium.org,mmenke@chromium.org,rdsmith@chromium.org

BUG=558224
BUG=553300
BUG=474859
BUG=329902

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

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

6 files changed, 58 insertions, 42 deletions

diff --git a/net/test/url_request/url_request_failed_job.cc b/net/test/url_request/url_request_failed_job.cc
index d7c479b..e4ac6a6 100644
--- a/net/test/url_request/url_request_failed_job.cc
+++ b/net/test/url_request/url_request_failed_job.cc
@@ -97,20 +97,40 @@ URLRequestFailedJob::URLRequestFailedJob(URLRequest* request,
 }
 
 void URLRequestFailedJob::Start() {
-  base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE,
-      base::Bind(&URLRequestFailedJob::StartAsync, weak_factory_.GetWeakPtr()));
+  if (phase_ == START) {
+    if (net_error_ != ERR_IO_PENDING) {
+      NotifyStartError(URLRequestStatus(URLRequestStatus::FAILED, net_error_));
+      return;
+    }
+    SetStatus(URLRequestStatus(URLRequestStatus::IO_PENDING, 0));
+    return;
+  }
+  response_info_.headers = new net::HttpResponseHeaders("HTTP/1.1 200 OK");
+  NotifyHeadersComplete();
 }
 
-int URLRequestFailedJob::ReadRawData(IOBuffer* buf, int buf_size) {
+bool URLRequestFailedJob::ReadRawData(IOBuffer* buf,
+                                      int buf_size,
+                                      int* bytes_read) {
   CHECK(phase_ == READ_SYNC || phase_ == READ_ASYNC);
-  if (net_error_ == ERR_IO_PENDING || phase_ == READ_SYNC)
-    return net_error_;
+  if (net_error_ != ERR_IO_PENDING && phase_ == READ_SYNC) {
+    NotifyDone(URLRequestStatus(URLRequestStatus::FAILED, net_error_));
+    return false;
+  }
+
+  SetStatus(URLRequestStatus(URLRequestStatus::IO_PENDING, 0));
+
+  if (net_error_ == ERR_IO_PENDING)
+    return false;
+
+  DCHECK_EQ(READ_ASYNC, phase_);
+  DCHECK_NE(ERR_IO_PENDING, net_error_);
 
   base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::Bind(&URLRequestFailedJob::ReadRawDataComplete,
-                            weak_factory_.GetWeakPtr(), net_error_));
-  return ERR_IO_PENDING;
+      FROM_HERE,
+      base::Bind(&URLRequestFailedJob::NotifyDone, weak_factory_.GetWeakPtr(),
+                 URLRequestStatus(URLRequestStatus::FAILED, net_error_)));
+  return false;
 }
 
 int URLRequestFailedJob::GetResponseCode() const {
@@ -175,17 +195,4 @@ GURL URLRequestFailedJob::GetMockHttpsUrlForHostname(
 URLRequestFailedJob::~URLRequestFailedJob() {
 }
 
-void URLRequestFailedJob::StartAsync() {
-  if (phase_ == START) {
-    if (net_error_ != ERR_IO_PENDING) {
-      NotifyStartError(URLRequestStatus(URLRequestStatus::FAILED, net_error_));
-      return;
-    }
-    SetStatus(URLRequestStatus(URLRequestStatus::IO_PENDING, 0));
-    return;
-  }
-  response_info_.headers = new net::HttpResponseHeaders("HTTP/1.1 200 OK");
-  NotifyHeadersComplete();
-}
-
 }  // namespace net
diff --git a/net/test/url_request/url_request_failed_job.h b/net/test/url_request/url_request_failed_job.h
index 45b1911..0413111 100644
--- a/net/test/url_request/url_request_failed_job.h
+++ b/net/test/url_request/url_request_failed_job.h
@@ -39,7 +39,7 @@ class URLRequestFailedJob : public URLRequestJob {
 
   // URLRequestJob implementation:
   void Start() override;
-  int ReadRawData(IOBuffer* buf, int buf_size) override;
+  bool ReadRawData(IOBuffer* buf, int buf_size, int* bytes_read) override;
   int GetResponseCode() const override;
   void GetResponseInfo(HttpResponseInfo* info) override;
 
@@ -71,7 +71,6 @@ class URLRequestFailedJob : public URLRequestJob {
 
  protected:
   ~URLRequestFailedJob() override;
-  void StartAsync();
 
  private:
   HttpResponseInfo response_info_;
diff --git a/net/test/url_request/url_request_mock_data_job.cc b/net/test/url_request/url_request_mock_data_job.cc
index b9ef385..9549242 100644
--- a/net/test/url_request/url_request_mock_data_job.cc
+++ b/net/test/url_request/url_request_mock_data_job.cc
@@ -104,12 +104,15 @@ void URLRequestMockDataJob::Start() {
 URLRequestMockDataJob::~URLRequestMockDataJob() {
 }
 
-int URLRequestMockDataJob::ReadRawData(IOBuffer* buf, int buf_size) {
-  int bytes_read =
-      std::min(static_cast<size_t>(buf_size), data_.length() - data_offset_);
-  memcpy(buf->data(), data_.c_str() + data_offset_, bytes_read);
-  data_offset_ += bytes_read;
-  return bytes_read;
+bool URLRequestMockDataJob::ReadRawData(IOBuffer* buf,
+                                        int buf_size,
+                                        int* bytes_read) {
+  DCHECK(bytes_read);
+  *bytes_read = static_cast<int>(
+      std::min(static_cast<size_t>(buf_size), data_.length() - data_offset_));
+  memcpy(buf->data(), data_.c_str() + data_offset_, *bytes_read);
+  data_offset_ += *bytes_read;
+  return true;
 }
 
 int URLRequestMockDataJob::GetResponseCode() const {
diff --git a/net/test/url_request/url_request_mock_data_job.h b/net/test/url_request/url_request_mock_data_job.h
index 14ad665..3d84e37 100644
--- a/net/test/url_request/url_request_mock_data_job.h
+++ b/net/test/url_request/url_request_mock_data_job.h
@@ -24,7 +24,7 @@ class URLRequestMockDataJob : public URLRequestJob {
                         int data_repeat_count);
 
   void Start() override;
-  int ReadRawData(IOBuffer* buf, int buf_size) override;
+  bool ReadRawData(IOBuffer* buf, int buf_size, int* bytes_read) override;
   int GetResponseCode() const override;
   void GetResponseInfo(HttpResponseInfo* info) override;
 
diff --git a/net/test/url_request/url_request_slow_download_job.cc b/net/test/url_request/url_request_slow_download_job.cc
index 52fd71c..f344feb 100644
--- a/net/test/url_request/url_request_slow_download_job.cc
+++ b/net/test/url_request/url_request_slow_download_job.cc
@@ -179,34 +179,39 @@ URLRequestSlowDownloadJob::FillBufferHelper(IOBuffer* buf,
   return REQUEST_COMPLETE;
 }
 
-int URLRequestSlowDownloadJob::ReadRawData(IOBuffer* buf, int buf_size) {
+bool URLRequestSlowDownloadJob::ReadRawData(IOBuffer* buf,
+                                            int buf_size,
+                                            int* bytes_read) {
   if (base::LowerCaseEqualsASCII(kFinishDownloadUrl,
                                  request_->url().spec().c_str()) ||
       base::LowerCaseEqualsASCII(kErrorDownloadUrl,
                                  request_->url().spec().c_str())) {
     VLOG(10) << __FUNCTION__ << " called w/ kFinish/ErrorDownloadUrl.";
-    return 0;
+    *bytes_read = 0;
+    return true;
   }
 
   VLOG(10) << __FUNCTION__ << " called at position " << bytes_already_sent_
            << " in the stream.";
-  int bytes_read = 0;
-  ReadStatus status = FillBufferHelper(buf, buf_size, &bytes_read);
+  ReadStatus status = FillBufferHelper(buf, buf_size, bytes_read);
   switch (status) {
     case BUFFER_FILLED:
-    case REQUEST_COMPLETE:
-      return bytes_read;
+      return true;
     case REQUEST_BLOCKED:
       buffer_ = buf;
       buffer_size_ = buf_size;
+      SetStatus(URLRequestStatus(URLRequestStatus::IO_PENDING, 0));
       base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
           FROM_HERE, base::Bind(&URLRequestSlowDownloadJob::CheckDoneStatus,
                                 weak_factory_.GetWeakPtr()),
           base::TimeDelta::FromMilliseconds(100));
-      return ERR_IO_PENDING;
+      return false;
+    case REQUEST_COMPLETE:
+      *bytes_read = 0;
+      return true;
   }
   NOTREACHED();
-  return OK;
+  return true;
 }
 
 void URLRequestSlowDownloadJob::CheckDoneStatus() {
@@ -218,10 +223,12 @@ void URLRequestSlowDownloadJob::CheckDoneStatus() {
         FillBufferHelper(buffer_.get(), buffer_size_, &bytes_written);
     DCHECK_EQ(BUFFER_FILLED, status);
     buffer_ = NULL;  // Release the reference.
-    ReadRawDataComplete(bytes_written);
+    SetStatus(URLRequestStatus());
+    NotifyReadComplete(bytes_written);
   } else if (should_error_download_) {
     VLOG(10) << __FUNCTION__ << " called w/ should_finish_ownload_ set.";
-    ReadRawDataComplete(ERR_CONNECTION_RESET);
+    NotifyDone(
+        URLRequestStatus(URLRequestStatus::FAILED, ERR_CONNECTION_RESET));
   } else {
     base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
         FROM_HERE, base::Bind(&URLRequestSlowDownloadJob::CheckDoneStatus,
diff --git a/net/test/url_request/url_request_slow_download_job.h b/net/test/url_request/url_request_slow_download_job.h
index fcd7f661..115a6ac 100644
--- a/net/test/url_request/url_request_slow_download_job.h
+++ b/net/test/url_request/url_request_slow_download_job.h
@@ -37,7 +37,7 @@ class URLRequestSlowDownloadJob : public URLRequestJob {
   void Start() override;
   bool GetMimeType(std::string* mime_type) const override;
   void GetResponseInfo(HttpResponseInfo* info) override;
-  int ReadRawData(IOBuffer* buf, int buf_size) override;
+  bool ReadRawData(IOBuffer* buf, int buf_size, int* bytes_read) override;
 
   // Returns the current number of URLRequestSlowDownloadJobs that have
   // not yet completed.