Remove void** from disk_cache interface.

net/disk_cache/disk_cache.h

 // 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.
 
 // Defines the public interface of the disk cache. For more details see
 // http://dev.chromium.org/developers/design-documents/network-stack/disk-cache
 
 #ifndef NET_DISK_CACHE_DISK_CACHE_H_
 #define NET_DISK_CACHE_DISK_CACHE_H_
 
 #include <string>
 #include <vector>
 
 #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"
 #include "net/base/net_export.h"
 
 namespace base {
 class FilePath;
 class SingleThreadTaskRunner;
 }
 
@@ skipping 29 matching lines @@
     int max_bytes,
     bool force,
     const scoped_refptr<base::SingleThreadTaskRunner>& thread,
     net::NetLog* net_log,
     scoped_ptr<Backend>* backend,
     const net::CompletionCallback& callback);
 
 // The root interface for a disk cache instance.
 class NET_EXPORT Backend {
  public:
+  class EnumerationState {
+   public:
+    virtual ~EnumerationState() = 0;
+  };
+  typedef scoped_ptr<EnumerationState> Iterator;
   typedef net::CompletionCallback CompletionCallback;
 
   // 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;
 
   // Returns the number of entries in the cache.
   virtual int32 GetEntryCount() const = 0;
 
   // Opens an existing entry. Upon success, |entry| holds a pointer to an Entry
   // object representing the specified disk cache entry. When the entry pointer
   // is no longer needed, its Close method should be called. The return value is
@@ skipping 32 matching lines @@
                                  base::Time end_time,
                                  const CompletionCallback& callback) = 0;
 
   // Marks all entries accessed since |initial_time| for deletion. The return
   // value is a net error code. If this method returns ERR_IO_PENDING, the
   // |callback| will be invoked when the operation completes.
   // Entries with |initial_time| <= access time are deleted.
   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
-  // 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,
+  // Enumerates the cache. Initialize |iter| to NULL for the first call, and
+  // provide it again for subsequent calls. For each successful step of
+  // enumeration, OpenNextEntry returns OK and provides |next_entry|, which is
+  // open and should be closed by the caller. This method returns ERR_FAILED at
+  // the end of enumeration. If any entry in a cache is modified during
+  // iteration, then the result of this function is thereafter undefined.

[pasko, 2014/09/05 09:53:50]

This explicit statement about undefined behavior basically means we cannot use
enumeration for anything except enumerating cache on the start, the way the
shader cache is using it. If shader cache is the only usecase for enumeration,
we may only have one memory leak per backend, right?

The original implementation by clamy@ made a snapshot of all entries in
StartEnumeration, and OpenNextEntry would iterate only through those in the
snapshot except the ones that were removed in the meantime, modulo hash
collisions. I am more comfortable with this definition being put as the API
spec, though I am not sure it is what the blockfile backend does. Simplecache
definition of iteration allows arbitrary number of concurrent iterations and
cache modifications. And I would argue that Camille's implementation for that is
pretty simple.

Big question: Do we plan to add more cache enumeration clients?

+  virtual int OpenNextEntry(Iterator* 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;
-
   // Return a list of cache statistics.
   virtual void GetStats(
       std::vector<std::pair<std::string, std::string> >* stats) = 0;
 
   // Called whenever an external cache in the system reuses the resource
   // referred to by |key|.
   virtual void OnExternalCacheHit(const std::string& key) = 0;
 };
 
 // This interface represents an entry in the disk cache.
@@ skipping 141 matching lines @@
   // OK although another IO operation cannot be issued at this time; in this
   // case the caller should just wait for the regular callback to be invoked
   // instead of using this method to provide another callback.
   //
   // Note that CancelSparseIO may have been called on another instance of this
   // object that refers to the same physical disk entry.
   // Note: This method is deprecated.
   virtual int ReadyForSparseIO(const CompletionCallback& callback) = 0;
 
  protected:
-  virtual ~Entry() {}
+  virtual ~Entry();
 };
 
 struct EntryDeleter {
   void operator()(Entry* entry) {
     // Note that |entry| is ref-counted.
     entry->Close();
   }
 };
 
 // Automatically closes an entry when it goes out of scope.
 typedef scoped_ptr<Entry, EntryDeleter> ScopedEntryPtr;
 
 }  // namespace disk_cache
 
 #endif  // NET_DISK_CACHE_DISK_CACHE_H_