Remove void** from disk_cache interface.

net/disk_cache/blockfile/backend_impl.cc

 // 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.
 
 #include "net/disk_cache/blockfile/backend_impl.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/file_util.h"
 #include "base/files/file.h"
@@ skipping 1181 matching lines @@
   return net::ERR_IO_PENDING;
 }
 
 int BackendImpl::DoomEntriesSince(const base::Time initial_time,
                                   const CompletionCallback& callback) {
   DCHECK(!callback.is_null());
   background_queue_.DoomEntriesSince(initial_time, callback);
   return net::ERR_IO_PENDING;
 }
 
-int BackendImpl::OpenNextEntry(void** iter, Entry** next_entry,
+int BackendImpl::OpenNextEntry(Iterator* iter, Entry** next_entry,
                                const CompletionCallback& callback) {
+  // TODO(gavinp): Remove all void** iter from cache.
   DCHECK(!callback.is_null());
-  background_queue_.OpenNextEntry(iter, next_entry, callback);
+  class State : public EnumerationState {

[clamy, 2014/09/04 18:22:01]

Maybe it's just me, but I find class definitions inside a function a bit hard to
read. Especially with a function argument called iter, and a class member iter_
- which are quite different, as far as I can tell.

[gavinp, 2014/09/04 18:31:46]

As it happens, I had to move this out of the function in BackendImpl, in order
to support building the dump_cache --upgrade feature, a windows only
functionality of dump_cache that depends on BackendImpl::OpenPrevEntry in the
blockfile backend.

So perhaps you'll like the latest upload; I renamed the confusing iter method
and member to the less confusing "data", which seems like it describes a void**
just fine.

I slightly prefer declaring these classes inside the functions; the reason is
just that we're doing some crazy stuff with static casts, and declaring it in
the function makes a strong statement about where you can expect that to occur,
and where the Backend::Iterator() will obey its normal interface. Do you have
the same concerns about the maybe more readable call sites in SimpleBackendImpl
or MemoryBackendImpl?

[clamy, 2014/09/04 19:01:37]

Yes the version with "data" is easier to follow. I think it should be fine for
the other call sites.

By the way, why is the upgrade feature using OpenPrevEntry instead of
OpenNextEntry? And is it really used?

+   public:
+    explicit State(base::WeakPtr<InFlightBackendIO> background_queue)
+        : background_queue_(background_queue),
+          iter_(NULL) {}
+
+    virtual ~State() {
+      if (background_queue_)
+        background_queue_->EndEnumeration(*iter());
+    }
+
+    void** iter() { return &iter_; }
+
+   private:
+    base::WeakPtr<InFlightBackendIO> background_queue_;
+    void* iter_;
+  };
+  if (!*iter)
+    iter->reset(new State(GetBackgroundQueue()));
+  State* state = static_cast<State*>(iter->get());
+
+  background_queue_.OpenNextEntry(state->iter(), next_entry, callback);
   return net::ERR_IO_PENDING;
 }
 
-void BackendImpl::EndEnumeration(void** iter) {
-  background_queue_.EndEnumeration(*iter);
-  *iter = NULL;
-}
-
 void BackendImpl::GetStats(StatsItems* stats) {
   if (disabled_)
     return;
 
   std::pair<std::string, std::string> item;
 
   item.first = "Entries";
   item.second = base::StringPrintf("%d", data_->header.num_entries);
   stats->push_back(item);
 
@@ skipping 871 matching lines @@
     if (total_memory > kMaxBuffersSize || total_memory <= 0)
       total_memory = kMaxBuffersSize;
 
     done = true;
   }
 
   return static_cast<int>(total_memory);
 }
 
 }  // namespace disk_cache