Improved iterator for persistent memory allocator.

base/metrics/persistent_memory_allocator_unittest.cc

Index: base/metrics/persistent_memory_allocator_unittest.cc
diff --git a/base/metrics/persistent_memory_allocator_unittest.cc b/base/metrics/persistent_memory_allocator_unittest.cc
index b34cf14de315d1bc464565df624cb824736f5313..548652dbfe5ca1adf60efda19155bc0f3d7019f8 100644
--- a/base/metrics/persistent_memory_allocator_unittest.cc
+++ b/base/metrics/persistent_memory_allocator_unittest.cc
@@ -68,12 +68,11 @@ class PersistentMemoryAllocatorTest : public testing::Test {
   }
 
   unsigned CountIterables() {
-    PersistentMemoryAllocator::Iterator iter;
+    PersistentMemoryAllocator::Iterator iter(allocator_.get());
     uint32_t type;
     unsigned count = 0;
-    for (allocator_->CreateIterator(&iter);
-         allocator_->GetNextIterable(&iter, &type) != 0;) {
-      count++;
+    while (iter.GetNext(&type) != 0) {
+      ++count;
     }
     return count;
   }
@@ -114,14 +113,13 @@ TEST_F(PersistentMemoryAllocatorTest, AllocateAndIterate) {
   EXPECT_GT(meminfo0.free, meminfo1.free);
 
   // Ensure that the test-object can be made iterable.
-  PersistentMemoryAllocator::Iterator iter;
+  PersistentMemoryAllocator::Iterator iter1a(allocator_.get());
   uint32_t type;
-  allocator_->CreateIterator(&iter);
-  EXPECT_EQ(0U, allocator_->GetNextIterable(&iter, &type));
+  EXPECT_EQ(0U, iter1a.GetNext(&type));
   allocator_->MakeIterable(block1);
-  EXPECT_EQ(block1, allocator_->GetNextIterable(&iter, &type));
+  EXPECT_EQ(block1, iter1a.GetNext(&type));
   EXPECT_EQ(1U, type);
-  EXPECT_EQ(0U, allocator_->GetNextIterable(&iter, &type));
+  EXPECT_EQ(0U, iter1a.GetNext(&type));
 
   // Create second test-object and ensure everything is good and it cannot
   // be confused with test-object of another type.
@@ -139,14 +137,14 @@ TEST_F(PersistentMemoryAllocatorTest, AllocateAndIterate) {
 
   // Ensure that second test-object can also be made iterable.
   allocator_->MakeIterable(block2);
-  EXPECT_EQ(block2, allocator_->GetNextIterable(&iter, &type));
+  EXPECT_EQ(block2, iter1a.GetNext(&type));
   EXPECT_EQ(2U, type);
-  EXPECT_EQ(0U, allocator_->GetNextIterable(&iter, &type));
+  EXPECT_EQ(0U, iter1a.GetNext(&type));
 
   // Check that iteration can begin after an arbitrary location.
-  allocator_->CreateIterator(&iter, block1);
-  EXPECT_EQ(block2, allocator_->GetNextIterable(&iter, &type));
-  EXPECT_EQ(0U, allocator_->GetNextIterable(&iter, &type));
+  PersistentMemoryAllocator::Iterator iter1b(allocator_.get(), block1);
+  EXPECT_EQ(block2, iter1b.GetNext(&type));
+  EXPECT_EQ(0U, iter1b.GetNext(&type));
 
   // Ensure nothing has gone noticably wrong.
   EXPECT_FALSE(allocator_->IsFull());
@@ -192,10 +190,10 @@ TEST_F(PersistentMemoryAllocatorTest, AllocateAndIterate) {
   EXPECT_NE(allocator2->allocs_histogram_, allocator_->allocs_histogram_);
 
   // Ensure that iteration and access through second allocator works.
-  allocator2->CreateIterator(&iter);
-  EXPECT_EQ(block1, allocator2->GetNextIterable(&iter, &type));
-  EXPECT_EQ(block2, allocator2->GetNextIterable(&iter, &type));
-  EXPECT_EQ(0U, allocator2->GetNextIterable(&iter, &type));
+  PersistentMemoryAllocator::Iterator iter2(allocator2.get());
+  EXPECT_EQ(block1, iter2.GetNext(&type));
+  EXPECT_EQ(block2, iter2.GetNext(&type));
+  EXPECT_EQ(0U, iter2.GetNext(&type));
   EXPECT_NE(nullptr, allocator2->GetAsObject<TestObject1>(block1, 1));
   EXPECT_NE(nullptr, allocator2->GetAsObject<TestObject2>(block2, 2));
 
@@ -208,12 +206,17 @@ TEST_F(PersistentMemoryAllocatorTest, AllocateAndIterate) {
   EXPECT_FALSE(allocator3->allocs_histogram_);
 
   // Ensure that iteration and access through third allocator works.
-  allocator3->CreateIterator(&iter);
-  EXPECT_EQ(block1, allocator3->GetNextIterable(&iter, &type));
-  EXPECT_EQ(block2, allocator3->GetNextIterable(&iter, &type));
-  EXPECT_EQ(0U, allocator3->GetNextIterable(&iter, &type));
+  PersistentMemoryAllocator::Iterator iter3(allocator3.get());
+  EXPECT_EQ(block1, iter3.GetNext(&type));
+  EXPECT_EQ(block2, iter3.GetNext(&type));
+  EXPECT_EQ(0U, iter3.GetNext(&type));
   EXPECT_NE(nullptr, allocator3->GetAsObject<TestObject1>(block1, 1));
   EXPECT_NE(nullptr, allocator3->GetAsObject<TestObject2>(block2, 2));
+
+  // Ensure that GetNextOfType works.
+  PersistentMemoryAllocator::Iterator iter1c(allocator_.get());
+  EXPECT_EQ(block2, iter1c.GetNextOfType(2));
+  EXPECT_EQ(0U, iter1c.GetNextOfType(2));
 }
 
 TEST_F(PersistentMemoryAllocatorTest, PageTest) {
@@ -427,12 +430,11 @@ TEST(SharedPersistentMemoryAllocatorTest, CreationTest) {
   EXPECT_FALSE(shalloc2.IsFull());
   EXPECT_FALSE(shalloc2.IsCorrupt());
 
-  PersistentMemoryAllocator::Iterator iter2;
+  PersistentMemoryAllocator::Iterator iter2(&shalloc2);
   uint32_t type;
-  shalloc2.CreateIterator(&iter2);
-  EXPECT_EQ(r123, shalloc2.GetNextIterable(&iter2, &type));
-  EXPECT_EQ(r789, shalloc2.GetNextIterable(&iter2, &type));
-  EXPECT_EQ(0U, shalloc2.GetNextIterable(&iter2, &type));
+  EXPECT_EQ(r123, iter2.GetNext(&type));
+  EXPECT_EQ(r789, iter2.GetNext(&type));
+  EXPECT_EQ(0U, iter2.GetNext(&type));
 
   EXPECT_EQ(123U, shalloc2.GetType(r123));
   EXPECT_EQ(654U, shalloc2.GetType(r456));
@@ -454,11 +456,10 @@ TEST(SharedPersistentMemoryAllocatorTest, CreationTest) {
   EXPECT_FALSE(shalloc3.IsFull());
   EXPECT_FALSE(shalloc3.IsCorrupt());
 
-  PersistentMemoryAllocator::Iterator iter3;
-  shalloc3.CreateIterator(&iter3);
-  EXPECT_EQ(r123, shalloc3.GetNextIterable(&iter3, &type));
-  EXPECT_EQ(r789, shalloc3.GetNextIterable(&iter3, &type));
-  EXPECT_EQ(0U, shalloc3.GetNextIterable(&iter3, &type));
+  PersistentMemoryAllocator::Iterator iter3(&shalloc3);
+  EXPECT_EQ(r123, iter3.GetNext(&type));
+  EXPECT_EQ(r789, iter3.GetNext(&type));
+  EXPECT_EQ(0U, iter3.GetNext(&type));
 
   EXPECT_EQ(123U, shalloc3.GetType(r123));
   EXPECT_EQ(654U, shalloc3.GetType(r456));
@@ -473,7 +474,7 @@ TEST(SharedPersistentMemoryAllocatorTest, CreationTest) {
   Reference obj = shalloc3.Allocate(42, 42);
   ASSERT_TRUE(obj);
   shalloc3.MakeIterable(obj);
-  EXPECT_EQ(obj, shalloc2.GetNextIterable(&iter2, &type));
+  EXPECT_EQ(obj, iter2.GetNext(&type));
   EXPECT_EQ(42U, type);
 }
 
@@ -517,12 +518,11 @@ TEST(FilePersistentMemoryAllocatorTest, CreationTest) {
   EXPECT_FALSE(file.IsFull());
   EXPECT_FALSE(file.IsCorrupt());
 
-  PersistentMemoryAllocator::Iterator iter;
+  PersistentMemoryAllocator::Iterator iter(&file);
   uint32_t type;
-  file.CreateIterator(&iter);
-  EXPECT_EQ(r123, file.GetNextIterable(&iter, &type));
-  EXPECT_EQ(r789, file.GetNextIterable(&iter, &type));
-  EXPECT_EQ(0U, file.GetNextIterable(&iter, &type));
+  EXPECT_EQ(r123, iter.GetNext(&type));
+  EXPECT_EQ(r789, iter.GetNext(&type));
+  EXPECT_EQ(0U, iter.GetNext(&type));
 
   EXPECT_EQ(123U, file.GetType(r123));
   EXPECT_EQ(654U, file.GetType(r456));
@@ -542,8 +542,8 @@ TEST(FilePersistentMemoryAllocatorTest, AcceptableTest) {
   FilePath file_path_base = temp_dir.path().AppendASCII("persistent_memory_");
 
   LocalPersistentMemoryAllocator local(TEST_MEMORY_SIZE, TEST_ID, "");
-  local.Allocate(1, 1);
-  local.Allocate(11, 11);
+  local.MakeIterable(local.Allocate(1, 1));
+  local.MakeIterable(local.Allocate(11, 11));
   const size_t minsize = local.used();
   scoped_ptr<char[]> garbage(new char[minsize]);
   RandBytes(garbage.get(), minsize);
@@ -568,12 +568,10 @@ TEST(FilePersistentMemoryAllocatorTest, AcceptableTest) {
       // Make sure construction doesn't crash.
       FilePersistentMemoryAllocator allocator(std::move(mmfile), 0, "");
       // Also make sure that iteration doesn't crash.
-      PersistentMemoryAllocator::Iterator iter;
-      allocator.CreateIterator(&iter);
-      for (;;) {
-        Reference ref = allocator.GetNextIterable(&iter, 0);
-        if (!ref)
-          break;
+      PersistentMemoryAllocator::Iterator iter(&allocator);
+      uint32_t type_id;
+      Reference ref;
+      while ((ref = iter.GetNext(&type_id)) != 0) {
         const char* data = allocator.GetAsObject<char>(ref, 0);
         uint32_t type = allocator.GetType(ref);
         size_t size = allocator.GetAllocSize(ref);
@@ -581,9 +579,9 @@ TEST(FilePersistentMemoryAllocatorTest, AcceptableTest) {
         (void)data;
         (void)type;
         (void)size;
-        // Ensure that corruption-detected flag gets properly set.
-        EXPECT_EQ(filesize != minsize, allocator.IsCorrupt());
       }
+      // Ensure that short files are detected as corrupt and full files are not.
+      DCHECK_EQ(filesize != minsize, allocator.IsCorrupt());

[Ilya Sherman, 2016/03/24 02:03:58]

Did you mean EXPECT_EQ?

[bcwhite, 2016/03/24 14:22:34]

Done.  (Was changed to make it easier to find a problem during development.)

     } else {
       // For filesize >= minsize, the file must be acceptable. This
       // else clause (file-not-acceptable) should be reached only if