Reland: URLRequestJob: change ReadRawData contract

content/browser/streams/stream_url_request_job.cc

Index: content/browser/streams/stream_url_request_job.cc
diff --git a/content/browser/streams/stream_url_request_job.cc b/content/browser/streams/stream_url_request_job.cc
index 0392d108fbf935868d7b225d54228a8bbeaae822..e26fe35a41a66b90eaae5ee414d5b06a1d3761f6 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;