Remove void** from disk_cache interface.

net/disk_cache/backend_unittest.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 "base/basictypes.h"
 #include "base/files/file_util.h"
 #include "base/metrics/field_trial.h"
 #include "base/port.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
@@ skipping 63 matching lines @@
   // Perform IO operations on the cache until there is pending IO.
   int GeneratePendingIO(net::TestCompletionCallback* cb);
 
   // Adds 5 sparse entries. |doomed_start| and |doomed_end| if not NULL,
   // will be filled with times, used by DoomEntriesSince and DoomEntriesBetween.
   // There are 4 entries after doomed_start and 2 after doomed_end.
   void InitSparseCache(base::Time* doomed_start, base::Time* doomed_end);
 
   bool CreateSetOfRandomEntries(std::set<std::string>* key_pool);
   bool EnumerateAndMatchKeys(int max_to_open,
-                             void** iter,
+                             const scoped_ptr<TestIterator>& iter,

[rvargas (doing something else), 2014/09/18 02:32:11]

This should not be a const ref as conceptually this method modifies the
argument. (the argument being the iterator, not the scoped_pointer)

[gavinp, 2014/09/18 18:13:04]

Hmm, but that's not what const means though, is it? You're thinking perhaps of a
scoped_ptr<const TestIterator>? The const in this case is promising that
EnumerateAndMatchKeys will not delete iter or otherwise mutate what iter points
to, not that it won't modify the pointed at value. It's akin to:

  TestIterator* const& ref_to_const_ptr_to_mutable_TestIterator;

and not as you might be suggesting above:

  const TestIterator*& ref_to_ptr_to_const_TestIterator;
  TestIterator const*& ditto;

The choices for arguments here were a few. Originally I wrote this like:

  bool EnumerateAndMatchKeys(int max_to_open, TestIterator* iter, ...);

But the frustration around that was that each call site had to call .get(),
making them a bit ugly. So by swallowing a bit of the confusing in this
signature, every call site was cleaner. My reading of
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Reference_Arg...
suggests that since this argument is in fact const, we should prefer passing it
as a reference.

Do you think we should change the argument type? One possibility that might be
clearer is "scoped_ptr<TestIterator> const&" which means the same thing. Another
is "TestIterator*", and I'm open to other suggestions?

[jkarlin, 2014/09/18 18:41:34]

The chromium guide says not to pass scoped_ptr by const ref toward the bottom:
http://www.chromium.org/developers/smart-pointer-guidelines

[gavinp, 2014/09/18 19:00:39]

Good catch. This forces it to TestIterator*.

I wonder why we don't programmatically enforce this (i.e. why is
scoped_ptr<>::get() a const method?), but that's a story for another day...

                              std::set<std::string>* keys_to_match,
                              size_t* count);
 
   // Actual tests:
   void BackendBasics();
   void BackendKeying();
   void BackendShutdownWithPendingFileIO(bool fast);
   void BackendShutdownWithPendingIO(bool fast);
   void BackendShutdownWithPendingCreate(bool fast);
   void BackendSetSize();
@@ skipping 154 matching lines @@
   }
   return key_pool->size() == implicit_cast<size_t>(cache_->GetEntryCount());
 }
 
 // Performs iteration over the backend and checks that the keys of entries
 // opened are in |keys_to_match|, then erases them. Up to |max_to_open| entries
 // will be opened, if it is positive. Otherwise, iteration will continue until
 // OpenNextEntry stops returning net::OK.
 bool DiskCacheBackendTest::EnumerateAndMatchKeys(
     int max_to_open,
-    void** iter,
+    const scoped_ptr<TestIterator>& iter,
     std::set<std::string>* keys_to_match,
     size_t* count) {
   disk_cache::Entry* entry;
 
-  while (OpenNextEntry(iter, &entry) == net::OK) {
+  if (!iter)
+    return false;
+  while (iter->OpenNextEntry(&entry) == net::OK) {
     if (!entry)
       return false;
     EXPECT_EQ(1U, keys_to_match->erase(entry->GetKey()));
     entry->Close();
     ++(*count);
     if (max_to_open >= 0 && implicit_cast<int>(*count) >= max_to_open)
       break;
   };
 
   return true;
@@ skipping 955 matching lines @@
   for (int i = 0; i < kNumEntries; i++) {
     std::string key = GenerateKey(true);
     disk_cache::Entry* entry;
     ASSERT_EQ(net::OK, CreateEntry(key, &entry));
     entry->Close();
   }
   EXPECT_EQ(kNumEntries, cache_->GetEntryCount());
   Time final = Time::Now();
 
   disk_cache::Entry* entry;
-  void* iter = NULL;
+  scoped_ptr<TestIterator> iter = CreateIterator();
   int count = 0;
   Time last_modified[kNumEntries];
   Time last_used[kNumEntries];
-  while (OpenNextEntry(&iter, &entry) == net::OK) {
+  while (iter->OpenNextEntry(&entry) == net::OK) {
     ASSERT_TRUE(NULL != entry);
     if (count < kNumEntries) {
       last_modified[count] = entry->GetLastModified();
       last_used[count] = entry->GetLastUsed();
       EXPECT_TRUE(initial <= last_modified[count]);
       EXPECT_TRUE(final >= last_modified[count]);
     }
 
     entry->Close();
     count++;
   };
   EXPECT_EQ(kNumEntries, count);
 
-  iter = NULL;
+  iter = CreateIterator();
   count = 0;
   // The previous enumeration should not have changed the timestamps.
-  while (OpenNextEntry(&iter, &entry) == net::OK) {
+  while (iter->OpenNextEntry(&entry) == net::OK) {
     ASSERT_TRUE(NULL != entry);
     if (count < kNumEntries) {
       EXPECT_TRUE(last_modified[count] == entry->GetLastModified());
       EXPECT_TRUE(last_used[count] == entry->GetLastUsed());
     }
     entry->Close();
     count++;
   };
   EXPECT_EQ(kNumEntries, count);
 }
@@ skipping 30 matching lines @@
   disk_cache::Entry *entry1, *entry2;
   ASSERT_EQ(net::OK, CreateEntry(first, &entry1));
   entry1->Close();
   ASSERT_EQ(net::OK, CreateEntry(second, &entry2));
   entry2->Close();
   FlushQueueForTest();
 
   // Make sure that the timestamp is not the same.
   AddDelay();
   ASSERT_EQ(net::OK, OpenEntry(second, &entry1));
-  void* iter = NULL;
-  ASSERT_EQ(net::OK, OpenNextEntry(&iter, &entry2));
+  scoped_ptr<TestIterator> iter = CreateIterator();
+  ASSERT_EQ(net::OK, iter->OpenNextEntry(&entry2));
   EXPECT_EQ(entry2->GetKey(), second);
 
   // Two entries and the iterator pointing at "first".
   entry1->Close();
   entry2->Close();
 
   // The iterator should still be valid, so we should not crash.
-  ASSERT_EQ(net::OK, OpenNextEntry(&iter, &entry2));
+  ASSERT_EQ(net::OK, iter->OpenNextEntry(&entry2));
   EXPECT_EQ(entry2->GetKey(), first);
   entry2->Close();
-  cache_->EndEnumeration(&iter);
+  iter = CreateIterator();
 
   // Modify the oldest entry and get the newest element.
   ASSERT_EQ(net::OK, OpenEntry(first, &entry1));
   EXPECT_EQ(0, WriteData(entry1, 0, 200, NULL, 0, false));
-  ASSERT_EQ(net::OK, OpenNextEntry(&iter, &entry2));
+  ASSERT_EQ(net::OK, iter->OpenNextEntry(&entry2));
   if (type_ == net::APP_CACHE) {
     // The list is not updated.
     EXPECT_EQ(entry2->GetKey(), second);
   } else {
     EXPECT_EQ(entry2->GetKey(), first);
   }
 
   entry1->Close();
   entry2->Close();
-  cache_->EndEnumeration(&iter);
 }
 
 TEST_F(DiskCacheBackendTest, Enumerations2) {
   BackendEnumerations2();
 }
 
 TEST_F(DiskCacheBackendTest, NewEvictionEnumerations2) {
   SetNewEviction();
   BackendEnumerations2();
 }
@@ skipping 34 matching lines @@
 
   FlushQueueForTest();
 
   // Make sure that the timestamp is not the same.
   AddDelay();
 
   // Read from the last item in the LRU.
   EXPECT_EQ(kSize, ReadData(entry1, 0, 0, buffer1.get(), kSize));
   entry1->Close();
 
-  void* iter = NULL;
-  ASSERT_EQ(net::OK, OpenNextEntry(&iter, &entry2));
+  scoped_ptr<TestIterator> iter = CreateIterator();
+  ASSERT_EQ(net::OK, iter->OpenNextEntry(&entry2));
   EXPECT_EQ(entry2->GetKey(), second);
   entry2->Close();
-  cache_->EndEnumeration(&iter);
 }
 
 #if !defined(LEAK_SANITIZER)
 // Verify handling of invalid entries while doing enumerations.
 // We'll be leaking memory from this test.
 void DiskCacheBackendTest::BackendInvalidEntryEnumeration() {
   InitCache();
 
   std::string key("Some key");
   disk_cache::Entry *entry, *entry1, *entry2;
   ASSERT_EQ(net::OK, CreateEntry(key, &entry1));
 
   const int kSize = 50;
   scoped_refptr<net::IOBuffer> buffer1(new net::IOBuffer(kSize));
   memset(buffer1->data(), 0, kSize);
   base::strlcpy(buffer1->data(), "And the data to save", kSize);
   EXPECT_EQ(kSize, WriteData(entry1, 0, 0, buffer1.get(), kSize, false));
   entry1->Close();
   ASSERT_EQ(net::OK, OpenEntry(key, &entry1));
   EXPECT_EQ(kSize, ReadData(entry1, 0, 0, buffer1.get(), kSize));
 
   std::string key2("Another key");
   ASSERT_EQ(net::OK, CreateEntry(key2, &entry2));
   entry2->Close();
   ASSERT_EQ(2, cache_->GetEntryCount());
 
   SimulateCrash();
 
-  void* iter = NULL;
+  scoped_ptr<TestIterator> iter = CreateIterator();
   int count = 0;
-  while (OpenNextEntry(&iter, &entry) == net::OK) {
+  while (iter->OpenNextEntry(&entry) == net::OK) {
     ASSERT_TRUE(NULL != entry);
     EXPECT_EQ(key2, entry->GetKey());
     entry->Close();
     count++;
   };
   EXPECT_EQ(1, count);
   EXPECT_EQ(1, cache_->GetEntryCount());
 }
 
 // We'll be leaking memory from this test.
@@ skipping 19 matching lines @@
   const int kNumEntries = 10;
   for (int i = 0; i < kNumEntries; i++) {
     std::string key = GenerateKey(true);
     disk_cache::Entry* entry;
     ASSERT_EQ(net::OK, CreateEntry(key, &entry));
     entry->Close();
   }
   EXPECT_EQ(kNumEntries, cache_->GetEntryCount());
 
   disk_cache::Entry *entry1, *entry2;
-  void* iter1 = NULL;
-  void* iter2 = NULL;
-  ASSERT_EQ(net::OK, OpenNextEntry(&iter1, &entry1));
+  scoped_ptr<TestIterator> iter1 = CreateIterator(), iter2 = CreateIterator();
+  ASSERT_EQ(net::OK, iter1->OpenNextEntry(&entry1));
   ASSERT_TRUE(NULL != entry1);
   entry1->Close();
   entry1 = NULL;
 
   // Let's go to the middle of the list.
   for (int i = 0; i < kNumEntries / 2; i++) {
     if (entry1)
       entry1->Close();
-    ASSERT_EQ(net::OK, OpenNextEntry(&iter1, &entry1));
+    ASSERT_EQ(net::OK, iter1->OpenNextEntry(&entry1));
     ASSERT_TRUE(NULL != entry1);
 
-    ASSERT_EQ(net::OK, OpenNextEntry(&iter2, &entry2));
+    ASSERT_EQ(net::OK, iter2->OpenNextEntry(&entry2));
     ASSERT_TRUE(NULL != entry2);
     entry2->Close();
   }
 
   // Messing up with entry1 will modify entry2->next.
   entry1->Doom();
-  ASSERT_EQ(net::OK, OpenNextEntry(&iter2, &entry2));
+  ASSERT_EQ(net::OK, iter2->OpenNextEntry(&entry2));
   ASSERT_TRUE(NULL != entry2);
 
   // The link entry2->entry1 should be broken.
   EXPECT_NE(entry2->GetKey(), entry1->GetKey());
   entry1->Close();
   entry2->Close();
 
   // And the second iterator should keep working.
-  ASSERT_EQ(net::OK, OpenNextEntry(&iter2, &entry2));
+  ASSERT_EQ(net::OK, iter2->OpenNextEntry(&entry2));
   ASSERT_TRUE(NULL != entry2);
   entry2->Close();
-
-  cache_->EndEnumeration(&iter1);
-  cache_->EndEnumeration(&iter2);
 }
 
 TEST_F(DiskCacheBackendTest, FixEnumerators) {
   BackendFixEnumerators();
 }
 
 TEST_F(DiskCacheBackendTest, NewEvictionFixEnumerators) {
   SetNewEviction();
   BackendFixEnumerators();
 }
@@ skipping 505 matching lines @@
 }
 
 // Tests that we don't crash or hang when enumerating this cache.
 void DiskCacheBackendTest::BackendInvalidEntry3() {
   SetMask(0x1);  // 2-entry table.
   SetMaxSize(0x3000);  // 12 kB.
   DisableFirstCleanup();
   InitCache();
 
   disk_cache::Entry* entry;
-  void* iter = NULL;
-  while (OpenNextEntry(&iter, &entry) == net::OK) {
+  scoped_ptr<TestIterator> iter = CreateIterator();
+  while (iter->OpenNextEntry(&entry) == net::OK) {
     entry->Close();
   }
 }
 
 TEST_F(DiskCacheBackendTest, InvalidEntry3) {
   ASSERT_TRUE(CopyTestCache("dirty_entry3"));
   BackendInvalidEntry3();
 }
 
 TEST_F(DiskCacheBackendTest, NewEvictionInvalidEntry3) {
@@ skipping 123 matching lines @@
   entry_impl->rankings()->Store();
   entry->Close();
   FlushQueueForTest();
   EXPECT_EQ(2, cache_->GetEntryCount());
 
   // This should detect the bad entry.
   EXPECT_NE(net::OK, OpenEntry(second, &entry));
   EXPECT_EQ(1, cache_->GetEntryCount());
 
   // We should delete the cache. The list still has a corrupt node.
-  void* iter = NULL;
-  EXPECT_NE(net::OK, OpenNextEntry(&iter, &entry));
+  scoped_ptr<TestIterator> iter = CreateIterator();
+  EXPECT_NE(net::OK, iter->OpenNextEntry(&entry));
   FlushQueueForTest();
   EXPECT_EQ(0, cache_->GetEntryCount());
 }
 
 TEST_F(DiskCacheBackendTest, InvalidEntry7) {
   BackendInvalidEntry7();
 }
 
 TEST_F(DiskCacheBackendTest, NewEvictionInvalidEntry7) {
   SetNewEviction();
@@ skipping 22 matching lines @@
   entry_impl->rankings()->Store();
   entry->Close();
   FlushQueueForTest();
   EXPECT_EQ(2, cache_->GetEntryCount());
 
   // This should detect the bad entry.
   EXPECT_NE(net::OK, OpenEntry(second, &entry));
   EXPECT_EQ(1, cache_->GetEntryCount());
 
   // We should not delete the cache.
-  void* iter = NULL;
-  ASSERT_EQ(net::OK, OpenNextEntry(&iter, &entry));
+  scoped_ptr<TestIterator> iter = CreateIterator();
+  ASSERT_EQ(net::OK, iter->OpenNextEntry(&entry));
   entry->Close();
-  EXPECT_NE(net::OK, OpenNextEntry(&iter, &entry));
+  EXPECT_NE(net::OK, iter->OpenNextEntry(&entry));
   EXPECT_EQ(1, cache_->GetEntryCount());
 }
 
 TEST_F(DiskCacheBackendTest, InvalidEntry8) {
   BackendInvalidEntry8();
 }
 
 TEST_F(DiskCacheBackendTest, NewEvictionInvalidEntry8) {
   SetNewEviction();
   BackendInvalidEntry8();
@@ skipping 26 matching lines @@
   EXPECT_EQ(2, cache_->GetEntryCount());
 
   if (eviction) {
     TrimForTest(false);
     EXPECT_EQ(1, cache_->GetEntryCount());
     TrimForTest(false);
     EXPECT_EQ(1, cache_->GetEntryCount());
   } else {
     // We should detect the problem through the list, but we should not delete
     // the entry, just fail the iteration.
-    void* iter = NULL;
-    EXPECT_NE(net::OK, OpenNextEntry(&iter, &entry));
+    scoped_ptr<TestIterator> iter = CreateIterator();
+    EXPECT_NE(net::OK, iter->OpenNextEntry(&entry));
 
     // Now a full iteration will work, and return one entry.
-    ASSERT_EQ(net::OK, OpenNextEntry(&iter, &entry));
+    ASSERT_EQ(net::OK, iter->OpenNextEntry(&entry));
     entry->Close();
-    EXPECT_NE(net::OK, OpenNextEntry(&iter, &entry));
+    EXPECT_NE(net::OK, iter->OpenNextEntry(&entry));
 
     // This should detect what's left of the bad entry.
     EXPECT_NE(net::OK, OpenEntry(second, &entry));
     EXPECT_EQ(2, cache_->GetEntryCount());
   }
   DisableIntegrityCheck();
 }
 
 TEST_F(DiskCacheBackendTest, InvalidEntry9) {
   BackendInvalidEntry9(false);
@@ skipping 50 matching lines @@
     TrimForTest(false);
     EXPECT_EQ(3, cache_->GetEntryCount());
     TrimForTest(false);
     EXPECT_EQ(2, cache_->GetEntryCount());
     TrimForTest(false);
     EXPECT_EQ(1, cache_->GetEntryCount());
   } else {
     // Detection order: third -> second -> first.
     // We should detect the problem through the list, but we should not delete
     // the entry.
-    void* iter = NULL;
-    ASSERT_EQ(net::OK, OpenNextEntry(&iter, &entry));
+    scoped_ptr<TestIterator> iter = CreateIterator();
+    ASSERT_EQ(net::OK, iter->OpenNextEntry(&entry));
     entry->Close();
-    ASSERT_EQ(net::OK, OpenNextEntry(&iter, &entry));
+    ASSERT_EQ(net::OK, iter->OpenNextEntry(&entry));
     EXPECT_EQ(first, entry->GetKey());
     entry->Close();
-    EXPECT_NE(net::OK, OpenNextEntry(&iter, &entry));
+    EXPECT_NE(net::OK, iter->OpenNextEntry(&entry));
   }
   DisableIntegrityCheck();
 }
 
 TEST_F(DiskCacheBackendTest, InvalidEntry10) {
   BackendInvalidEntry10(false);
 }
 
 TEST_F(DiskCacheBackendTest, TrimInvalidEntry10) {
   BackendInvalidEntry10(true);
@@ skipping 40 matching lines @@
     TrimForTest(false);
     EXPECT_EQ(2, cache_->GetEntryCount());
     TrimForTest(false);
     EXPECT_EQ(1, cache_->GetEntryCount());
     TrimForTest(false);
     EXPECT_EQ(1, cache_->GetEntryCount());
   } else {
     // Detection order: third -> second.
     // We should detect the problem through the list, but we should not delete
     // the entry, just fail the iteration.
-    void* iter = NULL;
-    ASSERT_EQ(net::OK, OpenNextEntry(&iter, &entry));
+    scoped_ptr<TestIterator> iter = CreateIterator();
+    ASSERT_EQ(net::OK, iter->OpenNextEntry(&entry));
     entry->Close();
-    EXPECT_NE(net::OK, OpenNextEntry(&iter, &entry));
+    EXPECT_NE(net::OK, iter->OpenNextEntry(&entry));
 
     // Now a full iteration will work, and return two entries.
-    ASSERT_EQ(net::OK, OpenNextEntry(&iter, &entry));
+    ASSERT_EQ(net::OK, iter->OpenNextEntry(&entry));
     entry->Close();
-    ASSERT_EQ(net::OK, OpenNextEntry(&iter, &entry));
+    ASSERT_EQ(net::OK, iter->OpenNextEntry(&entry));
     entry->Close();
-    EXPECT_NE(net::OK, OpenNextEntry(&iter, &entry));
+    EXPECT_NE(net::OK, iter->OpenNextEntry(&entry));
   }
   DisableIntegrityCheck();
 }
 
 TEST_F(DiskCacheBackendTest, InvalidEntry11) {
   BackendInvalidEntry11(false);
 }
 
 TEST_F(DiskCacheBackendTest, TrimInvalidEntry11) {
   BackendInvalidEntry11(true);
@@ skipping 57 matching lines @@
 }
 
 TEST_F(DiskCacheBackendTest, NewEvictionInvalidRankings2) {
   SetNewEviction();
   BackendInvalidRankings2();
 }
 
 // If the LRU is corrupt, we delete the cache.
 void DiskCacheBackendTest::BackendInvalidRankings() {
   disk_cache::Entry* entry;
-  void* iter = NULL;
-  ASSERT_EQ(net::OK, OpenNextEntry(&iter, &entry));
+  scoped_ptr<TestIterator> iter = CreateIterator();
+  ASSERT_EQ(net::OK, iter->OpenNextEntry(&entry));
   entry->Close();
   EXPECT_EQ(2, cache_->GetEntryCount());
 
-  EXPECT_NE(net::OK, OpenNextEntry(&iter, &entry));
+  EXPECT_NE(net::OK, iter->OpenNextEntry(&entry));
   FlushQueueForTest();  // Allow the restart to finish.
   EXPECT_EQ(0, cache_->GetEntryCount());
 }
 
 TEST_F(DiskCacheBackendTest, InvalidRankingsSuccess) {
   ASSERT_TRUE(CopyTestCache("bad_rankings"));
   DisableFirstCleanup();
   InitCache();
   BackendInvalidRankings();
 }
@@ skipping 19 matching lines @@
   DisableFirstCleanup();
   SetNewEviction();
   InitCache();
   SetTestMode();  // Fail cache reinitialization.
   BackendInvalidRankings();
 }
 
 // If the LRU is corrupt and we have open entries, we disable the cache.
 void DiskCacheBackendTest::BackendDisable() {
   disk_cache::Entry *entry1, *entry2;
-  void* iter = NULL;
-  ASSERT_EQ(net::OK, OpenNextEntry(&iter, &entry1));
+  scoped_ptr<TestIterator> iter = CreateIterator();
+  ASSERT_EQ(net::OK, iter->OpenNextEntry(&entry1));
 
-  EXPECT_NE(net::OK, OpenNextEntry(&iter, &entry2));
+  EXPECT_NE(net::OK, iter->OpenNextEntry(&entry2));
   EXPECT_EQ(0, cache_->GetEntryCount());
   EXPECT_NE(net::OK, CreateEntry("Something new", &entry2));
 
   entry1->Close();
   FlushQueueForTest();  // Flushing the Close posts a task to restart the cache.
   FlushQueueForTest();  // This one actually allows that task to complete.
 
   EXPECT_EQ(0, cache_->GetEntryCount());
 }
 
@@ skipping 27 matching lines @@
   InitCache();
   SetTestMode();  // Fail cache reinitialization.
   BackendDisable();
 }
 
 // This is another type of corruption on the LRU; disable the cache.
 void DiskCacheBackendTest::BackendDisable2() {
   EXPECT_EQ(8, cache_->GetEntryCount());
 
   disk_cache::Entry* entry;
-  void* iter = NULL;
+  scoped_ptr<TestIterator> iter = CreateIterator();
   int count = 0;
-  while (OpenNextEntry(&iter, &entry) == net::OK) {
+  while (iter->OpenNextEntry(&entry) == net::OK) {
     ASSERT_TRUE(NULL != entry);
     entry->Close();
     count++;
     ASSERT_LT(count, 9);
   };
 
   FlushQueueForTest();
   EXPECT_EQ(0, cache_->GetEntryCount());
 }
 
@@ skipping 25 matching lines @@
   DisableFirstCleanup();
   SetNewEviction();
   InitCache();
   SetTestMode();  // Fail cache reinitialization.
   BackendDisable2();
 }
 
 // If the index size changes when we disable the cache, we should not crash.
 void DiskCacheBackendTest::BackendDisable3() {
   disk_cache::Entry *entry1, *entry2;
-  void* iter = NULL;
+  scoped_ptr<TestIterator> iter = CreateIterator();
   EXPECT_EQ(2, cache_->GetEntryCount());
-  ASSERT_EQ(net::OK, OpenNextEntry(&iter, &entry1));
+  ASSERT_EQ(net::OK, iter->OpenNextEntry(&entry1));
   entry1->Close();
 
-  EXPECT_NE(net::OK, OpenNextEntry(&iter, &entry2));
+  EXPECT_NE(net::OK, iter->OpenNextEntry(&entry2));
   FlushQueueForTest();
 
   ASSERT_EQ(net::OK, CreateEntry("Something new", &entry2));
   entry2->Close();
 
   EXPECT_EQ(1, cache_->GetEntryCount());
 }
 
 TEST_F(DiskCacheBackendTest, DisableSuccess3) {
   ASSERT_TRUE(CopyTestCache("bad_rankings2"));
   DisableFirstCleanup();
   SetMaxSize(20 * 1024 * 1024);
   InitCache();
   BackendDisable3();
 }
 
 TEST_F(DiskCacheBackendTest, NewEvictionDisableSuccess3) {
   ASSERT_TRUE(CopyTestCache("bad_rankings2"));
   DisableFirstCleanup();
   SetMaxSize(20 * 1024 * 1024);
   SetNewEviction();
   InitCache();
   BackendDisable3();
 }
 
 // If we disable the cache, already open entries should work as far as possible.
 void DiskCacheBackendTest::BackendDisable4() {
   disk_cache::Entry *entry1, *entry2, *entry3, *entry4;
-  void* iter = NULL;
-  ASSERT_EQ(net::OK, OpenNextEntry(&iter, &entry1));
+  scoped_ptr<TestIterator> iter = CreateIterator();
+  ASSERT_EQ(net::OK, iter->OpenNextEntry(&entry1));
 
   char key2[2000];
   char key3[20000];
   CacheTestFillBuffer(key2, sizeof(key2), true);
   CacheTestFillBuffer(key3, sizeof(key3), true);
   key2[sizeof(key2) - 1] = '\0';
   key3[sizeof(key3) - 1] = '\0';
   ASSERT_EQ(net::OK, CreateEntry(key2, &entry2));
   ASSERT_EQ(net::OK, CreateEntry(key3, &entry3));
 
   const int kBufSize = 20000;
   scoped_refptr<net::IOBuffer> buf(new net::IOBuffer(kBufSize));
   memset(buf->data(), 0, kBufSize);
   EXPECT_EQ(100, WriteData(entry2, 0, 0, buf.get(), 100, false));
   EXPECT_EQ(kBufSize, WriteData(entry3, 0, 0, buf.get(), kBufSize, false));
 
   // This line should disable the cache but not delete it.
-  EXPECT_NE(net::OK, OpenNextEntry(&iter, &entry4));
+  EXPECT_NE(net::OK, iter->OpenNextEntry(&entry4));
   EXPECT_EQ(0, cache_->GetEntryCount());
 
   EXPECT_NE(net::OK, CreateEntry("cache is disabled", &entry4));
 
   EXPECT_EQ(100, ReadData(entry2, 0, 0, buf.get(), 100));
   EXPECT_EQ(100, WriteData(entry2, 0, 0, buf.get(), 100, false));
   EXPECT_EQ(100, WriteData(entry2, 1, 0, buf.get(), 100, false));
 
   EXPECT_EQ(kBufSize, ReadData(entry3, 0, 0, buf.get(), kBufSize));
   EXPECT_EQ(kBufSize, WriteData(entry3, 0, 0, buf.get(), kBufSize, false));
@@ skipping 660 matching lines @@
 // Tests basic functionality of the SimpleBackend implementation of the
 // enumeration API.
 TEST_F(DiskCacheBackendTest, SimpleCacheEnumerationBasics) {
   SetSimpleCacheMode();
   InitCache();
   std::set<std::string> key_pool;
   ASSERT_TRUE(CreateSetOfRandomEntries(&key_pool));
 
   // Check that enumeration returns all entries.
   std::set<std::string> keys_to_match(key_pool);
-  void* iter = NULL;
+  scoped_ptr<TestIterator> iter = CreateIterator();
   size_t count = 0;
-  ASSERT_TRUE(EnumerateAndMatchKeys(-1, &iter, &keys_to_match, &count));
-  cache_->EndEnumeration(&iter);
+  ASSERT_TRUE(EnumerateAndMatchKeys(-1, iter, &keys_to_match, &count));
+  iter.reset();
   EXPECT_EQ(key_pool.size(), count);
   EXPECT_TRUE(keys_to_match.empty());
 
   // Check that opening entries does not affect enumeration.
   keys_to_match = key_pool;
-  iter = NULL;
+  iter = CreateIterator();
   count = 0;
   disk_cache::Entry* entry_opened_before;
   ASSERT_EQ(net::OK, OpenEntry(*(key_pool.begin()), &entry_opened_before));
   ASSERT_TRUE(EnumerateAndMatchKeys(key_pool.size()/2,
-                                    &iter,
+                                    iter,
                                     &keys_to_match,
                                     &count));
 
   disk_cache::Entry* entry_opened_middle;
   ASSERT_EQ(net::OK,
             OpenEntry(*(keys_to_match.begin()), &entry_opened_middle));
-  ASSERT_TRUE(EnumerateAndMatchKeys(-1, &iter, &keys_to_match, &count));
-  cache_->EndEnumeration(&iter);
+  ASSERT_TRUE(EnumerateAndMatchKeys(-1, iter, &keys_to_match, &count));
+  iter.reset();
   entry_opened_before->Close();
   entry_opened_middle->Close();
 
   EXPECT_EQ(key_pool.size(), count);
   EXPECT_TRUE(keys_to_match.empty());
 }
 
 // Tests that the enumerations are not affected by dooming an entry in the
 // middle.
 TEST_F(DiskCacheBackendTest, SimpleCacheEnumerationWhileDoomed) {
   SetSimpleCacheMode();
   InitCache();
   std::set<std::string> key_pool;
   ASSERT_TRUE(CreateSetOfRandomEntries(&key_pool));
 
   // Check that enumeration returns all entries but the doomed one.
   std::set<std::string> keys_to_match(key_pool);
-  void* iter = NULL;
+  scoped_ptr<TestIterator> iter = CreateIterator();
   size_t count = 0;
   ASSERT_TRUE(EnumerateAndMatchKeys(key_pool.size()/2,
-                                    &iter,
+                                    iter,
                                     &keys_to_match,
                                     &count));
 
   std::string key_to_delete = *(keys_to_match.begin());
   DoomEntry(key_to_delete);
   keys_to_match.erase(key_to_delete);
   key_pool.erase(key_to_delete);
-  ASSERT_TRUE(EnumerateAndMatchKeys(-1, &iter, &keys_to_match, &count));
-  cache_->EndEnumeration(&iter);
+  ASSERT_TRUE(EnumerateAndMatchKeys(-1, iter, &keys_to_match, &count));
+  iter.reset();
 
   EXPECT_EQ(key_pool.size(), count);
   EXPECT_TRUE(keys_to_match.empty());
 }
 
 // Tests that enumerations are not affected by corrupt files.
 TEST_F(DiskCacheBackendTest, SimpleCacheEnumerationCorruption) {
   SetSimpleCacheMode();
   InitCache();
   std::set<std::string> key_pool;
@@ skipping 15 matching lines @@
   ASSERT_EQ(kSize, ReadData(corrupted_entry, 0, 0, buffer.get(), kSize));
   corrupted_entry->Close();
 
   EXPECT_TRUE(disk_cache::simple_util::CreateCorruptFileForTests(
       key, cache_path_));
   EXPECT_EQ(key_pool.size() + 1,
             implicit_cast<size_t>(cache_->GetEntryCount()));
 
   // Check that enumeration returns all entries but the corrupt one.
   std::set<std::string> keys_to_match(key_pool);
-  void* iter = NULL;
+  scoped_ptr<TestIterator> iter = CreateIterator();
   size_t count = 0;
-  ASSERT_TRUE(EnumerateAndMatchKeys(-1, &iter, &keys_to_match, &count));
-  cache_->EndEnumeration(&iter);
+  ASSERT_TRUE(EnumerateAndMatchKeys(-1, iter, &keys_to_match, &count));
+  iter.reset();
 
   EXPECT_EQ(key_pool.size(), count);
   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;
+  scoped_ptr<TestIterator> iter = CreateIterator();
   disk_cache::Entry* entry = NULL;
-  ASSERT_EQ(net::OK, OpenNextEntry(&iter, &entry));
+  ASSERT_EQ(net::OK, iter->OpenNextEntry(&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)