URLRequestJob: change ReadRawData contract

storage/browser/blob/blob_url_request_job.cc

Index: storage/browser/blob/blob_url_request_job.cc
diff --git a/storage/browser/blob/blob_url_request_job.cc b/storage/browser/blob/blob_url_request_job.cc
index 6ec2fe57ae7ca706b53171c9d8e1f8a24f1d3d3f..657098ddfc3a62745c4e27c9b004db7810ebab4b 100644
--- a/storage/browser/blob/blob_url_request_job.cc
+++ b/storage/browser/blob/blob_url_request_job.cc
@@ -75,41 +75,37 @@ void BlobURLRequestJob::Kill() {
   weak_factory_.InvalidateWeakPtrs();
 }
 
-bool BlobURLRequestJob::ReadRawData(net::IOBuffer* dest,
-                                    int dest_size,
-                                    int* bytes_read) {
+int BlobURLRequestJob::ReadRawData(net::IOBuffer* dest, int dest_size) {
   TRACE_EVENT_ASYNC_BEGIN1("Blob", "BlobRequest::ReadRawData", this, "uuid",
                            blob_handle_ ? blob_handle_->uuid() : "NotFound");
   DCHECK_NE(dest_size, 0);
-  DCHECK(bytes_read);
 
-  // Bail out immediately if we encounter an error.
-  if (error_) {
-    *bytes_read = 0;
-    return true;
-  }
+  // Bail out immediately if we encounter an error. This happens if a previous
+  // ReadRawData signalled an error to its caller but the caller called
+  // ReadRawData again anyway.
+  if (error_)
+    return 0;
 
+  int bytes_read = 0;
   BlobReader::Status read_status =
-      blob_reader_->Read(dest, dest_size, bytes_read,
+      blob_reader_->Read(dest, dest_size, &bytes_read,
                          base::Bind(&BlobURLRequestJob::DidReadRawData,
                                     weak_factory_.GetWeakPtr()));
 
   switch (read_status) {
     case BlobReader::Status::NET_ERROR:
-      NotifyFailure(blob_reader_->net_error());
       TRACE_EVENT_ASYNC_END1("Blob", "BlobRequest::ReadRawData", this, "uuid",
                              blob_handle_ ? blob_handle_->uuid() : "NotFound");
-      return false;
+      return 0;
     case BlobReader::Status::IO_PENDING:
-      SetStatus(net::URLRequestStatus(net::URLRequestStatus::IO_PENDING, 0));
-      return false;
+      return net::ERR_IO_PENDING;
     case BlobReader::Status::DONE:
       TRACE_EVENT_ASYNC_END1("Blob", "BlobRequest::ReadRawData", this, "uuid",
                              blob_handle_ ? blob_handle_->uuid() : "NotFound");
-      return true;
+      return bytes_read;
   }
   NOTREACHED();
-  return true;
+  return 0;
 }
 
 bool BlobURLRequestJob::GetMimeType(std::string* mime_type) const {
@@ -223,12 +219,11 @@ void BlobURLRequestJob::DidReadRawData(int result) {
   TRACE_EVENT_ASYNC_END1("Blob", "BlobRequest::ReadRawData", this, "uuid",
                          blob_handle_ ? blob_handle_->uuid() : "NotFound");
   if (result < 0) {

[Randy Smith (Not in Mondays), 2015/10/22 20:38:45]

I don't think this conditional is needed anymore; you can just call
ReadRawDataComplete(result); unilaterally, right?

[xunjieli, 2015/10/23 13:43:08]

Done.

-    NotifyFailure(result);
+    ReadRawDataComplete(result);
     return;
   }
   // Clear the IO_PENDING status

[Randy Smith (Not in Mondays), 2015/10/22 20:38:45]

nit: I think this comment is out of date now?

[xunjieli, 2015/10/23 13:43:08]

Done.

-  SetStatus(net::URLRequestStatus());
-  NotifyReadComplete(result);
+  ReadRawDataComplete(result);
 }
 
 void BlobURLRequestJob::NotifyFailure(int error_code) {
@@ -236,11 +231,7 @@ void BlobURLRequestJob::NotifyFailure(int error_code) {
 
   // If we already return the headers on success, we can't change the headers
   // now. Instead, we just error out.
-  if (response_info_) {
-    NotifyDone(
-        net::URLRequestStatus(net::URLRequestStatus::FAILED, error_code));
-    return;
-  }
+  DCHECK(!response_info_) << "Cannot NotifyFailure after headers.";
 
   net::HttpStatusCode status_code = net::HTTP_INTERNAL_SERVER_ERROR;
   switch (error_code) {