2 files changed, 80 insertions, 123 deletions

diff --git a/net/disk_cache/simple/simple_backend_impl.cc b/net/disk_cache/simple/simple_backend_impl.cc
index 2e16576..e84211b 100644
--- a/net/disk_cache/simple/simple_backend_impl.cc
+++ b/net/disk_cache/simple/simple_backend_impl.cc
@@ -473,18 +473,78 @@ int SimpleBackendImpl::DoomEntriesSince(
   return DoomEntriesBetween(initial_time, Time(), callback);
 }
 
-int SimpleBackendImpl::OpenNextEntry(void** iter,
-                                     Entry** next_entry,
-                                     const CompletionCallback& callback) {
-  CompletionCallback get_next_entry =
-      base::Bind(&SimpleBackendImpl::GetNextEntryInIterator, AsWeakPtr(), iter,
-                 next_entry, callback);
-  return index_->ExecuteWhenReady(get_next_entry);
-}
+class SimpleBackendImpl::SimpleIterator FINAL : public Iterator {
+ public:
+  explicit SimpleIterator(base::WeakPtr<SimpleBackendImpl> backend)
+      : backend_(backend),
+        weak_factory_(this) {
+  }
+
+  // From Backend::Iterator:
+  virtual int OpenNextEntry(Entry** next_entry,
+                            const CompletionCallback& callback) OVERRIDE {
+    CompletionCallback open_next_entry_impl =
+        base::Bind(&SimpleIterator::OpenNextEntryImpl,
+                   weak_factory_.GetWeakPtr(), next_entry, callback);
+    return backend_->index_->ExecuteWhenReady(open_next_entry_impl);
+  }
+
+  void OpenNextEntryImpl(Entry** next_entry,
+                         const CompletionCallback& callback,
+                         int index_initialization_error_code) {
+    if (!backend_) {
+      callback.Run(net::ERR_FAILED);
+      return;
+    }
+    if (index_initialization_error_code != net::OK) {
+      callback.Run(index_initialization_error_code);
+      return;
+    }
+    if (!hashes_to_enumerate_)
+      hashes_to_enumerate_ = backend_->index()->GetAllHashes().Pass();
+
+    while (!hashes_to_enumerate_->empty()) {
+      uint64 entry_hash = hashes_to_enumerate_->back();
+      hashes_to_enumerate_->pop_back();
+      if (backend_->index()->Has(entry_hash)) {
+        *next_entry = NULL;
+        CompletionCallback continue_iteration = base::Bind(
+            &SimpleIterator::CheckIterationReturnValue,
+            weak_factory_.GetWeakPtr(),
+            next_entry,
+            callback);
+        int error_code_open = backend_->OpenEntryFromHash(entry_hash,
+                                                          next_entry,
+                                                          continue_iteration);
+        if (error_code_open == net::ERR_IO_PENDING)
+          return;
+        if (error_code_open != net::ERR_FAILED) {
+          callback.Run(error_code_open);
+          return;
+        }
+      }
+    }
+    callback.Run(net::ERR_FAILED);
+  }
 
-void SimpleBackendImpl::EndEnumeration(void** iter) {
-  active_enumerations_.Remove(IteratorToEnumerationId(iter));
-  *iter = NULL;
+  void CheckIterationReturnValue(Entry** entry,
+                                 const CompletionCallback& callback,
+                                 int error_code) {
+    if (error_code == net::ERR_FAILED) {
+      OpenNextEntry(entry, callback);
+      return;
+    }
+    callback.Run(error_code);
+  }
+
+ private:
+  base::WeakPtr<SimpleBackendImpl> backend_;
+  scoped_ptr<std::vector<uint64> > hashes_to_enumerate_;
+  base::WeakPtrFactory<SimpleIterator> weak_factory_;
+};
+
+scoped_ptr<Backend::Iterator> SimpleBackendImpl::CreateIterator() {
+  return scoped_ptr<Iterator>(new SimpleIterator(AsWeakPtr()));
 }
 
 void SimpleBackendImpl::GetStats(
@@ -499,27 +559,6 @@ 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) {
@@ -633,49 +672,6 @@ int SimpleBackendImpl::DoomEntryFromHash(uint64 entry_hash,
   return net::ERR_IO_PENDING;
 }
 
-void SimpleBackendImpl::GetNextEntryInIterator(
-    void** iter,
-    Entry** next_entry,
-    const CompletionCallback& callback,
-    int error_code) {
-  if (error_code != net::OK) {
-    callback.Run(error_code);
-    return;
-  }
-  std::vector<uint64>* entry_list = NULL;
-  if (*iter == NULL) {
-    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));
-  }
-  while (entry_list->size() > 0) {
-    uint64 entry_hash = entry_list->back();
-    entry_list->pop_back();
-    if (index()->Has(entry_hash)) {
-      *next_entry = NULL;
-      CompletionCallback continue_iteration = base::Bind(
-          &SimpleBackendImpl::CheckIterationReturnValue,
-          AsWeakPtr(),
-          iter,
-          next_entry,
-          callback);
-      int error_code_open = OpenEntryFromHash(entry_hash,
-                                              next_entry,
-                                              continue_iteration);
-      if (error_code_open == net::ERR_IO_PENDING)
-        return;
-      if (error_code_open != net::ERR_FAILED) {
-        callback.Run(error_code_open);
-        return;
-      }
-    }
-  }
-  callback.Run(net::ERR_FAILED);
-}
-
 void SimpleBackendImpl::OnEntryOpenedFromHash(
     uint64 hash,
     Entry** entry,
@@ -729,18 +725,6 @@ void SimpleBackendImpl::OnEntryOpenedFromKey(
   callback.Run(final_code);
 }
 
-void SimpleBackendImpl::CheckIterationReturnValue(
-    void** iter,
-    Entry** entry,
-    const CompletionCallback& callback,
-    int error_code) {
-  if (error_code == net::ERR_FAILED) {
-    OpenNextEntry(iter, entry, callback);
-    return;
-  }
-  callback.Run(error_code);
-}
-
 void SimpleBackendImpl::DoomEntriesComplete(
     scoped_ptr<std::vector<uint64> > entry_hashes,
     const net::CompletionCallback& callback,
diff --git a/net/disk_cache/simple/simple_backend_impl.h b/net/disk_cache/simple/simple_backend_impl.h
index 907ee5a..48c422f 100644
--- a/net/disk_cache/simple/simple_backend_impl.h
+++ b/net/disk_cache/simple/simple_backend_impl.h
@@ -13,7 +13,6 @@
 #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"
@@ -35,6 +34,11 @@ namespace disk_cache {
 // files.
 // See http://www.chromium.org/developers/design-documents/network-stack/disk-cache/very-simple-backend
 //
+// The SimpleBackendImpl provides safe iteration; mutating entries during
+// iteration cannot cause a crash. It is undefined whether entries created or
+// destroyed during the iteration will be included in any pre-existing
+// iterations.
+//
 // The non-static functions below must be called on the IO thread unless
 // otherwise stated.
 
@@ -98,17 +102,16 @@ class NET_EXPORT_PRIVATE SimpleBackendImpl : public Backend,
                                  const CompletionCallback& callback) OVERRIDE;
   virtual int DoomEntriesSince(base::Time initial_time,
                                const CompletionCallback& callback) OVERRIDE;
-  virtual int OpenNextEntry(void** iter, Entry** next_entry,
-                            const CompletionCallback& callback) OVERRIDE;
-  virtual void EndEnumeration(void** iter) OVERRIDE;
+  virtual scoped_ptr<Iterator> CreateIterator() OVERRIDE;
   virtual void GetStats(
       std::vector<std::pair<std::string, std::string> >* stats) OVERRIDE;
   virtual void OnExternalCacheHit(const std::string& key) OVERRIDE;
 
  private:
-  typedef base::hash_map<uint64, SimpleEntryImpl*> EntryMap;
+  class SimpleIterator;
+  friend class SimpleIterator;
 
-  typedef IDMap<std::vector<uint64>, IDMapOwnPointer> ActiveEnumerationMap;
+  typedef base::hash_map<uint64, SimpleEntryImpl*> EntryMap;
 
   typedef base::Callback<void(base::Time mtime, uint64 max_size, int result)>
       InitializeIndexCallback;
@@ -124,17 +127,6 @@ 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);
 
@@ -169,14 +161,6 @@ class NET_EXPORT_PRIVATE SimpleBackendImpl : public Backend,
   // which is very important to prevent races in DoomEntries() above.
   int DoomEntryFromHash(uint64 entry_hash, const CompletionCallback & callback);
 
-  // Called when the index is initilized to find the next entry in the iterator
-  // |iter|. If there are no more hashes in the iterator list, net::ERR_FAILED
-  // is returned. Otherwise, calls OpenEntryFromHash.
-  void GetNextEntryInIterator(void** iter,
-                              Entry** next_entry,
-                              const CompletionCallback& callback,
-                              int error_code);
-
   // Called when we tried to open an entry with hash alone. When a blank entry
   // has been created and filled in with information from the disk - based on a
   // hash alone - this checks that a duplicate active entry was not created
@@ -195,14 +179,6 @@ class NET_EXPORT_PRIVATE SimpleBackendImpl : public Backend,
                             const CompletionCallback& callback,
                             int error_code);
 
-  // Called at the end of the asynchronous operation triggered by
-  // OpenEntryFromHash. Makes sure to continue iterating if the open entry was
-  // not a success.
-  void CheckIterationReturnValue(void** iter,
-                                 Entry** entry,
-                                 const CompletionCallback& callback,
-                                 int error_code);
-
   // A callback thunk used by DoomEntries to clear the |entries_pending_doom_|
   // after a mass doom.
   void DoomEntriesComplete(scoped_ptr<std::vector<uint64> > entry_hashes,
@@ -220,9 +196,6 @@ 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