Clarify some resizing terminology in ScrollView/FrameView
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Feb 2015 05:35:38 +0000 (05:35 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Feb 2015 05:35:38 +0000 (05:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=141996

Reviewed by Zalan Bujtas.

ScrollableArea/ScrollView/FrameView had some confusing terminology around
contentsResized/visibleContentsResized/fixedLayoutSizeChanged.

Clarify this by distinguishing between:
1. Available size changes because of
    i) non-overlay scrollbar presence
   ii) ScrollableArea frame change
2. Removing fixedLayoutSizeChanged() and just treating it like an
   available size change.

contentsResized() is relegated to simply being a hook that allows Mac to
flash overlay scrollbars.

The confusingly named visibleContentsResized() is now updateContentsSize(),
and is the way that a ScrollableArea tells its subclasss that it should recompute
the size of the contents (i.e. do a layout).

Source/WebCore:

* page/FrameView.cpp:
(WebCore::FrameView::setContentsSize): No longer mysteriously skip the
FrameView implementation of contentsResized(), which used to do a setNeedsLayout()
which we didn't want to do from setContentsSize(), which itself happens as a result of layout.
(WebCore::FrameView::adjustViewSize): Whitespace.
(WebCore::FrameView::layout): Ditto.
(WebCore::FrameView::availableContentSizeChanged): Called on frame size change, or scrollbar
change.
(WebCore::FrameView::updateContentsSize): This actually does the layout.
(WebCore::FrameView::scrollbarStyleChanged): Always call the base class;  ScrollView::scrollbarStyleChanged
will bail if not a forced update.
(WebCore::FrameView::setCustomFixedPositionLayoutRect): Forces a layout via updateContentsSize() now.
(WebCore::FrameView::contentsResized): Deleted.
(WebCore::FrameView::fixedLayoutSizeChanged): Deleted.
(WebCore::FrameView::visibleContentsResized): Deleted.
* page/FrameView.h:
* platform/ScrollView.cpp:
(WebCore::ScrollView::setFixedLayoutSize): Calls availableContentSizeChanged() now.
(WebCore::ScrollView::setUseFixedLayout): Ditto.
(WebCore::ScrollView::availableContentSizeChanged): Update scrollbars if that's not
the reason we are being called.
(WebCore::ScrollView::updateScrollbars): contentsResized() was the thing that caused setNeedsLayout();
replace it with availableContentSizeChanged(). visibleContentsResized() did the layout, and
replace with updateContentsSize().
(WebCore::ScrollView::setFrameRect): Call availableContentSizeChanged() now. This takes care of
updating scrollbars, so no need to explicitly do that.
(WebCore::ScrollView::scrollbarStyleChanged): Call the base class.
(WebCore::ScrollView::fixedLayoutSizeChanged): Deleted.
* platform/ScrollView.h:
* platform/ScrollableArea.cpp:
(WebCore::ScrollableArea::availableContentSizeChanged): Call scrollAnimator->contentsResized()
to flash the scrollbars.
(WebCore::ScrollableArea::scrolledToRight):
(WebCore::ScrollableArea::scrollbarStyleChanged): Call availableContentSizeChanged() since
scrollbar style affects available space.
* platform/ScrollableArea.h:
(WebCore::ScrollableArea::updateContentsSize):
(WebCore::ScrollableArea::scrollbarStyleChanged): Deleted.

Source/WebKit2:

* WebProcess/Plugins/PDF/PDFPlugin.mm:
(WebKit::PDFPlugin::scrollbarStyleChanged): Call the base class method,
which takes care of

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

Source/WebCore/ChangeLog
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/FrameView.h
Source/WebCore/platform/ScrollView.cpp
Source/WebCore/platform/ScrollView.h
Source/WebCore/platform/ScrollableArea.cpp
Source/WebCore/platform/ScrollableArea.h
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm

index 67f8a46..563c514 100644 (file)
@@ -1,3 +1,66 @@
+2015-02-24  Simon Fraser  <simon.fraser@apple.com>
+
+        Clarify some resizing terminology in ScrollView/FrameView
+        https://bugs.webkit.org/show_bug.cgi?id=141996
+
+        Reviewed by Zalan Bujtas.
+
+        ScrollableArea/ScrollView/FrameView had some confusing terminology around
+        contentsResized/visibleContentsResized/fixedLayoutSizeChanged.
+        
+        Clarify this by distinguishing between:
+        1. Available size changes because of
+            i) non-overlay scrollbar presence
+           ii) ScrollableArea frame change
+        2. Removing fixedLayoutSizeChanged() and just treating it like an
+           available size change.
+           
+        contentsResized() is relegated to simply being a hook that allows Mac to
+        flash overlay scrollbars.
+        
+        The confusingly named visibleContentsResized() is now updateContentsSize(),
+        and is the way that a ScrollableArea tells its subclasss that it should recompute
+        the size of the contents (i.e. do a layout).
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::setContentsSize): No longer mysteriously skip the
+        FrameView implementation of contentsResized(), which used to do a setNeedsLayout()
+        which we didn't want to do from setContentsSize(), which itself happens as a result of layout.
+        (WebCore::FrameView::adjustViewSize): Whitespace.
+        (WebCore::FrameView::layout): Ditto.
+        (WebCore::FrameView::availableContentSizeChanged): Called on frame size change, or scrollbar
+        change.
+        (WebCore::FrameView::updateContentsSize): This actually does the layout.
+        (WebCore::FrameView::scrollbarStyleChanged): Always call the base class;  ScrollView::scrollbarStyleChanged
+        will bail if not a forced update.
+        (WebCore::FrameView::setCustomFixedPositionLayoutRect): Forces a layout via updateContentsSize() now.
+        (WebCore::FrameView::contentsResized): Deleted.
+        (WebCore::FrameView::fixedLayoutSizeChanged): Deleted.
+        (WebCore::FrameView::visibleContentsResized): Deleted.
+        * page/FrameView.h:
+        * platform/ScrollView.cpp:
+        (WebCore::ScrollView::setFixedLayoutSize): Calls availableContentSizeChanged() now.
+        (WebCore::ScrollView::setUseFixedLayout): Ditto.
+        (WebCore::ScrollView::availableContentSizeChanged): Update scrollbars if that's not
+        the reason we are being called.
+        (WebCore::ScrollView::updateScrollbars): contentsResized() was the thing that caused setNeedsLayout();
+        replace it with availableContentSizeChanged(). visibleContentsResized() did the layout, and
+        replace with updateContentsSize().
+        (WebCore::ScrollView::setFrameRect): Call availableContentSizeChanged() now. This takes care of
+        updating scrollbars, so no need to explicitly do that.
+        (WebCore::ScrollView::scrollbarStyleChanged): Call the base class.
+        (WebCore::ScrollView::fixedLayoutSizeChanged): Deleted.
+        * platform/ScrollView.h:
+        * platform/ScrollableArea.cpp:
+        (WebCore::ScrollableArea::availableContentSizeChanged): Call scrollAnimator->contentsResized()
+        to flash the scrollbars.
+        (WebCore::ScrollableArea::scrolledToRight):
+        (WebCore::ScrollableArea::scrollbarStyleChanged): Call availableContentSizeChanged() since
+        scrollbar style affects available space.
+        * platform/ScrollableArea.h:
+        (WebCore::ScrollableArea::updateContentsSize):
+        (WebCore::ScrollableArea::scrollbarStyleChanged): Deleted.
+
 2015-02-24  Ryosuke Niwa  <rniwa@webkit.org>
 
         Unreviewed Mavericks build attempt fix after r180609.
index 06f946c..4333925 100644 (file)
@@ -566,7 +566,7 @@ void FrameView::setContentsSize(const IntSize& size)
     m_deferSetNeedsLayoutCount++;
 
     ScrollView::setContentsSize(size);
-    ScrollView::contentsResized();
+    contentsResized();
     
     Page* page = frame().page();
     if (!page)
@@ -597,7 +597,7 @@ void FrameView::adjustViewSize()
     const IntRect rect = renderView->documentRect();
     const IntSize& size = rect.size();
     ScrollView::setScrollOrigin(IntPoint(-rect.x(), -rect.y()), !frame().document()->printing(), size == contentsSize());
-    
+
     setContentsSize(size);
 }
 
@@ -1276,7 +1276,6 @@ void FrameView::layout(bool allowSubtree)
             }
 
             LayoutSize oldSize = m_size;
-
             m_size = layoutSize();
 
             if (oldSize != m_size) {
@@ -2290,19 +2289,10 @@ bool FrameView::renderedCharactersExceed(unsigned threshold)
     return countRenderedCharactersInRenderObjectWithThreshold(*m_frame->contentRenderer(), threshold) >= threshold;
 }
 
-void FrameView::contentsResized()
+void FrameView::availableContentSizeChanged(AvailableSizeChangeReason reason)
 {
-    ScrollView::contentsResized();
     setNeedsLayout();
-}
-
-void FrameView::fixedLayoutSizeChanged()
-{
-    // Can be triggered before the view is set, see comment in FrameView::visibleContentsResized().
-    // An ASSERT is triggered when a view schedules a layout before being attached to a frame.
-    if (!frame().view())
-        return;
-    ScrollView::fixedLayoutSizeChanged();
+    ScrollView::availableContentSizeChanged(reason);
 }
 
 bool FrameView::shouldLayoutAfterContentsResized() const
@@ -2310,7 +2300,7 @@ bool FrameView::shouldLayoutAfterContentsResized() const
     return !useFixedLayout() || useCustomFixedPositionLayoutRect();
 }
 
-void FrameView::visibleContentsResized()
+void FrameView::updateContentsSize()
 {
     // We check to make sure the view is attached to a frame() as this method can
     // be triggered before the view is attached by Frame::createView(...) setting
@@ -3493,8 +3483,7 @@ void FrameView::scrollbarStyleChanged(ScrollbarStyle newStyle, bool forceUpdate)
 
     frame().page()->chrome().client().recommendedScrollbarStyleDidChange(newStyle);
 
-    if (forceUpdate)
-        ScrollView::scrollbarStyleChanged(newStyle, forceUpdate);
+    ScrollView::scrollbarStyleChanged(newStyle, forceUpdate);
 }
 
 void FrameView::notifyPageThatContentAreaWillPaint() const
@@ -4466,7 +4455,7 @@ void FrameView::setCustomFixedPositionLayoutRect(const IntRect& rect)
         return;
     m_useCustomFixedPositionLayoutRect = true;
     m_customFixedPositionLayoutRect = rect;
-    visibleContentsResized();
+    updateContentsSize();
 }
 
 bool FrameView::updateFixedPositionLayoutRect()
index 4ac1216..969c009 100644 (file)
@@ -580,10 +580,9 @@ private:
     WEBCORE_EXPORT void adjustTiledBackingCoverage();
 
     virtual void repaintContentRectangle(const IntRect&) override;
-    virtual void contentsResized() override;
-    virtual void visibleContentsResized() override;
+    virtual void updateContentsSize() override;
+    virtual void availableContentSizeChanged(AvailableSizeChangeReason) override;
     virtual void addedOrRemovedScrollbar() override;
-    virtual void fixedLayoutSizeChanged() override;
 
     virtual void delegatesScrollingDidChange() override;
 
index 5df0920..7fcd12b 100644 (file)
@@ -346,7 +346,7 @@ void ScrollView::setFixedLayoutSize(const IntSize& newSize)
         return;
     m_fixedLayoutSize = newSize;
     if (m_useFixedLayout)
-        fixedLayoutSizeChanged();
+        availableContentSizeChanged(AvailableSizeChangeReason::AreaSizeChanged);
 }
 
 bool ScrollView::useFixedLayout() const
@@ -360,13 +360,14 @@ void ScrollView::setUseFixedLayout(bool enable)
         return;
     m_useFixedLayout = enable;
     if (!m_fixedLayoutSize.isEmpty())
-        fixedLayoutSizeChanged();
+        availableContentSizeChanged(AvailableSizeChangeReason::AreaSizeChanged);
 }
 
-void ScrollView::fixedLayoutSizeChanged()
+void ScrollView::availableContentSizeChanged(AvailableSizeChangeReason reason)
 {
-    updateScrollbars(scrollOffset());
-    contentsResized();
+    ScrollableArea::availableContentSizeChanged(reason);
+    if (reason != AvailableSizeChangeReason::ScrollbarsChanged)
+        updateScrollbars(scrollOffset());
 }
 
 IntSize ScrollView::contentsSize() const
@@ -600,11 +601,11 @@ void ScrollView::updateScrollbars(const IntSize& desiredOffset)
     bool hasOverlayScrollbars = (!m_horizontalScrollbar || m_horizontalScrollbar->isOverlayScrollbar()) && (!m_verticalScrollbar || m_verticalScrollbar->isOverlayScrollbar());
 
     // If we came in here with the view already needing a layout, then go ahead and do that
-    // first.  (This will be the common case, e.g., when the page changes due to window resizing for example).
+    // first. (This will be the common case, e.g., when the page changes due to window resizing for example).
     // This layout will not re-enter updateScrollbars and does not count towards our max layout pass total.
     if (!m_scrollbarsSuppressed && !hasOverlayScrollbars) {
         m_inUpdateScrollbars = true;
-        visibleContentsResized();
+        updateContentsSize();
         m_inUpdateScrollbars = false;
     }
 
@@ -696,8 +697,8 @@ void ScrollView::updateScrollbars(const IntSize& desiredOffset)
         const unsigned cMaxUpdateScrollbarsPass = 2;
         if ((sendContentResizedNotification || needAnotherPass) && m_updateScrollbarsPass < cMaxUpdateScrollbarsPass) {
             m_updateScrollbarsPass++;
-            contentsResized();
-            visibleContentsResized();
+            availableContentSizeChanged(AvailableSizeChangeReason::ScrollbarsChanged);
+            updateContentsSize();
             IntSize newDocSize = totalContentsSize();
             if (newDocSize == docSize) {
                 // The layout with the new scroll state had no impact on
@@ -1047,13 +1048,10 @@ void ScrollView::setFrameRect(const IntRect& newRect)
         return;
 
     Widget::setFrameRect(newRect);
-
     frameRectsChanged();
-
-    updateScrollbars(scrollOffset());
-
+    
     if (!m_useFixedLayout && oldRect.size() != newRect.size())
-        contentsResized();
+        availableContentSizeChanged(AvailableSizeChangeReason::AreaSizeChanged);
 }
 
 void ScrollView::frameRectsChanged()
@@ -1162,12 +1160,12 @@ bool ScrollView::isScrollCornerVisible() const
     return !scrollCornerRect().isEmpty();
 }
 
-void ScrollView::scrollbarStyleChanged(ScrollbarStyle, bool forceUpdate)
+void ScrollView::scrollbarStyleChanged(ScrollbarStyle newStyle, bool forceUpdate)
 {
+    ScrollableArea::scrollbarStyleChanged(newStyle, forceUpdate);
     if (!forceUpdate)
         return;
 
-    contentsResized();
     updateScrollbars(scrollOffset());
     positionScrollbarLayers();
 }
index 4d8620a..b7e899d 100644 (file)
@@ -389,10 +389,9 @@ protected:
 
     virtual void paintOverhangAreas(GraphicsContext*, const IntRect& horizontalOverhangArea, const IntRect& verticalOverhangArea, const IntRect& dirtyRect);
 
-    virtual void visibleContentsResized() = 0;
+    virtual void availableContentSizeChanged(AvailableSizeChangeReason) override;
     virtual void addedOrRemovedScrollbar() = 0;
     virtual void delegatesScrollingDidChange() { }
-    virtual void fixedLayoutSizeChanged();
 
     // These functions are used to create/destroy scrollbars.
     // They return true if the scrollbar was added or removed.
index 0d6ddfb..62cfa2e 100644 (file)
@@ -314,6 +314,12 @@ void ScrollableArea::contentsResized()
         scrollAnimator->contentsResized();
 }
 
+void ScrollableArea::availableContentSizeChanged(AvailableSizeChangeReason)
+{
+    if (ScrollAnimator* scrollAnimator = existingScrollAnimator())
+        scrollAnimator->contentsResized(); // This flashes overlay scrollbars.
+}
+
 bool ScrollableArea::hasOverlayScrollbars() const
 {
     return (verticalScrollbar() && verticalScrollbar()->isOverlayScrollbar())
@@ -479,6 +485,11 @@ bool ScrollableArea::scrolledToRight() const
     return scrollPosition().x() >= maximumScrollPosition().x();
 }
 
+void ScrollableArea::scrollbarStyleChanged(ScrollbarStyle, bool)
+{
+    availableContentSizeChanged(AvailableSizeChangeReason::ScrollbarsChanged);
+}
+
 IntSize ScrollableArea::totalContentsSize() const
 {
     IntSize totalContentsSize = contentsSize();
index 62d79af..58fc6f8 100644 (file)
@@ -112,6 +112,15 @@ public:
 
     WEBCORE_EXPORT virtual void contentsResized();
 
+    // Force the contents to recompute their size (i.e. do layout).
+    virtual void updateContentsSize() { }
+
+    enum class AvailableSizeChangeReason {
+        ScrollbarsChanged,
+        AreaSizeChanged
+    };
+    WEBCORE_EXPORT virtual void availableContentSizeChanged(AvailableSizeChangeReason);
+
     bool hasOverlayScrollbars() const;
     WEBCORE_EXPORT virtual void setScrollbarOverlayStyle(ScrollbarOverlayStyle);
     ScrollbarOverlayStyle scrollbarOverlayStyle() const { return static_cast<ScrollbarOverlayStyle>(m_scrollbarOverlayStyle); }
@@ -201,7 +210,7 @@ public:
     WEBCORE_EXPORT IntSize totalContentsSize() const;
 
     virtual bool shouldSuspendScrollAnimations() const { return true; }
-    virtual void scrollbarStyleChanged(ScrollbarStyle, bool /*forceUpdate*/) { }
+    WEBCORE_EXPORT virtual void scrollbarStyleChanged(ScrollbarStyle /*newStyle*/, bool /*forceUpdate*/);
     virtual void setVisibleScrollerThumbRect(const IntRect&) { }
     
     // Note that this only returns scrollable areas that can actually be scrolled.
index 86416fc..45154a9 100644 (file)
@@ -1,5 +1,33 @@
 2015-02-24  Simon Fraser  <simon.fraser@apple.com>
 
+        Clarify some resizing terminology in ScrollView/FrameView
+        https://bugs.webkit.org/show_bug.cgi?id=141996
+
+        Reviewed by Zalan Bujtas.
+
+        ScrollableArea/ScrollView/FrameView had some confusing terminology around
+        contentsResized/visibleContentsResized/fixedLayoutSizeChanged.
+        
+        Clarify this by distinguishing between:
+        1. Available size changes because of
+            i) non-overlay scrollbar presence
+           ii) ScrollableArea frame change
+        2. Removing fixedLayoutSizeChanged() and just treating it like an
+           available size change.
+           
+        contentsResized() is relegated to simply being a hook that allows Mac to
+        flash overlay scrollbars.
+        
+        The confusingly named visibleContentsResized() is now updateContentsSize(),
+        and is the way that a ScrollableArea tells its subclasss that it should recompute
+        the size of the contents (i.e. do a layout).
+
+        * WebProcess/Plugins/PDF/PDFPlugin.mm:
+        (WebKit::PDFPlugin::scrollbarStyleChanged): Call the base class method,
+        which takes care of 
+
+2015-02-24  Simon Fraser  <simon.fraser@apple.com>
+
         Use an enum for scrollbar style
         https://bugs.webkit.org/show_bug.cgi?id=141985
 
index b2f67eb..0ac9192 100644 (file)
@@ -724,7 +724,7 @@ IntRect PDFPlugin::scrollCornerRect() const
 ScrollableArea* PDFPlugin::enclosingScrollableArea() const
 {
     // FIXME: Walk up the frame tree and look for a scrollable parent frame or RenderLayer.
-    return 0;
+    return nullptr;
 }
 
 IntRect PDFPlugin::scrollableAreaBoundingBox() const
@@ -797,10 +797,9 @@ void PDFPlugin::scrollbarStyleChanged(ScrollbarStyle style, bool forceUpdate)
     IntPoint newScrollOffset = IntPoint(m_scrollOffset).shrunkTo(maximumScrollPosition());
     setScrollOffset(newScrollOffset);
     
+    ScrollableArea::scrollbarStyleChanged(style, forceUpdate);
     // As size of the content area changes, scrollbars may need to appear or to disappear.
     updateScrollbars();
-    
-    ScrollableArea::contentsResized();
 }
 
 void PDFPlugin::addArchiveResource()