HTTP cache: Add tests to improve coverage of AddTruncatedFlag.

There's also a small change in the logic that sets done_reading_, which
is used to determine if the truncated flag should be set or not, and should
not be used for sparse entries.

BUG=

Review URL: https://codereview.chromium.org/1346793002

Cr-Commit-Position: refs/heads/master@{#349528}

2 files changed, 85 insertions, 83 deletions

diff --git a/net/http/http_cache_transaction.cc b/net/http/http_cache_transaction.cc
index 450cc45..db3f496 100644
--- a/net/http/http_cache_transaction.cc
+++ b/net/http/http_cache_transaction.cc
@@ -1875,7 +1875,7 @@ int HttpCache::Transaction::DoCacheWriteDataComplete(int result) {
     // We want to ignore errors writing to disk and just keep reading from
     // the network.
     result = write_len_;
-  } else if (!done_reading_ && entry_) {
+  } else if (!done_reading_ && entry_ && (!partial_ || truncated_)) {
     int current_size = entry_->disk_entry->GetDataSize(kResponseContentIndex);
     int64 body_size = response_.headers->GetContentLength();
     if (body_size >= 0 && body_size <= current_size)
diff --git a/net/http/http_cache_unittest.cc b/net/http/http_cache_unittest.cc
index 2d4625f..3059663 100644
--- a/net/http/http_cache_unittest.cc
+++ b/net/http/http_cache_unittest.cc
@@ -508,6 +508,24 @@ void CreateTruncatedEntry(std::string raw_headers, MockHttpCache* cache) {
   entry->Close();
 }
 
+// Verifies that there's an entry with this |key| with the truncated flag set to
+// |flag_value|, and with an optional |data_size| (if not zero).
+void VerifyTruncatedFlag(MockHttpCache* cache,
+                         const std::string& key,
+                         bool flag_value,
+                         int data_size) {
+  disk_cache::Entry* entry;
+  ASSERT_TRUE(cache->OpenBackendEntry(key, &entry));
+  disk_cache::ScopedEntryPtr closer(entry);
+
+  HttpResponseInfo response;
+  bool truncated = !flag_value;
+  EXPECT_TRUE(MockHttpCache::ReadResponseInfo(entry, &response, &truncated));
+  EXPECT_EQ(flag_value, truncated);
+  if (data_size)
+    EXPECT_EQ(data_size, entry->GetDataSize(1));
+}
+
 // Helper to represent a network HTTP response.
 struct Response {
   // Set this response into |trans|.
@@ -5639,12 +5657,11 @@ TEST(HttpCache, DoomOnDestruction3) {
 TEST(HttpCache, SetTruncatedFlag) {
   MockHttpCache cache;
 
-  MockTransaction transaction(kSimpleGET_Transaction);
+  ScopedMockTransaction transaction(kSimpleGET_Transaction);
   transaction.response_headers =
       "Last-Modified: Wed, 28 Nov 2007 00:40:09 GMT\n"
       "Content-Length: 22\n"
       "Etag: \"foopy\"\n";
-  AddMockTransaction(&transaction);
   MockHttpRequest request(transaction);
 
   scoped_ptr<Context> c(new Context());
@@ -5686,27 +5703,18 @@ TEST(HttpCache, SetTruncatedFlag) {
   EXPECT_FALSE(c->callback.have_result());
 
   // Verify that the entry is marked as incomplete.
-  disk_cache::Entry* entry;
-  ASSERT_TRUE(cache.OpenBackendEntry(kSimpleGET_Transaction.url, &entry));
-  HttpResponseInfo response;
-  bool truncated = false;
-  EXPECT_TRUE(MockHttpCache::ReadResponseInfo(entry, &response, &truncated));
-  EXPECT_TRUE(truncated);
-  entry->Close();
-
-  RemoveMockTransaction(&transaction);
+  VerifyTruncatedFlag(&cache, kSimpleGET_Transaction.url, true, 0);
 }
 
 // Tests that we don't mark an entry as truncated when we read everything.
 TEST(HttpCache, DontSetTruncatedFlag) {
   MockHttpCache cache;
 
-  MockTransaction transaction(kSimpleGET_Transaction);
+  ScopedMockTransaction transaction(kSimpleGET_Transaction);
   transaction.response_headers =
       "Last-Modified: Wed, 28 Nov 2007 00:40:09 GMT\n"
       "Content-Length: 22\n"
       "Etag: \"foopy\"\n";
-  AddMockTransaction(&transaction);
   MockHttpRequest request(transaction);
 
   scoped_ptr<Context> c(new Context());
@@ -5725,21 +5733,66 @@ TEST(HttpCache, DontSetTruncatedFlag) {
   c->trans.reset();
 
   // Verify that the entry is not marked as truncated.
-  disk_cache::Entry* entry;
-  ASSERT_TRUE(cache.OpenBackendEntry(kSimpleGET_Transaction.url, &entry));
-  HttpResponseInfo response;
-  bool truncated = true;
-  EXPECT_TRUE(MockHttpCache::ReadResponseInfo(entry, &response, &truncated));
-  EXPECT_FALSE(truncated);
-  entry->Close();
+  VerifyTruncatedFlag(&cache, kSimpleGET_Transaction.url, false, 0);
+}
 
-  RemoveMockTransaction(&transaction);
+// Tests that sparse entries don't set the truncate flag.
+TEST(HttpCache, RangeGET_DontTruncate) {
+  MockHttpCache cache;
+
+  ScopedMockTransaction transaction(kRangeGET_TransactionOK);
+  transaction.request_headers = "Range: bytes = 0-19\r\n" EXTRA_HEADER;
+
+  scoped_ptr<MockHttpRequest> request(new MockHttpRequest(transaction));
+  scoped_ptr<HttpTransaction> trans;
+
+  int rv = cache.http_cache()->CreateTransaction(DEFAULT_PRIORITY, &trans);
+  EXPECT_EQ(OK, rv);
+
+  TestCompletionCallback cb;
+  rv = trans->Start(request.get(), cb.callback(), BoundNetLog());
+  EXPECT_EQ(0, cb.GetResult(rv));
+
+  scoped_refptr<IOBuffer> buf(new IOBuffer(10));
+  rv = trans->Read(buf.get(), 10, cb.callback());
+  EXPECT_EQ(10, cb.GetResult(rv));
+
+  // Should not trigger any DCHECK.
+  trans.reset();
+  VerifyTruncatedFlag(&cache, kRangeGET_TransactionOK.url, false, 0);
+}
+
+// Tests that sparse entries don't set the truncate flag (when the byte range
+//  starts after 0).
+TEST(HttpCache, RangeGET_DontTruncate2) {
+  MockHttpCache cache;
+
+  ScopedMockTransaction transaction(kRangeGET_TransactionOK);
+  transaction.request_headers = "Range: bytes = 30-49\r\n" EXTRA_HEADER;
+
+  scoped_ptr<MockHttpRequest> request(new MockHttpRequest(transaction));
+  scoped_ptr<HttpTransaction> trans;
+
+  int rv = cache.http_cache()->CreateTransaction(DEFAULT_PRIORITY, &trans);
+  EXPECT_EQ(OK, rv);
+
+  TestCompletionCallback cb;
+  rv = trans->Start(request.get(), cb.callback(), BoundNetLog());
+  EXPECT_EQ(0, cb.GetResult(rv));
+
+  scoped_refptr<IOBuffer> buf(new IOBuffer(10));
+  rv = trans->Read(buf.get(), 10, cb.callback());
+  EXPECT_EQ(10, cb.GetResult(rv));
+
+  // Should not trigger any DCHECK.
+  trans.reset();
+  VerifyTruncatedFlag(&cache, kRangeGET_TransactionOK.url, false, 0);
 }
 
 // Tests that we can continue with a request that was interrupted.
 TEST(HttpCache, GET_IncompleteResource) {
   MockHttpCache cache;
-  AddMockTransaction(&kRangeGET_TransactionOK);
+  ScopedMockTransaction transaction(kRangeGET_TransactionOK);
 
   std::string raw_headers("HTTP/1.1 200 OK\n"
                           "Last-Modified: Sat, 18 Apr 2007 01:10:43 GMT\n"
@@ -5750,7 +5803,6 @@ TEST(HttpCache, GET_IncompleteResource) {
 
   // Now make a regular request.
   std::string headers;
-  MockTransaction transaction(kRangeGET_TransactionOK);
   transaction.request_headers = EXTRA_HEADER;
   transaction.data = kFullRangeData;
   RunTransactionTestWithResponse(cache.http_cache(), transaction, &headers);
@@ -5769,16 +5821,7 @@ TEST(HttpCache, GET_IncompleteResource) {
   EXPECT_EQ(1, cache.disk_cache()->create_count());
 
   // Verify that the disk entry was updated.
-  disk_cache::Entry* entry;
-  ASSERT_TRUE(cache.OpenBackendEntry(kRangeGET_TransactionOK.url, &entry));
-  EXPECT_EQ(80, entry->GetDataSize(1));
-  bool truncated = true;
-  HttpResponseInfo response;
-  EXPECT_TRUE(MockHttpCache::ReadResponseInfo(entry, &response, &truncated));
-  EXPECT_FALSE(truncated);
-  entry->Close();
-
-  RemoveMockTransaction(&kRangeGET_TransactionOK);
+  VerifyTruncatedFlag(&cache, kRangeGET_TransactionOK.url, false, 80);
 }
 
 // Tests the handling of no-store when revalidating a truncated entry.
@@ -6044,7 +6087,7 @@ TEST(HttpCache, TransactionRetryLimit) {
 // Tests that we cache a 200 response to the validation request.
 TEST(HttpCache, GET_IncompleteResource4) {
   MockHttpCache cache;
-  AddMockTransaction(&kRangeGET_TransactionOK);
+  ScopedMockTransaction transaction(kRangeGET_TransactionOK);
 
   std::string raw_headers("HTTP/1.1 200 OK\n"
                           "Last-Modified: Sat, 18 Apr 2009 01:10:43 GMT\n"
@@ -6055,7 +6098,6 @@ TEST(HttpCache, GET_IncompleteResource4) {
 
   // Now make a regular request.
   std::string headers;
-  MockTransaction transaction(kRangeGET_TransactionOK);
   transaction.request_headers = EXTRA_HEADER;
   transaction.data = "Not a range";
   RangeTransactionServer handler;
@@ -6067,23 +6109,14 @@ TEST(HttpCache, GET_IncompleteResource4) {
   EXPECT_EQ(1, cache.disk_cache()->create_count());
 
   // Verify that the disk entry was updated.
-  disk_cache::Entry* entry;
-  ASSERT_TRUE(cache.OpenBackendEntry(kRangeGET_TransactionOK.url, &entry));
-  EXPECT_EQ(11, entry->GetDataSize(1));
-  bool truncated = true;
-  HttpResponseInfo response;
-  EXPECT_TRUE(MockHttpCache::ReadResponseInfo(entry, &response, &truncated));
-  EXPECT_FALSE(truncated);
-  entry->Close();
-
-  RemoveMockTransaction(&kRangeGET_TransactionOK);
+  VerifyTruncatedFlag(&cache, kRangeGET_TransactionOK.url, false, 11);
 }
 
 // Tests that when we cancel a request that was interrupted, we mark it again
 // as truncated.
 TEST(HttpCache, GET_CancelIncompleteResource) {
   MockHttpCache cache;
-  AddMockTransaction(&kRangeGET_TransactionOK);
+  ScopedMockTransaction transaction(kRangeGET_TransactionOK);
 
   std::string raw_headers("HTTP/1.1 200 OK\n"
                           "Last-Modified: Sat, 18 Apr 2009 01:10:43 GMT\n"
@@ -6093,7 +6126,6 @@ TEST(HttpCache, GET_CancelIncompleteResource) {
   CreateTruncatedEntry(raw_headers, &cache);
 
   // Now make a regular request.
-  MockTransaction transaction(kRangeGET_TransactionOK);
   transaction.request_headers = EXTRA_HEADER;
 
   MockHttpRequest request(transaction);
@@ -6120,15 +6152,7 @@ TEST(HttpCache, GET_CancelIncompleteResource) {
   EXPECT_EQ(1, cache.disk_cache()->create_count());
 
   // Verify that the disk entry was updated: now we have 30 bytes.
-  disk_cache::Entry* entry;
-  ASSERT_TRUE(cache.OpenBackendEntry(kRangeGET_TransactionOK.url, &entry));
-  EXPECT_EQ(30, entry->GetDataSize(1));
-  bool truncated = false;
-  HttpResponseInfo response;
-  EXPECT_TRUE(MockHttpCache::ReadResponseInfo(entry, &response, &truncated));
-  EXPECT_TRUE(truncated);
-  entry->Close();
-  RemoveMockTransaction(&kRangeGET_TransactionOK);
+  VerifyTruncatedFlag(&cache, kRangeGET_TransactionOK.url, true, 30);
 }
 
 // Tests that we can handle range requests when we have a truncated entry.
@@ -6875,7 +6899,7 @@ TEST(HttpCache, StopCachingSavesEntry) {
     ASSERT_EQ(OK, cache.CreateTransaction(&trans));
 
     // Force a response that can be resumed.
-    MockTransaction mock_transaction(kSimpleGET_Transaction);
+    ScopedMockTransaction mock_transaction(kSimpleGET_Transaction);
     AddMockTransaction(&mock_transaction);
     mock_transaction.response_headers = "Cache-Control: max-age=10000\n"
                                         "Content-Length: 42\n"
@@ -6895,18 +6919,10 @@ TEST(HttpCache, StopCachingSavesEntry) {
     EXPECT_GT(callback.GetResult(rv), 0);
     rv = trans->Read(buf.get(), 256, callback.callback());
     EXPECT_EQ(callback.GetResult(rv), 0);
-
-    RemoveMockTransaction(&mock_transaction);
   }
 
   // Verify that the entry is marked as incomplete.
-  disk_cache::Entry* entry;
-  ASSERT_TRUE(cache.OpenBackendEntry(kSimpleGET_Transaction.url, &entry));
-  HttpResponseInfo response;
-  bool truncated = false;
-  EXPECT_TRUE(MockHttpCache::ReadResponseInfo(entry, &response, &truncated));
-  EXPECT_TRUE(truncated);
-  entry->Close();
+  VerifyTruncatedFlag(&cache, kSimpleGET_Transaction.url, true, 0);
 }
 
 // Tests that we handle truncated enries when StopCaching is called.
@@ -6950,15 +6966,7 @@ TEST(HttpCache, StopCachingTruncatedEntry) {
   }
 
   // Verify that the disk entry was updated.
-  disk_cache::Entry* entry;
-  ASSERT_TRUE(cache.OpenBackendEntry(kRangeGET_TransactionOK.url, &entry));
-  EXPECT_EQ(80, entry->GetDataSize(1));
-  bool truncated = true;
-  HttpResponseInfo response;
-  EXPECT_TRUE(MockHttpCache::ReadResponseInfo(entry, &response, &truncated));
-  EXPECT_FALSE(truncated);
-  entry->Close();
-
+  VerifyTruncatedFlag(&cache, kRangeGET_TransactionOK.url, false, 80);
   RemoveMockTransaction(&kRangeGET_TransactionOK);
 }
 
@@ -6998,13 +7006,7 @@ TEST(HttpCache, TruncatedByContentLength2) {
   RemoveMockTransaction(&transaction);
 
   // Verify that the entry is marked as incomplete.
-  disk_cache::Entry* entry;
-  ASSERT_TRUE(cache.OpenBackendEntry(kSimpleGET_Transaction.url, &entry));
-  HttpResponseInfo response;
-  bool truncated = false;
-  EXPECT_TRUE(MockHttpCache::ReadResponseInfo(entry, &response, &truncated));
-  EXPECT_TRUE(truncated);
-  entry->Close();
+  VerifyTruncatedFlag(&cache, kSimpleGET_Transaction.url, true, 0);
 }
 
 // Make sure that calling SetPriority on a cache transaction passes on