Release the cache entry on deferred redirect.

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 "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/memory/scoped_vector.h"
 #include "base/message_loop/message_loop.h"
@@ skipping 5305 matching lines @@
 
   // force this transaction to validate the cache
   MockTransaction transaction(kETagGET_Transaction);
   transaction.load_flags |= net::LOAD_VALIDATE_CACHE;
   RunTransactionTest(cache.http_cache(), transaction);
 
   // read from the cache
   RunTransactionTest(cache.http_cache(), kETagGET_Transaction);
 }
 
 TEST(HttpCache, CachedRedirect) {

[rvargas (doing something else), 2013/09/17 19:53:25]

I think is better to write a new test closer to the actual behavior change: now
calling DoneReading() before Read() should release a transaction. Something
close to HttpCache.FilterCompletion but without deleting the first transaction.

[davidben, 2013/09/17 22:16:04]

Added an HttpCache.DiscardedResponse. Did you mean that it should replace
URLRequestTestHTTP.DeferredRedirect_ReleaseCacheEntry? (It's still in there for
now.)

   MockHttpCache cache;
 
   ScopedMockTransaction kTestTransaction(kSimpleGET_Transaction);
   kTestTransaction.status = "HTTP/1.1 301 Moved Permanently";
   kTestTransaction.response_headers = "Location: http://www.bar.com/\n";
 
   MockHttpRequest request(kTestTransaction);
   net::TestCompletionCallback callback;
 
-  // write to the cache
+  // Write to the cache.
   {
     scoped_ptr<net::HttpTransaction> trans;
     int rv = cache.http_cache()->CreateTransaction(
         net::DEFAULT_PRIORITY, &trans, NULL);
     EXPECT_EQ(net::OK, rv);
     ASSERT_TRUE(trans.get());
 
     rv = trans->Start(&request, callback.callback(), net::BoundNetLog());
     if (rv == net::ERR_IO_PENDING)
       rv = callback.WaitForResult();
     ASSERT_EQ(net::OK, rv);
 
     const net::HttpResponseInfo* info = trans->GetResponseInfo();
     ASSERT_TRUE(info);
 
     EXPECT_EQ(info->headers->response_code(), 301);
 
     std::string location;
     info->headers->EnumerateHeader(NULL, "Location", &location);
     EXPECT_EQ(location, "http://www.bar.com/");
 
+    // Mark the transaction as completed so it is cached.
+    trans->DoneReading();
+
     // Destroy transaction when going out of scope. We have not actually
     // read the response body -- want to test that it is still getting cached.
   }
   EXPECT_EQ(1, cache.network_layer()->transaction_count());
   EXPECT_EQ(0, cache.disk_cache()->open_count());
   EXPECT_EQ(1, cache.disk_cache()->create_count());
 
-  // read from the cache
+  // Active entries in the cache are not retired synchronously. Make
+  // sure the next run hits the MockHttpCache and open_count is
+  // correct.
+  base::MessageLoop::current()->RunUntilIdle();
+
+  // Read from the cache.
   {
     scoped_ptr<net::HttpTransaction> trans;
     int rv = cache.http_cache()->CreateTransaction(
         net::DEFAULT_PRIORITY, &trans, NULL);
     EXPECT_EQ(net::OK, rv);
     ASSERT_TRUE(trans.get());
 
     rv = trans->Start(&request, callback.callback(), net::BoundNetLog());
     if (rv == net::ERR_IO_PENDING)
       rv = callback.WaitForResult();
     ASSERT_EQ(net::OK, rv);
 
     const net::HttpResponseInfo* info = trans->GetResponseInfo();
     ASSERT_TRUE(info);
 
     EXPECT_EQ(info->headers->response_code(), 301);
 
     std::string location;
     info->headers->EnumerateHeader(NULL, "Location", &location);
     EXPECT_EQ(location, "http://www.bar.com/");
 
+    // Mark the transaction as completed so it is cached.
+    trans->DoneReading();
+
     // Destroy transaction when going out of scope. We have not actually
     // read the response body -- want to test that it is still getting cached.
   }
   EXPECT_EQ(1, cache.network_layer()->transaction_count());
   EXPECT_EQ(1, cache.disk_cache()->open_count());
   EXPECT_EQ(1, cache.disk_cache()->create_count());
 }
 
 // Verify that no-cache resources are stored in cache, but are not fetched from
 // cache during normal loads.
@@ skipping 765 matching lines @@
   trans->SetPriority(net::HIGHEST);
   // Should trigger a new network transaction and pick up the new
   // priority.
   ReadAndVerifyTransaction(trans.get(), transaction);
 
   EXPECT_EQ(net::HIGHEST,
             cache.network_layer()->last_create_transaction_priority());
 
   RemoveMockTransaction(&kRangeGET_TransactionOK);
 }