Remove void** from disk_cache interface.

net/disk_cache/disk_cache.h

Index: net/disk_cache/disk_cache.h
diff --git a/net/disk_cache/disk_cache.h b/net/disk_cache/disk_cache.h
index 4f64b9afb02ac4219532491f592d0c7b269985fb..4016dd3625b69b814567a4368bec0e312ed01e52 100644
--- a/net/disk_cache/disk_cache.h
+++ b/net/disk_cache/disk_cache.h
@@ -13,6 +13,7 @@
 
 #include "base/basictypes.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/time/time.h"
 #include "net/base/cache_type.h"
 #include "net/base/completion_callback.h"
@@ -64,13 +65,28 @@ class NET_EXPORT Backend {
  public:
   typedef net::CompletionCallback CompletionCallback;
 
+  class Iterator {
+   public:
+    virtual ~Iterator();
+
+    // OpenNextEntry returns |net::OK| and provides |next_entry| if there is an

[pasko, 2014/09/12 08:10:26]

Please briefly mention net::ERR_IO_PENDING behavior.

Please also document the lifetime constraints of the Iterator. Is it  required
that the iterator is functional after the backend is destructed?

[gavinp, 2014/09/12 14:15:32]

Done.

+    // entry to enumerate. It returns |net::ERR_FAILED| at the end of
+    // enumeration. If any entry in the cache is modified during iteration, the
+    // result of this function is thereafter undefined.
+    //
+    // Some cache backends make stronger guarantees about mutation during
+    // iteration, see top comment in simple_backend_impl.h for details.
+    virtual int OpenNextEntry(Entry** next_entry,
+                              const CompletionCallback& callback) = 0;
+  };
+
   // If the backend is destroyed when there are operations in progress (any
   // callback that has not been invoked yet), this method cancels said
   // operations so the callbacks are not invoked, possibly leaving the work
   // half way (for instance, dooming just a few entries). Note that pending IO
   // for a given Entry (as opposed to the Backend) will still generate a
   // callback from within this method.
-  virtual ~Backend() {}
+  virtual ~Backend();
 
   // Returns the type of this cache.
   virtual net::CacheType GetCacheType() const = 0;
@@ -123,31 +139,9 @@ class NET_EXPORT Backend {
   virtual int DoomEntriesSince(base::Time initial_time,
                                const CompletionCallback& callback) = 0;
 
-  // Enumerates the cache. Initialize |iter| to NULL before calling this method
-  // the first time. That will cause the enumeration to start at the head of
-  // the cache. For subsequent calls, pass the same |iter| pointer again without
-  // changing its value. This method returns ERR_FAILED when there are no more
-  // entries to enumerate. When the entry pointer is no longer needed, its
-  // Close method should be called. The return value is a net error code. If
-  // this method returns ERR_IO_PENDING, the |callback| will be invoked when the
-  // |next_entry| is available. The pointer to receive the |next_entry| must
-  // remain valid until the operation completes.
-  //
-  // NOTE: This method does not modify the last_used field of the entry, and

[pasko, 2014/09/12 08:10:26]

this comment clarifies some undesired behavior of the blockfile cache. It's not
doing so very clearly, but it's still better than nothing. Please move it to
net/disk_cache/blockfile/backend_impl.h

[gavinp, 2014/09/12 14:15:32]

Done. The revised comment in disk_cache.h summarizes this section as "If any
entry of the cache is modified during iteration, the result of this function is
thereafter undefined", which felt like about as much information as an API
consumer needs.

I've cut and pasted the longer original version in to disk_cache.h as you
suggested, editing only for the initial pronoun "This method" and reformatting
to 80 columns after that change.

-  // therefore it does not impact the eviction ranking of the entry. However,
-  // an enumeration will go through all entries on the cache only if the cache
-  // is not modified while the enumeration is taking place. Significantly
-  // altering the entry pointed by |iter| (for example, deleting the entry) will
-  // invalidate |iter|. Performing operations on an entry that modify the entry
-  // may result in loops in the iteration, skipped entries or similar.
-  virtual int OpenNextEntry(void** iter, Entry** next_entry,
-                            const CompletionCallback& callback) = 0;
-
-  // Releases iter without returning the next entry. Whenever OpenNextEntry()
-  // returns true, but the caller is not interested in continuing the
-  // enumeration by calling OpenNextEntry() again, the enumeration must be
-  // ended by calling this method with iter returned by OpenNextEntry().
-  virtual void EndEnumeration(void** iter) = 0;
+  // Returns an iterator which will enumerate all entries of the cache in an
+  // undefined order.
+  virtual scoped_ptr<Iterator> CreateIterator() = 0;
 
   // Return a list of cache statistics.
   virtual void GetStats(
@@ -310,7 +304,7 @@ class NET_EXPORT Entry {
   virtual int ReadyForSparseIO(const CompletionCallback& callback) = 0;
 
  protected:
-  virtual ~Entry() {}
+  virtual ~Entry();
 };
 
 struct EntryDeleter {