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

third_party/WebKit/Source/platform/graphics/paint/DisplayItemList.h

Index: third_party/WebKit/Source/platform/graphics/paint/DisplayItemList.h
diff --git a/third_party/WebKit/Source/platform/graphics/paint/DisplayItemList.h b/third_party/WebKit/Source/platform/graphics/paint/DisplayItemList.h
index 0a48df379a67a535725ba2698636f1bf3f6dc8fb..aaf7a3954205f69c08ea938efb4e09b76df60ee7 100644
--- a/third_party/WebKit/Source/platform/graphics/paint/DisplayItemList.h
+++ b/third_party/WebKit/Source/platform/graphics/paint/DisplayItemList.h
@@ -32,27 +32,32 @@ class PLATFORM_EXPORT DisplayItemList
   DisplayItemList(size_t initial_size_bytes)
       : ContiguousContainer(kMaximumDisplayItemSize, initial_size_bytes) {}
   DisplayItemList(DisplayItemList&& source)
-      : ContiguousContainer(std::move(source)),
-        visual_rects_(std::move(source.visual_rects_)) {}
+      : ContiguousContainer(std::move(source)) {}
 
   DisplayItemList& operator=(DisplayItemList&& source) {
     ContiguousContainer::operator=(std::move(source));
-    visual_rects_ = std::move(source.visual_rects_);
     return *this;
   }
 
-  DisplayItem& AppendByMoving(DisplayItem&);
-
-  bool HasVisualRect(size_t index) const {
-    return index < visual_rects_.size();
-  }
-  IntRect VisualRect(size_t index) const {
-    DCHECK(HasVisualRect(index));
-    return visual_rects_[index];
+  DisplayItem& AppendByMoving(DisplayItem& item) {

[Xianzhu, 2017/05/19 00:27:42]

This is inlined because this function is a hot spot during painting when most of
the display items are cached.

[wkorman, 2017/05/19 00:55:08]

When do we use ALWAYS_INLINE? I see PaintPropertyTreebuilder uses that macro.

[Xianzhu, 2017/05/19 01:25:11]

We added ALWAYS_INLINE for some big private functions whose implementations are
in .cpp that are called from few places. I'm not sure why the compiler didn't
inline them even with 'inline'. It seems related to the size of the functions
and/or separation of declaration and implementation. Or perhaps just because
profiling is enabled, in the case ALWAYS_INLINE can make the profiling build
more similar to the real release build.

+#ifndef NDEBUG
+    String original_debug_string = item.AsDebugString();
+#endif
+    DCHECK(item.HasValidClient());
+    DisplayItem& result =
+        ContiguousContainer::AppendByMoving(item, item.DerivedSize());
+    // ContiguousContainer::AppendByMoving() calls an in-place constructor
+    // on item which replaces it with a tombstone/"dead display item" that
+    // can be safely destructed but should never be used.
+    DCHECK(!item.HasValidClient());
+#ifndef NDEBUG
+    // Save original debug string in the old item to help debugging.
+    item.SetClientDebugString(original_debug_string);
+#endif
+    item.visual_rect_ = result.visual_rect_;

[Xianzhu, 2017/05/19 00:27:42]

The above line is new, to save the old visual rect.

+    return result;
   }
 
-  void AppendVisualRect(const IntRect& visual_rect);
-
   // Useful for iterating with a range-based for loop.
   template <typename Iterator>
   class Range {
@@ -81,9 +86,6 @@ class PLATFORM_EXPORT DisplayItemList
   std::unique_ptr<JSONArray> SubsequenceAsJSON(size_t begin_index,
                                                size_t end_index,
                                                JsonFlags options) const;
-
- private:
-  Vector<IntRect> visual_rects_;
 };
 
 }  // namespace blink