Fixing possible read on URLRequest when there is no more data.

chrome/browser/predictors/resource_prefetcher.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 <iterator>
 
 #include "chrome/browser/predictors/resource_prefetcher.h"
 
 #include "base/stl_util.h"
 #include "content/public/browser/browser_thread.h"
@@ skipping 151 matching lines @@
 }
 
 void ResourcePrefetcher::ReadFullResponse(net::URLRequest* request) {
   bool status = true;
   while (status) {
     int bytes_read = 0;
     scoped_refptr<net::IOBuffer> buffer(new net::IOBuffer(
         kResourceBufferSizeBytes));
     status = request->Read(buffer, kResourceBufferSizeBytes, &bytes_read);
 
-    if (status) {
-      if (request->status().error()) {
-        FinishRequest(request, Request::PREFETCH_STATUS_FAILED);
-        return;
-      } else if (bytes_read == 0) {
-        if (request->was_cached())
-          FinishRequest(request, Request::PREFETCH_STATUS_FROM_CACHE);
-        else
-          FinishRequest(request, Request::PREFETCH_STATUS_FROM_NETWORK);
-        return;
-      }
+    if (request->status().error()) {

[rvargas (doing something else), 2013/01/04 02:02:40]

The contract is not to inspect status() regardless of the return value. The
first thing to check is the return value of the function (as the old code does).

[Shishir, 2013/01/04 02:21:14]

From the comment on the Read function:
// If a read error occurs, Read returns false and the request->status
// will be set to an error.

So if read returns false, its either because its in async mode or there is an
error. Earlier we were checking error when status was true which would never
happen. This condition will rightly bail out in the case where there is an error
bit set and the read returns false.

+      FinishRequest(request, Request::PREFETCH_STATUS_FAILED);
+      return;
+    }
+
+    if (status && bytes_read == 0) {  // When bytes_read == 0, no more data.
+      if (request->was_cached())
+        FinishRequest(request, Request::PREFETCH_STATUS_FROM_CACHE);
+      else
+        FinishRequest(request, Request::PREFETCH_STATUS_FROM_NETWORK);
+      return;
     }
   }
 }
 
 void ResourcePrefetcher::OnReceivedRedirect(net::URLRequest* request,
                                             const GURL& new_url,
                                             bool* defer_redirect) {
   FinishRequest(request, Request::PREFETCH_STATUS_REDIRECTED);
 }
 
@@ skipping 23 matching lines @@
   ReadFullResponse(request);
 }
 
 void ResourcePrefetcher::OnReadCompleted(net::URLRequest* request,
                                          int bytes_read) {
   if (request->status().error()) {
     FinishRequest(request, Request::PREFETCH_STATUS_FAILED);
     return;
   }
 
+  // If the bytes_read is == 0, there is no more data to read.
+  if (bytes_read == 0) {
+    if (request->was_cached())
+      FinishRequest(request, Request::PREFETCH_STATUS_FROM_CACHE);
+    else
+      FinishRequest(request, Request::PREFETCH_STATUS_FROM_NETWORK);
+    return;
+  }
+
   ReadFullResponse(request);
 }
 
 }  // namespace predictors