Remove the zero-delay ScrollingCoordinatorMac commit timer
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Mar 2020 05:27:00 +0000 (05:27 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Mar 2020 05:27:00 +0000 (05:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=209164

Reviewed by Zalan Bujtas.

Source/WebCore:

The scrolling tree on macOS should just commit at rendering update time. There's no need
for a separate zero-delay timer.

Tested by existing tests.

* page/scrolling/mac/ScrollingCoordinatorMac.h:
* page/scrolling/mac/ScrollingCoordinatorMac.mm:
(WebCore::ScrollingCoordinatorMac::ScrollingCoordinatorMac):
(WebCore::ScrollingCoordinatorMac::pageDestroyed):
(WebCore::ScrollingCoordinatorMac::scheduleTreeStateCommit):
(WebCore::ScrollingCoordinatorMac::commitTreeStateIfNeeded):
(WebCore::ScrollingCoordinatorMac::commitTreeState): Deleted.

LayoutTests:

Dumping layers just made this test flakey. It's enough to test for scroll events on the overflow.

* tiled-drawing/scrolling/fast-scroll-div-latched-mainframe-expected.txt:
* tiled-drawing/scrolling/fast-scroll-div-latched-mainframe.html:

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

LayoutTests/ChangeLog
LayoutTests/tiled-drawing/scrolling/fast-scroll-div-latched-mainframe-expected.txt
LayoutTests/tiled-drawing/scrolling/fast-scroll-div-latched-mainframe.html
Source/WebCore/ChangeLog
Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.h
Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm

index f6c8ee6..581a7f4 100644 (file)
@@ -1,3 +1,15 @@
+2020-03-16  Simon Fraser  <simon.fraser@apple.com>
+
+        Remove the zero-delay ScrollingCoordinatorMac commit timer
+        https://bugs.webkit.org/show_bug.cgi?id=209164
+
+        Reviewed by Zalan Bujtas.
+
+        Dumping layers just made this test flakey. It's enough to test for scroll events on the overflow.
+
+        * tiled-drawing/scrolling/fast-scroll-div-latched-mainframe-expected.txt:
+        * tiled-drawing/scrolling/fast-scroll-div-latched-mainframe.html:
+
 2020-03-16  Lauro Moura  <lmoura@igalia.com>
 
         WPE and GTK gardening.
index fd7ff8f..874ae73 100644 (file)
@@ -53,26 +53,4 @@ PASS successfullyParsed is true
 
 TEST COMPLETE
 PASS Scrollable div did not receive wheel events.
-(GraphicsLayer
-  (anchor 0.00 0.00)
-  (bounds 2008.00 2266.00)
-  (visible rect 0.00, 70.00 785.00 x 585.00)
-  (coverage rect 0.00, 70.00 785.00 x 585.00)
-  (intersects coverage rect 1)
-  (contentsScale 1.00)
-  (children 1
-    (GraphicsLayer
-      (bounds 2008.00 2266.00)
-      (contentsOpaque 1)
-      (visible rect 0.00, 70.00 785.00 x 585.00)
-      (coverage rect 0.00, 0.00 1570.00 x 1755.00)
-      (intersects coverage rect 1)
-      (contentsScale 1.00)
-      (tile cache coverage 0, 0 2008 x 2048)
-      (tile size 512 x 512)
-      (top left tile 0, 0 tiles grid 4 x 4)
-      (in window 1)
-    )
-  )
-)
 
index 5858208..bbdac18 100644 (file)
@@ -63,11 +63,6 @@ function checkForScroll()
     else
         testPassed("Scrollable div did not receive wheel events.");
 
-    if (window.internals) {
-        document.getElementById('layers').innerText = internals.layerTreeAsText(document,
-            internals.LAYER_TREE_INCLUDES_VISIBLE_RECTS | internals.LAYER_TREE_INCLUDES_TILE_CACHES);
-    }
-
     testRunner.notifyDone();
 }
 
@@ -175,7 +170,6 @@ function setupTopLevel()
 <script>
 description("Tests that a scrollable div doesn't consume wheel events when scroll is latched to main frame.");
 </script>
-<pre id="layers">Layer tree goes here</p>
 <script src="../../resources/js-test-post.js"></script>
 </body>
 </html>
index 86769d8..b4b7df9 100644 (file)
@@ -1,5 +1,25 @@
 2020-03-16  Simon Fraser  <simon.fraser@apple.com>
 
+        Remove the zero-delay ScrollingCoordinatorMac commit timer
+        https://bugs.webkit.org/show_bug.cgi?id=209164
+
+        Reviewed by Zalan Bujtas.
+
+        The scrolling tree on macOS should just commit at rendering update time. There's no need
+        for a separate zero-delay timer.
+
+        Tested by existing tests.
+
+        * page/scrolling/mac/ScrollingCoordinatorMac.h:
+        * page/scrolling/mac/ScrollingCoordinatorMac.mm:
+        (WebCore::ScrollingCoordinatorMac::ScrollingCoordinatorMac):
+        (WebCore::ScrollingCoordinatorMac::pageDestroyed):
+        (WebCore::ScrollingCoordinatorMac::scheduleTreeStateCommit):
+        (WebCore::ScrollingCoordinatorMac::commitTreeStateIfNeeded):
+        (WebCore::ScrollingCoordinatorMac::commitTreeState): Deleted.
+
+2020-03-16  Simon Fraser  <simon.fraser@apple.com>
+
         Add a bit more UIHitTesting logging, and make it possible to dump EventRegions from WebKit
         https://bugs.webkit.org/show_bug.cgi?id=209058
 
index 0d5dfd7..7bfd811 100644 (file)
@@ -38,19 +38,15 @@ public:
 
     void pageDestroyed() override;
 
-    void commitTreeStateIfNeeded() override;
+    void commitTreeStateIfNeeded() final;
 
     // Handle the wheel event on the scrolling thread. Returns whether the event was handled or not.
     ScrollingEventResult handleWheelEvent(FrameView&, const PlatformWheelEvent&) override;
 
 private:
-    void scheduleTreeStateCommit() override;
+    void scheduleTreeStateCommit() final;
 
-    void commitTreeState();
-    
     void updateTiledScrollingIndicator();
-
-    Timer m_scrollingStateTreeCommitterTimer;
 };
 
 } // namespace WebCore
index 21ed837..42f2204 100644 (file)
@@ -53,7 +53,6 @@ Ref<ScrollingCoordinator> ScrollingCoordinator::create(Page* page)
 
 ScrollingCoordinatorMac::ScrollingCoordinatorMac(Page* page)
     : AsyncScrollingCoordinator(page)
-    , m_scrollingStateTreeCommitterTimer(*this, &ScrollingCoordinatorMac::commitTreeState)
 {
     setScrollingTree(ScrollingTreeMac::create(*this));
 }
@@ -67,8 +66,6 @@ void ScrollingCoordinatorMac::pageDestroyed()
 {
     AsyncScrollingCoordinator::pageDestroyed();
 
-    m_scrollingStateTreeCommitterTimer.stop();
-
     // Invalidating the scrolling tree will break the reference cycle between the ScrollingCoordinator and ScrollingTree objects.
     RefPtr<ThreadedScrollingTree> scrollingTree = static_pointer_cast<ThreadedScrollingTree>(releaseScrollingTree());
     ScrollingThread::dispatch([scrollingTree] {
@@ -76,12 +73,6 @@ void ScrollingCoordinatorMac::pageDestroyed()
     });
 }
 
-void ScrollingCoordinatorMac::commitTreeStateIfNeeded()
-{
-    commitTreeState();
-    m_scrollingStateTreeCommitterTimer.stop();
-}
-
 ScrollingEventResult ScrollingCoordinatorMac::handleWheelEvent(FrameView&, const PlatformWheelEvent& wheelEvent)
 {
     ASSERT(isMainThread());
@@ -99,16 +90,10 @@ ScrollingEventResult ScrollingCoordinatorMac::handleWheelEvent(FrameView&, const
 
 void ScrollingCoordinatorMac::scheduleTreeStateCommit()
 {
-    ASSERT(scrollingStateTree()->hasChangedProperties() || eventTrackingRegionsDirty());
-
-    if (m_scrollingStateTreeCommitterTimer.isActive())
-        return;
-
-    LOG(Scrolling, "ScrollingCoordinatorMac::scheduleTreeStateCommit");
-    m_scrollingStateTreeCommitterTimer.startOneShot(0_s);
+    m_page->scheduleRenderingUpdate();
 }
 
-void ScrollingCoordinatorMac::commitTreeState()
+void ScrollingCoordinatorMac::commitTreeStateIfNeeded()
 {
     willCommitTree();