Delete enumerations at Simple Cache backend deletion.

The memory allocated for an enumeration should be freed when a backend
is deleted.

R=clamy@chromium.org,jkarlin@chromium.org
BUG=410276

Review URL: https://codereview.chromium.org/533293002

Cr-Commit-Position: refs/heads/master@{#294705}

net/disk_cache/backend_unittest.cc

@@ -3473,4 +3473,22 @@ TEST_F(DiskCacheBackendTest, SimpleCacheEnumerationCorruption) {
   EXPECT_TRUE(keys_to_match.empty());
 }
 
+// Tests that enumerations don't leak memory when the backend is destructed
+// mid-enumeration.
+TEST_F(DiskCacheBackendTest, SimpleCacheEnumerationDestruction) {
+  SetSimpleCacheMode();
+  InitCache();
+  std::set<std::string> key_pool;
+  ASSERT_TRUE(CreateSetOfRandomEntries(&key_pool));
+
+  void* iter = NULL;
+  disk_cache::Entry* entry = NULL;
+  ASSERT_EQ(net::OK, OpenNextEntry(&iter, &entry));
+  EXPECT_TRUE(entry);
+  disk_cache::ScopedEntryPtr entry_closer(entry);
+
+  cache_.reset();
+  // This test passes if we don't leak memory.
+}
+
 #endif  // defined(OS_POSIX)

net/disk_cache/simple/simple_backend_impl.cc

@@ -483,9 +483,7 @@ int SimpleBackendImpl::OpenNextEntry(void** iter,
 }
 
 void SimpleBackendImpl::EndEnumeration(void** iter) {
-  SimpleIndex::HashList* entry_list =
-      static_cast<SimpleIndex::HashList*>(*iter);
-  delete entry_list;
+  active_enumerations_.Remove(IteratorToEnumerationId(iter));
   *iter = NULL;
 }
 
@@ -501,6 +499,27 @@ void SimpleBackendImpl::OnExternalCacheHit(const std::string& key) {
   index_->UseIfExists(simple_util::GetEntryHashKey(key));
 }
 
+// static
+SimpleBackendImpl::ActiveEnumerationMap::KeyType
+    SimpleBackendImpl::IteratorToEnumerationId(void** iter) {
+  COMPILE_ASSERT(sizeof(ptrdiff_t) >= sizeof(*iter),
+                 integer_type_must_fit_ptr_type_for_cast_to_be_reversible);
+  const ptrdiff_t ptrdiff_enumeration_id = reinterpret_cast<ptrdiff_t>(*iter);
+  const ActiveEnumerationMap::KeyType enumeration_id = ptrdiff_enumeration_id;
+  DCHECK_EQ(enumeration_id, ptrdiff_enumeration_id);
+  return enumeration_id;
+}
+
+// static
+void* SimpleBackendImpl::EnumerationIdToIterator(
+    ActiveEnumerationMap::KeyType enumeration_id) {
+  const ptrdiff_t ptrdiff_enumeration_id = enumeration_id;
+  DCHECK_EQ(enumeration_id, ptrdiff_enumeration_id);
+  COMPILE_ASSERT(sizeof(ptrdiff_t) >= sizeof(void*),
+                 integer_type_must_fit_ptr_type_for_cast_to_be_reversible);
+  return reinterpret_cast<void*>(ptrdiff_enumeration_id);
+}
+
 void SimpleBackendImpl::InitializeIndex(const CompletionCallback& callback,
                                         const DiskStatResult& result) {
   if (result.net_error == net::OK) {
@@ -623,11 +642,15 @@ void SimpleBackendImpl::GetNextEntryInIterator(
     callback.Run(error_code);
     return;
   }
+  std::vector<uint64>* entry_list = NULL;
   if (*iter == NULL) {
-    *iter = index()->GetAllHashes().release();
+    const ActiveEnumerationMap::KeyType new_enumeration_id =
+        active_enumerations_.Add(
+            entry_list = index()->GetAllHashes().release());
+    *iter = EnumerationIdToIterator(new_enumeration_id);
+  } else {
+    entry_list = active_enumerations_.Lookup(IteratorToEnumerationId(iter));
   }
-  SimpleIndex::HashList* entry_list =
-      static_cast<SimpleIndex::HashList*>(*iter);
   while (entry_list->size() > 0) {
     uint64 entry_hash = entry_list->back();
     entry_list->pop_back();

net/disk_cache/simple/simple_backend_impl.h

@@ -13,6 +13,7 @@
 #include "base/compiler_specific.h"
 #include "base/containers/hash_tables.h"
 #include "base/files/file_path.h"
+#include "base/id_map.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
@@ -107,6 +108,8 @@ class NET_EXPORT_PRIVATE SimpleBackendImpl : public Backend,
  private:
   typedef base::hash_map<uint64, SimpleEntryImpl*> EntryMap;
 
+  typedef IDMap<std::vector<uint64>, IDMapOwnPointer> ActiveEnumerationMap;
+
   typedef base::Callback<void(base::Time mtime, uint64 max_size, int result)>
       InitializeIndexCallback;
 
@@ -121,6 +124,17 @@ class NET_EXPORT_PRIVATE SimpleBackendImpl : public Backend,
     int net_error;
   };
 
+  // Convert an iterator from OpenNextEntry() to the key type for
+  // ActiveEnumerationMap. Note it takes a void** argument; this is for safety;
+  // if it took a void*, that would be type compatible with a void** permitting
+  // easy calls missing the dereference.
+  static ActiveEnumerationMap::KeyType IteratorToEnumerationId(void** iter);
+
+  // Convert a key from ActiveEnumerationMap back to a void*, suitable for
+  // storing in the iterator argument to OpenNextEntry().
+  static void* EnumerationIdToIterator(
+      ActiveEnumerationMap::KeyType enumeration_id);
+
   void InitializeIndex(const CompletionCallback& callback,
                        const DiskStatResult& result);
 
@@ -206,6 +220,9 @@ class NET_EXPORT_PRIVATE SimpleBackendImpl : public Backend,
 
   EntryMap active_entries_;
 
+  // One entry for every enumeration in progress.
+  ActiveEnumerationMap active_enumerations_;
+
   // The set of all entries which are currently being doomed. To avoid races,
   // these entries cannot have Doom/Create/Open operations run until the doom
   // is complete. The base::Closure map target is used to store deferred