Fix scrollbar damage accumulation issue

Source/platform/scroll/ScrollableArea.h

Index: Source/platform/scroll/ScrollableArea.h
diff --git a/Source/platform/scroll/ScrollableArea.h b/Source/platform/scroll/ScrollableArea.h
index 24e65351c7c8628e261d839f18ad546b3760c8ea..8584c2627c7f593a0859287016dcefee744a43ac 100644
--- a/Source/platform/scroll/ScrollableArea.h
+++ b/Source/platform/scroll/ScrollableArea.h
@@ -204,26 +204,25 @@ public:
     int maximumScrollPosition(ScrollbarOrientation orientation) { return orientation == HorizontalScrollbar ? maximumScrollPosition().x() : maximumScrollPosition().y(); }
     int clampScrollPosition(ScrollbarOrientation orientation, int pos)  { return std::max(std::min(pos, maximumScrollPosition(orientation)), minimumScrollPosition(orientation)); }
 
-    bool hasVerticalBarDamage() const { return m_hasVerticalBarDamage; }
-    bool hasHorizontalBarDamage() const { return m_hasHorizontalBarDamage; }
+    bool hasVerticalBarDamage() const { return !m_verticalBarDamage.isEmpty(); }
+    bool hasHorizontalBarDamage() const { return !m_horizontalBarDamage.isEmpty(); }
+    const IntRect& verticalBarDamage() const { return m_verticalBarDamage; }
+    const IntRect& horizontalBarDamage() const { return m_horizontalBarDamage; }
 
-    const IntRect& verticalBarDamage() const
+    void addScrollbarDamage(Scrollbar* scrollbar, const IntRect& rect)

[Julien - ping for review, 2014/07/23 20:46:23]

As discussed, the code seems to assume a single invalidation rectangle so it's
not really addScrollbardDamage, more a setScrollbarDamage.

As we make this change, we should add some
ASSERT_RELEASE(m_horizontalBarDamage.isEmpty()) in the horizontalBarDamage
branch. Same for vertical scrollbars.

[Xianzhu, 2014/07/23 22:45:31]

Just find a case that a scrollbar may be invalidated several times with
different rects.

The call stack (reproduced on Linux by undefining THUMB_POSITION_AFFECTS_BUTTONS
in Scrollbar.cpp is):
  RenderLayerScrollableArea::invalidateScrollbarRect()
  Scrollbar::invalidateRect()
  ScrollbarTheme::invalidatePart() <-- Called 3 times
  ScrollbarTheme::invalidateParts(ForwardTrackPart | BackTrackPart | ThumbPart)
  Scrollbar::updateThumb()
  Scrollbar::updateThumbPosition()
  ...

     {
-        ASSERT(m_hasVerticalBarDamage);
-        return m_verticalBarDamage;
-    }
-
-    const IntRect& horizontalBarDamage() const
-    {
-        ASSERT(m_hasHorizontalBarDamage);
-        return m_horizontalBarDamage;
+        if (scrollbar == horizontalScrollbar())
+            m_horizontalBarDamage.unite(rect);
+        else
+            m_verticalBarDamage.unite(rect);
     }
 
     void resetScrollbarDamage()

[Julien - ping for review, 2014/07/23 20:46:23]

I think we should remove this function.

All callers do:
- invalidate horizontal scrollbar.
- invalidate vertical scrollbar.
- resetScrollbarDamage.

That means that we should just have an API (invalidateScrollbars()) that takes
care of all the previous steps so that the callers doesn't have to take care of
it.

[Xianzhu, 2014/07/23 23:55:01]

That seems not feasible because the two callers (FrameView and RenderBox) have
different methods to invalidate rects. ScrollableArea doesn't (and should not)
know how to invalidate an area in its container.

In addition, I'd also like to make the damage flags be part of
paintInvalidationState, so that paintInvalidationStateIsDirty can check it.

[Xianzhu, 2014/07/24 00:09:36]

There is also a case that we need to invalidate and reset the damages
separately: when view is doing full repaint, we should not invalidate the
damages, but need to reset them.

I think this is also a reason that Dan and I wanted to make the damages be a
part of paintInvalidationState.

     {
-        m_hasVerticalBarDamage = false;
-        m_hasHorizontalBarDamage = false;
+        m_verticalBarDamage = IntRect();
+        m_horizontalBarDamage = IntRect();
     }
+
     virtual GraphicsLayer* layerForContainer() const;
     virtual GraphicsLayer* layerForScrolling() const { return 0; }
     virtual GraphicsLayer* layerForHorizontalScrollbar() const { return 0; }
@@ -245,13 +244,6 @@ protected:
     virtual void invalidateScrollbarRect(Scrollbar*, const IntRect&) = 0;
     virtual void invalidateScrollCornerRect(const IntRect&) = 0;
 
-    // For repaint after layout, stores the damage to be repainted for the
-    // scrollbars.
-    unsigned m_hasHorizontalBarDamage : 1;
-    unsigned m_hasVerticalBarDamage : 1;
-    IntRect m_horizontalBarDamage;
-    IntRect m_verticalBarDamage;
-
 private:
     void scrollPositionChanged(const IntPoint&);
 
@@ -268,6 +260,11 @@ private:
     virtual int documentStep(ScrollbarOrientation) const;
     virtual float pixelStep(ScrollbarOrientation) const;
 
+    // For repaint after layout, stores the damage to be repainted for the
+    // scrollbars.
+    IntRect m_horizontalBarDamage;
+    IntRect m_verticalBarDamage;
+
     struct ScrollableAreaAnimators {
         OwnPtr<ScrollAnimator> scrollAnimator;
         OwnPtr<ProgrammaticScrollAnimator> programmaticScrollAnimator;