<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[282629] trunk/Source/WebCore</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/282629">282629</a></dd>
<dt>Author</dt> <dd>simon.fraser@apple.com</dd>
<dt>Date</dt> <dd>2021-09-16 19:33:27 -0700 (Thu, 16 Sep 2021)</dd>
</dl>

<h3>Log Message</h3>
<pre>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):</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorepageFrameViewcpp">trunk/Source/WebCore/page/FrameView.cpp</a></li>
<li><a href="#trunkSourceWebCorepageFrameViewh">trunk/Source/WebCore/page/FrameView.h</a></li>
<li><a href="#trunkSourceWebCorepagePagecpp">trunk/Source/WebCore/page/Page.cpp</a></li>
<li><a href="#trunkSourceWebCoreplatformScrollViewcpp">trunk/Source/WebCore/platform/ScrollView.cpp</a></li>
<li><a href="#trunkSourceWebCoreplatformScrollViewh">trunk/Source/WebCore/platform/ScrollView.h</a></li>
<li><a href="#trunkSourceWebCoreplatformScrollableAreacpp">trunk/Source/WebCore/platform/ScrollableArea.cpp</a></li>
<li><a href="#trunkSourceWebCoreplatformmacScrollAnimatorMacmm">trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.mm</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (282628 => 282629)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,40 @@
</span><ins>+2021-09-16  Simon Fraser  <simon.fraser@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):
+
</ins><span class="cx"> 2021-09-16  Philip Chimento  <pchimento@igalia.com>
</span><span class="cx"> 
</span><span class="cx">         Fixes for build-webkit --minimal
</span></span></pre></div>
<a id="trunkSourceWebCorepageFrameViewcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/FrameView.cpp (282628 => 282629)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -3967,8 +3967,18 @@
</span><span class="cx">     ScrollView::scrollbarStyleChanged(newStyle, forceUpdate);
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-void FrameView::notifyPageThatContentAreaWillPaint() const
</del><ins>+void FrameView::notifyAllFramesThatContentAreaWillPaint() const
</ins><span class="cx"> {
</span><ins>+    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
+{
</ins><span class="cx">     Page* page = frame().page();
</span><span class="cx">     if (!page)
</span><span class="cx">         return;
</span><span class="lines">@@ -3978,8 +3988,11 @@
</span><span class="cx">     if (!m_scrollableAreas)
</span><span class="cx">         return;
</span><span class="cx"> 
</span><del>-    for (auto& scrollableArea : *m_scrollableAreas)
-        scrollableArea->contentAreaWillPaint();
</del><ins>+    for (auto& scrollableArea : *m_scrollableAreas) {
+        // ScrollView ScrollableAreas will be handled via the Frame tree traversal above.
+        if (!is<ScrollView>(scrollableArea))
+            scrollableArea->contentAreaWillPaint();
+    }
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> bool FrameView::scrollAnimatorEnabled() const
</span></span></pre></div>
<a id="trunkSourceWebCorepageFrameViewh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/FrameView.h (282628 => 282629)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -634,6 +634,8 @@
</span><span class="cx"> 
</span><span class="cx">     const HashSet<Widget*>& widgetsInRenderTree() const { return m_widgetsInRenderTree; }
</span><span class="cx"> 
</span><ins>+    void notifyAllFramesThatContentAreaWillPaint() const;
+
</ins><span class="cx">     void addTrackedRepaintRect(const FloatRect&);
</span><span class="cx"> 
</span><span class="cx">     // exposedRect represents WebKit's understanding of what part
</span><span class="lines">@@ -796,8 +798,6 @@
</span><span class="cx">     void updateScrollableAreaSet();
</span><span class="cx">     void updateLayoutViewport();
</span><span class="cx"> 
</span><del>-    void notifyPageThatContentAreaWillPaint() const final;
-
</del><span class="cx">     void enableSpeculativeTilingIfNeeded();
</span><span class="cx">     void speculativeTilingEnableTimerFired();
</span><span class="cx"> 
</span><span class="lines">@@ -814,6 +814,8 @@
</span><span class="cx">     void scheduleScrollEvent();
</span><span class="cx">     void resetScrollAnchor();
</span><span class="cx"> 
</span><ins>+    void notifyScrollableAreasThatContentAreaWillPaint() const;
+
</ins><span class="cx">     bool hasCustomScrollbars() const;
</span><span class="cx"> 
</span><span class="cx">     void updateScrollCorner() final;
</span></span></pre></div>
<a id="trunkSourceWebCorepagePagecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/Page.cpp (282628 => 282629)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1714,6 +1714,9 @@
</span><span class="cx">     }
</span><span class="cx"> #endif
</span><span class="cx"> 
</span><ins>+    if (auto* view = mainFrame().view())
+        view->notifyAllFramesThatContentAreaWillPaint();
+
</ins><span class="cx">     if (!m_sampledPageTopColor) {
</span><span class="cx">         m_sampledPageTopColor = PageColorSampler::sampleTop(*this);
</span><span class="cx">         if (m_sampledPageTopColor)
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformScrollViewcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/ScrollView.cpp (282628 => 282629)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -432,10 +432,6 @@
</span><span class="cx">     return scrollPosition() - IntSize(0, headerHeight());
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-void ScrollView::notifyPageThatContentAreaWillPaint() const
-{
-}
-
</del><span class="cx"> void ScrollView::setScrollOffset(const ScrollOffset& offset)
</span><span class="cx"> {
</span><span class="cx">     LOG_WITH_STREAM(Scrolling, stream << "\nScrollView::setScrollOffset " << offset << " constrains " << constrainsScrollingToContentEdge());
</span><span class="lines">@@ -1185,7 +1181,6 @@
</span><span class="cx">         return;
</span><span class="cx"> 
</span><span class="cx">     if (platformWidget()) {
</span><del>-        notifyPageThatContentAreaWillPaint();
</del><span class="cx">         platformRepaintContentRectangle(paintRect);
</span><span class="cx">         return;
</span><span class="cx">     }
</span><span class="lines">@@ -1282,8 +1277,6 @@
</span><span class="cx">     if (context.paintingDisabled() && !context.performingPaintInvalidation() && !eventRegionContext)
</span><span class="cx">         return;
</span><span class="cx"> 
</span><del>-    notifyPageThatContentAreaWillPaint();
-
</del><span class="cx">     IntRect documentDirtyRect = rect;
</span><span class="cx">     if (!paintsEntireContents()) {
</span><span class="cx">         IntRect visibleAreaWithoutScrollbars(locationOfContents(), visibleContentRect(LegacyIOSDocumentVisibleRect).size());
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformScrollViewh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/ScrollView.h (282628 => 282629)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -75,8 +75,6 @@
</span><span class="cx">     bool isScrollCornerVisible() const final;
</span><span class="cx">     void scrollbarStyleChanged(ScrollbarStyle, bool forceUpdate) override;
</span><span class="cx"> 
</span><del>-    virtual void notifyPageThatContentAreaWillPaint() const;
-
</del><span class="cx">     IntPoint locationOfContents() const;
</span><span class="cx"> 
</span><span class="cx">     // NOTE: This should only be called by the overridden setScrollOffset from ScrollableArea.
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformScrollableAreacpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/ScrollableArea.cpp (282628 => 282629)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -258,6 +258,7 @@
</span><span class="cx"> 
</span><span class="cx"> void ScrollableArea::contentAreaWillPaint() const
</span><span class="cx"> {
</span><ins>+    // This is used to flash overlay scrollbars in some circumstances.
</ins><span class="cx">     if (ScrollAnimator* scrollAnimator = existingScrollAnimator())
</span><span class="cx">         scrollAnimator->contentAreaWillPaint();
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformmacScrollAnimatorMacmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.mm (282628 => 282629)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -891,6 +891,8 @@
</span><span class="cx"> 
</span><span class="cx"> void ScrollAnimatorMac::contentAreaWillPaint() const
</span><span class="cx"> {
</span><ins>+    LOG_WITH_STREAM(OverlayScrollbars, stream << "ScrollAnimatorMac for [" << scrollableArea() << "] contentAreaWillPaint (scrollers locked " << [m_scrollerImpPair overlayScrollerStateIsLocked] << ")");
+
</ins><span class="cx">     if ([m_scrollerImpPair overlayScrollerStateIsLocked])
</span><span class="cx">         return;
</span><span class="cx"> 
</span></span></pre>
</div>
</div>

</body>
</html>