Combine LayoutObject::previousPaintOffset and paintOffset in paint properties

third_party/WebKit/Source/core/layout/LayoutObject.h

 /*
  * Copyright (C) 2000 Lars Knoll (knoll@kde.org)
  *           (C) 2000 Antti Koivisto (koivisto@kde.org)
  *           (C) 2000 Dirk Mueller (mueller@kde.org)
  *           (C) 2004 Allan Sandfeld Jensen (kde@carewolf.com)
  * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2012 Apple Inc.
  *               All rights reserved.
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
@@ skipping 1557 matching lines @@
   // container (*not* the graphics layer that paints this object).
   LayoutRect previousVisualRectIncludingCompositedScrolling(
       const LayoutBoxModelObject& paintInvalidationContainer) const;
 
   // The returned rect does *not* account for composited scrolling.
   const LayoutRect& previousVisualRect() const { return m_previousVisualRect; }
 
   // Called when the previous visual rect(s) is no longer valid.
   virtual void clearPreviousVisualRects();
 
-  const LayoutPoint& previousPaintOffset() const {
-    return m_previousPaintOffset;
-  }
+  const LayoutPoint& paintOffset() const { return m_paintOffset; }
 
   PaintInvalidationReason fullPaintInvalidationReason() const {
     return m_bitfields.fullPaintInvalidationReason();
   }
   bool shouldDoFullPaintInvalidation() const {
     return m_bitfields.fullPaintInvalidationReason() != PaintInvalidationNone;
   }
   void setShouldDoFullPaintInvalidation(
       PaintInvalidationReason = PaintInvalidationFull);
   void clearShouldDoFullPaintInvalidation() {
@@ skipping 81 matching lines @@
     void setBackgroundChangedSinceLastPaintInvalidation() {
       m_layoutObject.setBackgroundChangedSinceLastPaintInvalidation();
     }
     void ensureIsReadyForPaintInvalidation() {
       m_layoutObject.ensureIsReadyForPaintInvalidation();
     }
 
     void setPreviousVisualRect(const LayoutRect& r) {
       m_layoutObject.setPreviousVisualRect(r);
     }
-    void setPreviousPaintOffset(const LayoutPoint& p) {
+    void setPaintOffset(const LayoutPoint& p) {

[Xianzhu, 2017/01/06 17:24:39]

Drop "Previous" because we also use paintOffset() in the same cycle and
previousPaintOffset() would confuse in the situation.

Would also like to drop other "previous"s in e.g. previousVisualRect because
they have the same situation.

We could alternatively use 'Last' instead of 'Previous'. Would like to hear your
opinions.

[pdr., 2017/01/06 18:27:42]

I like these renames.

Can you add a comment like so:
// Store the current paint offset as calculated during the pre-paint tree walk.
And an assert here that this is only called during InPrePaint?

Similarly for paintOffset(), can you add a comment like so:
// Paint offset as calculated in the pre-paint tree walk.
And an assert that it is only accessed in or after PrePaint?

[Xianzhu, 2017/01/06 19:24:18]

Done.

       DCHECK(RuntimeEnabledFeatures::slimmingPaintInvalidationEnabled());
-      m_layoutObject.m_previousPaintOffset = p;
+      m_layoutObject.m_paintOffset = p;
     }
     void setHasPreviousLocationInBacking(bool b) {
       m_layoutObject.m_bitfields.setHasPreviousLocationInBacking(b);
     }
     void setHasPreviousSelectionVisualRect(bool b) {
       m_layoutObject.m_bitfields.setHasPreviousSelectionVisualRect(b);
     }
     void setHasPreviousBoxGeometries(bool b) {
       m_layoutObject.m_bitfields.setHasPreviousBoxGeometries(b);
     }
@@ skipping 740 matching lines @@
   // Store state between styleWillChange and styleDidChange
   static bool s_affectsParentBlock;
 
   // This stores the visual rect computed by the latest paint invalidation.
   // This rect does *not* account for composited scrolling. See
   // adjustVisualRectForCompositedScrolling().
   LayoutRect m_previousVisualRect;
 
   // This stores the paint offset computed by the latest paint property tree
   // building. It is relative to the containing transform space. It is the same
   // offset that will be used to paint the object on SPv2. It's used to detect

[pdr., 2017/01/06 18:27:42]

This comment is a little out of date. Can you update it?

[Xianzhu, 2017/01/06 19:24:18]

Can you point out the out of date part?

For paint invalidation and painting it still applies for SPv2 only because the
paint offset used in painting on SPv1 is sometimes different because of paint
invalidation containers.

[pdr., 2017/01/06 19:40:01]

You're right, this is up to date.

   // paint offset change for paint invalidation on SPv2, and partial paint
   // property tree update for SlimmingPaintInvalidation on SPv1 and SPv2.
-  LayoutPoint m_previousPaintOffset;
+  LayoutPoint m_paintOffset;
 
   // For SPv2 only. The ObjectPaintProperties structure holds references to the
   // property tree nodes that are created by the layout object for painting.
   std::unique_ptr<ObjectPaintProperties> m_paintProperties;
 };
 
 // FIXME: remove this once the layout object lifecycle ASSERTS are no longer
 // hit.
 class DeprecatedDisableModifyLayoutTreeStructureAsserts {
   STACK_ALLOCATED();
@@ skipping 211 matching lines @@
 CORE_EXPORT void showLineTree(const blink::LayoutObject*);
 CORE_EXPORT void showLayoutTree(const blink::LayoutObject* object1);
 // We don't make object2 an optional parameter so that showLayoutTree
 // can be called from gdb easily.
 CORE_EXPORT void showLayoutTree(const blink::LayoutObject* object1,
                                 const blink::LayoutObject* object2);
 
 #endif
 
 #endif  // LayoutObject_h