cc: Switch ListContainer to use composition instead of inheritance.

cc/base/list_container.h

Index: cc/base/list_container.h
diff --git a/cc/base/list_container.h b/cc/base/list_container.h
index e504c290df686d62339c484addfc24799723622f..c6d7ca960ad2ac7bfadb208f28a4886b5dde7417 100644
--- a/cc/base/list_container.h
+++ b/cc/base/list_container.h
@@ -21,15 +21,18 @@ namespace cc {
 // to hold DrawQuads, the allocations size of each element in this class is
 // LargestDrawQuadSize while BaseElementType is DrawQuad.
 
-// Base class for non-templated logic. All methods are protected, and only
-// exposed by ListContainer<BaseElementType>.
+// Helper class for non-templated logic. All methods are private, and only
+// exposed to friend classes.
 // For usage, see comments in ListContainer.
-class CC_EXPORT ListContainerBase {
- protected:
-  explicit ListContainerBase(size_t max_size_for_derived_class);
-  ListContainerBase(size_t max_size_for_derived_class,
-                    size_t num_of_elements_to_reserve_for);
-  ~ListContainerBase();
+class CC_EXPORT ListContainerHelper {
+ private:
+  template <typename T>
+  friend class CC_EXPORT ListContainer;
+
+  explicit ListContainerHelper(size_t max_size_for_derived_class);
+  ListContainerHelper(size_t max_size_for_derived_class,
+                      size_t num_of_elements_to_reserve_for);
+  ~ListContainerHelper();
 
   // This class deals only with char* and void*. It does allocation and passing
   // out raw pointers, as well as memory deallocation when being destroyed.
@@ -178,32 +181,30 @@ class CC_EXPORT ListContainerBase {
 
   scoped_ptr<ListContainerCharAllocator> data_;
 
- private:
-  DISALLOW_COPY_AND_ASSIGN(ListContainerBase);
+  DISALLOW_COPY_AND_ASSIGN(ListContainerHelper);
 };
 
 template <class BaseElementType>
-class ListContainer : public ListContainerBase {
+class CC_EXPORT ListContainer {

[danakj, 2015/09/14 21:25:59]

can you put this in its own file? (or rather, put the helper in its own file)
now, since you have another container class coming (and put that one in its own
too)

[vmpstr, 2015/09/15 17:32:33]

Done.

  public:
   // BaseElementType is the type of raw pointers this class hands out; however,
   // its derived classes might require different memory sizes.
   // max_size_for_derived_class the largest memory size required for all the
   // derived classes to use for allocation.
   explicit ListContainer(size_t max_size_for_derived_class)
-      : ListContainerBase(max_size_for_derived_class) {}
+      : helper_(max_size_for_derived_class) {}
 
   // This constructor omits input variable for max_size_for_derived_class. This
   // is used when there is no derived classes from BaseElementType we need to
   // worry about, and allocation size is just sizeof(BaseElementType).
-  ListContainer() : ListContainerBase(sizeof(BaseElementType)) {}
+  ListContainer() : helper_(sizeof(BaseElementType)) {}
 
   // This constructor reserves the requested memory up front so only single
   // allocation is needed. When num_of_elements_to_reserve_for is zero, use the
   // default size.
   ListContainer(size_t max_size_for_derived_class,
                 size_t num_of_elements_to_reserve_for)
-      : ListContainerBase(max_size_for_derived_class,
-                          num_of_elements_to_reserve_for) {}
+      : helper_(max_size_for_derived_class, num_of_elements_to_reserve_for) {}
 
   ~ListContainer() {
     for (Iterator i = begin(); i != end(); ++i) {
@@ -221,7 +222,7 @@ class ListContainer : public ListContainerBase {
   void RemoveLast() {
     DCHECK(!empty());
     back()->~BaseElementType();
-    ListContainerBase::RemoveLast();
+    helper_.RemoveLast();
   }
 
   // When called, all raw pointers that have been handed out are no longer
@@ -231,32 +232,26 @@ class ListContainer : public ListContainerBase {
   Iterator EraseAndInvalidateAllPointers(Iterator position) {
     BaseElementType* item = *position;
     item->~BaseElementType();
-    ListContainerBase::EraseAndInvalidateAllPointers(&position);
+    helper_.EraseAndInvalidateAllPointers(&position);
     return empty() ? end() : position;
   }
 
   ConstReverseIterator crbegin() const {
-    return ConstReverseIterator(ListContainerBase::crbegin());
+    return ConstReverseIterator(helper_.crbegin());
   }
   ConstReverseIterator crend() const {
-    return ConstReverseIterator(ListContainerBase::crend());
+    return ConstReverseIterator(helper_.crend());
   }
   ConstReverseIterator rbegin() const { return crbegin(); }
   ConstReverseIterator rend() const { return crend(); }
-  ReverseIterator rbegin() {
-    return ReverseIterator(ListContainerBase::rbegin());
-  }
-  ReverseIterator rend() { return ReverseIterator(ListContainerBase::rend()); }
-  ConstIterator cbegin() const {
-    return ConstIterator(ListContainerBase::cbegin());
-  }
-  ConstIterator cend() const {
-    return ConstIterator(ListContainerBase::cend());
-  }
+  ReverseIterator rbegin() { return ReverseIterator(helper_.rbegin()); }
+  ReverseIterator rend() { return ReverseIterator(helper_.rend()); }
+  ConstIterator cbegin() const { return ConstIterator(helper_.cbegin()); }
+  ConstIterator cend() const { return ConstIterator(helper_.cend()); }
   ConstIterator begin() const { return cbegin(); }
   ConstIterator end() const { return cend(); }
-  Iterator begin() { return Iterator(ListContainerBase::begin()); }
-  Iterator end() { return Iterator(ListContainerBase::end()); }
+  Iterator begin() { return Iterator(helper_.begin()); }
+  Iterator end() { return Iterator(helper_.end()); }
 
   // TODO(weiliangc): front(), back() and ElementAt() function should return
   // reference, consistent with container-of-object.
@@ -266,24 +261,25 @@ class ListContainer : public ListContainerBase {
   const BaseElementType* back() const { return *rbegin(); }
 
   BaseElementType* ElementAt(size_t index) {
-    return *Iterator(IteratorAt(index));
+    return *Iterator(helper_.IteratorAt(index));
   }
   const BaseElementType* ElementAt(size_t index) const {
-    return *ConstIterator(IteratorAt(index));
+    return *ConstIterator(helper_.IteratorAt(index));
   }
 
   // Take in derived element type and construct it at location generated by
   // Allocate().
   template <typename DerivedElementType>
   DerivedElementType* AllocateAndConstruct() {
-    return new (Allocate(sizeof(DerivedElementType))) DerivedElementType;
+    return new (helper_.Allocate(sizeof(DerivedElementType)))
+        DerivedElementType;
   }
 
   // Take in derived element type and copy construct it at location generated by
   // Allocate().
   template <typename DerivedElementType>
   DerivedElementType* AllocateAndCopyFrom(const DerivedElementType* source) {
-    return new (Allocate(sizeof(DerivedElementType)))
+    return new (helper_.Allocate(sizeof(DerivedElementType)))
         DerivedElementType(*source);
   }
 
@@ -299,7 +295,7 @@ class ListContainer : public ListContainerBase {
   // for the beginning of the newly inserted segment.
   template <typename DerivedElementType>
   Iterator InsertBeforeAndInvalidateAllPointers(Iterator at, size_t count) {
-    ListContainerBase::InsertBeforeAndInvalidateAllPointers(&at, count);
+    helper_.InsertBeforeAndInvalidateAllPointers(&at, count);
     Iterator result = at;
     for (size_t i = 0; i < count; ++i) {
       new (*at) DerivedElementType();
@@ -310,7 +306,7 @@ class ListContainer : public ListContainerBase {
 
   template <typename DerivedElementType>
   void swap(ListContainer<DerivedElementType>& other) {
-    data_.swap(other.data_);
+    helper_.data_.swap(other.helper_.data_);
   }
 
   // Appends a new item without copying. The original item will not be
@@ -320,39 +316,43 @@ class ListContainer : public ListContainerBase {
   // the moved element is returned.
   template <typename DerivedElementType>
   DerivedElementType* AppendByMoving(DerivedElementType* item) {
-    size_t max_size_for_derived_class = MaxSizeForDerivedClass();
-    void* new_item = Allocate(max_size_for_derived_class);
+    size_t max_size_for_derived_class = helper_.MaxSizeForDerivedClass();
+    void* new_item = helper_.Allocate(max_size_for_derived_class);
     memcpy(new_item, static_cast<void*>(item), max_size_for_derived_class);
     // Construct a new element in-place so it can be destructed safely.
     new (item) DerivedElementType;
     return static_cast<DerivedElementType*>(new_item);
   }
 
-  using ListContainerBase::size;
-  using ListContainerBase::empty;
-  using ListContainerBase::GetCapacityInBytes;
+  size_t size() const { return helper_.size(); }
+  bool empty() const { return helper_.empty(); }
+  size_t GetCapacityInBytes() const { return helper_.GetCapacityInBytes(); }
 
   void clear() {
     for (Iterator i = begin(); i != end(); ++i) {
       i->~BaseElementType();
     }
-    ListContainerBase::clear();
+    helper_.clear();
   }
 
-  using ListContainerBase::AvailableSizeWithoutAnotherAllocationForTesting;
+  size_t AvailableSizeWithoutAnotherAllocationForTesting() const {
+    return helper_.AvailableSizeWithoutAnotherAllocationForTesting();
+  }
 
   // Iterator classes that can be used to access data.
   /////////////////////////////////////////////////////////////////
-  class Iterator : public ListContainerBase::Iterator {
+  class Iterator : public ListContainerHelper::Iterator {
     // This class is only defined to forward iterate through
     // ListContainerCharAllocator.
    public:
-    Iterator(ListContainerCharAllocator* container,
+    Iterator(ListContainerHelper::ListContainerCharAllocator* container,
              size_t vector_ind,
              char* item_iter,
              size_t index)
-        : ListContainerBase::Iterator(container, vector_ind, item_iter, index) {
-    }
+        : ListContainerHelper::Iterator(container,

[danakj, 2015/09/14 21:25:59]

dat git cl format. surely this is a bug :P

+                                        vector_ind,
+                                        item_iter,
+                                        index) {}
     BaseElementType* operator->() const {
       return reinterpret_cast<BaseElementType*>(item_iterator);
     }
@@ -371,28 +371,28 @@ class ListContainer : public ListContainerBase {
     }
 
    private:
-    explicit Iterator(const ListContainerBase::Iterator& base_iterator)
-        : ListContainerBase::Iterator(base_iterator) {}
+    explicit Iterator(const ListContainerHelper::Iterator& base_iterator)
+        : ListContainerHelper::Iterator(base_iterator) {}
     friend Iterator ListContainer<BaseElementType>::begin();
     friend Iterator ListContainer<BaseElementType>::end();
     friend BaseElementType* ListContainer<BaseElementType>::ElementAt(
         size_t index);
   };
 
-  class ConstIterator : public ListContainerBase::ConstIterator {
+  class ConstIterator : public ListContainerHelper::ConstIterator {
     // This class is only defined to forward iterate through
     // ListContainerCharAllocator.
    public:
-    ConstIterator(ListContainerCharAllocator* container,
+    ConstIterator(ListContainerHelper::ListContainerCharAllocator* container,
                   size_t vector_ind,
                   char* item_iter,
                   size_t index)
-        : ListContainerBase::ConstIterator(container,
-                                           vector_ind,
-                                           item_iter,
-                                           index) {}
+        : ListContainerHelper::ConstIterator(container,
+                                             vector_ind,
+                                             item_iter,
+                                             index) {}
     ConstIterator(const Iterator& other)  // NOLINT
-        : ListContainerBase::ConstIterator(other) {}
+        : ListContainerHelper::ConstIterator(other) {}
     const BaseElementType* operator->() const {
       return reinterpret_cast<const BaseElementType*>(item_iterator);
     }
@@ -412,26 +412,26 @@ class ListContainer : public ListContainerBase {
 
    private:
     explicit ConstIterator(
-        const ListContainerBase::ConstIterator& base_iterator)
-        : ListContainerBase::ConstIterator(base_iterator) {}
+        const ListContainerHelper::ConstIterator& base_iterator)
+        : ListContainerHelper::ConstIterator(base_iterator) {}
     friend ConstIterator ListContainer<BaseElementType>::cbegin() const;
     friend ConstIterator ListContainer<BaseElementType>::cend() const;
     friend const BaseElementType* ListContainer<BaseElementType>::ElementAt(
         size_t index) const;
   };
 
-  class ReverseIterator : public ListContainerBase::ReverseIterator {
+  class ReverseIterator : public ListContainerHelper::ReverseIterator {
     // This class is only defined to reverse iterate through
     // ListContainerCharAllocator.
    public:
-    ReverseIterator(ListContainerCharAllocator* container,
+    ReverseIterator(ListContainerHelper::ListContainerCharAllocator* container,

[danakj, 2015/09/14 21:25:59]

Having ListContainer in the name twice, maybe a little verbose? Drop the second
one?

[vmpstr, 2015/09/15 17:32:33]

Done.

                     size_t vector_ind,
                     char* item_iter,
                     size_t index)
-        : ListContainerBase::ReverseIterator(container,
-                                             vector_ind,
-                                             item_iter,
-                                             index) {}
+        : ListContainerHelper::ReverseIterator(container,
+                                               vector_ind,
+                                               item_iter,
+                                               index) {}
     BaseElementType* operator->() const {
       return reinterpret_cast<BaseElementType*>(item_iterator);
     }
@@ -450,26 +450,28 @@ class ListContainer : public ListContainerBase {
     }
 
    private:
-    explicit ReverseIterator(ListContainerBase::ReverseIterator base_iterator)
-        : ListContainerBase::ReverseIterator(base_iterator) {}
+    explicit ReverseIterator(ListContainerHelper::ReverseIterator base_iterator)
+        : ListContainerHelper::ReverseIterator(base_iterator) {}
     friend ReverseIterator ListContainer<BaseElementType>::rbegin();
     friend ReverseIterator ListContainer<BaseElementType>::rend();
   };
 
-  class ConstReverseIterator : public ListContainerBase::ConstReverseIterator {
+  class ConstReverseIterator
+      : public ListContainerHelper::ConstReverseIterator {
     // This class is only defined to reverse iterate through
     // ListContainerCharAllocator.
    public:
-    ConstReverseIterator(ListContainerCharAllocator* container,
-                         size_t vector_ind,
-                         char* item_iter,
-                         size_t index)
-        : ListContainerBase::ConstReverseIterator(container,
-                                                  vector_ind,
-                                                  item_iter,
-                                                  index) {}
+    ConstReverseIterator(
+        ListContainerHelper::ListContainerCharAllocator* container,
+        size_t vector_ind,
+        char* item_iter,
+        size_t index)
+        : ListContainerHelper::ConstReverseIterator(container,
+                                                    vector_ind,
+                                                    item_iter,
+                                                    index) {}
     ConstReverseIterator(const ReverseIterator& other)  // NOLINT
-        : ListContainerBase::ConstReverseIterator(other) {}
+        : ListContainerHelper::ConstReverseIterator(other) {}
     const BaseElementType* operator->() const {
       return reinterpret_cast<const BaseElementType*>(item_iterator);
     }
@@ -489,11 +491,16 @@ class ListContainer : public ListContainerBase {
 
    private:
     explicit ConstReverseIterator(
-        ListContainerBase::ConstReverseIterator base_iterator)
-        : ListContainerBase::ConstReverseIterator(base_iterator) {}
+        ListContainerHelper::ConstReverseIterator base_iterator)
+        : ListContainerHelper::ConstReverseIterator(base_iterator) {}
     friend ConstReverseIterator ListContainer<BaseElementType>::crbegin() const;
     friend ConstReverseIterator ListContainer<BaseElementType>::crend() const;
   };
+
+ private:
+  ListContainerHelper helper_;
+
+  DISALLOW_COPY_AND_ASSIGN(ListContainer);
 };
 
 }  // namespace cc