Move selection invalidation to the invalidation phase

Source/core/rendering/RenderObject.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
  * modify it under the terms of the GNU Library General Public
@@ skipping 980 matching lines @@
     RenderObject* rendererForRootBackground();
 
     RespectImageOrientationEnum shouldRespectImageOrientation() const;
 
     bool isRelayoutBoundaryForInspector() const;
 
     // The previous paint invalidation rect in the object's previous paint backing.
     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; }
 
     bool shouldDoFullPaintInvalidation() const { return m_bitfields.fullPaintInvalidationReason() != PaintInvalidationNone; }
     void setShouldDoFullPaintInvalidation(PaintInvalidationReason = PaintInvalidationFull);
     void clearShouldDoFullPaintInvalidation() { m_bitfields.setFullPaintInvalidationReason(PaintInvalidationNone); }
 
     bool shouldInvalidateOverflowForPaint() const { return m_bitfields.shouldInvalidateOverflowForPaint(); }
 
@@ skipping 373 matching lines @@
     void setIsDragging(bool b) { m_bitfields.setIsDragging(b); }
     void setEverHadLayout(bool b) { m_bitfields.setEverHadLayout(b); }
     void setShouldInvalidateOverflowForPaint(bool b) { m_bitfields.setShouldInvalidateOverflowForPaint(b); }
     void setSelfNeedsOverflowRecalcAfterStyleChange(bool b) { m_bitfields.setSelfNeedsOverflowRecalcAfterStyleChange(b); }
     void setChildNeedsOverflowRecalcAfterStyleChange(bool b) { m_bitfields.setChildNeedsOverflowRecalcAfterStyleChange(b); }
 
 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;
 
     static unsigned s_instanceCount;
 };
 
 // FIXME: remove this once the render object lifecycle ASSERTS are no longer hit.
 class DeprecatedDisableModifyRenderTreeStructureAsserts {
     WTF_MAKE_NONCOPYABLE(DeprecatedDisableModifyRenderTreeStructureAsserts);
@@ skipping 206 matching lines @@
 void showTree(const blink::RenderObject*);
 void showLineTree(const blink::RenderObject*);
 void showRenderTree(const blink::RenderObject* object1);
 // We don't make object2 an optional parameter so that showRenderTree
 // can be called from gdb easily.
 void showRenderTree(const blink::RenderObject* object1, const blink::RenderObject* object2);
 
 #endif
 
 #endif // RenderObject_h