Fix several corner case issues of scrollbar paint invalidation

third_party/WebKit/Source/core/paint/PaintInvalidationCapableScrollableArea.cpp

Index: third_party/WebKit/Source/core/paint/PaintInvalidationCapableScrollableArea.cpp
diff --git a/third_party/WebKit/Source/core/paint/PaintInvalidationCapableScrollableArea.cpp b/third_party/WebKit/Source/core/paint/PaintInvalidationCapableScrollableArea.cpp
index 8b1831a065130648a8329c5998bcbce4a11851fa..24176d7100362745d4d5b9b1f376ca3129489d4f 100644
--- a/third_party/WebKit/Source/core/paint/PaintInvalidationCapableScrollableArea.cpp
+++ b/third_party/WebKit/Source/core/paint/PaintInvalidationCapableScrollableArea.cpp
@@ -12,6 +12,7 @@
 #include "core/layout/LayoutScrollbarPart.h"
 #include "core/layout/PaintInvalidationState.h"
 #include "core/paint/PaintLayer.h"
+#include "platform/graphics/GraphicsLayer.h"
 
 namespace blink {
 
@@ -24,37 +25,49 @@ void PaintInvalidationCapableScrollableArea::willRemoveScrollbar(Scrollbar* scro
     ScrollableArea::willRemoveScrollbar(scrollbar, orientation);
 }
 
-// Returns true if the scroll control is invalidated.
-static bool invalidatePaintOfScrollControlIfNeeded(const IntRect& scrollControlRect, LayoutRect& previousPaintInvalidationRect, bool needsPaintInvalidation, LayoutBox& box, const PaintInvalidationState& paintInvalidationState, const LayoutBoxModelObject& paintInvalidationContainer)
+static LayoutRect scrollControlPaintInvalidationRect(const IntRect& scrollControlRect, const LayoutBox& box, const PaintInvalidationState& paintInvalidationState, const LayoutBoxModelObject& paintInvalidationContainer)
 {
     LayoutRect paintInvalidationRect(scrollControlRect);
     if (!paintInvalidationRect.isEmpty())
         PaintLayer::mapRectToPaintInvalidationBacking(&box, &paintInvalidationContainer, paintInvalidationRect, &paintInvalidationState);
+    return paintInvalidationRect;
+}
 
-    if (paintInvalidationRect != previousPaintInvalidationRect) {
+// Returns true if the scroll control is invalidated.
+static bool invalidatePaintOfScrollControlIfNeeded(const LayoutRect& newPaintInvalidationRect, const LayoutRect& previousPaintInvalidationRect, bool needsPaintInvalidation, LayoutBox& box, const LayoutBoxModelObject& paintInvalidationContainer)
+{
+    if (newPaintInvalidationRect != previousPaintInvalidationRect) {
         box.invalidatePaintUsingContainer(paintInvalidationContainer, previousPaintInvalidationRect, PaintInvalidationScroll);
-        previousPaintInvalidationRect = paintInvalidationRect;
         needsPaintInvalidation = true;
     }
-
     if (needsPaintInvalidation) {
-        box.invalidatePaintUsingContainer(paintInvalidationContainer, paintInvalidationRect, PaintInvalidationScroll);
+        box.invalidatePaintUsingContainer(paintInvalidationContainer, newPaintInvalidationRect, PaintInvalidationScroll);
+        box.enclosingLayer()->setNeedsRepaint();
         return true;
     }
-
     return false;
 }
 
-static void invalidatePaintOfScrollbarIfNeeded(Scrollbar* scrollbar, GraphicsLayer* scrollbarGraphicsLayer, LayoutRect& previousPaintInvalidationRect, bool needsPaintInvalidation, LayoutBox& box, const PaintInvalidationState& paintInvalidationState, const LayoutBoxModelObject& paintInvalidationContainer)
+static void invalidatePaintOfScrollbarIfNeeded(Scrollbar* scrollbar, GraphicsLayer* scrollbarGraphicsLayer, const LayoutRect& newPaintInvalidationRect, const LayoutRect& previousPaintInvalidationRect, bool needsPaintInvalidation, LayoutBox& box, const LayoutBoxModelObject& paintInvalidationContainer)
 {
-    IntRect scrollbarRect = scrollbar && !scrollbarGraphicsLayer ? scrollbar->frameRect() : IntRect();
-    if (!invalidatePaintOfScrollControlIfNeeded(scrollbarRect, previousPaintInvalidationRect, needsPaintInvalidation, box, paintInvalidationState, paintInvalidationContainer))
+    if (scrollbarGraphicsLayer && needsPaintInvalidation) {
+        scrollbarGraphicsLayer->setNeedsDisplay();
+        scrollbarGraphicsLayer->setContentsNeedsDisplay();
+        // If the scrollbar needs paint invalidation but didn't change location/size or the scrollbar is an
+        // overlay scrollbar (paint invalidation rect is empty), invalidating the graphics layer is enough.
+        // Otherwise invalidatePaintOfScrollControlIfNeeded() below will invalidate the old and new location
+        // of the scrollbar on the box's paint invalidation container to ensure newly expanded/shrunk areas
+        // of the box to be invalidated.
+        needsPaintInvalidation = false;
+    }
+
+    if (!invalidatePaintOfScrollControlIfNeeded(newPaintInvalidationRect, previousPaintInvalidationRect, needsPaintInvalidation, box, paintInvalidationContainer))
         return;
-    if (!scrollbar)
+
+    if (!scrollbar || scrollbarGraphicsLayer)
         return;
 
     paintInvalidationContainer.invalidateDisplayItemClientOnBacking(*scrollbar, PaintInvalidationScroll, &previousPaintInvalidationRect);
-    box.enclosingLayer()->setNeedsRepaint();
     if (scrollbar->isCustomScrollbar())
         toLayoutScrollbar(scrollbar)->invalidateDisplayItemClientsOfScrollbarParts(paintInvalidationContainer, previousPaintInvalidationRect);
 }
@@ -62,18 +75,40 @@ static void invalidatePaintOfScrollbarIfNeeded(Scrollbar* scrollbar, GraphicsLay
 void PaintInvalidationCapableScrollableArea::invalidatePaintOfScrollControlsIfNeeded(const PaintInvalidationState& paintInvalidationState, const LayoutBoxModelObject& paintInvalidationContainer)

[chrishtr, 2015/12/03 18:52:36]

There looks to be a lot of repeating code in this method. Also, it's hard to
read. Would it help to factor out an invalidation object that is repeated for
horizontal and vertical scrollbars? It would store invalidation rects and have
the other business logic below.

Or maybe a template/helper method?

[Xianzhu, 2015/12/03 22:13:27]

Done.

 {
     LayoutBox& box = boxForScrollControlPaintInvalidation();
-    LayoutRect oldRect = m_horizontalScrollbarPreviousPaintInvalidationRect;
-    invalidatePaintOfScrollbarIfNeeded(horizontalScrollbar(), layerForHorizontalScrollbar(), m_horizontalScrollbarPreviousPaintInvalidationRect, horizontalScrollbarNeedsPaintInvalidation(), box, paintInvalidationState, paintInvalidationContainer);
-    bool scrollbarGeometryChanged = oldRect != m_horizontalScrollbarPreviousPaintInvalidationRect;
-
-    oldRect = m_verticalScrollbarPreviousPaintInvalidationRect;
-    invalidatePaintOfScrollbarIfNeeded(verticalScrollbar(), layerForVerticalScrollbar(), m_verticalScrollbarPreviousPaintInvalidationRect, verticalScrollbarNeedsPaintInvalidation(), box, paintInvalidationState, paintInvalidationContainer);
-    scrollbarGeometryChanged |= oldRect != m_verticalScrollbarPreviousPaintInvalidationRect;
-
-    if (scrollbarGeometryChanged)
+    Scrollbar* horizontalScrollbar = this->horizontalScrollbar();
+    Scrollbar* verticalScrollbar = this->verticalScrollbar();
+    GraphicsLayer* layerForHorizontalScrollbar = this->layerForHorizontalScrollbar();
+    GraphicsLayer* layerForVerticalScrollbar = this->layerForVerticalScrollbar();
+    bool horizontalScrollbarIsOverlay = horizontalScrollbar && horizontalScrollbar->isOverlayScrollbar();
+    bool verticalScrollbarIsOverlay = verticalScrollbar && verticalScrollbar->isOverlayScrollbar();
+
+    // Calculate paint invalidation rects of the scrollbars, except overlay composited scrollbars because we invalidate the graphics layer only.
+    LayoutRect horizontalScrollbarPaintInvalidationRect;
+    if (horizontalScrollbar && !(layerForHorizontalScrollbar && horizontalScrollbarIsOverlay))
+        horizontalScrollbarPaintInvalidationRect = scrollControlPaintInvalidationRect(horizontalScrollbar->frameRect(), box, paintInvalidationState, paintInvalidationContainer);
+    LayoutRect verticalScrollbarPaintInvalidationRect;
+    if (verticalScrollbar && !(layerForVerticalScrollbar && verticalScrollbarIsOverlay))
+        verticalScrollbarPaintInvalidationRect = scrollControlPaintInvalidationRect(verticalScrollbar->frameRect(), box, paintInvalidationState, paintInvalidationContainer);
+
+    invalidatePaintOfScrollbarIfNeeded(horizontalScrollbar, layerForHorizontalScrollbar, horizontalScrollbarPaintInvalidationRect, m_horizontalScrollbarPreviousPaintInvalidationRect, horizontalScrollbarNeedsPaintInvalidation(), box, paintInvalidationContainer);
+    invalidatePaintOfScrollbarIfNeeded(verticalScrollbar, layerForVerticalScrollbar, verticalScrollbarPaintInvalidationRect, m_verticalScrollbarPreviousPaintInvalidationRect, verticalScrollbarNeedsPaintInvalidation(), box, paintInvalidationContainer);
+
+    // Invalidate the box's display item client if the box's padding box size is affected by change of non-overlay scrollbar widths.
+    LayoutSize horizontalScrollbarSize = horizontalScrollbarIsOverlay ? LayoutSize() : horizontalScrollbarPaintInvalidationRect.size();
+    LayoutSize verticalScrollbarSize = verticalScrollbarIsOverlay ? LayoutSize() : verticalScrollbarPaintInvalidationRect.size();
+    LayoutSize horizontalScrollbarPreviousSize = m_horizontalScrollbarPreviouslyWasOverlay ? LayoutSize() : m_horizontalScrollbarPreviousPaintInvalidationRect.size();
+    LayoutSize verticalScrollbarPreviousSize = m_verticalScrollbarPreviouslyWasOverlay ? LayoutSize() : m_verticalScrollbarPreviousPaintInvalidationRect.size();
+    if (horizontalScrollbarSize != horizontalScrollbarPreviousSize || verticalScrollbarSize != verticalScrollbarPreviousSize)
         paintInvalidationContainer.invalidateDisplayItemClientOnBacking(box, PaintInvalidationScroll, nullptr);
 
-    if (invalidatePaintOfScrollControlIfNeeded(scrollCornerRect(), m_scrollCornerPreviousPaintInvalidationRect, scrollCornerNeedsPaintInvalidation(), box, paintInvalidationState, paintInvalidationContainer)) {
+    m_horizontalScrollbarPreviousPaintInvalidationRect = horizontalScrollbarPaintInvalidationRect;
+    m_horizontalScrollbarPreviouslyWasOverlay = horizontalScrollbarIsOverlay;
+    m_verticalScrollbarPreviousPaintInvalidationRect = verticalScrollbarPaintInvalidationRect;
+    m_verticalScrollbarPreviouslyWasOverlay = verticalScrollbarIsOverlay;
+
+    LayoutRect scrollCornerPaintInvalidationRect = scrollControlPaintInvalidationRect(scrollCornerRect(), box, paintInvalidationState, paintInvalidationContainer);
+    if (invalidatePaintOfScrollControlIfNeeded(scrollCornerPaintInvalidationRect, m_scrollCornerPreviousPaintInvalidationRect, scrollCornerNeedsPaintInvalidation(), box, paintInvalidationContainer)) {
+        m_scrollCornerPreviousPaintInvalidationRect = scrollCornerPaintInvalidationRect;
         if (LayoutScrollbarPart* scrollCorner = this->scrollCorner())
             scrollCorner->invalidateDisplayItemClientsIncludingNonCompositingDescendants(&paintInvalidationContainer, PaintInvalidationScroll, &m_scrollCornerPreviousPaintInvalidationRect);
         if (LayoutScrollbarPart* resizer = this->resizer())
@@ -83,4 +118,11 @@ void PaintInvalidationCapableScrollableArea::invalidatePaintOfScrollControlsIfNe
     clearNeedsPaintInvalidationForScrollControls();
 }
 
+void PaintInvalidationCapableScrollableArea::clearPreviousPaintInvalidationRects()
+{
+    m_horizontalScrollbarPreviousPaintInvalidationRect = LayoutRect();
+    m_verticalScrollbarPreviousPaintInvalidationRect = LayoutRect();
+    m_scrollCornerPreviousPaintInvalidationRect = LayoutRect();
+}
+
 } // namespace blink