DoomPartialEntry should not attempt to doom an already doomed entry.

net/http/http_cache_unittest.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 "net/http/http_cache.h"
 
 #include <stdint.h>
 
 #include <algorithm>
 #include <memory>
@@ skipping 129 matching lines @@
   // same value.
   EXPECT_FALSE(load_timing_info.send_start.is_null());
   EXPECT_EQ(load_timing_info.send_start, load_timing_info.send_end);
 
   // Set by URLRequest / URLRequestHttpJob, at a higher level.
   EXPECT_TRUE(load_timing_info.request_start_time.is_null());
   EXPECT_TRUE(load_timing_info.request_start.is_null());
   EXPECT_TRUE(load_timing_info.receive_headers_end.is_null());
 }
 
-void DeferNetworkStart(bool* defer) {
+void DeferCallback(bool* defer) {
   *defer = true;
 }
 
 class DeleteCacheCompletionCallback : public TestCompletionCallbackBase {
  public:
   explicit DeleteCacheCompletionCallback(MockHttpCache* cache)
       : cache_(cache),
         callback_(base::Bind(&DeleteCacheCompletionCallback::OnComplete,
                              base::Unretained(this))) {
   }
@@ skipping 1310 matching lines @@
       c->result = c->callback.WaitForResult();
 
     ReadAndVerifyTransaction(c->trans.get(), kRangeGET_TransactionOK);
   }
 
   EXPECT_EQ(5, cache.network_layer()->transaction_count());
   EXPECT_EQ(0, cache.disk_cache()->open_count());
   EXPECT_EQ(5, cache.disk_cache()->create_count());
 }
 
+// Tests that if a transaction is dooming the entry and the entry was doomed by
+// another transaction that was not part of the entry and created a new entry,
+// the new entry should not be incorrectly doomed. (crbug.com/736993)
+TEST(HttpCache, RangeGET_ParallelValidationNoMatchDoomEntry) {
+  MockHttpCache cache;
+
+  ScopedMockTransaction transaction(kRangeGET_TransactionOK);
+  MockHttpRequest request(transaction);
+
+  MockTransaction dooming_transaction(kRangeGET_TransactionOK);
+  dooming_transaction.load_flags |= LOAD_BYPASS_CACHE;
+  MockHttpRequest dooming_request(dooming_transaction);
+
+  std::vector<std::unique_ptr<Context>> context_list;
+  const int kNumTransactions = 3;
+
+  MockDiskEntry* first_entry = nullptr;
+  MockDiskEntry* second_entry = nullptr;
+  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());
+
+    MockHttpRequest* this_request = &request;
+
+    if (i == 2)
+      this_request = &dooming_request;
+
+    if (i == 1) {
+      ASSERT_TRUE(first_entry);
+      first_entry->SetDefer(MockDiskEntry::DEFER_READ);
+    }
+
+    c->result = c->trans->Start(this_request, c->callback.callback(),
+                                NetLogWithSource());
+
+    // Continue the transactions. 2nd will pause at the cache reading state and
+    // 3rd transaction will doom the entry.
+    base::RunLoop().RunUntilIdle();
+
+    // Check status of the first and second entries after every transaction.
+    switch (i) {
+      case 0:
+        first_entry =
+            cache.disk_cache()->GetDiskEntryRef(kRangeGET_TransactionOK.url);
+        break;
+      case 1:
+        EXPECT_FALSE(first_entry->is_doomed());
+        break;
+      case 2:
+        EXPECT_TRUE(first_entry->is_doomed());
+        second_entry =
+            cache.disk_cache()->GetDiskEntryRef(kRangeGET_TransactionOK.url);
+        EXPECT_FALSE(second_entry->is_doomed());
+        break;
+    }
+  }
+  // Resume cache read by 1st transaction which will lead to dooming the entry
+  // as well since the entry cannot be validated. This double dooming should not
+  // lead to an assertion.
+  first_entry->ResumeCacheOperation();
+  base::RunLoop().RunUntilIdle();
+
+  // Since second_entry is already created, when 1st transaction goes on to
+  // create an entry, it will get ERR_CACHE_RACE leading to dooming of
+  // second_entry and creation of a third entry.
+  EXPECT_TRUE(second_entry->is_doomed());
+
+  EXPECT_EQ(3, cache.network_layer()->transaction_count());
+  EXPECT_EQ(0, cache.disk_cache()->open_count());
+  EXPECT_EQ(3, cache.disk_cache()->create_count());
+
+  for (auto& context : context_list) {
+    EXPECT_EQ(LOAD_STATE_IDLE, context->trans->GetLoadState());
+  }
+
+  for (auto& c : context_list) {
+    ReadAndVerifyTransaction(c->trans.get(), kRangeGET_TransactionOK);
+  }
+
+  EXPECT_EQ(3, cache.network_layer()->transaction_count());
+  EXPECT_EQ(0, cache.disk_cache()->open_count());
+  EXPECT_EQ(3, cache.disk_cache()->create_count());
+
+  if (first_entry)
+    first_entry->Close();
+  if (second_entry)
+    second_entry->Close();
+}
+
+// Same as above but tests that the 2nd transaction does not do anything if
+// there is nothing to doom. (crbug.com/736993)
+TEST(HttpCache, RangeGET_ParallelValidationNoMatchDoomEntry1) {
+  MockHttpCache cache;
+
+  ScopedMockTransaction transaction(kRangeGET_TransactionOK);
+  MockHttpRequest request(transaction);
+
+  MockTransaction dooming_transaction(kRangeGET_TransactionOK);
+  dooming_transaction.load_flags |= LOAD_BYPASS_CACHE;
+  MockHttpRequest dooming_request(dooming_transaction);
+
+  std::vector<std::unique_ptr<Context>> context_list;
+  const int kNumTransactions = 3;
+
+  MockDiskEntry* first_entry = nullptr;
+  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());
+
+    MockHttpRequest* this_request = &request;
+
+    if (i == 2) {
+      this_request = &dooming_request;
+      cache.disk_cache()->SetDefer(MockDiskEntry::DEFER_CREATE);
+    }
+
+    if (i == 1) {
+      ASSERT_TRUE(first_entry != nullptr);
+      first_entry->SetDefer(MockDiskEntry::DEFER_READ);
+    }
+
+    c->result = c->trans->Start(this_request, c->callback.callback(),
+                                NetLogWithSource());
+
+    // Continue the transactions. 2nd will pause at the cache reading state and
+    // 3rd transaction will doom the entry and pause before creating a new
+    // entry.
+    base::RunLoop().RunUntilIdle();
+
+    // Check status of the entry after every transaction.
+    switch (i) {
+      case 0:
+        first_entry =
+            cache.disk_cache()->GetDiskEntryRef(kRangeGET_TransactionOK.url);
+        break;
+      case 1:
+        EXPECT_FALSE(first_entry->is_doomed());
+        break;
+      case 2:
+        EXPECT_TRUE(first_entry->is_doomed());
+        break;
+    }
+  }
+  // Resume cache read by 2nd transaction which will lead to dooming the entry
+  // as well since the entry cannot be validated. This double dooming should not
+  // lead to an assertion.
+  first_entry->ResumeCacheOperation();
+  base::RunLoop().RunUntilIdle();
+
+  // Resume creation of entry by 3rd transaction.
+  cache.disk_cache()->ResumeCacheOperation();
+  base::RunLoop().RunUntilIdle();
+
+  // Note that since 3rd transaction's entry is already created but its
+  // callback is deferred, MockDiskCache's implementation returns
+  // ERR_CACHE_CREATE_FAILURE when 2nd transaction tries to create an entry
+  // during that time, leading to it switching over to pass-through mode.
+  // Thus the number of entries is 2 below.
+  EXPECT_EQ(3, cache.network_layer()->transaction_count());
+  EXPECT_EQ(0, cache.disk_cache()->open_count());
+  EXPECT_EQ(2, cache.disk_cache()->create_count());
+
+  for (auto& context : context_list) {
+    EXPECT_EQ(LOAD_STATE_IDLE, context->trans->GetLoadState());
+  }
+
+  for (auto& c : context_list) {
+    ReadAndVerifyTransaction(c->trans.get(), kRangeGET_TransactionOK);
+  }
+
+  EXPECT_EQ(3, cache.network_layer()->transaction_count());
+  EXPECT_EQ(0, cache.disk_cache()->open_count());
+  EXPECT_EQ(2, cache.disk_cache()->create_count());
+
+  if (first_entry)
+    first_entry->Close();
+}
+
 // Tests parallel validation on range requests with non-overlapping ranges.
 TEST(HttpCache, RangeGET_ParallelValidationDifferentRanges) {
   MockHttpCache cache;
 
   ScopedMockTransaction transaction(kRangeGET_TransactionOK);
 
   std::vector<std::unique_ptr<Context>> context_list;
   const int kNumTransactions = 2;
 
   for (int i = 0; i < kNumTransactions; ++i) {
@@ skipping 661 matching lines @@
   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());
 
     MockHttpRequest* this_request = &request;
     if (i == 3) {
       this_request = &validate_request;
-      c->trans->SetBeforeNetworkStartCallback(base::Bind(&DeferNetworkStart));
+      c->trans->SetBeforeNetworkStartCallback(base::Bind(&DeferCallback));
     }
 
     c->result = c->trans->Start(this_request, c->callback.callback(),
                                 NetLogWithSource());
   }
 
   // Allow all requests to move from the Create queue to the active entry.
   base::RunLoop().RunUntilIdle();
 
   EXPECT_EQ(2, cache.network_layer()->transaction_count());
@@ skipping 77 matching lines @@
   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());
 
     MockHttpRequest* this_request = &request;
     if (i == 2) {
       this_request = &validate_request;
-      c->trans->SetBeforeNetworkStartCallback(base::Bind(&DeferNetworkStart));
+      c->trans->SetBeforeNetworkStartCallback(base::Bind(&DeferCallback));
     }
 
     c->result = c->trans->Start(this_request, c->callback.callback(),
                                 NetLogWithSource());
   }
 
   // Allow all requests to move from the Create queue to the active entry.
   base::RunLoop().RunUntilIdle();
 
   EXPECT_EQ(2, cache.network_layer()->transaction_count());
@@ skipping 117 matching lines @@
   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());
 
     MockHttpRequest* this_request = &request;
     if (i == 2) {
       this_request = &validate_request;
-      c->trans->SetBeforeNetworkStartCallback(base::Bind(&DeferNetworkStart));
+      c->trans->SetBeforeNetworkStartCallback(base::Bind(&DeferCallback));
     }
 
     c->result = c->trans->Start(this_request, c->callback.callback(),
                                 NetLogWithSource());
   }
 
   // Allow all requests to move from the Create queue to the active entry.
   base::RunLoop().RunUntilIdle();
 
   EXPECT_EQ(2, cache.network_layer()->transaction_count());
@@ skipping 45 matching lines @@
   std::vector<std::unique_ptr<Context>> context_list;
 
   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());
 
     if (i == 0)
-      c->trans->SetBeforeNetworkStartCallback(base::Bind(&DeferNetworkStart));
+      c->trans->SetBeforeNetworkStartCallback(base::Bind(&DeferCallback));
 
     c->result =
         c->trans->Start(&request, c->callback.callback(), NetLogWithSource());
   }
 
   base::RunLoop().RunUntilIdle();
 
   EXPECT_TRUE(cache.IsHeadersTransactionPresent(kSimpleGET_Transaction.url));
   EXPECT_EQ(1, cache.GetCountAddToEntryQueue(kSimpleGET_Transaction.url));
 
@@ skipping 6884 matching lines @@
   ASSERT_TRUE(attrs->GetDictionary(
       base::trace_event::MemoryAllocatorDump::kNameSize, &size_attrs));
   std::string size;
   ASSERT_TRUE(size_attrs->GetString("value", &size));
   int actual_size = 0;
   ASSERT_TRUE(base::HexStringToInt(size, &actual_size));
   ASSERT_LT(0, actual_size);
 }
 
 }  // namespace net