Cursor should not update on a 20ms timer
authorgraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 May 2020 12:31:01 +0000 (12:31 +0000)
committergraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 May 2020 12:31:01 +0000 (12:31 +0000)
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:

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

LayoutTests/ChangeLog
LayoutTests/fast/events/mouse-cursor-no-mousemove.html
LayoutTests/platform/mac-wk1/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/page/EventHandler.cpp
Source/WebCore/page/EventHandler.h
Source/WebCore/page/Page.cpp

index d7e7aae..bbd0eda 100644 (file)
@@ -1,3 +1,17 @@
+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>
 
         [GStreamer] webrtc/disable-encryption.html is a crashing flaky
index c0720a4..c176390 100644 (file)
@@ -14,7 +14,6 @@
 <br/>
 <div id="console"></div>
 <script>
-var CURSOR_UPDATE_DELAY = 50;
 
 description("Test that there is no mousemove event fired when changing cursor.");
 
@@ -28,8 +27,12 @@ if (window.testRunner) {
     window.jsTestIsAsync = true;
 }
 
-// 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);
     eventSender.mouseMoveTo(node.offsetLeft + 3, node.offsetTop + 3);
@@ -39,17 +42,17 @@ if (window.eventSender) {
         finishJSTest();
     });
     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>
 <script src="../../resources/js-test-post.js"></script>
index f27b9be..cddb9ba 100644 (file)
@@ -936,8 +936,6 @@ webkit.org/b/209182 [ Debug ] inspector/debugger/setShouldBlackboxURL.html [ Pas
 
 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 ]
 
 webkit.org/b/209882 [ Debug ] inspector/page/overrideSetting-ICECandidateFilteringEnabled.html [ Pass Timeout ]
index 8147e5f..da979d7 100644 (file)
@@ -1,3 +1,24 @@
+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>
 
         [GStreamer] webrtc/disable-encryption.html is a crashing flaky
index cf97ea4..2cdee17 100644 (file)
@@ -395,7 +395,6 @@ bool EventHandler::eventLoopHandleMouseDragged(const MouseEventWithHitTestResult
 EventHandler::EventHandler(Frame& frame)
     : m_frame(frame)
     , m_hoverTimer(*this, &EventHandler::hoverTimerFired)
-    , m_cursorUpdateTimer(*this, &EventHandler::cursorUpdateTimerFired)
 #if PLATFORM(MAC)
     , m_clearLatchingStateTimer(*this, &EventHandler::clearLatchedStateTimerFired)
 #endif
@@ -432,7 +431,7 @@ DragState& EventHandler::dragState()
 void EventHandler::clear()
 {
     m_hoverTimer.stop();
-    m_cursorUpdateTimer.stop();
+    m_hasScheduledCursorUpdate = false;
 #if !ENABLE(IOS_TOUCH_EVENTS)
     m_fakeMouseMoveEventTimer.stop();
 #endif
@@ -1416,10 +1415,10 @@ bool EventHandler::useHandCursor(Node* node, bool isOverLink, bool shiftKey)
     return ((isOverLink || isSubmitImage(node)) && (!editable || editableLinkEnabled));
 }
 
-void EventHandler::cursorUpdateTimerFired()
+void EventHandler::updateCursorIfNeeded()
 {
-    ASSERT(m_frame.document());
-    updateCursor();
+    if (std::exchange(m_hasScheduledCursorUpdate, false))
+        updateCursor();
 }
 
 void EventHandler::updateCursor()
@@ -1982,7 +1981,7 @@ bool EventHandler::handleMouseMoveEvent(const PlatformMouseEvent& platformMouseE
     if (m_hoverTimer.isActive())
         m_hoverTimer.stop();
 
-    m_cursorUpdateTimer.stop();
+    m_hasScheduledCursorUpdate = false;
 
 #if !ENABLE(IOS_TOUCH_EVENTS)
     cancelFakeMouseMoveEvent();
@@ -3141,13 +3140,18 @@ void EventHandler::scheduleHoverStateUpdate()
 
 void EventHandler::scheduleCursorUpdate()
 {
-    if (Page* page = m_frame.page()) {
-        if (!page->chrome().client().supportsSettingCursor())
-            return;
-    }
+    if (m_hasScheduledCursorUpdate)
+        return;
+
+    auto* page = m_frame.page();
+    if (!page)
+        return;
+
+    if (!page->chrome().client().supportsSettingCursor())
+        return;
 
-    if (!m_cursorUpdateTimer.isActive())
-        m_cursorUpdateTimer.startOneShot(cursorUpdateInterval);
+    m_hasScheduledCursorUpdate = true;
+    page->scheduleRenderingUpdate();
 }
 
 void EventHandler::dispatchFakeMouseMoveEventSoon()
index ef1d48e..6ffc83c 100644 (file)
@@ -319,6 +319,7 @@ public:
 #endif
 
     bool useHandCursor(Node*, bool isOverLink, bool shiftKey);
+    void updateCursorIfNeeded();
     void updateCursor();
 
     bool isHandlingWheelEvent() const { return m_isHandlingWheelEvent; }
@@ -375,7 +376,6 @@ private:
     void updateCursor(FrameView&, const HitTestResult&, bool shiftKey);
 
     void hoverTimerFired();
-    void cursorUpdateTimerFired();
 
     bool logicalScrollOverflow(ScrollLogicalDirection, ScrollGranularity, Node* startingNode = nullptr);
     
@@ -533,7 +533,7 @@ private:
 #endif
 
     Timer m_hoverTimer;
-    Timer m_cursorUpdateTimer;
+    bool m_hasScheduledCursorUpdate { false };
 
 #if PLATFORM(MAC)
     Timer m_clearLatchingStateTimer;
index 876fa71..4abc7e7 100644 (file)
@@ -52,6 +52,7 @@
 #include "EditorClient.h"
 #include "EmptyClients.h"
 #include "Event.h"
+#include "EventHandler.h"
 #include "EventNames.h"
 #include "ExtensionStyleSheets.h"
 #include "FocusController.h"
@@ -1363,6 +1364,11 @@ void Page::updateRendering()
     // Flush autofocus candidates
 
     forEachDocument([] (Document& document) {
+        if (auto* frame = document.frame())
+            frame->eventHandler().updateCursorIfNeeded();
+    });
+
+    forEachDocument([] (Document& document) {
         document.runResizeSteps();
     });