HTTP cache: Add tests to improve coverage of AddTruncatedFlag.

net/http/http_cache_unittest.cc

Index: net/http/http_cache_unittest.cc
diff --git a/net/http/http_cache_unittest.cc b/net/http/http_cache_unittest.cc
index 2d4625f7559d296dcb3df1f3314c5259025c92be..def8cada9f08a65f11fe2977cbe154b513a8ea7a 100644
--- a/net/http/http_cache_unittest.cc
+++ b/net/http/http_cache_unittest.cc
@@ -5635,16 +5635,37 @@ TEST(HttpCache, DoomOnDestruction3) {
   RemoveMockTransaction(&transaction);
 }
 
+namespace {
+
+// 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,

[asanka, 2015/09/17 14:43:34]

Nit: I'd move this to the top since it's not specific to the test or cluster
immediately following.

(Though I admit I might occasionally forget follow this guidance myself)

[rvargas (doing something else), 2015/09/17 20:18:37]

I only placed it here 'cause I saw you doing that :P

done.

[asanka, 2015/09/17 22:26:00]

Hehe. I caught one of these strays in my CL and moved it. My general rule still
stands though (that some accessory function or class that's specific to a test
gets placed adjacent to the test instead of at the top of the file).

+                         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));
+}
+
+}  // namespace
+
 // Tests that we mark an entry as incomplete when the request is cancelled.
 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 +5707,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 +5737,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 +5807,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 +5825,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 +6091,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 +6102,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 +6113,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 +6130,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 +6156,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 +6903,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 +6923,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 +6970,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 +7010,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