Changeset 156201 in webkit

Timestamp:

Sep 20, 2013, 2:47:47 PM (12 years ago)

Author:

Simon Fraser

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:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• page/FrameView.cpp (modified) (1 diff)
• page/FrameView.h (modified) (1 diff)
• platform/ScrollView.cpp (modified) (10 diffs)
• platform/ScrollView.h (modified) (1 diff)
• rendering/RenderLayerCompositor.cpp (modified) (2 diffs)
• rendering/RenderLayerCompositor.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-09-20  Simon Fraser  <simon.fraser@apple.com>
+
+        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  <simon.fraser@apple.com>
 

trunk/Source/WebCore/page/FrameView.cpp

@@ -2182 +2182 @@
 }
 
+void FrameView::addedOrRemovedScrollbar()
+{
+#if USE(ACCELERATED_COMPOSITING)
+    if (RenderView* renderView = this->renderView()) {
+        if (renderView->usesCompositing())
+            renderView->compositor().frameViewDidAddOrRemoveScrollbars();
+    }
+#endif
+}
+
 void FrameView::beginDeferredRepaints()
 {

trunk/Source/WebCore/page/FrameView.h

@@ -484 +484 @@
     virtual void contentsResized() OVERRIDE;
     virtual void visibleContentsResized() OVERRIDE;
+    virtual void addedOrRemovedScrollbar() OVERRIDE;
     virtual void fixedLayoutSizeChanged() OVERRIDE;
 

trunk/Source/WebCore/platform/ScrollView.cpp

@@ -85 +85 @@
 }
 
-bool ScrollView::setHasHorizontalScrollbar(bool hasBar)
+bool ScrollView::setHasHorizontalScrollbar(bool hasBar, bool* contentSizeAffected)
 {
     ASSERT(!hasBar || !avoidScrollbarCreation());
@@ -93 +93 @@
         didAddScrollbar(m_horizontalScrollbar.get(), HorizontalScrollbar);
         m_horizontalScrollbar->styleChanged();
-        return !m_horizontalScrollbar->isOverlayScrollbar();
+        if (contentSizeAffected)
+            *contentSizeAffected = !m_horizontalScrollbar->isOverlayScrollbar();
+        return true;
     }
     
@@ -101 +103 @@
         removeChild(m_horizontalScrollbar.get());
         m_horizontalScrollbar = 0;
-        return !wasOverlayScrollbar;
+        if (contentSizeAffected)
+            *contentSizeAffected = !wasOverlayScrollbar;
+        return true;
     }
 
@@ -107 +111 @@
 }
 
-bool ScrollView::setHasVerticalScrollbar(bool hasBar)
+bool ScrollView::setHasVerticalScrollbar(bool hasBar, bool* contentSizeAffected)
 {
     ASSERT(!hasBar || !avoidScrollbarCreation());
@@ -115 +119 @@
         didAddScrollbar(m_verticalScrollbar.get(), VerticalScrollbar);
         m_verticalScrollbar->styleChanged();
-        return !m_verticalScrollbar->isOverlayScrollbar();
+        if (contentSizeAffected)
+            *contentSizeAffected = !m_verticalScrollbar->isOverlayScrollbar();
+        return true;
     }
     
@@ -123 +129 @@
         removeChild(m_verticalScrollbar.get());
         m_verticalScrollbar = 0;
-        return !wasOverlayScrollbar;
+        if (contentSizeAffected)
+            *contentSizeAffected = !wasOverlayScrollbar;
+        return true;
     }
 
@@ -523 +531 @@
         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;
@@ -558 +573 @@
                 m_horizontalScrollbar->invalidate();
 
-            sendContentResizedNotification = setHasHorizontalScrollbar(newHasHorizontalScrollbar);
+            bool changeAffectsContentSize = false;
+            if (setHasHorizontalScrollbar(newHasHorizontalScrollbar, &changeAffectsContentSize)) {
+                scrollbarAddedOrRemoved = true;
+                sendContentResizedNotification |= changeAffectsContentSize;
+            }
         }
 
@@ -567 +586 @@
                 m_verticalScrollbar->invalidate();
 
-            sendContentResizedNotification = setHasVerticalScrollbar(newHasVerticalScrollbar);
+            bool changeAffectsContentSize = false;
+            if (setHasVerticalScrollbar(newHasVerticalScrollbar, &changeAffectsContentSize)) {
+                scrollbarAddedOrRemoved = true;
+                sendContentResizedNotification |= changeAffectsContentSize;
+            }
         }
 
@@ -583 +606 @@
             m_updateScrollbarsPass--;
         }
+
+        if (scrollbarAddedOrRemoved)
+            addedOrRemovedScrollbar();
     }
     

trunk/Source/WebCore/platform/ScrollView.h

@@ -312 +312 @@
 
     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();

trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp

@@ -1341 +1341 @@
 }
 
+void RenderLayerCompositor::frameViewDidAddOrRemoveScrollbars()
+{
+    updateOverflowControlsLayers();
+}
+
 void RenderLayerCompositor::frameViewDidLayout()
 {
@@ -2741 +2746 @@
 #endif
 #if PLATFORM(MAC) && USE(CA)
-        m_layerForVerticalScrollbar->setAcceleratesDrawing(acceleratedDrawingEnabled());
+            m_layerForVerticalScrollbar->setAcceleratesDrawing(acceleratedDrawingEnabled());
 #endif
             m_overflowControlsHostLayer->addChild(m_layerForVerticalScrollbar.get());

trunk/Source/WebCore/rendering/RenderLayerCompositor.h

@@ -236 +236 @@
     void frameViewDidChangeSize();
     void frameViewDidScroll();
+    void frameViewDidAddOrRemoveScrollbars();
     void frameViewDidLayout();
     void rootFixedBackgroundsChanged();