URLRequestJob::Read can return false but still leave the request in state URLRequestState::SUCCESS

This CL fixes the problem, where URLRequestJob::Read indicates that either IO is pending or an error occurred but still leaves the state of the URLRequest as URLRequestState::SUCCESS.

I have added a DCHECK in lieu of a more complex unit test.

BUG=88085
TEST=no

Review URL: http://codereview.chromium.org/7290009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@91549 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 42 insertions, 25 deletions

diff --git a/net/url_request/url_request.cc b/net/url_request/url_request.cc
index f784c7b..7b55201 100644
--- a/net/url_request/url_request.cc
+++ b/net/url_request/url_request.cc
@@ -501,7 +501,10 @@ bool URLRequest::Read(IOBuffer* dest, int dest_size, int* bytes_read) {
     return true;
   }
 
-  return job_->Read(dest, dest_size, bytes_read);
+  bool rv = job_->Read(dest, dest_size, bytes_read);
+  // If rv is false, the status cannot be success.
+  DCHECK(rv || status_.status() != URLRequestStatus::SUCCESS);
+  return rv;
 }
 
 void URLRequest::StopCaching() {
diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index 52e330e..d36de09 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -323,28 +323,7 @@ void URLRequestHttpJob::NotifyHeadersComplete() {
   URLRequestJob::NotifyHeadersComplete();
 }
 
-void URLRequestHttpJob::NotifyDone(const URLRequestStatus& original_status) {
-  URLRequestStatus status(original_status);
-  // Some servers send the body compressed, but specify the content length as
-  // the uncompressed size.  Although this violates the HTTP spec we want to
-  // support it (as IE and FireFox do), but *only* for an exact match.
-  // See http://crbug.com/79694.
-  if (status.os_error() == net::ERR_CONNECTION_CLOSED) {
-    if (request_ && request_->response_headers()) {
-      int64 expected_length = request_->response_headers()->GetContentLength();
-      VLOG(1) << __FUNCTION__ << "() "
-              << "\"" << request_->url().spec() << "\""
-              << " content-length = " << expected_length
-              << " pre total = " << prefilter_bytes_read()
-              << " post total = " << postfilter_bytes_read();
-      if (postfilter_bytes_read() == expected_length) {
-        // Clear the error.
-        status.set_status(URLRequestStatus::SUCCESS);
-        status.set_os_error(0);
-      }
-    }
-  }
-
+void URLRequestHttpJob::NotifyDone(const URLRequestStatus& status) {
   DoneWithRequest(FINISHED);
   URLRequestJob::NotifyDone(status);
 }
@@ -765,6 +744,9 @@ void URLRequestHttpJob::OnStartCompleted(int result) {
 void URLRequestHttpJob::OnReadCompleted(int result) {
   read_in_progress_ = false;
 
+  if (ShouldFixMismatchedContentLength(result))
+    result = 0;
+
   if (result == 0) {
     NotifyDone(URLRequestStatus());
   } else if (result < 0) {
@@ -1124,6 +1106,28 @@ void URLRequestHttpJob::ContinueDespiteLastError() {
           &URLRequestHttpJob::OnStartCompleted, rv));
 }
 
+bool URLRequestHttpJob::ShouldFixMismatchedContentLength(int rv) const {
+  // Some servers send the body compressed, but specify the content length as
+  // the uncompressed size.  Although this violates the HTTP spec we want to
+  // support it (as IE and FireFox do), but *only* for an exact match.
+  // See http://crbug.com/79694.
+  if (rv == net::ERR_CONNECTION_CLOSED) {
+    if (request_ && request_->response_headers()) {
+      int64 expected_length = request_->response_headers()->GetContentLength();
+      VLOG(1) << __FUNCTION__ << "() "
+              << "\"" << request_->url().spec() << "\""
+              << " content-length = " << expected_length
+              << " pre total = " << prefilter_bytes_read()
+              << " post total = " << postfilter_bytes_read();
+      if (postfilter_bytes_read() == expected_length) {
+        // Clear the error.
+        return true;
+      }
+    }
+  }
+  return false;
+}
+
 bool URLRequestHttpJob::ReadRawData(IOBuffer* buf, int buf_size,
                                     int *bytes_read) {
   DCHECK_NE(buf_size, 0);
@@ -1131,6 +1135,10 @@ bool URLRequestHttpJob::ReadRawData(IOBuffer* buf, int buf_size,
   DCHECK(!read_in_progress_);
 
   int rv = transaction_->Read(buf, buf_size, &read_callback_);
+
+  if (ShouldFixMismatchedContentLength(rv))
+    rv = 0;
+
   if (rv >= 0) {
     *bytes_read = rv;
     if (!rv)
diff --git a/net/url_request/url_request_http_job.h b/net/url_request/url_request_http_job.h
index de29f1a..67632cb 100644
--- a/net/url_request/url_request_http_job.h
+++ b/net/url_request/url_request_http_job.h
@@ -161,6 +161,13 @@ class URLRequestHttpJob : public URLRequestJob {
   void RecordPerfHistograms(CompletionCause reason);
   void DoneWithRequest(CompletionCause reason);
 
+  // Some servers send the body compressed, but specify the content length as
+  // the uncompressed size. If this is the case, we return true in order
+  // to request to work around this non-adherence to the HTTP standard.
+  // |rv| is the standard return value of a read function indicating the number
+  // of bytes read or, if negative, an error code.
+  bool ShouldFixMismatchedContentLength(int rv) const;
+
   base::Time request_creation_time_;
 
   // Data used for statistics gathering. This data is only used for histograms
diff --git a/net/url_request/url_request_job.cc b/net/url_request/url_request_job.cc
index 96db7a1..b664c6c 100644
--- a/net/url_request/url_request_job.cc
+++ b/net/url_request/url_request_job.cc
@@ -416,8 +416,7 @@ bool URLRequestJob::ReadRawData(IOBuffer* buf, int buf_size,
                                 int *bytes_read) {
   DCHECK(bytes_read);
   *bytes_read = 0;
-  NotifyDone(URLRequestStatus());
-  return false;
+  return true;
 }
 
 void URLRequestJob::FilteredDataRead(int bytes_read) {