Merge 33580 - Http cache: Don't access entry_ if an error switches

the transaction to mode NONE and deletes the entry_.

BUG=28994
TEST=unittests


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

TBR=rvargas@google.com
Review URL: http://codereview.chromium.org/465018

git-svn-id: svn://svn.chromium.org/chrome/branches/249/src@33637 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 39 insertions, 1 deletions

diff --git a/net/http/http_cache_transaction.cc b/net/http/http_cache_transaction.cc
index 047bc73..b42bf22 100644
--- a/net/http/http_cache_transaction.cc
+++ b/net/http/http_cache_transaction.cc
@@ -1233,9 +1233,10 @@ int HttpCache::Transaction::DoPartialCacheReadCompleted(int result) {
     if (partial_.get() && mode_ == READ_WRITE) {
       // We need to move on to the next range.
       result = ContinuePartialCacheValidation();
-      if (result != OK)
+      if (result != OK || !entry_) {
         // Any error was already handled.
         return result;
+      }
       cache_->ConvertWriterToReader(entry_);
     }
     cache_->DoneReadingFromEntry(entry_, this);
diff --git a/net/http/http_cache_unittest.cc b/net/http/http_cache_unittest.cc
index 0e693cf..0497df4 100644
--- a/net/http/http_cache_unittest.cc
+++ b/net/http/http_cache_unittest.cc
@@ -629,10 +629,12 @@ class RangeTransactionServer {
   RangeTransactionServer() {
     not_modified_ = false;
     modified_ = false;
+    bad_200_ = false;
   }
   ~RangeTransactionServer() {
     not_modified_ = false;
     modified_ = false;
+    bad_200_ = false;
   }
 
   // Returns only 416 or 304 when set.
@@ -641,6 +643,9 @@ class RangeTransactionServer {
   // Returns 206 when revalidating a range (instead of 304).
   void set_modified(bool value) { modified_ = value; }
 
+  // Returns 200 instead of 206 (a malformed response overall).
+  void set_bad_200(bool value) { bad_200_ = value; }
+
   static void RangeHandler(const net::HttpRequestInfo* request,
                            std::string* response_status,
                            std::string* response_headers,
@@ -649,10 +654,12 @@ class RangeTransactionServer {
  private:
   static bool not_modified_;
   static bool modified_;
+  static bool bad_200_;
   DISALLOW_COPY_AND_ASSIGN(RangeTransactionServer);
 };
 bool RangeTransactionServer::not_modified_ = false;
 bool RangeTransactionServer::modified_ = false;
+bool RangeTransactionServer::bad_200_ = false;
 
 // A dummy extra header that must be preserved on a given request.
 #define EXTRA_HEADER "Extra: header\r\n"
@@ -712,6 +719,10 @@ void RangeTransactionServer::RangeHandler(const net::HttpRequestInfo* request,
       response_headers->replace(response_headers->find("Content-Length:"),
                                 content_length.size(), content_length);
     }
+    if (bad_200_) {
+      // We return a range, but with a response code of 200.
+      response_status->assign("HTTP/1.1 200 Success");
+    }
   } else {
     response_status->assign("HTTP/1.1 304 Not Modified");
     response_data->clear();
@@ -3069,6 +3080,32 @@ TEST(HttpCache, RangeHEAD) {
   RemoveMockTransaction(&kRangeGET_TransactionOK);
 }
 
+// Tests that we don't crash when after reading from the cache we issue a
+// request for the next range and the server gives us a 200 synchronously.
+TEST(HttpCache, RangeGET_FastFlakyServer) {
+  MockHttpCache cache;
+  cache.http_cache()->set_enable_range_support(true);
+
+  MockTransaction transaction(kRangeGET_TransactionOK);
+  transaction.request_headers = "Range: bytes = 40-\r\n" EXTRA_HEADER;
+  transaction.test_mode = TEST_MODE_SYNC_NET_START;
+  AddMockTransaction(&transaction);
+
+  // Write to the cache.
+  RunTransactionTest(cache.http_cache(), kRangeGET_TransactionOK);
+
+  // And now read from the cache and the network.
+  RangeTransactionServer handler;
+  handler.set_bad_200(true);
+  RunTransactionTest(cache.http_cache(), transaction);
+
+  EXPECT_EQ(3, cache.network_layer()->transaction_count());
+  EXPECT_EQ(1, cache.disk_cache()->open_count());
+  EXPECT_EQ(1, cache.disk_cache()->create_count());
+
+  RemoveMockTransaction(&transaction);
+}
+
 #ifdef NDEBUG
 // This test hits a NOTREACHED so it is a release mode only test.
 TEST(HttpCache, RangeGET_OK_LoadOnlyFromCache) {