When there are multiple requests for the same resource, it is

possible that cancelling a request that is currently a reader
may be racing with another reader being completed.  In that
case, we were not removing the transaction for the cancelled
request so all queued requests were blocked forever. 

R=wtc
BUG=4769
TEST=unittest

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

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

2 files changed, 77 insertions, 3 deletions

diff --git a/net/http/http_cache.cc b/net/http/http_cache.cc
index fbc2b0f..2ac62ef 100644
--- a/net/http/http_cache.cc
+++ b/net/http/http_cache.cc
@@ -1255,9 +1255,9 @@ int HttpCache::AddTransactionToEntry(ActiveEntry* entry, Transaction* trans) {
 }
 
 void HttpCache::DoneWithEntry(ActiveEntry* entry, Transaction* trans) {
-  // If we already posted a task to move on to the next transaction, there is
-  // nothing to cancel.
-  if (entry->will_process_pending_queue)
+  // If we already posted a task to move on to the next transaction and this was
+  // the writer, there is nothing to cancel.
+  if (entry->will_process_pending_queue && entry->readers.empty())
     return;
 
   if (entry->writer) {
diff --git a/net/http/http_cache_unittest.cc b/net/http/http_cache_unittest.cc
index ab4e26b..be6986e 100644
--- a/net/http/http_cache_unittest.cc
+++ b/net/http/http_cache_unittest.cc
@@ -532,6 +532,80 @@ TEST(HttpCache, SimpleGET_ManyReaders) {
   }
 }
 
+// This is a test for http://code.google.com/p/chromium/issues/detail?id=4769.
+// If cancelling a request is racing with another request for the same resource
+// finishing, we have to make sure that we remove both transactions from the
+// entry.
+TEST(HttpCache, SimpleGET_RacingReaders) {
+  MockHttpCache cache;
+
+  MockHttpRequest request(kSimpleGET_Transaction);
+  MockHttpRequest reader_request(kSimpleGET_Transaction);
+  reader_request.load_flags = net::LOAD_ONLY_FROM_CACHE;
+
+  std::vector<Context*> context_list;
+  const int kNumTransactions = 5;
+
+  for (int i = 0; i < kNumTransactions; ++i) {
+    context_list.push_back(
+        new Context(cache.http_cache()->CreateTransaction()));
+
+    Context* c = context_list[i];
+    MockHttpRequest* this_request = &request;
+    if (i == 1 || i == 2)
+      this_request = &reader_request;
+
+    int rv = c->trans->Start(this_request, &c->callback);
+    if (rv != net::ERR_IO_PENDING)
+      c->result = rv;
+  }
+
+  // The first request should be a writer at this point, and the subsequent
+  // requests should be pending.
+
+  EXPECT_EQ(1, cache.network_layer()->transaction_count());
+  EXPECT_EQ(0, cache.disk_cache()->open_count());
+  EXPECT_EQ(1, cache.disk_cache()->create_count());
+
+  Context* c = context_list[0];
+  ASSERT_EQ(net::ERR_IO_PENDING, c->result);
+  c->result = c->callback.WaitForResult();
+  ReadAndVerifyTransaction(c->trans.get(), kSimpleGET_Transaction);
+
+  // Now we have 2 active readers and two queued transactions.
+
+  c = context_list[1];
+  ASSERT_EQ(net::ERR_IO_PENDING, c->result);
+  c->result = c->callback.WaitForResult();
+  ReadAndVerifyTransaction(c->trans.get(), kSimpleGET_Transaction);
+
+  // At this point we have one reader, two pending transactions and a task on
+  // the queue to move to the next transaction. Now we cancel the request that
+  // is the current reader, and expect the queued task to be able to start the
+  // next request.
+
+  c = context_list[2];
+  c->trans.reset();
+
+  for (int i = 3; i < kNumTransactions; ++i) {
+    Context* c = context_list[i];
+    if (c->result == net::ERR_IO_PENDING)
+      c->result = c->callback.WaitForResult();
+    ReadAndVerifyTransaction(c->trans.get(), kSimpleGET_Transaction);
+  }
+
+  // We should not have had to re-open the disk entry.
+
+  EXPECT_EQ(1, cache.network_layer()->transaction_count());
+  EXPECT_EQ(0, cache.disk_cache()->open_count());
+  EXPECT_EQ(1, cache.disk_cache()->create_count());
+
+  for (int i = 0; i < kNumTransactions; ++i) {
+    Context* c = context_list[i];
+    delete c;
+  }
+}
+
 TEST(HttpCache, SimpleGET_ManyWriters_CancelFirst) {
   MockHttpCache cache;