Changeset 261686 in webkit

Timestamp:

May 14, 2020 5:31:01 AM (4 years ago)

Author:

graouts@webkit.org

Message:

Cursor should not update on a 20ms timer
​https://bugs.webkit.org/show_bug.cgi?id=211884
<rdar://problem/63220368>

Reviewed by Antti Koivisto.

Source/WebCore:

Update the cursor during page rendering rather than using a 20ms timer.

• page/EventHandler.cpp:

(WebCore::EventHandler::EventHandler):
(WebCore::EventHandler::clear):
(WebCore::EventHandler::updateCursorIfNeeded):
(WebCore::EventHandler::handleMouseMoveEvent):
(WebCore::EventHandler::scheduleCursorUpdate):
(WebCore::EventHandler::cursorUpdateTimerFired): Deleted.

• page/EventHandler.h:
• page/Page.cpp:

(WebCore::Page::updateRendering):

LayoutTests:

Update a (flaky) test to not use setTimeout() to test a cursor change and instead use requestAnimationFrame()
since cursor updates are now guaranteed to be run before the next requestAnimationFrame() callback.

• fast/events/mouse-cursor-no-mousemove.html:
• platform/mac-wk1/TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/events/mouse-cursor-no-mousemove.html (modified) (3 diffs)
• LayoutTests/platform/mac-wk1/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/page/EventHandler.cpp (modified) (5 diffs)
• Source/WebCore/page/EventHandler.h (modified) (3 diffs)
• Source/WebCore/page/Page.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2020-05-14  Antoine Quint  <graouts@apple.com>
+
+        Cursor should not update on a 20ms timer
+        https://bugs.webkit.org/show_bug.cgi?id=211884
+        <rdar://problem/63220368>
+
+        Reviewed by Antti Koivisto.
+
+        Update a (flaky) test to not use setTimeout() to test a cursor change and instead use requestAnimationFrame()
+        since cursor updates are now guaranteed to be run before the next requestAnimationFrame() callback.
+
+        * fast/events/mouse-cursor-no-mousemove.html:
+        * platform/mac-wk1/TestExpectations:
+
 2020-05-14  Philippe Normand  <pnormand@igalia.com>
 

trunk/LayoutTests/fast/events/mouse-cursor-no-mousemove.html

@@ -15 +15 @@
 <div id="console"></div>
 <script>
-var CURSOR_UPDATE_DELAY = 50;
 
 description("Test that there is no mousemove event fired when changing cursor.");
@@ -29 +28 @@
 }
 
-// Can't do anything useful here without eventSender
-if (window.eventSender) {
+(async function()
+{
+    // Can't do anything useful here without eventSender
+    if (!window.eventSender)
+        return;
+
     var node = document.getElementById('target');
     debug('TEST CASE: ' + node.textContent);
@@ -40 +43 @@
     });
     node.style.cursor = 'help';
-    setTimeout(function() {
-        debug('Cursor Info: ' + window.internals.getCurrentCursorInfo());
-        debug('');
-    }, CURSOR_UPDATE_DELAY);
+
+    // Cursor will be updated during the next animation frame.
+    await new Promise(requestAnimationFrame);
+    debug('Cursor Info: ' + window.internals.getCurrentCursorInfo());
+    debug('');
 
     // Give some time for the change to resolve. Fake mousemove event that caused bug, is using a timer
-    setTimeout(function() {
-        document.getElementById('test-container').style.display = 'none';
-        finishJSTest();
-    }, 150);
-}
+    await new Promise(resolve => setTimeout(resolve, 100));
+    document.getElementById('test-container').style.display = 'none';
+    finishJSTest();
+})();
 
 </script>

trunk/LayoutTests/platform/mac-wk1/TestExpectations

@@ -937 +937 @@
 webkit.org/b/209353 [ Release ] media/video-background-tab-playback.html [ Pass Failure ]
 
-webkit.org/b/209494 fast/events/mouse-cursor-no-mousemove.html [ Pass Failure ]
-
 webkit.org/b/209560 imported/w3c/web-platform-tests/xhr/send-response-upload-event-progress.htm [ Pass Crash Failure ]
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-05-14  Antoine Quint  <graouts@apple.com>
+
+        Cursor should not update on a 20ms timer
+        https://bugs.webkit.org/show_bug.cgi?id=211884
+        <rdar://problem/63220368>
+
+        Reviewed by Antti Koivisto.
+
+        Update the cursor during page rendering rather than using a 20ms timer.
+
+        * page/EventHandler.cpp:
+        (WebCore::EventHandler::EventHandler):
+        (WebCore::EventHandler::clear):
+        (WebCore::EventHandler::updateCursorIfNeeded):
+        (WebCore::EventHandler::handleMouseMoveEvent):
+        (WebCore::EventHandler::scheduleCursorUpdate):
+        (WebCore::EventHandler::cursorUpdateTimerFired): Deleted.
+        * page/EventHandler.h:
+        * page/Page.cpp:
+        (WebCore::Page::updateRendering):
+
 2020-05-14  Philippe Normand  <pnormand@igalia.com>
 

trunk/Source/WebCore/page/EventHandler.cpp

@@ -396 +396 @@
     : m_frame(frame)
     , m_hoverTimer(*this, &EventHandler::hoverTimerFired)
-    , m_cursorUpdateTimer(*this, &EventHandler::cursorUpdateTimerFired)
 #if PLATFORM(MAC)
     , m_clearLatchingStateTimer(*this, &EventHandler::clearLatchedStateTimerFired)
@@ -433 +432 @@
 {
     m_hoverTimer.stop();
-    m_cursorUpdateTimer.stop();
+    m_hasScheduledCursorUpdate = false;
 #if !ENABLE(IOS_TOUCH_EVENTS)
     m_fakeMouseMoveEventTimer.stop();
@@ -1417 +1416 @@
 }
 
-void EventHandler::cursorUpdateTimerFired()
-{
-    ASSERT(m_frame.document());
-    updateCursor();
+void EventHandler::updateCursorIfNeeded()
+{
+    if (std::exchange(m_hasScheduledCursorUpdate, false))
+        updateCursor();
 }
 
@@ -1983 +1982 @@
         m_hoverTimer.stop();
 
-    m_cursorUpdateTimer.stop();
+    m_hasScheduledCursorUpdate = false;
 
 #if !ENABLE(IOS_TOUCH_EVENTS)
@@ -3142 +3141 @@
 void EventHandler::scheduleCursorUpdate()
 {
-    if (Page* page = m_frame.page()) {
-        if (!page->chrome().client().supportsSettingCursor())
-            return;
-    }
-
-    if (!m_cursorUpdateTimer.isActive())
-        m_cursorUpdateTimer.startOneShot(cursorUpdateInterval);
+    if (m_hasScheduledCursorUpdate)
+        return;
+
+    auto* page = m_frame.page();
+    if (!page)
+        return;
+
+    if (!page->chrome().client().supportsSettingCursor())
+        return;
+
+    m_hasScheduledCursorUpdate = true;
+    page->scheduleRenderingUpdate();
 }
 

trunk/Source/WebCore/page/EventHandler.h

@@ -320 +320 @@
 
     bool useHandCursor(Node*, bool isOverLink, bool shiftKey);
+    void updateCursorIfNeeded();
     void updateCursor();
 
@@ -376 +377 @@
 
     void hoverTimerFired();
-    void cursorUpdateTimerFired();
 
     bool logicalScrollOverflow(ScrollLogicalDirection, ScrollGranularity, Node* startingNode = nullptr);
@@ -534 +534 @@
 
     Timer m_hoverTimer;
-    Timer m_cursorUpdateTimer;
+    bool m_hasScheduledCursorUpdate { false };
 
 #if PLATFORM(MAC)

trunk/Source/WebCore/page/Page.cpp

@@ -53 +53 @@
 #include "EmptyClients.h"
 #include "Event.h"
+#include "EventHandler.h"
 #include "EventNames.h"
 #include "ExtensionStyleSheets.h"
@@ -1364 +1365 @@
 
     forEachDocument([] (Document& document) {
+        if (auto* frame = document.frame())
+            frame->eventHandler().updateCursorIfNeeded();
+    });
+
+    forEachDocument([] (Document& document) {
         document.runResizeSteps();
     });