Don't access DisplayItemClient::VisualRect() for cached display items.

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"
 #include "platform/wtf/Allocator.h"
 #include "platform/wtf/Assertions.h"
 #include "platform/wtf/Noncopyable.h"
 
 #ifndef NDEBUG
 #include "platform/wtf/text/StringBuilder.h"
 #include "platform/wtf/text/WTFString.h"
 #endif
 
 namespace blink {
 
 class GraphicsContext;
-class IntRect;
+class LayoutSize;

[pdr., 2017/05/19 17:11:36]

Curious: why a LayoutSize instead of an IntSize or a FloatSize?

[Xianzhu, 2017/05/19 18:06:33]

This is for the "const LayoutSize& visual_rect_offset" parameter of
AppendToWebDisplayItemList(). Previously the parameter was "const IntRect&
visual_rect".

The offset is from
GraphicsLayer::OffsetFromLayoutObjectWithSubpixelAccumulation() which is a
LayoutSize.

Added comments about the parameter.

 class WebDisplayItemList;
 
 class PLATFORM_EXPORT DisplayItem {
   DISALLOW_NEW_EXCEPT_PLACEMENT_NEW();
 
  public:
   enum {
     // Must be kept in sync with core/paint/PaintPhase.h.
     kPaintPhaseMax = 11,
   };
@@ skipping 158 matching lines @@
   // border type.
   enum TableCollapsedBorderSides {
     kTableCollapsedBorderTop = 1 << 0,
     kTableCollapsedBorderRight = 1 << 1,
     kTableCollapsedBorderBottom = 1 << 2,
     kTableCollapsedBorderLeft = 1 << 3,
   };
 
   DisplayItem(const DisplayItemClient& client, Type type, size_t derived_size)
       : client_(&client),
+        visual_rect_(client.VisualRect()),
         type_(type),
         derived_size_(derived_size),
         skipped_cache_(false)
 #ifndef NDEBUG
         ,
         client_debug_string_(client.DebugName())
 #endif
   {
     // derivedSize must fit in m_derivedSize.
     // If it doesn't, enlarge m_derivedSize and fix this assert.
@@ skipping 14 matching lines @@
   };
 
   Id GetId() const { return Id(*client_, type_); }
 
   virtual void Replay(GraphicsContext&) const {}
 
   const DisplayItemClient& Client() const {
     DCHECK(client_);
     return *client_;
   }
+
+  const LayoutRect& VisualRect() const { return visual_rect_; }

[pdr., 2017/05/19 17:11:36]

Can you add a comment here about what space this rect is in?

[Xianzhu, 2017/05/19 18:06:33]

Done.

+
   Type GetType() const { return 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 derived_size_; }
 
   // For PaintController only. Painters should use DisplayItemCacheSkipper
   // instead.
   void SetSkippedCache() { skipped_cache_ = true; }
   bool SkippedCache() const { return skipped_cache_; }
 
-  // TODO(wkorman): Only DrawingDisplayItem needs the visual rect argument.
-  // Consider refactoring class hierarchy to make this more explicit.
-  virtual void AppendToWebDisplayItemList(const IntRect&,
+  virtual void AppendToWebDisplayItemList(const LayoutSize&,
                                           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 >= k##Category##First && type <= k##Category##Last; \
   }                                                                 \
   bool Is##Category() const { return Is##Category##Type(GetType()); }
 
 #define DEFINE_CONVERSION_METHODS(Category1, category1, Category2, category2) \
@@ skipping 82 matching lines @@
   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;
+  friend class DisplayItemList;
 
   DisplayItem()
       : client_(nullptr),
         type_(kUninitializedType),
         derived_size_(sizeof(*this)),
         skipped_cache_(false) {}
 
   const DisplayItemClient* client_;
+  LayoutRect visual_rect_;
+
   static_assert(kTypeLast < (1 << 16),
                 "DisplayItem::Type should fit in 16 bits");
   const Type type_ : 16;
   const unsigned derived_size_ : 8;  // size of the actual derived class
   unsigned skipped_cache_ : 1;
 
 #ifndef NDEBUG
   WTF::String client_debug_string_;
 #endif
 };
@@ skipping 28 matching lines @@
   bool IsEndAndPairedWith(DisplayItem::Type other_type) const override = 0;
 #endif
 
  private:
   bool IsEnd() const final { return true; }
 };
 
 }  // namespace blink
 
 #endif  // DisplayItem_h