Doom and create new entry when validation is not a match

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 626f46a9a68c93b9f804e1369e150e91839b0b39..1c4d30b63dbec858e3f16d31776491cb64d3f94c 100644
--- a/net/http/http_cache_unittest.cc
+++ b/net/http/http_cache_unittest.cc
@@ -1415,8 +1415,9 @@ TEST(HttpCache, SimpleGET_ManyReaders) {
   EXPECT_EQ(1, cache.disk_cache()->create_count());
 }
 
-// Parallel validation results in 200.
-TEST(HttpCache, SimpleGET_ParallelValidationNoMatch) {
+// Parallel validation results in 200. Creation of a new entry after dooming
+// leads to a race between 2 transactions trying to create the new entry.
+TEST(HttpCache, SimpleGET_ParallelValidationNoMatchCacheCreateRace) {
   MockHttpCache cache;
 
   MockHttpRequest request(kSimpleGET_Transaction);
@@ -1450,11 +1451,9 @@ TEST(HttpCache, SimpleGET_ParallelValidationNoMatch) {
   // entries since none of them matched the headers of the earlier one.
   EXPECT_TRUE(cache.IsWriterPresent(kSimpleGET_Transaction.url));
 
-  // Note that there are only 3 entries created and not 5 since every other
-  // transaction would have gone to the network.
   EXPECT_EQ(5, cache.network_layer()->transaction_count());
   EXPECT_EQ(0, cache.disk_cache()->open_count());
-  EXPECT_EQ(3, cache.disk_cache()->create_count());
+  EXPECT_EQ(5, cache.disk_cache()->create_count());
 
   // All requests depend on the writer, and the writer is between Start and
   // Read, i.e. idle.
@@ -1470,7 +1469,64 @@ TEST(HttpCache, SimpleGET_ParallelValidationNoMatch) {
 
   EXPECT_EQ(5, cache.network_layer()->transaction_count());
   EXPECT_EQ(0, cache.disk_cache()->open_count());
-  EXPECT_EQ(3, cache.disk_cache()->create_count());
+  EXPECT_EQ(5, cache.disk_cache()->create_count());
+}
+
+// Parallel validation results in 200. Similar to above except here there are
+// only 2 transactions so no cache create race happens.
+TEST(HttpCache, SimpleGET_ParallelValidationNoMatch) {
+  MockHttpCache cache;
+
+  MockHttpRequest request(kSimpleGET_Transaction);
+  request.load_flags |= LOAD_VALIDATE_CACHE;
+
+  std::vector<std::unique_ptr<Context>> context_list;
+  const int kNumTransactions = 2;
+
+  for (int i = 0; i < kNumTransactions; ++i) {
+    context_list.push_back(base::MakeUnique<Context>());
+    auto& c = context_list[i];
+
+    c->result = cache.CreateTransaction(&c->trans);
+    ASSERT_THAT(c->result, IsOk());
+    EXPECT_EQ(LOAD_STATE_IDLE, c->trans->GetLoadState());
+
+    c->result =
+        c->trans->Start(&request, c->callback.callback(), NetLogWithSource());
+  }
+
+  // All requests are waiting for the active entry.
+  for (auto& context : context_list) {
+    EXPECT_EQ(LOAD_STATE_WAITING_FOR_CACHE, context->trans->GetLoadState());
+  }
+
+  // Allow all requests to move from the Create queue to the active entry.
+  base::RunLoop().RunUntilIdle();
+
+  // The first request should be a writer at this point, and the subsequent
+  // requests should have passed the validation phase and created their own
+  // entries since none of them matched the headers of the earlier one.
+  EXPECT_TRUE(cache.IsWriterPresent(kSimpleGET_Transaction.url));
+
+  EXPECT_EQ(2, cache.network_layer()->transaction_count());
+  EXPECT_EQ(0, cache.disk_cache()->open_count());
+  EXPECT_EQ(2, cache.disk_cache()->create_count());
+
+  // All requests depend on the writer, and the writer is between Start and
+  // Read, i.e. idle.
+  for (auto& context : context_list) {
+    EXPECT_EQ(LOAD_STATE_IDLE, context->trans->GetLoadState());
+  }
+
+  for (auto& context : context_list) {
+    if (context->result == ERR_IO_PENDING)
+      context->result = context->callback.WaitForResult();
+    ReadAndVerifyTransaction(context->trans.get(), kSimpleGET_Transaction);
+  }
+
+  EXPECT_EQ(2, cache.network_layer()->transaction_count());
+  EXPECT_EQ(0, cache.disk_cache()->open_count());
+  EXPECT_EQ(2, cache.disk_cache()->create_count());
 }
 
 // Tests that a GET followed by a DELETE results in DELETE immediately starting
@@ -1672,7 +1728,7 @@ TEST(HttpCache, SimpleGET_ParallelValidationCancelReader) {
   // The pending transactions will be added to a new entry.
   base::RunLoop().RunUntilIdle();
 
-  EXPECT_EQ(1, cache.GetCountDoneHeadersQueue(kSimpleGET_Transaction.url));
+  EXPECT_EQ(2, cache.GetCountDoneHeadersQueue(kSimpleGET_Transaction.url));
   EXPECT_TRUE(cache.IsWriterPresent(kSimpleGET_Transaction.url));
 
   // Complete the rest of the transactions.
@@ -1681,7 +1737,7 @@ TEST(HttpCache, SimpleGET_ParallelValidationCancelReader) {
     ReadAndVerifyTransaction(c->trans.get(), kSimpleGET_Transaction);
   }
 
-  EXPECT_EQ(3, cache.network_layer()->transaction_count());
+  EXPECT_EQ(2, cache.network_layer()->transaction_count());
   EXPECT_EQ(0, cache.disk_cache()->open_count());
   EXPECT_EQ(2, cache.disk_cache()->create_count());
 }
@@ -1746,10 +1802,6 @@ TEST(HttpCache, SimpleGET_ParallelValidationCancelWriter) {
   c->trans->ResumeNetworkStart();
   base::RunLoop().RunUntilIdle();
 
-  // Headers transaction would have doomed the new entry created.
-  EXPECT_TRUE(
-      cache.disk_cache()->IsDiskEntryDoomed(kSimpleGET_Transaction.url));
-
   // Complete the rest of the transactions.
   for (auto& context : context_list) {
     if (!context)
@@ -1759,7 +1811,9 @@ TEST(HttpCache, SimpleGET_ParallelValidationCancelWriter) {
 
   EXPECT_EQ(4, cache.network_layer()->transaction_count());
   EXPECT_EQ(0, cache.disk_cache()->open_count());
-  EXPECT_EQ(2, cache.disk_cache()->create_count());
+  // Headers transaction would have doomed the new entry created and created
+  // another new entry.
+  EXPECT_EQ(3, cache.disk_cache()->create_count());
 }
 
 // Tests that a transaction is currently in headers phase and is destroyed
@@ -2058,7 +2112,7 @@ TEST(HttpCache, FastNoStoreGET_DoneWithPending) {
 
   EXPECT_EQ(3, cache.network_layer()->transaction_count());
   EXPECT_EQ(0, cache.disk_cache()->open_count());
-  EXPECT_EQ(2, cache.disk_cache()->create_count());
+  EXPECT_EQ(3, cache.disk_cache()->create_count());
 
   // Now, make sure that the second request asks for the entry not to be stored.
   request_handler.set_no_store(true);
@@ -2073,7 +2127,7 @@ TEST(HttpCache, FastNoStoreGET_DoneWithPending) {
 
   EXPECT_EQ(3, cache.network_layer()->transaction_count());
   EXPECT_EQ(0, cache.disk_cache()->open_count());
-  EXPECT_EQ(2, cache.disk_cache()->create_count());
+  EXPECT_EQ(3, cache.disk_cache()->create_count());
 
   RemoveMockTransaction(&kFastNoStoreGET_Transaction);
 }