URLRequestJob: change ReadRawData contract

chrome/browser/chromeos/fileapi/external_file_url_request_job.cc

Index: chrome/browser/chromeos/fileapi/external_file_url_request_job.cc
diff --git a/chrome/browser/chromeos/fileapi/external_file_url_request_job.cc b/chrome/browser/chromeos/fileapi/external_file_url_request_job.cc
index 213211377f192621a57a32919c58159adc29bffe..69e2fb87b4a44a30a13cc4389fbe68967d974a3a 100644
--- a/chrome/browser/chromeos/fileapi/external_file_url_request_job.cc
+++ b/chrome/browser/chromeos/fileapi/external_file_url_request_job.cc
@@ -163,10 +163,14 @@ ExternalFileURLRequestJob::ExternalFileURLRequestJob(
     net::NetworkDelegate* network_delegate)
     : net::URLRequestJob(request, network_delegate),
       profile_id_(profile_id),
+      range_parse_result_(net::OK),
       remaining_bytes_(0),
-      weak_ptr_factory_(this) {
-}
+      weak_ptr_factory_(this) {}
 
+// Extracts headers that this job cares about from the supplied request headers.
+// Currently this job only cares about the Range header. Note that validation is
+// deferred to Start, because NotifyStartError is not legal to call since the
+// job has not started.

[mmenke, 2015/10/27 19:41:07]

See comment about this in another file.

[xunjieli, 2015/10/27 21:24:06]

Done.

 void ExternalFileURLRequestJob::SetExtraRequestHeaders(
     const net::HttpRequestHeaders& headers) {
   std::string range_header;
@@ -177,9 +181,9 @@ void ExternalFileURLRequestJob::SetExtraRequestHeaders(
         ranges.size() == 1) {
       byte_range_ = ranges[0];
     } else {
-      // Failed to parse Range: header, so notify the error.
-      NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED,
-                                       net::ERR_REQUEST_RANGE_NOT_SATISFIABLE));
+      // Failed to parse Range: header, so save the error whih will be reported
+      // in Start.
+      range_parse_result_ = net::ERR_REQUEST_RANGE_NOT_SATISFIABLE;
     }
   }
 }
@@ -189,6 +193,12 @@ void ExternalFileURLRequestJob::Start() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(!stream_reader_);
 
+  if (range_parse_result_ != net::OK) {
+    NotifyStartError(net::URLRequestStatus(net::URLRequestStatus::FAILED,
+                                           range_parse_result_));

[mmenke, 2015/10/27 19:41:07]

Not really safe to call NotifyStartError synchronously on start (NotifyDone was
safe).  :(

[xunjieli, 2015/10/27 21:24:06]

Done. I created a helper method to be invoked asynchronously. Does it look okay?

+    return;
+  }
+
   // We only support GET request.
   if (request()->method() != "GET") {
     LOG(WARNING) << "Failed to start request: " << request()->method()
@@ -326,38 +336,23 @@ bool ExternalFileURLRequestJob::IsRedirectResponse(GURL* location,
   return true;
 }
 
-bool ExternalFileURLRequestJob::ReadRawData(net::IOBuffer* buf,
-                                            int buf_size,
-                                            int* bytes_read) {
+int ExternalFileURLRequestJob::ReadRawData(net::IOBuffer* buf, int buf_size) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(stream_reader_);
 
-  if (remaining_bytes_ == 0) {
-    *bytes_read = 0;
-    return true;
-  }
+  if (remaining_bytes_ == 0)
+    return 0;
 
   const int result = stream_reader_->Read(
-      buf,
-      std::min<int64>(buf_size, remaining_bytes_),
+      buf, std::min<int64>(buf_size, remaining_bytes_),
       base::Bind(&ExternalFileURLRequestJob::OnReadCompleted,
                  weak_ptr_factory_.GetWeakPtr()));
 
-  if (result == net::ERR_IO_PENDING) {
-    // The data is not yet available.
-    SetStatus(net::URLRequestStatus(net::URLRequestStatus::IO_PENDING, 0));
-    return false;
-  }
-  if (result < 0) {
-    // An error occurs.
-    NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED, result));
-    return false;
-  }
+  if (result < 0)
+    return result;
 
-  // Reading has been finished immediately.
-  *bytes_read = result;
   remaining_bytes_ -= result;
-  return true;
+  return result;
 }
 
 ExternalFileURLRequestJob::~ExternalFileURLRequestJob() {
@@ -366,15 +361,10 @@ ExternalFileURLRequestJob::~ExternalFileURLRequestJob() {
 void ExternalFileURLRequestJob::OnReadCompleted(int read_result) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  if (read_result < 0) {
-    DCHECK_NE(read_result, net::ERR_IO_PENDING);
-    NotifyDone(
-        net::URLRequestStatus(net::URLRequestStatus::FAILED, read_result));
-  }
+  if (read_result > 0)
+    remaining_bytes_ -= read_result;
 
-  remaining_bytes_ -= read_result;
-  SetStatus(net::URLRequestStatus());  // Clear the IO_PENDING status.
-  NotifyReadComplete(read_result);
+  ReadRawDataComplete(read_result);
 }
 
 }  // namespace chromeos