Fix alignment issue of ContiguousContainer

third_party/WebKit/Source/platform/graphics/paint/DisplayItem.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 DisplayItem_h
 #define DisplayItem_h
 
 #include "platform/PlatformExport.h"
 #include "platform/graphics/ContiguousContainer.h"
 #include "platform/graphics/paint/DisplayItemClient.h"
@@ skipping 180 matching lines @@
     static_assert(TableCollapsedBorderBase >= TableCollapsedBorderUnalignedBase, "TableCollapsedBorder types overlap with other types");
     static_assert((TableCollapsedBorderBase & 0xf) == 0, "The lowest 4 bits of TableCollapsedBorderBase should be zero");
     // Bits or'ed onto TableCollapsedBorderBase to generate a real table collapsed border type.
     enum TableCollapsedBorderSides {
         TableCollapsedBorderTop = 1 << 0,
         TableCollapsedBorderRight = 1 << 1,
         TableCollapsedBorderBottom = 1 << 2,
         TableCollapsedBorderLeft = 1 << 3,
     };
 
-    DisplayItem(const DisplayItemClient& client, Type type, size_t derivedSize)
+    DisplayItem(const DisplayItemClient& client, Type type, size_t derivedSize, unsigned derivedLog2Alignment)
         : m_client(&client)
         , m_type(type)
         , m_derivedSize(derivedSize)
+        , m_derivedLog2Alignment(derivedLog2Alignment)
         , m_skippedCache(false)
 #ifndef NDEBUG
         , m_clientDebugString(client.debugName())
 #endif
     {
         // derivedSize must fit in m_derivedSize.
         // If it doesn't, enlarge m_derivedSize and fix this assert.
-        ASSERT_WITH_SECURITY_IMPLICATION(derivedSize < (1 << 8));
-        ASSERT_WITH_SECURITY_IMPLICATION(derivedSize >= sizeof(*this));
+        SECURITY_CHECK(derivedSize == m_derivedSize);

[jbroman, 2016/07/04 19:21:35]

isn't the replacement for ASSERT_WITH_SECURITY_IMPLICATION SECURITY_DCHECK,
rather than CHECK? (The difference being that SECURITY_CHECK still compiles in
release builds.)

+        SECURITY_CHECK(derivedSize >= sizeof(*this));
+        // derivedLog2Alignment must fit in m_derivedLog2Alignment.
+        SECURITY_CHECK(derivedLog2Alignment == m_derivedLog2Alignment);
     }
 
     virtual ~DisplayItem() { }
 
     // Ids are for matching new DisplayItems with existing DisplayItems.
     struct Id {
         STACK_ALLOCATED();
         Id(const DisplayItemClient& client, const Type type)
             : client(client)
             , type(type) { }
@@ skipping 30 matching lines @@
 
     const DisplayItemClient& client() const { ASSERT(m_client); return *m_client; }
     Type getType() const { return m_type; }
 
     // Size of this object in memory, used to move it with memcpy.
     // This is not sizeof(*this), because it needs to account for the size of
     // the derived class (i.e. runtime type). Derived classes are expected to
     // supply this to the DisplayItem constructor.
     size_t derivedSize() const { return m_derivedSize; }
 
+    // log2(alignment (in bytes)) of this object when this object is allocated in memory.
+    size_t derivedLog2Alignment() const { return m_derivedLog2Alignment; }
+
     // For PaintController only. Painters should use DisplayItemCacheSkipper instead.
     void setSkippedCache() { m_skippedCache = true; }
     bool skippedCache() const { return m_skippedCache; }
 
     virtual void appendToWebDisplayItemList(const IntRect&, WebDisplayItemList*) const { }
 
     // See comments of enum Type for usage of the following macros.
 #define DEFINE_CATEGORY_METHODS(Category) \
     static bool is##Category##Type(Type type) { return type >= Category##First && type <= Category##Last; } \
     bool is##Category() const { return is##Category##Type(getType()); }
@@ skipping 50 matching lines @@
     static bool isCacheableType(Type type) { return isDrawingType(type) || type == Subsequence; }
     bool isCacheable() const { return !skippedCache() && isCacheableType(m_type); }
 
     virtual bool isBegin() const { return false; }
     virtual bool isEnd() const { return false; }
 
 #if DCHECK_IS_ON()
     virtual bool isEndAndPairedWith(DisplayItem::Type otherType) const { return false; }
     virtual bool equals(const DisplayItem& other) const
     {
-        return m_client == other.m_client
-            && m_type == other.m_type
-            && m_derivedSize == other.m_derivedSize
-            && m_skippedCache == other.m_skippedCache;
+        if (m_type != other.m_type)
+            return false;
+        DCHECK(m_derivedSize == other.m_derivedSize);
+        DCHECK(m_derivedLog2Alignment == other.m_derivedLog2Alignment);
+        return m_client == other.m_client && m_skippedCache == other.m_skippedCache;
     }
 #endif
 
     // True if the client is non-null. Because m_client is const, this should
     // never be false except when we explicitly create a tombstone/"dead display
     // item" as part of moving an item from one list to another (see:
     // DisplayItemList::appendByMoving).
     bool hasValidClient() const { return m_client; }
 
     virtual bool drawsContent() const { return false; }
 
     // Override to implement specific analysis strategies.
     virtual void analyzeForGpuRasterization(SkPictureGpuAnalyzer&) const { }
 
 #ifndef NDEBUG
     static WTF::String typeAsDebugString(DisplayItem::Type);
     const WTF::String clientDebugString() const { return m_clientDebugString; }
     void setClientDebugString(const WTF::String& s) { m_clientDebugString = s; }
     WTF::String asDebugString() const;
     virtual void dumpPropertiesAsDebugString(WTF::StringBuilder&) const;
 #endif
 
 private:
     // The default DisplayItem constructor is only used by
     // ContiguousContainer::appendByMoving where an invalid DisplaItem is
     // constructed at the source location.
-    template <typename T, unsigned alignment> friend class ContiguousContainer;
+    template <typename T> friend class ContiguousContainer;
 
     DisplayItem()
         : m_client(nullptr)
         , m_type(UninitializedType)
         , m_derivedSize(sizeof(*this))
+        , m_derivedLog2Alignment(log2Alignment<DisplayItem>())
         , m_skippedCache(false)
     { }
 
     const DisplayItemClient* m_client;
     static_assert(TypeLast < (1 << 16), "DisplayItem::Type should fit in 16 bits");
     const Type m_type : 16;
     const unsigned m_derivedSize : 8; // size of the actual derived class
+    const unsigned m_derivedLog2Alignment : 3; // log2(alignment of the actual derived class)
     unsigned m_skippedCache : 1;
 
 #ifndef NDEBUG
     WTF::String m_clientDebugString;
 #endif
 };
 
-class PLATFORM_EXPORT PairedBeginDisplayItem : public DisplayItem {
+template <typename T>
+class DisplayItemBase : public DisplayItem {

[jbroman, 2016/07/04 19:21:35]

nit: it seems a little weird to have DisplayItemBase be a derived class of
DisplayItem, rather than its base class.

 protected:
-    PairedBeginDisplayItem(const DisplayItemClient& client, Type type, size_t derivedSize) : DisplayItem(client, type, derivedSize) { }
+    DisplayItemBase(const DisplayItemClient& client, Type type)
+        : DisplayItem(client, type, sizeof(T), log2Alignment<T>()) { }
+};
+
+template <typename T>
+class PairedBeginDisplayItem : public DisplayItemBase<T> {
+protected:
+    PairedBeginDisplayItem(const DisplayItemClient& client, DisplayItem::Type type)
+        : DisplayItemBase<T>(client, type) { }
 
 private:
     bool isBegin() const final { return true; }
 };
 
-class PLATFORM_EXPORT PairedEndDisplayItem : public DisplayItem {
+template <typename T>
+class PairedEndDisplayItem : public DisplayItemBase<T> {
 protected:
-    PairedEndDisplayItem(const DisplayItemClient& client, Type type, size_t derivedSize) : DisplayItem(client, type, derivedSize) { }
+    PairedEndDisplayItem(const DisplayItemClient& client, DisplayItem::Type type)
+        : DisplayItemBase<T>(client, type) { }
 
 #if ENABLE(ASSERT)
     bool isEndAndPairedWith(DisplayItem::Type otherType) const override = 0;
 #endif
 
 private:
     bool isEnd() const final { return true; }
 };
 
 } // namespace blink
 
 #endif // DisplayItem_h