Pixel result shows that compositing/iframes/repaint-after-losing-scrollbars.html...

https://bugs.webkit.org/show_bug.cgi?id=67858

Reviewed by Darin Adler.

When non-overlay scrollbars are hidden on a composited iframe, nothing invalidated
the scrollbar areas or the scroll corner, so the scrollbars appear to remain.

Fix by invalidating the scrollbars and scroll corner when they are removed. Invalidation
on scrollbar creation appears to happen via updating the scrollbar style.

Tested by compositing/iframes/repaint-after-losing-scrollbars.html which no longer shows
stale scrollbars when run manually, even though the green squares are missing from the
pixel result (bug 67878).

* page/FrameView.cpp:
(WebCore::FrameView::updateScrollCorner): Pass the corner rect into invalidateScrollCorner().
* platform/ScrollView.cpp:
(WebCore::ScrollView::setHasHorizontalScrollbar): Invalidate the scrollbar area if hiding it.
(WebCore::ScrollView::setHasVerticalScrollbar): Ditto.
(WebCore::ScrollView::updateScrollbars): In the case where both scrollbars are going away,
compute the scroll corner rect while we still have scrollbars, and then invalidate it
explicitly. (updateScrollCorner() doesn't, because it doesn't have access to the old corner
rect.)
* platform/ScrollableArea.cpp:
(WebCore::ScrollableArea::invalidateScrollCorner): Pass the rect in, because we can't
compute it in the case where the scrollbars are going away.
* platform/ScrollableArea.h: Pass in a rect to invalidateScrollCorner(), which matches
invalidateScrollbar().
* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::destroyRootLayer): Pass the corner rect into invalidateScrollCorner().
* rendering/RenderScrollbarPart.cpp: Ditto.
(WebCore::RenderScrollbarPart::imageChanged): Ditto.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@96070 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/compositing/iframes/repaint-after-losing-scrollbars-expected.png b/LayoutTests/compositing/iframes/repaint-after-losing-scrollbars-expected.png
index ea36292..3e311a1 100644 (file)
Binary files a/LayoutTests/compositing/iframes/repaint-after-losing-scrollbars-expected.png and b/LayoutTests/compositing/iframes/repaint-after-losing-scrollbars-expected.png differ

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 75d3af5..00238e5 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,5 +1,41 @@
 2011-09-09  Simon Fraser  <simon.fraser@apple.com>
 
+        Pixel result shows that compositing/iframes/repaint-after-losing-scrollbars.html is failing
+        https://bugs.webkit.org/show_bug.cgi?id=67858
+
+        Reviewed by Darin Adler.
+        
+        When non-overlay scrollbars are hidden on a composited iframe, nothing invalidated
+        the scrollbar areas or the scroll corner, so the scrollbars appear to remain.
+        
+        Fix by invalidating the scrollbars and scroll corner when they are removed. Invalidation
+        on scrollbar creation appears to happen via updating the scrollbar style.
+
+        Tested by compositing/iframes/repaint-after-losing-scrollbars.html which no longer shows
+        stale scrollbars when run manually, even though the green squares are missing from the
+        pixel result (bug 67878).
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::updateScrollCorner): Pass the corner rect into invalidateScrollCorner().
+        * platform/ScrollView.cpp:
+        (WebCore::ScrollView::setHasHorizontalScrollbar): Invalidate the scrollbar area if hiding it.
+        (WebCore::ScrollView::setHasVerticalScrollbar): Ditto.
+        (WebCore::ScrollView::updateScrollbars): In the case where both scrollbars are going away,
+        compute the scroll corner rect while we still have scrollbars, and then invalidate it
+        explicitly. (updateScrollCorner() doesn't, because it doesn't have access to the old corner
+        rect.)
+        * platform/ScrollableArea.cpp:
+        (WebCore::ScrollableArea::invalidateScrollCorner): Pass the rect in, because we can't
+        compute it in the case where the scrollbars are going away.
+        * platform/ScrollableArea.h: Pass in a rect to invalidateScrollCorner(), which matches
+        invalidateScrollbar().
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::destroyRootLayer): Pass the corner rect into invalidateScrollCorner().
+        * rendering/RenderScrollbarPart.cpp: Ditto.
+        (WebCore::RenderScrollbarPart::imageChanged): Ditto.
+
+2011-09-09  Simon Fraser  <simon.fraser@apple.com>
+
         Translucent scrollbars on composited layers render incorrectly
         https://bugs.webkit.org/show_bug.cgi?id=58515
 

diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp
index b2264a7..46ff100 100644 (file)
--- a/Source/WebCore/page/FrameView.cpp
+++ b/Source/WebCore/page/FrameView.cpp
@@ -2467,8 +2467,9 @@ void FrameView::updateScrollCorner()
 {
     RenderObject* renderer = 0;
     RefPtr<RenderStyle> cornerStyle;
+    IntRect cornerRect = scrollCornerRect();
     
-    if (!scrollCornerRect().isEmpty()) {
+    if (!cornerRect.isEmpty()) {
         // Try the <body> element first as a scroll corner source.
         Document* doc = m_frame->document();
         Element* body = doc ? doc->body() : 0;
@@ -2497,7 +2498,7 @@ void FrameView::updateScrollCorner()
         if (!m_scrollCorner)
             m_scrollCorner = new (renderer->renderArena()) RenderScrollbarPart(renderer->document());
         m_scrollCorner->setStyle(cornerStyle.release());
-        invalidateScrollCorner();
+        invalidateScrollCorner(cornerRect);
     } else if (m_scrollCorner) {
         m_scrollCorner->destroy();
         m_scrollCorner = 0;

diff --git a/Source/WebCore/platform/ScrollView.cpp b/Source/WebCore/platform/ScrollView.cpp
index fd07a8e..5124dc6 100644 (file)
--- a/Source/WebCore/platform/ScrollView.cpp
+++ b/Source/WebCore/platform/ScrollView.cpp
@@ -95,6 +95,7 @@ void ScrollView::setHasHorizontalScrollbar(bool hasBar)
         didAddHorizontalScrollbar(m_horizontalScrollbar.get());
         m_horizontalScrollbar->styleChanged();
     } else if (!hasBar && m_horizontalScrollbar) {
+        m_horizontalScrollbar->invalidate();
         willRemoveHorizontalScrollbar(m_horizontalScrollbar.get());
         removeChild(m_horizontalScrollbar.get());
         m_horizontalScrollbar = 0;
@@ -113,6 +114,7 @@ void ScrollView::setHasVerticalScrollbar(bool hasBar)
         didAddVerticalScrollbar(m_verticalScrollbar.get());
         m_verticalScrollbar->styleChanged();
     } else if (!hasBar && m_verticalScrollbar) {
+        m_verticalScrollbar->invalidate();
         willRemoveVerticalScrollbar(m_verticalScrollbar.get());
         removeChild(m_verticalScrollbar.get());
         m_verticalScrollbar = 0;
@@ -447,6 +449,8 @@ void ScrollView::updateScrollbars(const IntSize& desiredOffset)
         m_inUpdateScrollbars = false;
     }
 
+    IntRect oldScrollCornerRect = scrollCornerRect();
+
     bool hasHorizontalScrollbar = m_horizontalScrollbar;
     bool hasVerticalScrollbar = m_verticalScrollbar;
     
@@ -573,6 +577,8 @@ void ScrollView::updateScrollbars(const IntSize& desiredOffset)
         frameRectsChanged();
         positionScrollbarLayers();
         updateScrollCorner();
+        if (!m_horizontalScrollbar && !m_verticalScrollbar)
+            invalidateScrollCornerRect(oldScrollCornerRect);
     }
 
     IntPoint scrollPoint = adjustScrollPositionWithinRange(IntPoint(desiredOffset)) + IntSize(m_scrollOrigin.x(), m_scrollOrigin.y());

diff --git a/Source/WebCore/platform/ScrollableArea.cpp b/Source/WebCore/platform/ScrollableArea.cpp
index 4b51a65..c7a3997 100644 (file)
--- a/Source/WebCore/platform/ScrollableArea.cpp
+++ b/Source/WebCore/platform/ScrollableArea.cpp
@@ -274,7 +274,7 @@ void ScrollableArea::invalidateScrollbar(Scrollbar* scrollbar, const IntRect& re
     invalidateScrollbarRect(scrollbar, rect);
 }
 
-void ScrollableArea::invalidateScrollCorner()
+void ScrollableArea::invalidateScrollCorner(const IntRect& rect)
 {
 #if USE(ACCELERATED_COMPOSITING)
     if (GraphicsLayer* graphicsLayer = layerForScrollCorner()) {
@@ -282,7 +282,7 @@ void ScrollableArea::invalidateScrollCorner()
         return;
     }
 #endif
-    invalidateScrollCornerRect(scrollCornerRect());
+    invalidateScrollCornerRect(rect);
 }
 
 bool ScrollableArea::hasLayerForHorizontalScrollbar() const

diff --git a/Source/WebCore/platform/ScrollableArea.h b/Source/WebCore/platform/ScrollableArea.h
index 12a3523..5b54cbc 100644 (file)
--- a/Source/WebCore/platform/ScrollableArea.h
+++ b/Source/WebCore/platform/ScrollableArea.h
@@ -89,7 +89,7 @@ public:
     void invalidateScrollbar(Scrollbar*, const IntRect&);
     virtual bool isScrollCornerVisible() const = 0;
     virtual IntRect scrollCornerRect() const = 0;
-    void invalidateScrollCorner();
+    void invalidateScrollCorner(const IntRect&);
     virtual void getTickmarks(Vector<IntRect>&) const { }
 
     // This function should be overriden by subclasses to perform the actual

diff --git a/Source/WebCore/rendering/RenderLayerCompositor.cpp b/Source/WebCore/rendering/RenderLayerCompositor.cpp
index 3252b57..64754c5 100644 (file)
--- a/Source/WebCore/rendering/RenderLayerCompositor.cpp
+++ b/Source/WebCore/rendering/RenderLayerCompositor.cpp
@@ -1786,7 +1786,7 @@ void RenderLayerCompositor::destroyRootLayer()
 
     if (m_layerForScrollCorner) {
         m_layerForScrollCorner = nullptr;
-        m_renderView->frameView()->invalidateScrollCorner();
+        m_renderView->frameView()->invalidateScrollCorner(m_renderView->frameView()->scrollCornerRect());
     }
 
     if (m_overflowControlsHostLayer) {

diff --git a/Source/WebCore/rendering/RenderScrollbarPart.cpp b/Source/WebCore/rendering/RenderScrollbarPart.cpp
index 5a54de1..cca2727 100644 (file)
--- a/Source/WebCore/rendering/RenderScrollbarPart.cpp
+++ b/Source/WebCore/rendering/RenderScrollbarPart.cpp
@@ -150,7 +150,7 @@ void RenderScrollbarPart::imageChanged(WrappedImagePtr image, const IntRect* rec
     else {
         if (FrameView* frameView = view()->frameView()) {
             if (frameView->isFrameViewScrollCorner(this)) {
-                frameView->invalidateScrollCorner();
+                frameView->invalidateScrollCorner(frameView->scrollCornerRect());
                 return;
             }
         }