Changeset 258753 in webkit

Timestamp:

Mar 19, 2020 11:03:34 PM (4 years ago)

Author:

Simon Fraser

Message:

Some scroll snapping tests are still flaky
​https://bugs.webkit.org/show_bug.cgi?id=165196

Reviewed by Wenson Hsieh.

WheelEventTestMonitor could trigger too early if the main thread was bogged down, delaying
the firing of the m_updateNodeScrollPositionTimer scheduled from
AsyncScrollingCoordinator::scheduleUpdateScrollPositionAfterAsyncScroll().

Fix by extending the life of the "ScrollingThreadSyncNeeded" reason until after the m_updateNodeScrollPositionTimer
has fired

Fixes flakiness of tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-slow-vertical.html
and others.

• page/scrolling/AsyncScrollingCoordinator.cpp:

(WebCore::AsyncScrollingCoordinator::noteScrollingThreadSyncCompleteForNode):
(WebCore::AsyncScrollingCoordinator::scheduleUpdateScrollPositionAfterAsyncScroll):
(WebCore::AsyncScrollingCoordinator::updateScrollPositionAfterAsyncScrollTimerFired):

• page/scrolling/AsyncScrollingCoordinator.h:
• page/scrolling/ThreadedScrollingTree.cpp:

(WebCore::ThreadedScrollingTree::scrollingTreeNodeDidScroll):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• page/scrolling/AsyncScrollingCoordinator.cpp (modified) (3 diffs)
• page/scrolling/AsyncScrollingCoordinator.h (modified) (1 diff)
• page/scrolling/ThreadedScrollingTree.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-03-19  Simon Fraser  <simon.fraser@apple.com>
+
+        Some scroll snapping tests are still flaky
+        https://bugs.webkit.org/show_bug.cgi?id=165196
+
+        Reviewed by Wenson Hsieh.
+
+        WheelEventTestMonitor could trigger too early if the main thread was bogged down, delaying
+        the firing of the m_updateNodeScrollPositionTimer scheduled from
+        AsyncScrollingCoordinator::scheduleUpdateScrollPositionAfterAsyncScroll().
+
+        Fix by extending the life of the "ScrollingThreadSyncNeeded" reason until after the m_updateNodeScrollPositionTimer
+        has fired
+
+        Fixes flakiness of tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-slow-vertical.html
+        and others.
+
+        * page/scrolling/AsyncScrollingCoordinator.cpp:
+        (WebCore::AsyncScrollingCoordinator::noteScrollingThreadSyncCompleteForNode):
+        (WebCore::AsyncScrollingCoordinator::scheduleUpdateScrollPositionAfterAsyncScroll):
+        (WebCore::AsyncScrollingCoordinator::updateScrollPositionAfterAsyncScrollTimerFired):
+        * page/scrolling/AsyncScrollingCoordinator.h:
+        * page/scrolling/ThreadedScrollingTree.cpp:
+        (WebCore::ThreadedScrollingTree::scrollingTreeNodeDidScroll):
+
 2020-03-19  Peng Liu  <peng.liu6@apple.com>
 

trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp

@@ -270 +270 @@
 }
 
+void AsyncScrollingCoordinator::noteScrollingThreadSyncCompleteForNode(ScrollingNodeID nodeID)
+{
+#if PLATFORM(MAC)
+    if (m_page && m_page->isMonitoringWheelEvents()) {
+        LOG_WITH_STREAM(WheelEventTestMonitor, stream << "    (!) AsyncScrollingCoordinator::scheduleUpdateScrollPositionAfterAsyncScroll: Removing deferral on " << nodeID << " for reason " << WheelEventTestMonitor::ScrollingThreadSyncNeeded);
+        m_page->wheelEventTestMonitor()->removeDeferralForReason(reinterpret_cast<WheelEventTestMonitor::ScrollableAreaIdentifier>(nodeID), WheelEventTestMonitor::ScrollingThreadSyncNeeded);
+    }
+#else
+    UNUSED_PARAM(nodeID);
+#endif
+}
+
 void AsyncScrollingCoordinator::scheduleUpdateScrollPositionAfterAsyncScroll(ScrollingNodeID nodeID, const FloatPoint& scrollPosition, const Optional<FloatPoint>& layoutViewportOrigin, ScrollingLayerPositionAction scrollingLayerPositionAction)
 {
@@ -285 +297 @@
         updateScrollPositionAfterAsyncScroll(m_scheduledScrollUpdate.nodeID, m_scheduledScrollUpdate.scrollPosition, m_scheduledScrollUpdate.layoutViewportOrigin, ScrollType::User, m_scheduledScrollUpdate.updateLayerPositionAction);
         updateScrollPositionAfterAsyncScroll(nodeID, scrollPosition, layoutViewportOrigin, ScrollType::User, scrollingLayerPositionAction);
+        
+        if (m_scheduledScrollUpdate.nodeID != nodeID)
+            noteScrollingThreadSyncCompleteForNode(m_scheduledScrollUpdate.nodeID);
+        noteScrollingThreadSyncCompleteForNode(nodeID);
         return;
     }
@@ -295 +311 @@
 {
     updateScrollPositionAfterAsyncScroll(m_scheduledScrollUpdate.nodeID, m_scheduledScrollUpdate.scrollPosition, m_scheduledScrollUpdate.layoutViewportOrigin, ScrollType::User, m_scheduledScrollUpdate.updateLayerPositionAction);
+    noteScrollingThreadSyncCompleteForNode(m_scheduledScrollUpdate.nodeID);
 }
 

trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h

@@ -142 +142 @@
     void updateEventTrackingRegions();
     
+    void noteScrollingThreadSyncCompleteForNode(ScrollingNodeID);
+    
     FrameView* frameViewForScrollingNode(ScrollingNodeID) const;
 

trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp

@@ -117 +117 @@
 #endif
 
-    RunLoop::main().dispatch([strongThis = makeRef(*this), nodeID = node.scrollingNodeID(), scrollPosition, layoutViewportOrigin, scrollingLayerPositionAction, monitoringWheelEvents] {
+    RunLoop::main().dispatch([strongThis = makeRef(*this), nodeID = node.scrollingNodeID(), scrollPosition, layoutViewportOrigin, scrollingLayerPositionAction] {
         strongThis->m_scrollingCoordinator->scheduleUpdateScrollPositionAfterAsyncScroll(nodeID, scrollPosition, layoutViewportOrigin, scrollingLayerPositionAction);
-#if PLATFORM(MAC)
-        if (monitoringWheelEvents)
-            strongThis->removeWheelEventTestCompletionDeferralForReason(reinterpret_cast<WheelEventTestMonitor::ScrollableAreaIdentifier>(nodeID), WheelEventTestMonitor::ScrollingThreadSyncNeeded);
-#else
-        UNUSED_PARAM(monitoringWheelEvents);
-#endif
     });
 }