Doom and create new entry when validation is not a match

net/http/http_cache.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 <algorithm>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 928 matching lines @@
 void HttpCache::DoneReadingFromEntry(ActiveEntry* entry,
                                      Transaction* transaction) {
   DCHECK(!entry->writer);
   auto it = entry->readers.find(transaction);
   DCHECK(it != entry->readers.end());
   entry->readers.erase(it);
 
   ProcessQueuedTransactions(entry);
 }
 
+void HttpCache::DoomEntryValidationNoMatch(ActiveEntry* entry,
+                                           Transaction* transaction) {
+  // Validating transaction received a non-matching response.
+
+  // TODO(shivanisha) Can transaction be a writer in case of partial requests?

[jkarlin, 2017/06/09 17:04:49]

s/a writer/the writer/

[shivanisha, 2017/06/12 18:01:22]

done

+  // Handlie the writer case if that's possible.

[jkarlin, 2017/06/09 17:04:49]

Seems like we need to answer that question before we land this CL.

[jkarlin, 2017/06/09 17:04:49]

s/Handlie/Handle/

[shivanisha, 2017/06/12 18:01:22]

Handled this TODO in CanTransactionWriteResponseHeaders in the parent branch so
that this function should not be invoked in that scenario.

+  CHECK(transaction == entry->headers_transaction);

[jkarlin, 2017/06/09 17:04:49]

Let's DCHECK this and save CHECK for cases where we're debugging known problems.

[shivanisha, 2017/06/12 18:01:22]

done

+
+  entry->headers_transaction = nullptr;
+  if (entry->HasNoTransactions() && !entry->will_process_queued_transactions) {
+    entry->disk_entry->Doom();
+    DestroyEntry(entry);
+    return;
+  }
+
+  // Restart only add_to_entry_queue transactions.
+
+  DoomActiveEntry(entry->disk_entry->GetKey());
+  // Post task here to avoid a race in creating the entry between |transaction|
+  // and the add_to_entry_queue transactions. Reset the queued transactions'
+  // cache pending state so that in case their destructor is invoked, it's ok
+  // for them to not be found in this entry.
+  for (auto* transaction : entry->add_to_entry_queue) {
+    transaction->ResetCachePendingState();

[jkarlin, 2017/06/09 17:04:49]

Why is this necessary? Won't they all have this reset in DoAddToEntryComplete?

[shivanisha, 2017/06/12 18:01:23]

I am thinking of the race where we remove them from entry here and post the
task. Before the task is processed , that transaction's destructor is invoked.
The destructor assumes that if cache_pending is true then the transaction must
be present in the entry.

+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::Bind(transaction->io_callback(), net::ERR_CACHE_RACE));
+  }
+  entry->add_to_entry_queue.clear();
+}
+
 void HttpCache::RemoveAllQueuedTransactions(ActiveEntry* entry,
                                             TransactionList* list) {
   // Process done_headers_queue before add_to_entry_queue to maintain FIFO
   // order.
 
   for (auto* transaction : entry->done_headers_queue)
     list->push_back(transaction);
   entry->done_headers_queue.clear();
 
   for (auto* pending_transaction : entry->add_to_entry_queue)
     list->push_back(pending_transaction);
   entry->add_to_entry_queue.clear();
 }
 
 void HttpCache::ProcessEntryFailure(ActiveEntry* entry,
                                     Transaction* transaction) {
-  // Failure case is either writer failing to completely write the response to
-  // the cache or validating transaction received a non-304 response.
+  // Writer failed to completely write the response to
+  // the cache.
 
   if (entry->headers_transaction && transaction != entry->headers_transaction)
     RestartHeadersTransaction(entry, transaction);
 
   TransactionList list;
   RemoveAllQueuedTransactions(entry, &list);
 
   if (entry->HasNoTransactions() && !entry->will_process_queued_transactions) {
     entry->disk_entry->Doom();
     DestroyEntry(entry);
@@ skipping 374 matching lines @@
     building_backend_ = false;
     DeletePendingOp(pending_op);
   }
 
   // The cache may be gone when we return from the callback.
   if (!item->DoCallback(result, disk_cache_.get()))
     item->NotifyTransaction(result, NULL);
 }
 
 }  // namespace net