Http cache: Make sure that we remove pending transactions

when they belong to a doomed entry.

BUG=25588
TEST=unittests

Review URL: http://codereview.chromium.org/335015

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@30054 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 70 insertions, 3 deletions

diff --git a/net/http/http_cache.cc b/net/http/http_cache.cc
index eee9397..f9cdd5e 100644
--- a/net/http/http_cache.cc
+++ b/net/http/http_cache.cc
@@ -979,6 +979,9 @@ int HttpCache::Transaction::ValidateEntryHeadersAndContinue(
     bool byte_range_requested) {
   DCHECK(mode_ == READ_WRITE);
 
+  if (!cache_)
+    return HandleResult(ERR_UNEXPECTED);
+
   if (!partial_->UpdateFromStoredHeaders(response_.headers, entry_->disk_entry,
                                          truncated_)) {
     // The stored data cannot be used. Get rid of it and restart this request.
@@ -2044,17 +2047,29 @@ void HttpCache::ConvertWriterToReader(ActiveEntry* entry) {
 
 void HttpCache::RemovePendingTransaction(Transaction* trans) {
   ActiveEntriesMap::const_iterator i = active_entries_.find(trans->key());
-  if (i == active_entries_.end())
+  bool found = false;
+  if (i != active_entries_.end())
+    found = RemovePendingTransactionFromEntry(i->second, trans);
+
+  if (found)
     return;
 
-  TransactionList& pending_queue = i->second->pending_queue;
+  ActiveEntriesSet::iterator it = doomed_entries_.begin();
+  for (; it != doomed_entries_.end() && !found; ++it)
+    found = RemovePendingTransactionFromEntry(*it, trans);
+}
+
+bool HttpCache::RemovePendingTransactionFromEntry(ActiveEntry* entry,
+                                                  Transaction* trans) {
+  TransactionList& pending_queue = entry->pending_queue;
 
   TransactionList::iterator j =
       find(pending_queue.begin(), pending_queue.end(), trans);
   if (j == pending_queue.end())
-    return;
+    return false;
 
   pending_queue.erase(j);
+  return true;
 }
 
 void HttpCache::ProcessPendingQueue(ActiveEntry* entry) {
diff --git a/net/http/http_cache.h b/net/http/http_cache.h
index fe77912..68347f9 100644
--- a/net/http/http_cache.h
+++ b/net/http/http_cache.h
@@ -176,6 +176,8 @@ class HttpCache : public HttpTransactionFactory,
   void DoneReadingFromEntry(ActiveEntry* entry, Transaction* trans);
   void ConvertWriterToReader(ActiveEntry* entry);
   void RemovePendingTransaction(Transaction* trans);
+  bool RemovePendingTransactionFromEntry(ActiveEntry* entry,
+                                         Transaction* trans);
   void ProcessPendingQueue(ActiveEntry* entry);
 
 
diff --git a/net/http/http_cache_unittest.cc b/net/http/http_cache_unittest.cc
index 371ef2586..1206598 100644
--- a/net/http/http_cache_unittest.cc
+++ b/net/http/http_cache_unittest.cc
@@ -6,6 +6,7 @@
 
 #include "base/hash_tables.h"
 #include "base/message_loop.h"
+#include "base/scoped_vector.h"
 #include "base/string_util.h"
 #include "net/base/net_errors.h"
 #include "net/base/load_flags.h"
@@ -1140,6 +1141,55 @@ TEST(HttpCache, SimpleGET_RacingReaders) {
   }
 }
 
+// Tests that we can doom an entry with pending transactions and delete one of
+// the pending transactions before the first one completes.
+// See http://code.google.com/p/chromium/issues/detail?id=25588
+TEST(HttpCache, SimpleGET_DoomWithPending) {
+  // We need simultaneous doomed / not_doomed entries so let's use a real cache.
+  disk_cache::Backend* disk_cache =
+      disk_cache::CreateInMemoryCacheBackend(1024 * 1024);
+  MockHttpCache cache(disk_cache);
+
+  MockHttpRequest request(kSimpleGET_Transaction);
+  MockHttpRequest writer_request(kSimpleGET_Transaction);
+  writer_request.load_flags = net::LOAD_BYPASS_CACHE;
+
+  ScopedVector<Context> context_list;
+  const int kNumTransactions = 4;
+
+  for (int i = 0; i < kNumTransactions; ++i) {
+    context_list.push_back(new Context());
+    Context* c = context_list[i];
+
+    c->result = cache.http_cache()->CreateTransaction(&c->trans);
+    EXPECT_EQ(net::OK, c->result);
+
+    MockHttpRequest* this_request = &request;
+    if (i == 3)
+      this_request = &writer_request;
+
+    c->result = c->trans->Start(this_request, &c->callback, NULL);
+  }
+
+  // The first request should be a writer at this point, and the two subsequent
+  // requests should be pending. The last request doomed the first entry.
+
+  EXPECT_EQ(2, cache.network_layer()->transaction_count());
+
+  // Cancel the first queued transaction.
+  delete context_list[1];
+  context_list.get()[1] = NULL;
+
+  for (int i = 0; i < kNumTransactions; ++i) {
+    if (i == 1)
+      continue;
+    Context* c = context_list[i];
+    ASSERT_EQ(net::ERR_IO_PENDING, c->result);
+    c->result = c->callback.WaitForResult();
+    ReadAndVerifyTransaction(c->trans.get(), kSimpleGET_Transaction);
+  }
+}
+
 // This is a test for http://code.google.com/p/chromium/issues/detail?id=4731.
 // We may attempt to delete an entry synchronously with the act of adding a new
 // transaction to said entry.