[webkit-changes] [282629] trunk/Source/WebCore

Thu, 16 Sep 2021 19:34:10 -0700

Title: [282629] trunk/Source/WebCore
Revision
282629
Author
simon.fra...@apple.com
Date
2021-09-16 19:33:27 -0700 (Thu, 16 Sep 2021)

Log Message

Fix some issues with the code paths that call into ScrollAnimator::contentAreaWillPaint()
https://bugs.webkit.org/show_bug.cgi?id=230372

Reviewed by Tim Horton.

ScrollAnimator::contentAreaWillPaint() is a hook used to flash overlay scrollbars in some
situations (e.g. a view becomes newly visible). AppKit wants `[m_scrollerImpPair contentAreaWillDraw]`
to be called at the equivalent of "viewWillDraw" time, which in WebCore terminology is
the end of the Page rendering update stage.

However, existing WebKitLegacy-only code called notifyPageThatContentAreaWillPaint()
from repaint code paths, including updateControlTints(), which was wrong, and caused
ordering issues in tests between the calls to setUsesMockScrollAnimator(true) and
accessing the scroll animator (see also webkit.org/b/230371).

Fix by calling FrameView::notifyAllFramesThatContentAreaWillPaint() near the end
of Page::doAfterUpdateRendering(), and having it do a correct Frame tree traversal.

* page/FrameView.cpp:
(WebCore::FrameView::notifyAllFramesThatContentAreaWillPaint const):
(WebCore::FrameView::notifyScrollableAreasThatContentAreaWillPaint const):
(WebCore::FrameView::notifyPageThatContentAreaWillPaint const): Deleted.
* page/FrameView.h:
* page/Page.cpp:
(WebCore::Page::doAfterUpdateRendering):
* platform/ScrollView.cpp:
(WebCore::ScrollView::repaintContentRectangle):
(WebCore::ScrollView::paint):
(WebCore::ScrollView::notifyPageThatContentAreaWillPaint const): Deleted.
* platform/ScrollView.h:
* platform/ScrollableArea.cpp:
(WebCore::ScrollableArea::contentAreaWillPaint const):
* platform/mac/ScrollAnimatorMac.mm:
(WebCore::ScrollAnimatorMac::contentAreaWillPaint const):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (282628 => 282629)


--- trunk/Source/WebCore/ChangeLog	2021-09-17 02:29:15 UTC (rev 282628)
+++ trunk/Source/WebCore/ChangeLog	2021-09-17 02:33:27 UTC (rev 282629)
@@ -1,3 +1,40 @@
+2021-09-16  Simon Fraser  <simon.fra...@apple.com>
+
+        Fix some issues with the code paths that call into ScrollAnimator::contentAreaWillPaint()
+        https://bugs.webkit.org/show_bug.cgi?id=230372
+
+        Reviewed by Tim Horton.
+
+        ScrollAnimator::contentAreaWillPaint() is a hook used to flash overlay scrollbars in some
+        situations (e.g. a view becomes newly visible). AppKit wants `[m_scrollerImpPair contentAreaWillDraw]`
+        to be called at the equivalent of "viewWillDraw" time, which in WebCore terminology is
+        the end of the Page rendering update stage.
+
+        However, existing WebKitLegacy-only code called notifyPageThatContentAreaWillPaint()
+        from repaint code paths, including updateControlTints(), which was wrong, and caused
+        ordering issues in tests between the calls to setUsesMockScrollAnimator(true) and
+        accessing the scroll animator (see also webkit.org/b/230371).
+
+        Fix by calling FrameView::notifyAllFramesThatContentAreaWillPaint() near the end
+        of Page::doAfterUpdateRendering(), and having it do a correct Frame tree traversal.
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::notifyAllFramesThatContentAreaWillPaint const):
+        (WebCore::FrameView::notifyScrollableAreasThatContentAreaWillPaint const):
+        (WebCore::FrameView::notifyPageThatContentAreaWillPaint const): Deleted.
+        * page/FrameView.h:
+        * page/Page.cpp:
+        (WebCore::Page::doAfterUpdateRendering):
+        * platform/ScrollView.cpp:
+        (WebCore::ScrollView::repaintContentRectangle):
+        (WebCore::ScrollView::paint):
+        (WebCore::ScrollView::notifyPageThatContentAreaWillPaint const): Deleted.
+        * platform/ScrollView.h:
+        * platform/ScrollableArea.cpp:
+        (WebCore::ScrollableArea::contentAreaWillPaint const):
+        * platform/mac/ScrollAnimatorMac.mm:
+        (WebCore::ScrollAnimatorMac::contentAreaWillPaint const):
+
 2021-09-16  Philip Chimento  <pchime...@igalia.com>
 
         Fixes for build-webkit --minimal

Modified: trunk/Source/WebCore/page/FrameView.cpp (282628 => 282629)


--- trunk/Source/WebCore/page/FrameView.cpp	2021-09-17 02:29:15 UTC (rev 282628)
+++ trunk/Source/WebCore/page/FrameView.cpp	2021-09-17 02:33:27 UTC (rev 282629)
@@ -3967,8 +3967,18 @@
     ScrollView::scrollbarStyleChanged(newStyle, forceUpdate);
 }
 
-void FrameView::notifyPageThatContentAreaWillPaint() const
+void FrameView::notifyAllFramesThatContentAreaWillPaint() const
 {
+    notifyScrollableAreasThatContentAreaWillPaint();
+
+    for (auto* child = frame().tree().firstRenderedChild(); child; child = child->tree().traverseNextRendered(m_frame.ptr())) {
+        if (auto* frameView = child->view())
+            frameView->notifyScrollableAreasThatContentAreaWillPaint();
+    }
+}
+
+void FrameView::notifyScrollableAreasThatContentAreaWillPaint() const
+{
     Page* page = frame().page();
     if (!page)
         return;
@@ -3978,8 +3988,11 @@
     if (!m_scrollableAreas)
         return;
 
-    for (auto& scrollableArea : *m_scrollableAreas)
-        scrollableArea->contentAreaWillPaint();
+    for (auto& scrollableArea : *m_scrollableAreas) {
+        // ScrollView ScrollableAreas will be handled via the Frame tree traversal above.
+        if (!is<ScrollView>(scrollableArea))
+            scrollableArea->contentAreaWillPaint();
+    }
 }
 
 bool FrameView::scrollAnimatorEnabled() const

Modified: trunk/Source/WebCore/page/FrameView.h (282628 => 282629)


--- trunk/Source/WebCore/page/FrameView.h	2021-09-17 02:29:15 UTC (rev 282628)
+++ trunk/Source/WebCore/page/FrameView.h	2021-09-17 02:33:27 UTC (rev 282629)
@@ -634,6 +634,8 @@
 
     const HashSet<Widget*>& widgetsInRenderTree() const { return m_widgetsInRenderTree; }
 
+    void notifyAllFramesThatContentAreaWillPaint() const;
+
     void addTrackedRepaintRect(const FloatRect&);
 
     // exposedRect represents WebKit's understanding of what part
@@ -796,8 +798,6 @@
     void updateScrollableAreaSet();
     void updateLayoutViewport();
 
-    void notifyPageThatContentAreaWillPaint() const final;
-
     void enableSpeculativeTilingIfNeeded();
     void speculativeTilingEnableTimerFired();
 
@@ -814,6 +814,8 @@
     void scheduleScrollEvent();
     void resetScrollAnchor();
 
+    void notifyScrollableAreasThatContentAreaWillPaint() const;
+
     bool hasCustomScrollbars() const;
 
     void updateScrollCorner() final;

Modified: trunk/Source/WebCore/page/Page.cpp (282628 => 282629)


--- trunk/Source/WebCore/page/Page.cpp	2021-09-17 02:29:15 UTC (rev 282628)
+++ trunk/Source/WebCore/page/Page.cpp	2021-09-17 02:33:27 UTC (rev 282629)
@@ -1714,6 +1714,9 @@
     }
 #endif
 
+    if (auto* view = mainFrame().view())
+        view->notifyAllFramesThatContentAreaWillPaint();
+
     if (!m_sampledPageTopColor) {
         m_sampledPageTopColor = PageColorSampler::sampleTop(*this);
         if (m_sampledPageTopColor)

Modified: trunk/Source/WebCore/platform/ScrollView.cpp (282628 => 282629)


--- trunk/Source/WebCore/platform/ScrollView.cpp	2021-09-17 02:29:15 UTC (rev 282628)
+++ trunk/Source/WebCore/platform/ScrollView.cpp	2021-09-17 02:33:27 UTC (rev 282629)
@@ -432,10 +432,6 @@
     return scrollPosition() - IntSize(0, headerHeight());
 }
 
-void ScrollView::notifyPageThatContentAreaWillPaint() const
-{
-}
-
 void ScrollView::setScrollOffset(const ScrollOffset& offset)
 {
     LOG_WITH_STREAM(Scrolling, stream << "\nScrollView::setScrollOffset " << offset << " constrains " << constrainsScrollingToContentEdge());
@@ -1185,7 +1181,6 @@
         return;
 
     if (platformWidget()) {
-        notifyPageThatContentAreaWillPaint();
         platformRepaintContentRectangle(paintRect);
         return;
     }
@@ -1282,8 +1277,6 @@
     if (context.paintingDisabled() && !context.performingPaintInvalidation() && !eventRegionContext)
         return;
 
-    notifyPageThatContentAreaWillPaint();
-
     IntRect documentDirtyRect = rect;
     if (!paintsEntireContents()) {
         IntRect visibleAreaWithoutScrollbars(locationOfContents(), visibleContentRect(LegacyIOSDocumentVisibleRect).size());

Modified: trunk/Source/WebCore/platform/ScrollView.h (282628 => 282629)


--- trunk/Source/WebCore/platform/ScrollView.h	2021-09-17 02:29:15 UTC (rev 282628)
+++ trunk/Source/WebCore/platform/ScrollView.h	2021-09-17 02:33:27 UTC (rev 282629)
@@ -75,8 +75,6 @@
     bool isScrollCornerVisible() const final;
     void scrollbarStyleChanged(ScrollbarStyle, bool forceUpdate) override;
 
-    virtual void notifyPageThatContentAreaWillPaint() const;
-
     IntPoint locationOfContents() const;
 
     // NOTE: This should only be called by the overridden setScrollOffset from ScrollableArea.

Modified: trunk/Source/WebCore/platform/ScrollableArea.cpp (282628 => 282629)


--- trunk/Source/WebCore/platform/ScrollableArea.cpp	2021-09-17 02:29:15 UTC (rev 282628)
+++ trunk/Source/WebCore/platform/ScrollableArea.cpp	2021-09-17 02:33:27 UTC (rev 282629)
@@ -258,6 +258,7 @@
 
 void ScrollableArea::contentAreaWillPaint() const
 {
+    // This is used to flash overlay scrollbars in some circumstances.
     if (ScrollAnimator* scrollAnimator = existingScrollAnimator())
         scrollAnimator->contentAreaWillPaint();
 }

Modified: trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.mm (282628 => 282629)


--- trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.mm	2021-09-17 02:29:15 UTC (rev 282628)
+++ trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.mm	2021-09-17 02:33:27 UTC (rev 282629)
@@ -891,6 +891,8 @@
 
 void ScrollAnimatorMac::contentAreaWillPaint() const
 {
+    LOG_WITH_STREAM(OverlayScrollbars, stream << "ScrollAnimatorMac for [" << scrollableArea() << "] contentAreaWillPaint (scrollers locked " << [m_scrollerImpPair overlayScrollerStateIsLocked] << ")");
+
     if ([m_scrollerImpPair overlayScrollerStateIsLocked])
         return;
 

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to