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}

2 files changed, 39 insertions, 31 deletions

diff --git a/android_webview/browser/net/android_stream_reader_url_request_job.cc b/android_webview/browser/net/android_stream_reader_url_request_job.cc
index dc288fa..173bdbb 100644
--- a/android_webview/browser/net/android_stream_reader_url_request_job.cc
+++ b/android_webview/browser/net/android_stream_reader_url_request_job.cc
@@ -21,6 +21,7 @@
 #include "content/public/browser/browser_thread.h"
 #include "net/base/io_buffer.h"
 #include "net/base/mime_util.h"
+#include "net/base/net_errors.h"
 #include "net/base/net_util.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_response_info.h"
@@ -89,7 +90,6 @@ AndroidStreamReaderURLRequestJob::AndroidStreamReaderURLRequestJob(
     net::NetworkDelegate* network_delegate,
     scoped_ptr<Delegate> delegate)
     : URLRequestJob(request, network_delegate),
-      range_parse_result_(net::OK),
       delegate_(delegate.Pass()),
       weak_factory_(this) {
   DCHECK(delegate_);
@@ -101,7 +101,6 @@ AndroidStreamReaderURLRequestJob::AndroidStreamReaderURLRequestJob(
     scoped_ptr<DelegateObtainer> delegate_obtainer,
     bool)
     : URLRequestJob(request, network_delegate),
-      range_parse_result_(net::OK),
       delegate_obtainer_(delegate_obtainer.Pass()),
       weak_factory_(this) {
   DCHECK(delegate_obtainer_);
@@ -142,10 +141,7 @@ void AndroidStreamReaderURLRequestJob::Start() {
         base::Bind(&AndroidStreamReaderURLRequestJob::DelegateObtained,
                    weak_factory_.GetWeakPtr()));
   } else {
-    // Run DoStart asynchronously to avoid re-entering the delegate.
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::Bind(&AndroidStreamReaderURLRequestJob::DoStart,
-                              weak_factory_.GetWeakPtr()));
+    DoStart();
   }
 }
 
@@ -206,29 +202,45 @@ void AndroidStreamReaderURLRequestJob::OnReaderSeekCompleted(int result) {
     set_expected_content_size(result);
     HeadersComplete(kHTTPOk, kHTTPOkText);
   } else {
-    NotifyStartError(
-        net::URLRequestStatus(net::URLRequestStatus::FAILED, result));
+    NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED, result));
   }
 }
 
 void AndroidStreamReaderURLRequestJob::OnReaderReadCompleted(int result) {
   DCHECK(thread_checker_.CalledOnValidThread());
-
-  ReadRawDataComplete(result);
+  // The URLRequest API contract requires that:
+  // * NotifyDone be called once, to set the status code, indicate the job is
+  //   finished (there will be no further IO),
+  // * NotifyReadComplete be called if false is returned from ReadRawData to
+  //   indicate that the IOBuffer will not be used by the job anymore.
+  // There might be multiple calls to ReadRawData (and thus multiple calls to
+  // NotifyReadComplete), which is why NotifyDone is called only on errors
+  // (result < 0) and end of data (result == 0).
+  if (result == 0) {
+    NotifyDone(net::URLRequestStatus());
+  } else if (result < 0) {
+    NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED, result));
+  } else {
+    // Clear the IO_PENDING status.
+    SetStatus(net::URLRequestStatus());
+  }
+  NotifyReadComplete(result);
 }
 
 base::TaskRunner* AndroidStreamReaderURLRequestJob::GetWorkerThreadRunner() {
   return static_cast<base::TaskRunner*>(BrowserThread::GetBlockingPool());
 }
 
-int AndroidStreamReaderURLRequestJob::ReadRawData(net::IOBuffer* dest,
-                                                  int dest_size) {
+bool AndroidStreamReaderURLRequestJob::ReadRawData(net::IOBuffer* dest,
+                                                   int dest_size,
+                                                   int* bytes_read) {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (!input_stream_reader_wrapper_.get()) {
     // This will happen if opening the InputStream fails in which case the
     // error is communicated by setting the HTTP response status header rather
     // than failing the request during the header fetch phase.
-    return 0;
+    *bytes_read = 0;
+    return true;
   }
 
   PostTaskAndReplyWithResult(
@@ -239,7 +251,9 @@ int AndroidStreamReaderURLRequestJob::ReadRawData(net::IOBuffer* dest,
       base::Bind(&AndroidStreamReaderURLRequestJob::OnReaderReadCompleted,
                  weak_factory_.GetWeakPtr()));
 
-  return net::ERR_IO_PENDING;
+  SetStatus(net::URLRequestStatus(net::URLRequestStatus::IO_PENDING,
+                                  net::ERR_IO_PENDING));
+  return false;
 }
 
 bool AndroidStreamReaderURLRequestJob::GetMimeType(
@@ -289,11 +303,6 @@ void AndroidStreamReaderURLRequestJob::DelegateObtained(
 
 void AndroidStreamReaderURLRequestJob::DoStart() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  if (range_parse_result_ != net::OK) {
-    NotifyStartError(net::URLRequestStatus(net::URLRequestStatus::FAILED,
-                                           range_parse_result_));
-    return;
-  }
   // Start reading asynchronously so that all error reporting and data
   // callbacks happen as they would for network requests.
   SetStatus(net::URLRequestStatus(net::URLRequestStatus::IO_PENDING,
@@ -374,18 +383,19 @@ void AndroidStreamReaderURLRequestJob::SetExtraRequestHeaders(
     const net::HttpRequestHeaders& headers) {
   std::string range_header;
   if (headers.GetHeader(net::HttpRequestHeaders::kRange, &range_header)) {
-    // This job only cares about the Range header so that we know how many bytes
-    // in the stream to skip and how many to read after that. Note that
-    // validation is deferred to DoStart(), because NotifyStartError() is not
-    // legal to call since the job has not started.
+    // We only extract the "Range" header so that we know how many bytes in the
+    // stream to skip and how many to read after that.
     std::vector<net::HttpByteRange> ranges;
     if (net::HttpUtil::ParseRangeHeader(range_header, &ranges)) {
-      if (ranges.size() == 1)
+      if (ranges.size() == 1) {
         byte_range_ = ranges[0];
-    } else {
-      // We don't support multiple range requests in one single URL request,
-      // because we need to do multipart encoding here.
-      range_parse_result_ = net::ERR_REQUEST_RANGE_NOT_SATISFIABLE;
+      } else {
+        // We don't support multiple range requests in one single URL request,
+        // because we need to do multipart encoding here.
+        NotifyDone(
+            net::URLRequestStatus(net::URLRequestStatus::FAILED,
+                                  net::ERR_REQUEST_RANGE_NOT_SATISFIABLE));
+      }
     }
   }
 }
diff --git a/android_webview/browser/net/android_stream_reader_url_request_job.h b/android_webview/browser/net/android_stream_reader_url_request_job.h
index 645be13..e21d765 100644
--- a/android_webview/browser/net/android_stream_reader_url_request_job.h
+++ b/android_webview/browser/net/android_stream_reader_url_request_job.h
@@ -15,7 +15,6 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/threading/thread_checker.h"
-#include "net/base/net_errors.h"
 #include "net/http/http_byte_range.h"
 #include "net/url_request/url_request_job.h"
 
@@ -97,7 +96,7 @@ class AndroidStreamReaderURLRequestJob : public net::URLRequestJob {
   // URLRequestJob:
   void Start() override;
   void Kill() override;
-  int ReadRawData(net::IOBuffer* buf, int buf_size) override;
+  bool ReadRawData(net::IOBuffer* buf, int buf_size, int* bytes_read) override;
   void SetExtraRequestHeaders(const net::HttpRequestHeaders& headers) override;
   bool GetMimeType(std::string* mime_type) const override;
   bool GetCharset(std::string* charset) override;
@@ -132,7 +131,6 @@ class AndroidStreamReaderURLRequestJob : public net::URLRequestJob {
   void OnReaderReadCompleted(int bytes_read);
 
   net::HttpByteRange byte_range_;
-  net::Error range_parse_result_;
   scoped_ptr<net::HttpResponseInfo> response_info_;
   scoped_ptr<Delegate> delegate_;
   scoped_ptr<DelegateObtainer> delegate_obtainer_;