From a44f2bb4d6dbbf518664379440dc9f220e3c035b Mon Sep 17 00:00:00 2001 From: Euclid Ye Date: Tue, 10 Mar 2026 11:50:26 +0800 Subject: [PATCH 1/3] Remove loop and fmt Signed-off-by: Euclid Ye --- components/webdriver_server/actions.rs | 130 +++++++++++++------------ 1 file changed, 66 insertions(+), 64 deletions(-) diff --git a/components/webdriver_server/actions.rs b/components/webdriver_server/actions.rs index b6362c39f3301..65b3b13ef6597 100644 --- a/components/webdriver_server/actions.rs +++ b/components/webdriver_server/actions.rs @@ -765,85 +765,87 @@ impl Handler { #[expect(clippy::too_many_arguments)] fn perform_scroll( &mut self, + input_id: &str, duration: u64, x: f64, y: f64, target_delta_x: f64, target_delta_y: f64, - mut curr_delta_x: f64, - mut curr_delta_y: f64, + mut current_delta_x: f64, + mut current_delta_y: f64, tick_start: &Instant, ) { - loop { - // Step 1. Let time delta be the time since the beginning of the current tick, - // measured in milliseconds on a monotonic clock. - let time_delta = tick_start.elapsed().as_millis(); - - // Step 2. Let duration ratio be the ratio of time delta and duration, - // if duration is greater than 0, or 1 otherwise. - let duration_ratio = if duration > 0 { - time_delta as f64 / duration as f64 - } else { - 1.0 - }; + // Step 1. Let time delta be the time since the beginning of the current tick, + // measured in milliseconds on a monotonic clock. + let time_delta = tick_start.elapsed().as_millis(); - // Step 3. If duration ratio is 1, or close enough to 1 that - // the implementation will not further subdivide the move action, - // let last be true. Otherwise let last be false. - let last = 1.0 - duration_ratio < 0.001; - - // Step 4. If last is true, - // let delta x equal target delta x - current delta x and delta y equal target delta y - current delta y. - // Otherwise - // let delta x equal an approximation to duration ratio × target delta x - current delta x, - // and delta y equal an approximation to duration ratio × target delta y - current delta y. - let (delta_x, delta_y) = if last { - (target_delta_x - curr_delta_x, target_delta_y - curr_delta_y) - } else { - ( - duration_ratio * target_delta_x - curr_delta_x, - duration_ratio * target_delta_y - curr_delta_y, - ) - }; + // Step 2. Let duration ratio be the ratio of time delta and duration, + // if duration is greater than 0, or 1 otherwise. + let duration_ratio = if duration > 0 { + time_delta as f64 / duration as f64 + } else { + 1.0 + }; - // Step 5. If delta x != 0 or delta y != 0, run the following steps: - // Actually "last" should not be checked here based on spec. - // However, we need to send the webdriver id at the final perform. - if delta_x != 0.0 || delta_y != 0.0 || last { - // Step 5.1. Perform implementation-specific action dispatch steps - let delta = WheelDelta { - x: -delta_x, - y: -delta_y, - z: 0.0, - mode: WheelMode::DeltaPixel, - }; - let point = WebViewPoint::Page(Point2D::new(x as f32, y as f32)); - let input_event = InputEvent::Wheel(WheelEvent::new(delta, point)); - if last { - self.send_blocking_input_event_to_embedder(input_event); - } else { - self.send_input_event_to_embedder(input_event); - } + // Step 3. If duration ratio is 1, or close enough to 1 that + // the implementation will not further subdivide the move action, + // let last be true. Otherwise let last be false. + let last = 1.0 - duration_ratio < 0.001; - // Step 5.2. Let current delta x property equal delta x + current delta x - // and current delta y property equal delta y + current delta y. - curr_delta_x += delta_x; - curr_delta_y += delta_y; - } + // Step 4. If last is true, + // let delta x equal target delta x - current delta x and delta y equal target delta y - current delta y. + // Otherwise + // let delta x equal an approximation to duration ratio × target delta x - current delta x, + // and delta y equal an approximation to duration ratio × target delta y - current delta y. + let (delta_x, delta_y) = if last { + ( + target_delta_x - current_delta_x, + target_delta_y - current_delta_y, + ) + } else { + ( + duration_ratio * target_delta_x - current_delta_x, + duration_ratio * target_delta_y - current_delta_y, + ) + }; - // Step 6. If last is true, return. + // Step 5. If delta x != 0 or delta y != 0, run the following steps: + // Actually "last" should not be checked here based on spec. + // However, we need to send the webdriver id at the final perform. + if delta_x != 0.0 || delta_y != 0.0 || last { + // Step 5.1. Perform implementation-specific action dispatch steps + let delta = WheelDelta { + x: -delta_x, + y: -delta_y, + z: 0.0, + mode: WheelMode::DeltaPixel, + }; + let point = WebViewPoint::Page(Point2D::new(x as f32, y as f32)); + let input_event = InputEvent::Wheel(WheelEvent::new(delta, point)); if last { - return; + self.send_blocking_input_event_to_embedder(input_event); + } else { + self.send_input_event_to_embedder(input_event); } - // Step 7 - // TODO: The two steps should be done in parallel - // 7.1. Asynchronously wait for an implementation defined amount of time to pass. - thread::sleep(Duration::from_millis(WHEELSCROLL_INTERVAL)); - // 7.2. Perform a scroll with arguments duration, x, y, target delta x, - // target delta y, current delta x, current delta y. - // Notice that this simply repeat what we have done until last is true. + // Step 5.2. Let current delta x property equal delta x + current delta x + // and current delta y property equal delta y + current delta y. + current_delta_x += delta_x; + current_delta_y += delta_y; + } + + // Step 6. If last is true, return. + if last { + return; } + + // Step 7 + // TODO: The two steps should be done in parallel + // 7.1. Asynchronously wait for an implementation defined amount of time to pass. + thread::sleep(Duration::from_millis(MOVESCROLL_INTERVAL)); + // 7.2. Perform a scroll with arguments duration, x, y, target delta x, + // target delta y, current delta x, current delta y. + // Notice that this simply repeat what we have done until last is true. } /// Verify that the given coordinates are within the boundary of the viewport. From 6329e43da0827701e4ba74d06f070528c7cd315e Mon Sep 17 00:00:00 2001 From: Euclid Ye Date: Tue, 10 Mar 2026 12:19:29 +0800 Subject: [PATCH 2/3] event loop Signed-off-by: Euclid Ye --- components/webdriver_server/actions.rs | 145 ++++++++++++++++++------- components/webdriver_server/lib.rs | 11 +- 2 files changed, 113 insertions(+), 43 deletions(-) diff --git a/components/webdriver_server/actions.rs b/components/webdriver_server/actions.rs index 65b3b13ef6597..6b18a9c7badc0 100644 --- a/components/webdriver_server/actions.rs +++ b/components/webdriver_server/actions.rs @@ -28,8 +28,7 @@ use webdriver::error::{ErrorStatus, WebDriverError}; use crate::{Handler, VerifyBrowsingContextIsOpen, WebElement, wait_for_oneshot_response}; /// Interval between wheelScroll and pointerMove increments in ms, based on common vsync -static POINTERMOVE_INTERVAL: u64 = 16; -static WHEELSCROLL_INTERVAL: u64 = 16; +static MOVESCROLL_INTERVAL: u64 = 16; /// /// This is hard-coded as 0 in spec. @@ -66,7 +65,28 @@ pub(crate) enum InputSourceState { Wheel, } -pub(crate) struct PendingPointerMove { +#[expect(private_interfaces)] +pub(crate) enum PendingActions { + Scroll(PendingScroll), + PointerMove(PendingPointerMove), +} + +/// +/// For some unknown reason, unlike Pointer Actions, +/// Wheel Actions in spec has precision of integer. +/// We just use double precision, since our [`WheelEvent`] also uses double precision. +struct PendingScroll { + input_id: String, + duration: u64, + x: f64, + y: f64, + current_delta_x: f64, + current_delta_y: f64, + target_delta_x: f64, + target_delta_y: f64, +} + +struct PendingPointerMove { input_id: String, duration: u64, start_x: f64, @@ -181,7 +201,7 @@ impl Handler { // Step 1.3. Try to dispatch tick actions self.dispatch_tick_actions(tick_actions, tick_duration, &tick_start)?; - self.process_pending_pointer_moves(&tick_start); + self.process_pending_actions(&tick_start); // Step 1.4.1 // There are no pending asynchronous waits arising // from the last invocation of the dispatch tick actions steps. @@ -215,22 +235,52 @@ impl Handler { /// Step 9.1. Asynchronously wait for an implementation defined amount of time to pass. /// Step 9.2. Perform a pointer move with arguments input state, /// duration, start x, start y, target x, target y. - fn process_pending_pointer_moves(&mut self, tick_start: &Instant) { - while !self.pending_pointer_moves.is_empty() { - let moves = std::mem::take(&mut self.pending_pointer_moves); - thread::sleep(Duration::from_millis(POINTERMOVE_INTERVAL)); - for PendingPointerMove { - input_id, - duration, - start_x, - start_y, - target_x, - target_y, - } in moves - { - self.perform_pointer_move( - &input_id, duration, start_x, start_y, target_x, target_y, tick_start, - ); + /// + /// Step 7. Run the following substeps in parallel: + /// Step 7.1. Asynchronously wait for an implementation defined amount of time to pass. + /// Step 7.2. Perform a scroll with arguments duration, x, y, + /// target delta x, target delta y, current delta x, current delta y. + fn process_pending_actions(&mut self, tick_start: &Instant) { + while !self.pending_actions.is_empty() { + let pending_actions = std::mem::take(&mut self.pending_actions); + thread::sleep(Duration::from_millis(MOVESCROLL_INTERVAL)); + for action in pending_actions { + match action { + PendingActions::PointerMove(PendingPointerMove { + input_id, + duration, + start_x, + start_y, + target_x, + target_y, + }) => { + self.perform_pointer_move( + &input_id, duration, start_x, start_y, target_x, target_y, tick_start, + ); + }, + PendingActions::Scroll(PendingScroll { + input_id, + duration, + x, + y, + current_delta_x, + current_delta_y, + target_delta_x, + target_delta_y, + }) => { + self.perform_scroll( + &input_id, + duration, + x, + y, + target_delta_x, + target_delta_y, + current_delta_x, + current_delta_y, + tick_start, + ); + }, + } } } } @@ -540,7 +590,7 @@ impl Handler { // Asynchronously wait means do not block browser to process event loop. // In the context of Servo, it means block the webdriver server thread. if duration > 0 { - thread::sleep(Duration::from_millis(POINTERMOVE_INTERVAL)); + thread::sleep(Duration::from_millis(MOVESCROLL_INTERVAL)); } let (start_x, start_y) = { @@ -669,14 +719,15 @@ impl Handler { // We use [`PendingPointerMove`] to achieve the same effect as asynchronous wait and // parallelism required by spec. // This conveniently unify the wait interval between ticks. - self.pending_pointer_moves.push(PendingPointerMove { - input_id: input_id.to_owned(), - duration, - start_x, - start_y, - target_x, - target_y, - }); + self.pending_actions + .push(PendingActions::PointerMove(PendingPointerMove { + input_id: input_id.to_owned(), + duration, + start_x, + start_y, + target_x, + target_y, + })); } /// @@ -742,11 +793,12 @@ impl Handler { // Step 10. If duration is greater than 0 and inside any implementation-defined bounds, // asynchronously wait for an implementation defined amount of time to pass. if duration > 0 { - thread::sleep(Duration::from_millis(WHEELSCROLL_INTERVAL)); + thread::sleep(Duration::from_millis(MOVESCROLL_INTERVAL)); } // Step 11. Perform a scroll with arguments global key state, duration, x, y, delta x, delta y, 0, 0. self.perform_scroll( + input_id, duration, x, y, @@ -839,13 +891,32 @@ impl Handler { return; } - // Step 7 - // TODO: The two steps should be done in parallel - // 7.1. Asynchronously wait for an implementation defined amount of time to pass. - thread::sleep(Duration::from_millis(MOVESCROLL_INTERVAL)); - // 7.2. Perform a scroll with arguments duration, x, y, target delta x, - // target delta y, current delta x, current delta y. - // Notice that this simply repeat what we have done until last is true. + // Step 7. Run the following substeps in parallel: + // Step 7.1. Asynchronously wait for an implementation defined amount of time to pass. + // Step 7.2. Perform a scroll with arguments duration, x, y, + // target delta x, target delta y, current delta x, current delta y. + // This is done in `fn process_pending_actions`. + + // NOTE: The initial scroll is performed synchronously. + // This ensures determinism in the sequence of the first event + // triggered by each action in the tick. + // Subsequent scrolls (if any) are performed asynchronously. + // This allows events from two scroll actions in the tick to be interspersed. + + // We use [`PendingScroll`] to achieve the same effect as asynchronous wait and + // parallelism required by spec. + // This conveniently unify the wait interval between ticks. + self.pending_actions + .push(PendingActions::Scroll(PendingScroll { + input_id: input_id.to_owned(), + duration, + x, + y, + current_delta_x, + current_delta_y, + target_delta_x, + target_delta_y, + })); } /// Verify that the given coordinates are within the boundary of the viewport. diff --git a/components/webdriver_server/lib.rs b/components/webdriver_server/lib.rs index 185b394bb69aa..c0ccb0692da84 100644 --- a/components/webdriver_server/lib.rs +++ b/components/webdriver_server/lib.rs @@ -73,9 +73,7 @@ use webdriver::response::{ }; use webdriver::server::{self, Session, SessionTeardownKind, WebDriverHandler}; -use crate::actions::{ - ELEMENT_CLICK_BUTTON, InputSourceState, PendingPointerMove, PointerInputState, -}; +use crate::actions::{ELEMENT_CLICK_BUTTON, InputSourceState, PendingActions, PointerInputState}; use crate::session::{PageLoadStrategy, WebDriverSession}; use crate::timeout::{DEFAULT_IMPLICIT_WAIT, DEFAULT_PAGE_LOAD_TIMEOUT, SCREENSHOT_TIMEOUT}; @@ -183,8 +181,9 @@ struct Handler { /// TODO: Once we upgrade crossbeam-channel this can be replaced with a `WaitGroup`. pending_input_event_receivers: Vec>, - /// Moves that are currently in-progress and need to be ticked. - pending_pointer_moves: Vec, + /// Ongoing [`PointerMoveAction`] or [`WheelScrollAction`] that are being incrementally + /// processed across multiple execution cycles within the current tick. + pending_actions: Vec, /// The base set of preferences to treat as default when resetting. default_preferences: Preferences, @@ -468,7 +467,7 @@ impl Handler { event_loop_waker, default_preferences, pending_input_event_receivers: Default::default(), - pending_pointer_moves: Default::default(), + pending_actions: Default::default(), } } From 7c724b1adea40639c7169ac143c9f92b6da1a42e Mon Sep 17 00:00:00 2001 From: Euclid Ye Date: Tue, 10 Mar 2026 19:14:58 +0800 Subject: [PATCH 3/3] Add wdspec interspersed wheel/pointermove event test Signed-off-by: Euclid Ye --- tests/wpt/meta/MANIFEST.json | 2 +- .../tests/classic/perform_actions/perform.py | 30 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/tests/wpt/meta/MANIFEST.json b/tests/wpt/meta/MANIFEST.json index 5b7efc8a16ecd..40a70b1502bbf 100644 --- a/tests/wpt/meta/MANIFEST.json +++ b/tests/wpt/meta/MANIFEST.json @@ -990350,7 +990350,7 @@ ] ], "perform.py": [ - "30333942189c6f01ece57e49e45be9c5a5412e70", + "55aca8fd84ae0ef80f65245bbeade0cd462cf4c6", [ null, {} diff --git a/tests/wpt/tests/webdriver/tests/classic/perform_actions/perform.py b/tests/wpt/tests/webdriver/tests/classic/perform_actions/perform.py index 30333942189c6..55aca8fd84ae0 100644 --- a/tests/wpt/tests/webdriver/tests/classic/perform_actions/perform.py +++ b/tests/wpt/tests/webdriver/tests/classic/perform_actions/perform.py @@ -53,3 +53,33 @@ def test_input_source_action_sequence_pointer_parameters_not_processed( ] response = perform_actions(session, actions) assert_success(response) + + +def test_interspersed_wheel_pointermove(session, wheel_chain, mouse_chain, inline): + session.url = inline(""" +
+
+
+ + """) + + target = session.find.css("#target", all=False) + + wheel_chain.scroll(0, 0, 0, 150, duration=500, origin=target) + mouse_chain.pointer_move(80, 80, duration=500, origin=target) + + session.actions.perform([wheel_chain.dict, mouse_chain.dict]) + + events = session.execute_script("return window.events") + + assert "wheel" in events + assert "move" in events + + first_move = events.index("move") + last_wheel = len(events) - 1 - events[::-1].index("wheel") + + assert first_move < last_wheel, f"Events were not interspersed: {events}"