Http cache: Don't delete sparse entries when we

cancel the request.

BUG=20930
TEST=unittests

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

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

2 files changed, 54 insertions, 11 deletions

diff --git a/net/http/http_cache.cc b/net/http/http_cache.cc
index 35d51478..9cb9786 100644
--- a/net/http/http_cache.cc
+++ b/net/http/http_cache.cc
@@ -253,8 +253,9 @@ class HttpCache::Transaction
   int EntryAvailable(ActiveEntry* entry);
 
   // This transaction is being deleted and we are not done writing to the cache.
-  // We need to indicate that the response data was truncated.
-  void AddTruncatedFlag();
+  // We need to indicate that the response data was truncated.  Returns true on
+  // success.
+  bool AddTruncatedFlag();
 
  private:
   // This is a helper function used to trigger a completion callback.  It may
@@ -408,9 +409,9 @@ class HttpCache::Transaction
 HttpCache::Transaction::~Transaction() {
   if (!revoked()) {
     if (entry_) {
-      bool cancel_request = reading_ && !partial_.get() &&
-                            enable_range_support_ &&
-                            response_.headers->response_code() == 200;
+      bool cancel_request = reading_ && enable_range_support_;
+      if (cancel_request && !partial_.get())
+        cancel_request &= (response_.headers->response_code() == 200);
 
       cache_->DoneWithEntry(entry_, this, cancel_request);
     } else {
@@ -719,10 +720,20 @@ int HttpCache::Transaction::EntryAvailable(ActiveEntry* entry) {
   return rv;
 }
 
-void HttpCache::Transaction::AddTruncatedFlag() {
+bool HttpCache::Transaction::AddTruncatedFlag() {
   DCHECK(mode_ & WRITE);
+
+  // Don't set the flag for sparse entries.
+  if (partial_.get())
+    return true;
+
+  // Double check that there is something worth keeping.
+  if (!entry_->disk_entry->GetDataSize(kResponseContentIndex))
+    return false;
+
   truncated_ = true;
   WriteResponseInfoToEntry(true);
+  return true;
 }
 
 void HttpCache::Transaction::DoCallback(int rv) {
@@ -2003,11 +2014,7 @@ void HttpCache::DoneWithEntry(ActiveEntry* entry, Transaction* trans,
       DCHECK(entry->disk_entry);
       // This is a successful operation in the sense that we want to keep the
       // entry.
-      success = true;
-      // Double check that there is something worth keeping.
-      if (!entry->disk_entry->GetDataSize(kResponseContentIndex))
-        success = false;
-      trans->AddTruncatedFlag();
+      success = trans->AddTruncatedFlag();
     }
     DoneWritingToEntry(entry, success);
   } else {
diff --git a/net/http/http_cache_unittest.cc b/net/http/http_cache_unittest.cc
index f2acda9..aada69b 100644
--- a/net/http/http_cache_unittest.cc
+++ b/net/http/http_cache_unittest.cc
@@ -2298,6 +2298,42 @@ TEST(HttpCache, RangeGET_MoreThanCurrentSize) {
   RemoveMockTransaction(&kRangeGET_TransactionOK);
 }
 
+// Tests that we don't delete a sparse entry when we cancel a request.
+TEST(HttpCache, RangeGET_Cancel) {
+  MockHttpCache cache;
+  cache.http_cache()->set_enable_range_support(true);
+  AddMockTransaction(&kRangeGET_TransactionOK);
+
+  MockHttpRequest request(kRangeGET_TransactionOK);
+
+  Context* c =  new Context(cache.http_cache()->CreateTransaction());
+
+  int rv = c->trans->Start(&request, &c->callback, NULL);
+  if (rv == net::ERR_IO_PENDING)
+    rv = c->callback.WaitForResult();
+
+  EXPECT_EQ(1, cache.network_layer()->transaction_count());
+  EXPECT_EQ(0, cache.disk_cache()->open_count());
+  EXPECT_EQ(1, cache.disk_cache()->create_count());
+
+  // Make sure that the entry has some data stored.
+  scoped_refptr<net::IOBufferWithSize> buf = new net::IOBufferWithSize(10);
+  rv = c->trans->Read(buf, buf->size(), &c->callback);
+  if (rv == net::ERR_IO_PENDING)
+    rv = c->callback.WaitForResult();
+  EXPECT_EQ(buf->size(), rv);
+
+  // Destroy the transaction.
+  delete c;
+
+  // Verify that the entry has not been deleted.
+  disk_cache::Entry* entry;
+  ASSERT_TRUE(cache.disk_cache()->OpenEntry(kRangeGET_TransactionOK.url,
+                                            &entry));
+  entry->Close();
+  RemoveMockTransaction(&kRangeGET_TransactionOK);
+}
+
 #ifdef NDEBUG
 // This test hits a NOTREACHED so it is a release mode only test.
 TEST(HttpCache, RangeGET_OK_LoadOnlyFromCache) {