`moveOverTime` timer delays can cause intermediate events to be skipped
Categories
(Remote Protocol :: Agent, defect, P3)
Tracking
(firefox156 fixed)
| Tracking | Status | |
|---|---|---|
| firefox156 | --- | fixed |
People
(Reporter: whimboo, Assigned: whimboo)
References
(Blocks 1 open bug)
Details
(Whiteboard: [webdriver:m21][wptsync upstream])
Attachments
(3 files)
The moveOverTime function in Actions.sys.mjs uses an nsITimer (TYPE_ONE_SHOT) with a 17ms delay to slow-down incremental actions transitions. Before the first intermediate transition, it waits for this timer to fire. However, the timer callback is delivered as a Normal priority (4) runnable and as such gets starved by higher-priority work on the parent process main thread (Control priority IPC messages, Vsync priority RefreshDriver ticks), causing delays of ~140ms instead of the expected 17ms.
This was observed in CI mainly on macOS aarch64 workers where a wheel scroll action with duration=100 sometimes only dispatches a single wheel event instead of multiple, because the initial timer delay exceeds already the specified duration. For more details see bug 1852529 comment 7.
From a Gecko Profile (https://share.firefox.dev/4hacYcF):
- Timer initialized at ~t=22ms with a 17ms delay
- Parent process main thread is not CPU-bound between t=22ms and t=160ms but it's processing
vsyncticks andIPC - Timer callback fires at ~t=160ms (139ms late)
- The actual wheel event synthesis + dispatch takes only ~10ms after the timer fires
As per the WebDriver specification (here the case for wheel and scroll) the endpoint should do the following before any dispatching of events:
If duration is greater than 0 and inside any implementation-defined bounds, asynchronously wait for an implementation defined amount of time to pass.
This wait allows the implementation to model the overall wheel scroll as a series of small scroll occurring at an implementation defined rate (e.g. one scroll per vsync).
Later on in Perform a scroll there is the same logic for delaying individual scroll events repeated.
Why do we have the initial wait before dispatching the events? Is that required at all? If not lets remove it and maybe we can live with the lower priority of timers for the rest of the events? We may then still not dispatch the right amount of events, but at least we have 2 of them and not just a single one.
|
Assignee | |
Comment 1•1 month ago
|
||
As discussed the initial delay was most likely expected because at time zero no event should be fired. So we should leave it in.
Instead lets check with requestAnimationFrame if that makes a difference and how its priority is actually set.
|
|
Assignee | |
Comment 2•1 month ago
|
||
Together with the new test from bug 1852529 I pushed a new try build by using requestAnimationFrame instead of the timer:
https://treeherder.mozilla.org/jobs?repo=try&landoInstance=lando-prod-2025&landoCommitID=68585
|
|
Assignee | |
Comment 3•1 month ago
|
||
(In reply to Henrik Skupin [:whimboo][⌚️UTC+2] (away July 24th - August 16th) from comment #2)
Together with the new test from bug 1852529 I pushed a new try build by using
requestAnimationFrameinstead of the timer:
https://treeherder.mozilla.org/jobs?repo=try&landoInstance=lando-prod-2025&landoCommitID=68585
The results look great and we as well dispatch the events at a very constant interval of 17ms:
https://treeherder.mozilla.org/logviewer?job_id=579192426&repo=try&task=EVkNuhl8T9q-hmSUNndU7A.0&lineNumber=13191
Now I only wonder what would happen when the renderer is blocked (see bug 2045643). Could we actually rely on requestAnimationFrame given that normal timers are that unreliably due to downgraded priority, or would it cause issues when the renderer is blocked? Maybe we could fallback to the timer in such a case? Is there a way to detect if the renderer is blocked?
When we could go ahead with requestAnimationFrame I would add the test from bug 1852529 and as well need to extend it to check the final delta values as well. Further we should add the same test for pointer actions.
|
||
Comment 4•1 month ago
|
||
If rendering is blocked, wheel and scroll events also don't fire. So it's unclear to me what you'd like to do? Obviously the frame callbacks won't run.
|
|
Assignee | |
Comment 5•1 month ago
|
||
(In reply to Emilio Cobos Álvarez [:emilio] from comment #4)
If rendering is blocked, wheel and scroll events also don't fire. So it's unclear to me what you'd like to do? Obviously the frame callbacks won't run.
With a duration greater than 0, we split the entire scroll or pointer move action into a sequence of sub-actions, each dispatching an incremental scroll or move event at the best possible interval. Currently, this is implemented using a timer, but the timing is highly unpredictable because of the timer's lower priority (as mentioned in the initial comment). By switching to requestAnimationFrame, we seem to achieve the behavior we actually want, and I'm not aware of another mechanism that provides the same level of consistency.
If the renderer is blocked and a WebDriver client issues a Perform Actions command with a duration greater than 0, we should avoid blocking the WebDriver command itself. If no events would be dispatched anyway, would it make sense to skip dispatching them altogether? If so, is there a reliable way to determine whether the renderer is currently blocked?
|
|
||
Comment 6•1 month ago
|
||
Maybe? Whenever it gets unblocked the callbacks will start firing, which might be more like what you want?
|
|
Assignee | |
Comment 7•18 days ago
|
||
Here a try build. Lets see how it works:
https://treeherder.mozilla.org/jobs?repo=try&landoInstance=lando-prod-2025&landoCommitID=80825
|
|
Assignee | |
Comment 8•18 days ago
|
||
Ok, so there are a couple of failures. One obvious issue is that by using requestAnimationFrame, we're now dependent on the device's refresh rate, which can vary. Our implementation currently assumes a default tick duration of 16 ms, and the test runner makes the same assumption. On a device with a 120 Hz display, we'd end up dispatching roughly twice as many events as on a 60 Hz display.
I don't think that's the behavior we want. Instead, we should throttle event dispatching to maintain the default 60 Hz cadence, regardless of the display's refresh rate.
Here a new try build:
https://treeherder.mozilla.org/jobs?repo=try&landoInstance=lando-prod-2025&landoCommitID=80885
|
|
Assignee | |
Comment 9•18 days ago
|
||
|
||
Updated•18 days ago
|
|
|
Assignee | |
Comment 10•18 days ago
|
||
A pointer move, touch move, or wheel scroll action with a duration greater
than zero has to be split into multiple incremental events, dispatched roughly
once per animation frame, instead of collapsing into a single event.
Add tests for all three input types, for both WebDriver classic and BiDi, that
record the dispatched events and assert that more than one is received.
|
||
Comment 11•12 days ago
|
||
|
||
Comment 12•12 days ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/5a770d835c2d
https://hg.mozilla.org/mozilla-central/rev/9bae90b8c643
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/62203 for changes under testing/web-platform/tests
Upstream PR merged by moz-wptsync-bot
|
|
Assignee | |
Comment 15•11 days ago
|
||
|
|
Assignee | |
Updated•11 days ago
|
|
||
Comment 16•11 days ago
|
||
A patch has been attached on this bug, which was already closed. Filing a separate bug will ensure better tracking. If this was not by mistake and further action is needed, please alert the appropriate party. (Or: if the patch doesn't change behavior -- e.g. landing a test case, or fixing a typo -- then feel free to disregard this message)
|
|
||
Comment 17•11 days ago
|
||
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/62219 for changes under testing/web-platform/tests
|
||
Comment 19•11 days ago
|
||
| bugherder | ||
Upstream PR merged by moz-wptsync-bot
Description
•