Attachment 400303 Details for Bug 212338 – Updated patch initializing m_time to MonotonicTime::now

[patch] Updated patch initializing m_time to MonotonicTime::now

bug-212338-20200527001528.patch (text/plain), 13.50 KB, created by Lauro Moura on 2020-05-26 20:15:29 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Lauro Moura

Created: 2020-05-26 20:15:29 PDT

Size: 13.50 KB

patch

obsolete

>Subversion Revision: 262133
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 7f7de8d6884ed37a12f68ec967106566017a0030..11dcb834e296718f8ba86f57f8e8abf2369e6133 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,24 @@
>+2020-05-25  Lauro Moura  <lmoura@igalia.com>
>+
>+        [WPE] Avoid possible WTR event timing going back in time and fix conversion from s to ms
>+        https://bugs.webkit.org/show_bug.cgi?id=212338
>+
>+        Reviewed by Michael Catanzaro.
>+
>+        When converting the input timestamps to WallTime, we must somehow
>+        adjust them to give meaningful timestamps (due to Dom::Event
>+        conversion from timestamp wall time to the time origin from document
>+        loading).
>+
>+        As WPE timestamps are just intended to be compared among themselves,
>+        we use the first processed event as the origin, comparing the
>+        following events to it to get an actual time.
>+
>+        Test: fast/events/monotonic-event-time.html
>+
>+        * Shared/libwpe/WebEventFactory.cpp:
>+        (WebKit::wallTimeForEventTime):
>+
> 2020-05-25  Alex Christensen  <achristensen@webkit.org>
> 
>         Expose more network metrics to WebCoreNSURLSession
>diff --git a/Source/WebKit/Shared/libwpe/WebEventFactory.cpp b/Source/WebKit/Shared/libwpe/WebEventFactory.cpp
>index 615562597ede2de58e7cc735cde3841030972728..59fcb0ae991f2a458a9ae95359b0c7a5802affca 100644
>--- a/Source/WebKit/Shared/libwpe/WebEventFactory.cpp
>+++ b/Source/WebKit/Shared/libwpe/WebEventFactory.cpp
>@@ -52,14 +52,25 @@ static OptionSet<WebEvent::Modifier> modifiersForEventModifiers(unsigned eventMo
>     return modifiers;
> }
> 
>-WallTime wallTimeForEventTime(uint64_t timestamp)
>+WallTime wallTimeForEventTime(uint64_t msTimeStamp)
> {
>-    // This works if and only if the WPE backend uses CLOCK_MONOTONIC for its
>-    // event timestamps, and so long as g_get_monotonic_time() continues to do
>-    // so as well, and so long as WTF::MonotonicTime continues to use
>-    // g_get_monotonic_time(). It also assumes the event timestamp is in
>-    // milliseconds.
>-    return timestamp ? MonotonicTime::fromRawSeconds(timestamp / 1000.).approximateWallTime() : WallTime::now();
>+    // WPE event time field is an uint32_t, too small for full ms timestamps since
>+    // the epoch, and are expected to be just timestamps with monotonic behavior
>+    // to be compared among themselves, not against WallTime-like measurements.
>+    // Thus the need to define a reference origin based on the first event received.
>+
>+    static uint64_t firstEventTimeStamp;
>+    static WallTime firstEventWallTime;
>+    static std::once_flag once;
>+
>+    std::call_once(once, [msTimeStamp]() {
>+        firstEventTimeStamp = msTimeStamp;
>+        firstEventWallTime = WallTime::now();
>+    });
>+
>+    uint64_t delta = msTimeStamp - firstEventTimeStamp;
>+
>+    return firstEventWallTime + Seconds(delta / 1000.);
> }
> 
> WebKeyboardEvent WebEventFactory::createWebKeyboardEvent(struct wpe_input_keyboard_event* event, const String& text, bool handledByInputMethod, Optional<Vector<WebCore::CompositionUnderline>>&& preeditUnderlines, Optional<EditingRange>&& preeditSelectionRange)
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 8e0f1113cbbd4f77ca55cab6ab8fa04d9c518aa1..a0d315eff0be8142e0393397e1aee142e22bf4bd 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,27 @@
>+2020-05-25  Lauro Moura  <lmoura@igalia.com>
>+
>+        [WPE] Avoid possible WTR event timing going back in time and fix conversion from s to ms
>+        https://bugs.webkit.org/show_bug.cgi?id=212338
>+
>+        Reviewed by Michael Catanzaro.
>+
>+        Make sure we cast the double m_time timestamp from seconds to uint32_t
>+        milliseconds expected by WPE input events.
>+
>+        * WebKitTestRunner/wpe/EventSenderProxyWPE.cpp:
>+        (WTR::secToMsTimestamp):
>+        (WTR::EventSenderProxy::EventSenderProxy): Use monotonic time instead
>+        of zero as the time origin. This is used only in a relative manner,
>+        and we need monotonic behavior so conversion to DOM event timestamps
>+        makes sense.
>+        (WTR::EventSenderProxy::mouseDown):
>+        (WTR::EventSenderProxy::mouseUp):
>+        (WTR::EventSenderProxy::mouseMoveTo):
>+        (WTR::EventSenderProxy::mouseScrollBy):
>+        (WTR::EventSenderProxy::updateTouchPoint):
>+        (WTR::EventSenderProxy::prepareAndDispatchTouchEvent):
>+        (WTR::EventSenderProxy::releaseTouchPoint):
>+
> 2020-05-24  Sam Weinig  <weinig@apple.com>
> 
>         Extended Color Cleanup: Use the name SimpleColor consistently
>diff --git a/Tools/WebKitTestRunner/wpe/EventSenderProxyWPE.cpp b/Tools/WebKitTestRunner/wpe/EventSenderProxyWPE.cpp
>index c4b1915ba731094ffd80d04c99cb53c87324f768..eb1aaadb9ee35cd9636fa32db77d685db372036d 100644
>--- a/Tools/WebKitTestRunner/wpe/EventSenderProxyWPE.cpp
>+++ b/Tools/WebKitTestRunner/wpe/EventSenderProxyWPE.cpp
>@@ -31,6 +31,7 @@
> #include "TestController.h"
> #include <WebCore/NotImplemented.h>
> #include <wpe/wpe.h>
>+#include <wtf/MonotonicTime.h>
> #include <wtf/UniqueArray.h>
> 
> namespace WTR {
>@@ -58,9 +59,16 @@ struct wpe_view_backend* viewBackend(TestController& controller)
>     return controller.mainWebView()->platformWindow()->backend();
> }
> 
>+static uint32_t secToMsTimestamp(double currentEventTime)
>+{
>+    return static_cast<uint32_t>(currentEventTime * 1000);
>+}
>+
> EventSenderProxy::EventSenderProxy(TestController* testController)
>     : m_testController(testController)
>-    , m_time(0)
>+    // WPE event timestamps are just MonotonicTime, not actual WallTime, so we can
>+    // use any point of origin, as long as we are consistent.
>+    , m_time(MonotonicTime::now().secondsSinceEpoch().value())
>     , m_leftMouseButtonDown(false)
>     , m_clickCount(0)
>     , m_clickTime(0)
>@@ -135,7 +143,7 @@ void EventSenderProxy::mouseDown(unsigned button, WKEventModifiers wkModifiers)
>     m_mouseButtonsCurrentlyDown |= modifierForButton(wpeButton);
>     uint32_t modifiers = wkEventModifiersToWPE(wkModifiers);
> 
>-    struct wpe_input_pointer_event event { wpe_input_pointer_event_type_button, static_cast<uint32_t>(m_time), static_cast<int>(m_position.x), static_cast<int>(m_position.y), wpeButton, m_buttonState, m_mouseButtonsCurrentlyDown | modifiers };
>+    struct wpe_input_pointer_event event { wpe_input_pointer_event_type_button, secToMsTimestamp(m_time), static_cast<int>(m_position.x), static_cast<int>(m_position.y), wpeButton, m_buttonState, m_mouseButtonsCurrentlyDown | modifiers };
>     wpe_view_backend_dispatch_pointer_event(viewBackend(*m_testController), &event);
> }
> 
>@@ -148,7 +156,7 @@ void EventSenderProxy::mouseUp(unsigned button, WKEventModifiers wkModifiers)
>     m_mouseButtonsCurrentlyDown &= ~modifierForButton(wpeButton);
>     uint32_t modifiers = wkEventModifiersToWPE(wkModifiers);
> 
>-    struct wpe_input_pointer_event event { wpe_input_pointer_event_type_button, static_cast<uint32_t>(m_time), static_cast<int>(m_position.x), static_cast<int>(m_position.y), wpeButton, m_buttonState, m_mouseButtonsCurrentlyDown | modifiers };
>+    struct wpe_input_pointer_event event { wpe_input_pointer_event_type_button, secToMsTimestamp(m_time), static_cast<int>(m_position.x), static_cast<int>(m_position.y), wpeButton, m_buttonState, m_mouseButtonsCurrentlyDown | modifiers };
>     wpe_view_backend_dispatch_pointer_event(viewBackend(*m_testController), &event);
> }
> 
>@@ -157,7 +165,7 @@ void EventSenderProxy::mouseMoveTo(double x, double y)
>     m_position.x = x;
>     m_position.y = y;
> 
>-    struct wpe_input_pointer_event event { wpe_input_pointer_event_type_motion, static_cast<uint32_t>(m_time), static_cast<int>(m_position.x), static_cast<int>(m_position.y), static_cast<uint32_t>(m_clickButton), m_buttonState, m_mouseButtonsCurrentlyDown };
>+    struct wpe_input_pointer_event event { wpe_input_pointer_event_type_motion, secToMsTimestamp(m_time), static_cast<int>(m_position.x), static_cast<int>(m_position.y), static_cast<uint32_t>(m_clickButton), m_buttonState, m_mouseButtonsCurrentlyDown };
>     wpe_view_backend_dispatch_pointer_event(viewBackend(*m_testController), &event);
> }
> 
>@@ -168,11 +176,11 @@ void EventSenderProxy::mouseScrollBy(int horizontal, int vertical)
>         return;
> 
>     if (horizontal) {
>-        struct wpe_input_axis_event event = { wpe_input_axis_event_type_motion, static_cast<uint32_t>(m_time), static_cast<int>(m_position.x), static_cast<int>(m_position.y), HorizontalScroll, horizontal, 0};
>+        struct wpe_input_axis_event event = { wpe_input_axis_event_type_motion, secToMsTimestamp(m_time), static_cast<int>(m_position.x), static_cast<int>(m_position.y), HorizontalScroll, horizontal, 0};
>         wpe_view_backend_dispatch_axis_event(viewBackend(*m_testController), &event);
>     }
>     if (vertical) {
>-        struct wpe_input_axis_event event =  { wpe_input_axis_event_type_motion, static_cast<uint32_t>(m_time), static_cast<int>(m_position.x), static_cast<int>(m_position.y), VerticalScroll, vertical, 0};
>+        struct wpe_input_axis_event event =  { wpe_input_axis_event_type_motion, secToMsTimestamp(m_time), static_cast<int>(m_position.x), static_cast<int>(m_position.y), VerticalScroll, vertical, 0};
>         wpe_view_backend_dispatch_axis_event(viewBackend(*m_testController), &event);
>     }
> }
>@@ -327,7 +335,7 @@ void EventSenderProxy::updateTouchPoint(int index, int x, int y)
>     auto& rawEvent = m_touchEvents[index];
>     rawEvent.x = x;
>     rawEvent.y = y;
>-    rawEvent.time = m_time;
>+    rawEvent.time = secToMsTimestamp(m_time);
>     rawEvent.type = wpe_input_touch_event_type_motion;
>     m_updatedTouchEvents.add(index);
> }
>@@ -362,7 +370,7 @@ void EventSenderProxy::removeUpdatedTouchEvents()
> void EventSenderProxy::prepareAndDispatchTouchEvent(enum wpe_input_touch_event_type eventType)
> {
>     auto updatedEvents = getUpdatedTouchEvents();
>-    struct wpe_input_touch_event event = { updatedEvents.data(), updatedEvents.size(), eventType, 0, static_cast<uint32_t>(m_time), 0 };
>+    struct wpe_input_touch_event event = { updatedEvents.data(), updatedEvents.size(), eventType, 0, secToMsTimestamp(m_time), 0 };
>     wpe_view_backend_dispatch_touch_event(viewBackend(*m_testController), &event);
>     if (eventType == wpe_input_touch_event_type_up)
>         removeUpdatedTouchEvents();
>@@ -400,7 +408,7 @@ void EventSenderProxy::releaseTouchPoint(int index)
>     ASSERT(index >= 0 && static_cast<size_t>(index) <= m_touchEvents.size());
> 
>     auto& rawEvent = m_touchEvents[index];
>-    rawEvent.time = m_time;
>+    rawEvent.time = secToMsTimestamp(m_time);
>     rawEvent.type = wpe_input_touch_event_type_up;
>     m_updatedTouchEvents.add(index);
> }
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 8d8b84703d778606f36a93cbbe369d9b48230c34..95632cf4dea43c958821d834af4ff22579915720 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,16 @@
>+2020-05-25  Lauro Moura  <lmoura@igalia.com>
>+
>+        [WPE] Avoid possible WTR event timing going back in time and fix conversion from s to ms
>+        https://bugs.webkit.org/show_bug.cgi?id=212338
>+
>+        Reviewed by Michael Catanzaro.
>+
>+        Added new test to check if input events timestamps are actually
>+        monotonically increasing since the start.
>+
>+        * fast/events/monotonic-event-time-expected.txt: Added.
>+        * fast/events/monotonic-event-time.html: Added.
>+
> 2020-05-25  Oriol Brufau  <obrufau@igalia.com>
> 
>         [css-grid] Prevent grid-template-rows from serializing adjacent <line-names>
>diff --git a/LayoutTests/fast/events/monotonic-event-time-expected.txt b/LayoutTests/fast/events/monotonic-event-time-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..b1af633906324ed8db678c58c871fffd9d74007b
>--- /dev/null
>+++ b/LayoutTests/fast/events/monotonic-event-time-expected.txt
>@@ -0,0 +1,13 @@
>+This test checks if DOM event timestamps are monotonically increasing when leaping forward.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS lastTimeStamp < event.timeStamp is true
>+PASS lastTimeStamp < event.timeStamp is true
>+PASS lastTimeStamp < event.timeStamp is true
>+PASS lastTimeStamp < event.timeStamp is true
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/fast/events/monotonic-event-time.html b/LayoutTests/fast/events/monotonic-event-time.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..91ef621d7cc6171709fd51b6612043dbb85005cd
>--- /dev/null
>+++ b/LayoutTests/fast/events/monotonic-event-time.html
>@@ -0,0 +1,47 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<script src="../../resources/js-test.js"></script>
>+<script>
>+
>+var lastTimeStamp = undefined;
>+
>+function mouseDown(event)
>+{
>+    if (typeof(lastTimeStamp) != 'undefined') {
>+        shouldBeTrue('lastTimeStamp < event.timeStamp');
>+    }
>+    lastTimeStamp = event.timeStamp;
>+}
>+
>+function test()
>+{
>+    description(`This test checks if DOM event timestamps are monotonically
>+    increasing when leaping forward.`);
>+
>+    if (window.eventSender) {
>+        var testEle = document.getElementById("target");
>+        eventSender.mouseMoveTo(testEle.offsetLeft+testEle.offsetWidth/2, testEle.offsetTop+testEle.offsetHeight/2);
>+        testClick();
>+    } else if (window.testRunner) {
>+        testFailed('This test requires eventSender');
>+    }
>+}
>+
>+function testClick()
>+{
>+    eventSender.mouseDown();
>+    eventSender.mouseUp();
>+    for (var i = 1; i < 5; i++) {
>+        eventSender.leapForward(1000);
>+        eventSender.mouseDown();
>+        eventSender.mouseUp();
>+    }
>+}
>+</script>
>+</head>
>+<body onload="test()">
>+
>+<div id="target" style="width: 100px; height: 100px; background: red;" onmousedown="mouseDown(event)"></div>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 212338: 400185 | 400303 | 400363