Move selection invalidation to the invalidation phase

Source/core/rendering/RenderObject.h

Index: Source/core/rendering/RenderObject.h
diff --git a/Source/core/rendering/RenderObject.h b/Source/core/rendering/RenderObject.h
index bdeea0899053be872952974f06ddaaccbb26027c..6aa2a76b9dbd42f1bda0dfd19c11a21027477a36 100644
--- a/Source/core/rendering/RenderObject.h
+++ b/Source/core/rendering/RenderObject.h
@@ -998,6 +998,9 @@ public:
     const LayoutRect& previousPaintInvalidationRect() const { return m_previousPaintInvalidationRect; }
     void setPreviousPaintInvalidationRect(const LayoutRect& rect) { m_previousPaintInvalidationRect = rect; }
 
+    const LayoutRect& previousSelectionRectForPaintInvalidation() const { return m_previousSelectionRectForPaintInvalidation; }
+    void setpreviousSelectionRectForPaintInvalidation(const LayoutRect& rect) { m_previousSelectionRectForPaintInvalidation = rect; setShouldInvalidateSelection(); }

[Xianzhu, 2014/10/30 23:35:42]

/setprevious/setPrevious/

And it seems better to make it one statement per line.

[Julien - ping for review, 2014/10/31 22:31:03]

Doh', fixed!
Good point, I refactored the logic, moving the implementation into the cpp file.

+
     // The previous position of the top-left corner of the object in its previous paint backing.
     const LayoutPoint& previousPositionFromPaintInvalidationBacking() const { return m_previousPositionFromPaintInvalidationBacking; }
     void setPreviousPositionFromPaintInvalidationBacking(const LayoutPoint& positionFromPaintInvalidationBacking) { m_previousPositionFromPaintInvalidationBacking = positionFromPaintInvalidationBacking; }
@@ -1391,9 +1394,12 @@ private:
     // Store state between styleWillChange and styleDidChange
     static bool s_affectsParentBlock;
 
-    // This stores the paint invalidation rect from the previous layout.
+    // This stores the paint invalidation rect from the previous frame.
     LayoutRect m_previousPaintInvalidationRect;
 
+    // This stores the selection invalidation rect from the previous frame.
+    LayoutRect m_previousSelectionRectForPaintInvalidation;

[Xianzhu, 2014/10/30 23:35:42]

Seems not the best to add another rect :(

As only a very small set of object can have selection at a certain time, can we
use the global map method (like RenderBox's
gOverrideContainingBlockLogicalHeightMap) to store it?

[dsinclair, 2014/10/31 14:03:16]

Global maps make things more confusing. Do we know if this will cause noticeable
performance or memory issues?

[Xianzhu, 2014/10/31 15:52:35]

Got the feeling that every bit in RenderObject is precious when I tried to add a
m_previousBorderBox field and got an objection. I'm feeling a bit unfair that I
couldn't add that field which is also critical for correct incremental
invalidation for boxes with children visual overflow :)

No objection to the new field if we have data about RenderObject memory usage.
(And I believe these would be gone with slimming paint because there would be no
more paintInvalidationRects.)

[Julien - ping for review, 2014/10/31 22:31:03]

I think I have found a way to make a map work (didn't think it was possible but
I overlooked some details). That will prevent another RenderObject size
increase.

+
     // This stores the position in the paint invalidation backing's coordinate.
     // It is used to detect renderer shifts that forces a full invalidation.
     LayoutPoint m_previousPositionFromPaintInvalidationBacking;