Blink-side contiguous allocation of display items.

Source/platform/graphics/ListContainer.h

 // Copyright 2015 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 ListContainer_h
 #define ListContainer_h
 
 #include "platform/PlatformExport.h"
 #include "wtf/Forward.h"
 #include "wtf/Noncopyable.h"
@@ skipping 216 matching lines @@
 
     // 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 *iteratorAt(index);

[danakj, 2015/06/29 17:43:51]

Is this a change we could/should make in cc/ too?

[pdr., 2015/06/29 22:20:03]

Done in https://codereview.chromium.org/1219753003.

     }
+
     const BaseElementType* elementAt(size_t index) const
     {
-        return *ConstIterator(iteratorAt(index));
+        return *iteratorAt(index);
+    }
+
+    Iterator iteratorAt(size_t index)
+    {
+        return Iterator(ListContainerBase::iteratorAt(index));
+    }
+
+    ConstIterator iteratorAt(size_t index) const
+    {
+        return ConstIterator(ListContainerBase::iteratorAt(index));
+    }
+
+    BaseElementType& operator[](size_t index)

[danakj, 2015/06/29 17:43:51]

I recommend not adding these. It looks like it should be cheap or O(1), like
arrays or vectors but it is not.

The elementAt() was added to ListContainer mostly for tests. Production code is
probably doing something wrong if it is using them.

Similar thoughts about iteratorAt().

I didn't see where these are used in the delta, can you point it out?

[pdr., 2015/06/29 22:20:03]

I added these for similar reasons. These are heavily used in both tests and
assert code, but not release code, and the patch would be difficult to review
with these changes. Would you be okay landing these for the same reasons they
were added in cc?

[danakj, 2015/06/29 22:29:47]

ah OK. iteratorat sounds OK to me. operator[] less so.

+    {
+        return *elementAt(index);
+    }
+
+    const BaseElementType& operator[](size_t index) const
+    {
+        return *elementAt(index);
+    }
+
+    // Allocate a new, uninitialized DerivedElementType. Use with caution!

[danakj, 2015/06/29 17:43:51]

Why do we need this?

[pdr., 2015/06/29 22:20:03]

We no longer do because I've made allocateAndConstruct take arguments.

+    template <typename DerivedElementType>
+    DerivedElementType* allocateWithoutConstruction()
+    {
+        return static_cast<DerivedElementType*>(allocate(sizeof(DerivedElementType)));
     }
 
     // Take in derived element type and construct it at location generated by Allocate().
     template <typename DerivedElementType>
     DerivedElementType* allocateAndConstruct()
     {
         return new (allocate(sizeof(DerivedElementType))) DerivedElementType;
     }
 
     // Take in derived element type and copy construct it at location generated by
@@ skipping 14 matching lines @@
 
     template <typename DerivedElementType>
     void swap(ListContainer<DerivedElementType>& other)
     {
         m_data.swap(other.m_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()).
+    // of memory will be moved (up to maxSizeForDerivedClass()). A pointer
+    // to the moved element is returned.
     template <typename DerivedElementType>
-    void appendByMoving(DerivedElementType* item)
+    DerivedElementType* appendByMoving(DerivedElementType* item)

[danakj, 2015/06/29 17:43:51]

Can we do this to the cc/ class too?

[pdr., 2015/06/29 22:20:03]

Done in https://codereview.chromium.org/1219753003.

     {
         size_t maxSize = maxSizeForDerivedClass();
         void* newItem = allocate(maxSize);
         memcpy(newItem, static_cast<void*>(item), maxSize);
         // Construct a new element in-place so it can be destructed safely.
         new (item) DerivedElementType;
+        return static_cast<DerivedElementType*>(newItem);
     }
 
     using ListContainerBase::size;
     using ListContainerBase::empty;
 
     void clear()
     {
         for (Iterator i = begin(); i != end(); ++i)
             i->~BaseElementType();
         ListContainerBase::clear();
@@ skipping 24 matching lines @@
             ++m_index;
             return *this;
         }
 
     private:
         explicit Iterator(const ListContainerBase::Iterator& baseIterator)
             : ListContainerBase::Iterator(baseIterator) {}
         friend Iterator ListContainer<BaseElementType>::begin();
         friend Iterator ListContainer<BaseElementType>::end();
         friend BaseElementType* ListContainer<BaseElementType>::elementAt(size_t index);
+        friend Iterator ListContainer<BaseElementType>::iteratorAt(size_t index);
     };
 
     class ConstIterator : public ListContainerBase::ConstIterator {
     // This class is only defined to forward iterate through
     // ListContainerCharAllocator.
     public:
         ConstIterator(ListContainerCharAllocator* container, size_t vectorIndex, char* itemIter, size_t index)
             : ListContainerBase::ConstIterator(container, vectorIndex, itemIter, index) {}
         ConstIterator(const Iterator& other)
             : ListContainerBase::ConstIterator(other) {}
@@ skipping 18 matching lines @@
             ++m_index;
             return *this;
         }
 
     private:
         explicit ConstIterator(const ListContainerBase::ConstIterator& baseIterator)
             : ListContainerBase::ConstIterator(baseIterator) {}
         friend ConstIterator ListContainer<BaseElementType>::cbegin() const;
         friend ConstIterator ListContainer<BaseElementType>::cend() const;
         friend const BaseElementType* ListContainer<BaseElementType>::elementAt(size_t index) const;
+        friend ConstIterator ListContainer<BaseElementType>::iteratorAt(size_t index) const;
     };
 
     class ReverseIterator : public ListContainerBase::ReverseIterator {
     // This class is only defined to reverse iterate through
     // ListContainerCharAllocator.
     public:
         ReverseIterator(ListContainerCharAllocator* container, size_t vectorIndex, char* itemIter, size_t index)
             : ListContainerBase::ReverseIterator(container, vectorIndex, itemIter, index) {}
 
         BaseElementType* operator->() const
@@ skipping 58 matching lines @@
         explicit ConstReverseIterator(ListContainerBase::ConstReverseIterator baseIterator)
             : ListContainerBase::ConstReverseIterator(baseIterator) {}
         friend ConstReverseIterator ListContainer<BaseElementType>::crbegin() const;
         friend ConstReverseIterator ListContainer<BaseElementType>::crend() const;
     };
 };
 
 } // namespace blink
 
 #endif // ListContainer_h