cc: Switch ListContainer to use composition instead of inheritance.

cc/base/list_container.h

 // Copyright 2014 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.
 
 #ifndef CC_BASE_LIST_CONTAINER_H_
 #define CC_BASE_LIST_CONTAINER_H_
 
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "cc/base/cc_export.h"
 
 namespace cc {
 
 // ListContainer is a container type that handles allocating contiguous memory
 // for new elements and traversing through elements with either iterator or
 // reverse iterator. Since this container hands out raw pointers of its
 // elements, it is very important that this container never reallocate its
 // memory so those raw pointer will continue to be valid.  This class is used to
 // contain SharedQuadState or DrawQuad. Since the size of each DrawQuad varies,
 // 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;

[weiliangc, 2015/09/14 19:57:13]

I'm kinda torn on the friend idea. Maybe stop CC_EXPORT-ing Helper class is
enough? 

Just that putting entire public interface into private w/ friend seems like
exact same level of data encapsulation?

[vmpstr, 2015/09/14 20:05:07]

I'm not really sure. Wouldn't that still allow any cc code to create and use the
helper directly?
Ya, I intended to keep the same level of encapsulation, since I don't want to
expose ListContainerHelper functionality to other classes other than the ones
explicitly specified. 

Are you proposing making the helper's functionality public and then removing
CC_EXPORT or keeping the inheritance and removing CC_EXPORT?

+
+  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.
   class ListContainerCharAllocator;
 
   // This class points to a certain position inside memory of
   // ListContainerCharAllocator. It is a base class for ListContainer iterators.
   struct CC_EXPORT PositionInListContainerCharAllocator {
     ListContainerCharAllocator* ptr_to_container;
     size_t vector_index;
@@ skipping 128 matching lines @@
   // destructors.
   void clear();
 
   size_t AvailableSizeWithoutAnotherAllocationForTesting() const;
 
   // Hands out memory location for an element at the end of data structure.
   void* Allocate(size_t size_of_actual_element_in_bytes);
 
   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 {
  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) {
       i->~BaseElementType();
     }
   }
 
   class Iterator;
   class ConstIterator;
   class ReverseIterator;
   class ConstReverseIterator;
 
   // Removes the last element of the list and makes its space available for
   // allocation.
   void RemoveLast() {
     DCHECK(!empty());
     back()->~BaseElementType();
-    ListContainerBase::RemoveLast();
+    helper_.RemoveLast();
   }
 
   // When called, all raw pointers that have been handed out are no longer
   // valid. Use with caution.
   // Returns a valid Iterator pointing to the element after the erased element.
   // This function does not deallocate memory.
   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.
   BaseElementType* front() { return *begin(); }
   BaseElementType* back() { return *rbegin(); }
   const BaseElementType* front() const { return *begin(); }
   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);
   }
 
   // Construct a new element on top of an existing one.
   template <typename DerivedElementType>
   DerivedElementType* ReplaceExistingElement(Iterator at) {
     at->~BaseElementType();
     return new (*at) DerivedElementType();
   }
 
   // Insert |count| new elements of |DerivedElementType| before |at|. This will
   // invalidate all outstanding pointers and iterators. Return a valid iterator
   // 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();
       ++at;
     }
     return result;
   }
 
   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
   // destructed and will be replaced with a new DerivedElementType. The
   // DerivedElementType does not have to match the moved type as a full block
   // of memory will be moved (up to MaxSizeForDerivedClass()). A pointer to
   // 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,
+                                        vector_ind,
+                                        item_iter,
+                                        index) {}
     BaseElementType* operator->() const {
       return reinterpret_cast<BaseElementType*>(item_iterator);
     }
     BaseElementType* operator*() const {
       return reinterpret_cast<BaseElementType*>(item_iterator);
     }
     Iterator operator++(int unused_post_increment) {
       Iterator tmp = *this;
       operator++();
       return tmp;
     }
     Iterator& operator++() {
       Increment();
       ++index_;
       return *this;
     }
 
    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);
     }
     const BaseElementType* operator*() const {
       return reinterpret_cast<const BaseElementType*>(item_iterator);
     }
     ConstIterator operator++(int unused_post_increment) {
       ConstIterator tmp = *this;
       operator++();
       return tmp;
     }
     ConstIterator& operator++() {
       Increment();
       ++index_;
       return *this;
     }
 
    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,
                     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);
     }
     BaseElementType* operator*() const {
       return reinterpret_cast<BaseElementType*>(item_iterator);
     }
     ReverseIterator operator++(int unused_post_increment) {
       ReverseIterator tmp = *this;
       operator++();
       return tmp;
     }
     ReverseIterator& operator++() {
       ReverseIncrement();
       ++index_;
       return *this;
     }
 
    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);
     }
     const BaseElementType* operator*() const {
       return reinterpret_cast<const BaseElementType*>(item_iterator);
     }
     ConstReverseIterator operator++(int unused_post_increment) {
       ConstReverseIterator tmp = *this;
       operator++();
       return tmp;
     }
     ConstReverseIterator& operator++() {
       ReverseIncrement();
       ++index_;
       return *this;
     }
 
    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_;
 };
 
 }  // namespace cc
 
 #endif  // CC_BASE_LIST_CONTAINER_H_