Title: [156201] trunk/Source/WebCore
Revision
156201
Author
[email protected]
Date
2013-09-20 14:47:47 -0700 (Fri, 20 Sep 2013)

Log Message

Scrollbars don't appear on long pages after r155660
https://bugs.webkit.org/show_bug.cgi?id=121714
<rdar://problem/15042478>

Reviewed by Beth Dakin.

After r155660, we don't do any additional layouts when scrollbars are
added or removed. That caused us to not enter the code that creates or
destroys composting layers for scrollbars in RenderLayerCompositor, resulting
in missing scrollbars.

Fix by having ScrollView::updateScrollbars() call addedOrRemovedScrollbar()
when a scrollbar was added or removed, which is overridden in FrameView
to call through to RenderLayerCompositor::frameViewDidAddOrRemoveScrollbars().

Not testable since scrollbar layers aren't contained in layer tree dumps
(to reduce diffs between platforms).

* page/FrameView.cpp:
(WebCore::FrameView::addedOrRemovedScrollbar): New function to tell the compositor
that scrollbars changed.
* page/FrameView.h:
* platform/ScrollView.cpp:
(WebCore::ScrollView::setHasHorizontalScrollbar): Change the meaning of the return
value to say if the scrollbar was added or removed. Add an out param that indicates
whether the scrollbar change affects content size (not true for overlay scrollbars).
(WebCore::ScrollView::setHasVerticalScrollbar): Ditto.
(WebCore::ScrollView::updateScrollbars): Keep track of whether scrollbars were
added or removed, and call addedOrRemovedScrollbar() if true.
* platform/ScrollView.h:
* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::frameViewDidAddOrRemoveScrollbars): Call updateOverflowControlsLayers().
(WebCore::RenderLayerCompositor::updateOverflowControlsLayers):
* rendering/RenderLayerCompositor.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (156200 => 156201)


--- trunk/Source/WebCore/ChangeLog	2013-09-20 21:47:38 UTC (rev 156200)
+++ trunk/Source/WebCore/ChangeLog	2013-09-20 21:47:47 UTC (rev 156201)
@@ -1,5 +1,42 @@
 2013-09-20  Simon Fraser  <[email protected]>
 
+        Scrollbars don't appear on long pages after r155660
+        https://bugs.webkit.org/show_bug.cgi?id=121714
+        <rdar://problem/15042478>
+
+        Reviewed by Beth Dakin.
+        
+        After r155660, we don't do any additional layouts when scrollbars are 
+        added or removed. That caused us to not enter the code that creates or
+        destroys composting layers for scrollbars in RenderLayerCompositor, resulting
+        in missing scrollbars.
+        
+        Fix by having ScrollView::updateScrollbars() call addedOrRemovedScrollbar()
+        when a scrollbar was added or removed, which is overridden in FrameView
+        to call through to RenderLayerCompositor::frameViewDidAddOrRemoveScrollbars().
+
+        Not testable since scrollbar layers aren't contained in layer tree dumps
+        (to reduce diffs between platforms).
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::addedOrRemovedScrollbar): New function to tell the compositor
+        that scrollbars changed.
+        * page/FrameView.h:
+        * platform/ScrollView.cpp:
+        (WebCore::ScrollView::setHasHorizontalScrollbar): Change the meaning of the return
+        value to say if the scrollbar was added or removed. Add an out param that indicates
+        whether the scrollbar change affects content size (not true for overlay scrollbars).
+        (WebCore::ScrollView::setHasVerticalScrollbar): Ditto.
+        (WebCore::ScrollView::updateScrollbars): Keep track of whether scrollbars were
+        added or removed, and call addedOrRemovedScrollbar() if true.
+        * platform/ScrollView.h:
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::frameViewDidAddOrRemoveScrollbars): Call updateOverflowControlsLayers().
+        (WebCore::RenderLayerCompositor::updateOverflowControlsLayers):
+        * rendering/RenderLayerCompositor.h:
+
+2013-09-20  Simon Fraser  <[email protected]>
+
         Ensure that TileController tile coverage is updated on scrolling even when we don't have scrollbars
         https://bugs.webkit.org/show_bug.cgi?id=121700
         <rdar://problem/15030589>

Modified: trunk/Source/WebCore/page/FrameView.cpp (156200 => 156201)


--- trunk/Source/WebCore/page/FrameView.cpp	2013-09-20 21:47:38 UTC (rev 156200)
+++ trunk/Source/WebCore/page/FrameView.cpp	2013-09-20 21:47:47 UTC (rev 156201)
@@ -2181,6 +2181,16 @@
 #endif
 }
 
+void FrameView::addedOrRemovedScrollbar()
+{
+#if USE(ACCELERATED_COMPOSITING)
+    if (RenderView* renderView = this->renderView()) {
+        if (renderView->usesCompositing())
+            renderView->compositor().frameViewDidAddOrRemoveScrollbars();
+    }
+#endif
+}
+
 void FrameView::beginDeferredRepaints()
 {
     Page* page = frame().page();

Modified: trunk/Source/WebCore/page/FrameView.h (156200 => 156201)


--- trunk/Source/WebCore/page/FrameView.h	2013-09-20 21:47:38 UTC (rev 156200)
+++ trunk/Source/WebCore/page/FrameView.h	2013-09-20 21:47:47 UTC (rev 156201)
@@ -483,6 +483,7 @@
     virtual void repaintContentRectangle(const IntRect&, bool immediate) OVERRIDE;
     virtual void contentsResized() OVERRIDE;
     virtual void visibleContentsResized() OVERRIDE;
+    virtual void addedOrRemovedScrollbar() OVERRIDE;
     virtual void fixedLayoutSizeChanged() OVERRIDE;
 
     virtual void delegatesScrollingDidChange() OVERRIDE;

Modified: trunk/Source/WebCore/platform/ScrollView.cpp (156200 => 156201)


--- trunk/Source/WebCore/platform/ScrollView.cpp	2013-09-20 21:47:38 UTC (rev 156200)
+++ trunk/Source/WebCore/platform/ScrollView.cpp	2013-09-20 21:47:47 UTC (rev 156201)
@@ -84,7 +84,7 @@
         platformRemoveChild(child);
 }
 
-bool ScrollView::setHasHorizontalScrollbar(bool hasBar)
+bool ScrollView::setHasHorizontalScrollbar(bool hasBar, bool* contentSizeAffected)
 {
     ASSERT(!hasBar || !avoidScrollbarCreation());
     if (hasBar && !m_horizontalScrollbar) {
@@ -92,7 +92,9 @@
         addChild(m_horizontalScrollbar.get());
         didAddScrollbar(m_horizontalScrollbar.get(), HorizontalScrollbar);
         m_horizontalScrollbar->styleChanged();
-        return !m_horizontalScrollbar->isOverlayScrollbar();
+        if (contentSizeAffected)
+            *contentSizeAffected = !m_horizontalScrollbar->isOverlayScrollbar();
+        return true;
     }
     
     if (!hasBar && m_horizontalScrollbar) {
@@ -100,13 +102,15 @@
         willRemoveScrollbar(m_horizontalScrollbar.get(), HorizontalScrollbar);
         removeChild(m_horizontalScrollbar.get());
         m_horizontalScrollbar = 0;
-        return !wasOverlayScrollbar;
+        if (contentSizeAffected)
+            *contentSizeAffected = !wasOverlayScrollbar;
+        return true;
     }
 
     return false;
 }
 
-bool ScrollView::setHasVerticalScrollbar(bool hasBar)
+bool ScrollView::setHasVerticalScrollbar(bool hasBar, bool* contentSizeAffected)
 {
     ASSERT(!hasBar || !avoidScrollbarCreation());
     if (hasBar && !m_verticalScrollbar) {
@@ -114,7 +118,9 @@
         addChild(m_verticalScrollbar.get());
         didAddScrollbar(m_verticalScrollbar.get(), VerticalScrollbar);
         m_verticalScrollbar->styleChanged();
-        return !m_verticalScrollbar->isOverlayScrollbar();
+        if (contentSizeAffected)
+            *contentSizeAffected = !m_verticalScrollbar->isOverlayScrollbar();
+        return true;
     }
     
     if (!hasBar && m_verticalScrollbar) {
@@ -122,7 +128,9 @@
         willRemoveScrollbar(m_verticalScrollbar.get(), VerticalScrollbar);
         removeChild(m_verticalScrollbar.get());
         m_verticalScrollbar = 0;
-        return !wasOverlayScrollbar;
+        if (contentSizeAffected)
+            *contentSizeAffected = !wasOverlayScrollbar;
+        return true;
     }
 
     return false;
@@ -522,11 +530,18 @@
     if (vScroll != ScrollbarAuto)
         newHasVerticalScrollbar = (vScroll == ScrollbarAlwaysOn);
 
+    bool scrollbarAddedOrRemoved = false;
+
     if (m_scrollbarsSuppressed || (hScroll != ScrollbarAuto && vScroll != ScrollbarAuto)) {
-        if (hasHorizontalScrollbar != newHasHorizontalScrollbar && (hasHorizontalScrollbar || !avoidScrollbarCreation()))
-            setHasHorizontalScrollbar(newHasHorizontalScrollbar);
-        if (hasVerticalScrollbar != newHasVerticalScrollbar && (hasVerticalScrollbar || !avoidScrollbarCreation()))
-            setHasVerticalScrollbar(newHasVerticalScrollbar);
+        if (hasHorizontalScrollbar != newHasHorizontalScrollbar && (hasHorizontalScrollbar || !avoidScrollbarCreation())) {
+            if (setHasHorizontalScrollbar(newHasHorizontalScrollbar))
+                scrollbarAddedOrRemoved = true;
+        }
+
+        if (hasVerticalScrollbar != newHasVerticalScrollbar && (hasVerticalScrollbar || !avoidScrollbarCreation())) {
+            if (setHasVerticalScrollbar(newHasVerticalScrollbar))
+                scrollbarAddedOrRemoved = true;
+        }
     } else {
         bool sendContentResizedNotification = false;
         
@@ -557,7 +572,11 @@
             if (m_horizontalScrollbar)
                 m_horizontalScrollbar->invalidate();
 
-            sendContentResizedNotification = setHasHorizontalScrollbar(newHasHorizontalScrollbar);
+            bool changeAffectsContentSize = false;
+            if (setHasHorizontalScrollbar(newHasHorizontalScrollbar, &changeAffectsContentSize)) {
+                scrollbarAddedOrRemoved = true;
+                sendContentResizedNotification |= changeAffectsContentSize;
+            }
         }
 
         if (hasVerticalScrollbar != newHasVerticalScrollbar && (hasVerticalScrollbar || !avoidScrollbarCreation())) {
@@ -566,7 +585,11 @@
             if (m_verticalScrollbar)
                 m_verticalScrollbar->invalidate();
 
-            sendContentResizedNotification = setHasVerticalScrollbar(newHasVerticalScrollbar);
+            bool changeAffectsContentSize = false;
+            if (setHasVerticalScrollbar(newHasVerticalScrollbar, &changeAffectsContentSize)) {
+                scrollbarAddedOrRemoved = true;
+                sendContentResizedNotification |= changeAffectsContentSize;
+            }
         }
 
         if (sendContentResizedNotification && m_updateScrollbarsPass < cMaxUpdateScrollbarsPass) {
@@ -582,6 +605,9 @@
             }
             m_updateScrollbarsPass--;
         }
+
+        if (scrollbarAddedOrRemoved)
+            addedOrRemovedScrollbar();
     }
     
     // Set up the range (and page step/line step), but only do this if we're not in a nested call (to avoid

Modified: trunk/Source/WebCore/platform/ScrollView.h (156200 => 156201)


--- trunk/Source/WebCore/platform/ScrollView.h	2013-09-20 21:47:38 UTC (rev 156200)
+++ trunk/Source/WebCore/platform/ScrollView.h	2013-09-20 21:47:47 UTC (rev 156201)
@@ -311,13 +311,14 @@
     virtual void paintOverhangAreas(GraphicsContext*, const IntRect& horizontalOverhangArea, const IntRect& verticalOverhangArea, const IntRect& dirtyRect);
 
     virtual void visibleContentsResized() = 0;
+    virtual void addedOrRemovedScrollbar() = 0;
     virtual void delegatesScrollingDidChange() { }
     virtual void fixedLayoutSizeChanged();
 
     // These functions are used to create/destroy scrollbars.
-    // They return true if the space taken up by the scrollbar changed.
-    bool setHasHorizontalScrollbar(bool);
-    bool setHasVerticalScrollbar(bool);
+    // They return true if the scrollbar was added or removed.
+    bool setHasHorizontalScrollbar(bool, bool* contentSizeAffected = 0);
+    bool setHasVerticalScrollbar(bool, bool* contentSizeAffected = 0);
 
     virtual void updateScrollCorner();
     virtual void invalidateScrollCornerRect(const IntRect&) OVERRIDE;

Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp (156200 => 156201)


--- trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp	2013-09-20 21:47:38 UTC (rev 156200)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp	2013-09-20 21:47:47 UTC (rev 156201)
@@ -1340,6 +1340,11 @@
         fixedBackgroundLayer->setPosition(IntPoint(frameView.scrollOffsetForFixedPosition()));
 }
 
+void RenderLayerCompositor::frameViewDidAddOrRemoveScrollbars()
+{
+    updateOverflowControlsLayers();
+}
+
 void RenderLayerCompositor::frameViewDidLayout()
 {
     RenderLayerBacking* renderViewBacking = m_renderView.layer()->backing();
@@ -2740,7 +2745,7 @@
             m_layerForVerticalScrollbar->setName("vertical scrollbar");
 #endif
 #if PLATFORM(MAC) && USE(CA)
-        m_layerForVerticalScrollbar->setAcceleratesDrawing(acceleratedDrawingEnabled());
+            m_layerForVerticalScrollbar->setAcceleratesDrawing(acceleratedDrawingEnabled());
 #endif
             m_overflowControlsHostLayer->addChild(m_layerForVerticalScrollbar.get());
 

Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.h (156200 => 156201)


--- trunk/Source/WebCore/rendering/RenderLayerCompositor.h	2013-09-20 21:47:38 UTC (rev 156200)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.h	2013-09-20 21:47:47 UTC (rev 156201)
@@ -235,6 +235,7 @@
     void frameViewDidChangeLocation(const IntPoint& contentsOffset);
     void frameViewDidChangeSize();
     void frameViewDidScroll();
+    void frameViewDidAddOrRemoveScrollbars();
     void frameViewDidLayout();
     void rootFixedBackgroundsChanged();
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to