Fix a bug with LazilyDeallocatedDeque iterator

The bug was operator++ shouln't assume index zero is the front of
the ring's buffer.

This patch also improves test coverage of LazilyDeallocatedDeque.

Change-Id: I8d8a6dd50177a44ab78ef4262a794d6175898864
Reviewed-on: https://chromium-review.googlesource.com/1253982Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Commit-Queue: Alex Clarke <alexclarke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595435}

base/task/sequence_manager/lazily_deallocated_deque.h

@@ -79,6 +79,7 @@ class LazilyDeallocatedDeque {
   // Assumed to be an uncommon operation.
   void push_front(T t) {
     if (!head_) {
+      DCHECK(!tail_);
       head_ = std::make_unique<Ring>(kMinimumRingSize);
       tail_ = head_.get();
     }
@@ -97,6 +98,7 @@ class LazilyDeallocatedDeque {
   // Assumed to be a common operation.
   void push_back(T t) {
     if (!head_) {
+      DCHECK(!tail_);
       head_ = std::make_unique<Ring>(kMinimumRingSize);
       tail_ = head_.get();
     }
@@ -132,6 +134,8 @@ class LazilyDeallocatedDeque {
   }
 
   void pop_front() {
+    DCHECK(head_);
+    DCHECK(!head_->empty());
     DCHECK(tail_);
     DCHECK_GT(size_, 0u);
     head_->pop_front();
@@ -174,8 +178,8 @@ class LazilyDeallocatedDeque {
     // reclaiming it next time.
     max_size_ = size_;
 
-    // Only realloc if the current capacity is sufficiently the observed maximum
-    // size for the previous period.
+    // Only realloc if the current capacity is sufficiently greater than the
+    // observed maximum size for the previous period.
     if (new_capacity + kReclaimThreshold >= capacity())
       return;
 
@@ -313,7 +317,7 @@ class LazilyDeallocatedDeque {
     Iterator& operator++() {
       if (index_ == ring_->back_index_) {
         ring_ = ring_->next_.get();
-        index_ = 0;
+        index_ = ring_ ? ring_->CircularIncrement(ring_->front_index_) : 0;
       } else {
         index_ = ring_->CircularIncrement(index_);
       }

base/task/sequence_manager/lazily_deallocated_deque_unittest.cc

@@ -254,7 +254,7 @@ TEST_F(LazilyDeallocatedDequeTest, PushBackAndFront) {
   }
 }
 
-TEST_F(LazilyDeallocatedDequeTest, SetCapacity) {
+TEST_F(LazilyDeallocatedDequeTest, PushBackThenSetCapacity) {
   LazilyDeallocatedDeque<int> d;
   for (int i = 0; i < 1000; i++) {
     d.push_back(i);
@@ -265,7 +265,77 @@ TEST_F(LazilyDeallocatedDequeTest, SetCapacity) {
   // We need 1 more spot than the size due to the way the Ring works.
   d.SetCapacity(1001);
 
+  EXPECT_EQ(1000u, d.size());
+  EXPECT_EQ(0, d.front());
+  EXPECT_EQ(999, d.back());
+
+  for (int i = 0; i < 1000; i++) {
+    EXPECT_EQ(d.front(), i);
+    d.pop_front();
+  }
+}
+
+TEST_F(LazilyDeallocatedDequeTest, PushFrontThenSetCapacity) {
+  LazilyDeallocatedDeque<int> d;
+  for (int i = 0; i < 1000; i++) {
+    d.push_front(i);
+  }
+
+  EXPECT_EQ(1336u, d.capacity());
+
+  // We need 1 more spot than the size due to the way the Ring works.
+  d.SetCapacity(1001);
+
+  EXPECT_EQ(1000u, d.size());
+  EXPECT_EQ(999, d.front());
+  EXPECT_EQ(0, d.back());
+
+  for (int i = 0; i < 1000; i++) {
+    EXPECT_EQ(d.front(), 999 - i);
+    d.pop_front();
+  }
+}
+
+TEST_F(LazilyDeallocatedDequeTest, PushFrontThenSetCapacity2) {
+  LazilyDeallocatedDeque<std::unique_ptr<int>> d;
+  for (int i = 0; i < 1000; i++) {
+    d.push_front(std::make_unique<int>(i));
+  }
+
+  EXPECT_EQ(1336u, d.capacity());
+
+  // We need 1 more spot than the size due to the way the Ring works.
+  d.SetCapacity(1001);
+
+  EXPECT_EQ(1000u, d.size());
+  EXPECT_EQ(999, *d.front());
+  EXPECT_EQ(0, *d.back());
+
+  for (int i = 0; i < 1000; i++) {
+    EXPECT_EQ(*d.front(), 999 - i);
+    d.pop_front();
+  }
+}
+
+TEST_F(LazilyDeallocatedDequeTest, PushBackAndFrontThenSetCapacity) {
+  LazilyDeallocatedDeque<int> d;
+
+  int j = 1;
   for (int i = 0; i < 1000; i++) {
+    d.push_back(j++);
+    d.push_back(j++);
+    d.push_back(j++);
+    d.push_back(j++);
+    d.push_front(-i);
+  }
+
+  d.SetCapacity(5001);
+
+  EXPECT_EQ(5000u, d.size());
+  EXPECT_EQ(-999, d.front());
+  EXPECT_EQ(4000, d.back());
+
+  for (int i = -999; i < 4000; i++) {
     EXPECT_EQ(d.front(), i);
     d.pop_front();
   }
@@ -354,6 +424,86 @@ TEST_F(LazilyDeallocatedDequeTest, RingPushPopPushPop) {
   EXPECT_FALSE(r.CanPop());
 }
 
+TEST_F(LazilyDeallocatedDequeTest, PushAndIterate) {
+  LazilyDeallocatedDeque<int> d;
+
+  for (int i = 0; i < 1000; i++) {
+    d.push_front(i);
+  }
+
+  int expected = 999;
+  for (int value : d) {
+    EXPECT_EQ(expected, value);
+    expected--;
+  }
+}
+
+TEST_F(LazilyDeallocatedDequeTest, Swap) {
+  LazilyDeallocatedDeque<int> a;
+  LazilyDeallocatedDeque<int> b;
+
+  a.push_back(1);
+  a.push_back(2);
+
+  for (int i = 0; i < 1000; i++) {
+    b.push_back(i);
+  }
+
+  EXPECT_EQ(2u, a.size());
+  EXPECT_EQ(1, a.front());
+  EXPECT_EQ(2, a.back());
+  EXPECT_EQ(1000u, b.size());
+  EXPECT_EQ(0, b.front());
+  EXPECT_EQ(999, b.back());
+
+  a.swap(b);
+
+  EXPECT_EQ(1000u, a.size());
+  EXPECT_EQ(0, a.front());
+  EXPECT_EQ(999, a.back());
+  EXPECT_EQ(2u, b.size());
+  EXPECT_EQ(1, b.front());
+  EXPECT_EQ(2, b.back());
+}
+
+class DestructorTestItem {
+ public:
+  DestructorTestItem() : v_(-1) {}
+
+  DestructorTestItem(int v) : v_(v) {}
+
+  ~DestructorTestItem() { destructor_count_++; }
+
+  int v_;
+  static int destructor_count_;
+};
+
+int DestructorTestItem::destructor_count_ = 0;
+
+TEST_F(LazilyDeallocatedDequeTest, PopFrontCallsDestructor) {
+  LazilyDeallocatedDeque<DestructorTestItem> a;
+
+  a.push_front(DestructorTestItem(123));
+
+  DestructorTestItem::destructor_count_ = 0;
+  a.pop_front();
+  EXPECT_EQ(1, DestructorTestItem::destructor_count_);
+}
+
+TEST_F(LazilyDeallocatedDequeTest, ExpectedNumberOfDestructorsCalled) {
+  {
+    LazilyDeallocatedDeque<DestructorTestItem> a;
+
+    for (int i = 0; i < 100; i++) {
+      a.push_front(DestructorTestItem(i));
+    }
+
+    DestructorTestItem::destructor_count_ = 0;
+  }
+
+  EXPECT_EQ(100, DestructorTestItem::destructor_count_);
+}
+
 }  // namespace internal
 }  // namespace sequence_manager
 }  // namespace base