URLRequestJob: change ReadRawData contract

android_webview/browser/net/android_stream_reader_url_request_job.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "android_webview/browser/net/android_stream_reader_url_request_job.h"
 
 #include <string>
 
 #include "android_webview/browser/input_stream.h"
 #include "android_webview/browser/net/input_stream_reader.h"
@@ skipping 72 matching lines @@
   scoped_ptr<InputStreamReader> input_stream_reader_;
 
   DISALLOW_COPY_AND_ASSIGN(InputStreamReaderWrapper);
 };
 
 AndroidStreamReaderURLRequestJob::AndroidStreamReaderURLRequestJob(
     net::URLRequest* request,
     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_);
 }
 
 AndroidStreamReaderURLRequestJob::AndroidStreamReaderURLRequestJob(
     net::URLRequest* request,
     net::NetworkDelegate* network_delegate,
     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_);
 }
 
 AndroidStreamReaderURLRequestJob::~AndroidStreamReaderURLRequestJob() {
 }
 
 namespace {
 
@@ skipping 81 matching lines @@
 }
 
 void AndroidStreamReaderURLRequestJob::OnReaderSeekCompleted(int result) {
   DCHECK(thread_checker_.CalledOnValidThread());
   // Clear the IO_PENDING status set in Start().
   SetStatus(net::URLRequestStatus());
   if (result >= 0) {
     set_expected_content_size(result);
     HeadersComplete(kHTTPOk, kHTTPOkText);
   } else {
-    NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED, result));
+    NotifyStartError(
+        net::URLRequestStatus(net::URLRequestStatus::FAILED, result));
   }
 }
 
 void AndroidStreamReaderURLRequestJob::OnReaderReadCompleted(int result) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  // 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);
+
+  ReadRawDataComplete(result);
 }
 
 base::TaskRunner* AndroidStreamReaderURLRequestJob::GetWorkerThreadRunner() {
   return static_cast<base::TaskRunner*>(BrowserThread::GetBlockingPool());
 }
 
-bool AndroidStreamReaderURLRequestJob::ReadRawData(net::IOBuffer* dest,
-                                                   int dest_size,
-                                                   int* bytes_read) {
+int AndroidStreamReaderURLRequestJob::ReadRawData(net::IOBuffer* dest,
+                                                  int dest_size) {
   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.
-    *bytes_read = 0;
-    return true;
+    return 0;
   }
 
   PostTaskAndReplyWithResult(
-      GetWorkerThreadRunner(),
-      FROM_HERE,
+      GetWorkerThreadRunner(), FROM_HERE,
       base::Bind(&InputStreamReaderWrapper::ReadRawData,
-                 input_stream_reader_wrapper_,
-                 make_scoped_refptr(dest),
+                 input_stream_reader_wrapper_, make_scoped_refptr(dest),
                  dest_size),
       base::Bind(&AndroidStreamReaderURLRequestJob::OnReaderReadCompleted,
                  weak_factory_.GetWeakPtr()));
 
-  SetStatus(net::URLRequestStatus(net::URLRequestStatus::IO_PENDING,
-                                  net::ERR_IO_PENDING));
-  return false;
+  return net::ERR_IO_PENDING;
 }
 
 bool AndroidStreamReaderURLRequestJob::GetMimeType(
     std::string* mime_type) const {
   DCHECK(thread_checker_.CalledOnValidThread());
   JNIEnv* env = AttachCurrentThread();
   DCHECK(env);
 
   if (!input_stream_reader_wrapper_.get())
     return false;
@@ skipping 29 matching lines @@
   if (delegate) {
     delegate_.swap(delegate);
     DoStart();
   } else {
     NotifyRestartRequired();
   }
 }
 
 void AndroidStreamReaderURLRequestJob::DoStart() {
   DCHECK(thread_checker_.CalledOnValidThread());
+  if (range_parse_result_ != net::OK) {
+    NotifyStartError(net::URLRequestStatus(net::URLRequestStatus::FAILED,
+                                           range_parse_result_));

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

It's not really safe to call NotifyStartError synchronously from Start(),
although we don't currently have any protection against it.  Suggest just
posting a task to run DoStart on construction, using a weak pointer, instead of
calling DoStart synchronously.

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

Done. Good to know. Thanks!

+    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,
                                   net::ERR_IO_PENDING));
 
   // This could be done in the InputStreamReader but would force more
   // complex synchronization in the delegate.
   GetWorkerThreadRunner()->PostTask(
       FROM_HERE,
       base::Bind(
@@ skipping 56 matching lines @@
     return response_info_->headers->response_code();
   return URLRequestJob::GetResponseCode();
 }
 
 void AndroidStreamReaderURLRequestJob::GetResponseInfo(
     net::HttpResponseInfo* info) {
   if (response_info_)
     *info = *response_info_;
 }
 
+// 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 DoStart(), because NotifyStartError is not legal to call since
+// the job has not started.

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

See comment in other file (*file_dir_job.cc?)

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

Done.

 void AndroidStreamReaderURLRequestJob::SetExtraRequestHeaders(
     const net::HttpRequestHeaders& headers) {
   std::string range_header;
   if (headers.GetHeader(net::HttpRequestHeaders::kRange, &range_header)) {
     // 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.
-        NotifyDone(net::URLRequestStatus(
-            net::URLRequestStatus::FAILED,
-            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.
+      // Saves the failure which will be reported in DoStart().
+      range_parse_result_ = net::ERR_REQUEST_RANGE_NOT_SATISFIABLE;
     }
   }
 }
 
 }  // namespace android_webview