From 15eadb56a4383c397f86893be8ba568934fb2901 Mon Sep 17 00:00:00 2001 From: Kenzie Raditya Tirtarahardja Date: Wed, 11 Jun 2025 17:03:08 +0800 Subject: [PATCH] [webdriver] Add synchronization for wheel action (#37260) Implement action synchronization for wheel event. Previously only done for pointer here https://github.com/servo/servo/pull/36932. Testing: `tests/wpt/meta/webdriver/tests/classic/perform_actions/wheel.py` --------- Signed-off-by: PotatoCP --- components/compositing/compositor.rs | 19 +++++++--- components/constellation/constellation.rs | 14 ++++++- components/shared/compositing/lib.rs | 6 +-- components/shared/embedder/input_events.rs | 17 ++++++++- components/shared/embedder/webdriver.rs | 19 ++++++++-- components/webdriver_server/actions.rs | 37 ++++++++++++++----- ports/servoshell/desktop/headed_window.rs | 2 +- .../classic/perform_actions/wheel.py.ini | 9 ----- 8 files changed, 89 insertions(+), 34 deletions(-) diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs index a4afeb01b3c..09e6ac0b317 100644 --- a/components/compositing/compositor.rs +++ b/components/compositing/compositor.rs @@ -673,7 +673,7 @@ impl IOCompositor { let point = dppx.transform_point(Point2D::new(x, y)); webview_renderer.dispatch_point_input_event( InputEvent::MouseButton(MouseButtonEvent::new(action, button, point)) - .with_webdriver_message_id(Some(message_id)), + .with_webdriver_message_id(message_id), ); }, @@ -686,11 +686,18 @@ impl IOCompositor { let point = dppx.transform_point(Point2D::new(x, y)); webview_renderer.dispatch_point_input_event( InputEvent::MouseMove(MouseMoveEvent::new(point)) - .with_webdriver_message_id(Some(message_id)), + .with_webdriver_message_id(message_id), ); }, - CompositorMsg::WebDriverWheelScrollEvent(webview_id, x, y, delta_x, delta_y) => { + CompositorMsg::WebDriverWheelScrollEvent( + webview_id, + x, + y, + delta_x, + delta_y, + message_id, + ) => { let Some(webview_renderer) = self.webview_renderers.get_mut(webview_id) else { warn!("Handling input event for unknown webview: {webview_id}"); return; @@ -705,8 +712,10 @@ impl IOCompositor { let point = dppx.transform_point(Point2D::new(x, y)); let scroll_delta = dppx.transform_vector(Vector2D::new(delta_x as f32, delta_y as f32)); - webview_renderer - .dispatch_point_input_event(InputEvent::Wheel(WheelEvent { delta, point })); + webview_renderer.dispatch_point_input_event( + InputEvent::Wheel(WheelEvent::new(delta, point)) + .with_webdriver_message_id(message_id), + ); webview_renderer.on_webdriver_wheel_action(scroll_delta, point); }, diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index a583a75d1d9..1d0d8150f00 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -4899,10 +4899,20 @@ where webview_id, x, y, msg_id, )); }, - WebDriverCommandMsg::WheelScrollAction(webview, x, y, delta_x, delta_y) => { + WebDriverCommandMsg::WheelScrollAction( + webview_id, + x, + y, + delta_x, + delta_y, + msg_id, + response_sender, + ) => { + self.webdriver.input_command_response_sender = Some(response_sender); + self.compositor_proxy .send(CompositorMsg::WebDriverWheelScrollEvent( - webview, x, y, delta_x, delta_y, + webview_id, x, y, delta_x, delta_y, msg_id, )); }, WebDriverCommandMsg::TakeScreenshot(webview_id, rect, response_sender) => { diff --git a/components/shared/compositing/lib.rs b/components/shared/compositing/lib.rs index 8dadf93d628..c2acb83f240 100644 --- a/components/shared/compositing/lib.rs +++ b/components/shared/compositing/lib.rs @@ -111,12 +111,12 @@ pub enum CompositorMsg { MouseButton, f32, f32, - WebDriverMessageId, + Option, ), /// WebDriver mouse move event - WebDriverMouseMoveEvent(WebViewId, f32, f32, WebDriverMessageId), + WebDriverMouseMoveEvent(WebViewId, f32, f32, Option), // Webdriver wheel scroll event - WebDriverWheelScrollEvent(WebViewId, f32, f32, f64, f64), + WebDriverWheelScrollEvent(WebViewId, f32, f32, f64, f64, Option), /// Inform WebRender of the existence of this pipeline. SendInitialTransaction(WebRenderPipelineId), diff --git a/components/shared/embedder/input_events.rs b/components/shared/embedder/input_events.rs index ff48bd7b7d8..d5c0c7bc46a 100644 --- a/components/shared/embedder/input_events.rs +++ b/components/shared/embedder/input_events.rs @@ -54,7 +54,7 @@ impl InputEvent { InputEvent::MouseButton(event) => event.webdriver_id, InputEvent::MouseMove(event) => event.webdriver_id, InputEvent::Touch(..) => None, - InputEvent::Wheel(..) => None, + InputEvent::Wheel(event) => event.webdriver_id, } } @@ -71,7 +71,9 @@ impl InputEvent { event.webdriver_id = webdriver_id; }, InputEvent::Touch(..) => {}, - InputEvent::Wheel(..) => {}, + InputEvent::Wheel(ref mut event) => { + event.webdriver_id = webdriver_id; + }, }; self @@ -275,6 +277,17 @@ pub struct WheelDelta { pub struct WheelEvent { pub delta: WheelDelta, pub point: DevicePoint, + webdriver_id: Option, +} + +impl WheelEvent { + pub fn new(delta: WheelDelta, point: DevicePoint) -> Self { + WheelEvent { + delta, + point, + webdriver_id: None, + } + } } #[derive(Clone, Debug, Deserialize, Serialize)] diff --git a/components/shared/embedder/webdriver.rs b/components/shared/embedder/webdriver.rs index f4f9cd5d509..4333fa9fe2b 100644 --- a/components/shared/embedder/webdriver.rs +++ b/components/shared/embedder/webdriver.rs @@ -50,7 +50,8 @@ pub enum WebDriverCommandMsg { MouseButton, f32, f32, - WebDriverMessageId, + // Should never be None. + Option, IpcSender, ), /// Act as if the mouse was moved in the browsing context with the given ID. @@ -58,11 +59,23 @@ pub enum WebDriverCommandMsg { WebViewId, f32, f32, - WebDriverMessageId, + // None if it's not the last `perform_pointer_move` since we only + // expect one response from constellation for each tick actions. + Option, IpcSender, ), /// Act as if the mouse wheel is scrolled in the browsing context given the given ID. - WheelScrollAction(WebViewId, f32, f32, f64, f64), + WheelScrollAction( + WebViewId, + f32, + f32, + f64, + f64, + // None if it's not the last `perform_wheel_scroll` since we only + // expect one response from constellation for each tick actions. + Option, + IpcSender, + ), /// Set the window size. SetWindowSize(WebViewId, DeviceIntSize, IpcSender>), /// Take a screenshot of the window. diff --git a/components/webdriver_server/actions.rs b/components/webdriver_server/actions.rs index b84cbb4c2f8..774ed41e3cb 100644 --- a/components/webdriver_server/actions.rs +++ b/components/webdriver_server/actions.rs @@ -156,12 +156,16 @@ impl Handler { &self, tick_actions: &TickActions, ) -> Result<(), ErrorStatus> { - // TODO: Add matches! for wheel and key actions - // after implmenting webdriver id for wheel and key events. + // TODO: Add matches! for key actions + // after implmenting webdriver id for key events. let count_non_null_actions_in_tick = tick_actions .iter() .filter(|(_, action)| { - matches!(action, ActionItem::Pointer(PointerActionItem::Pointer(_))) + matches!( + action, + ActionItem::Pointer(PointerActionItem::Pointer(_)) | + ActionItem::Wheel(WheelActionItem::Wheel(_)) + ) }) .count(); @@ -176,7 +180,7 @@ impl Handler { let current_waiting_id = self .current_action_id .get() - .expect("Current id should be set before dispat_actions_inner is called"); + .expect("Current id should be set before dispatch_actions_inner is called"); if current_waiting_id != response.id { dbg!("Dispatch actions completed with wrong id in response"); @@ -345,7 +349,7 @@ impl Handler { } pointer_input_state.pressed.insert(action.button); - let msg_id = self.current_action_id.get().unwrap(); + let msg_id = self.current_action_id.get(); let cmd_msg = WebDriverCommandMsg::MouseButtonAction( session.webview_id, MouseButtonAction::Down, @@ -375,7 +379,7 @@ impl Handler { } pointer_input_state.pressed.remove(&action.button); - let msg_id = self.current_action_id.get().unwrap(); + let msg_id = self.current_action_id.get(); let cmd_msg = WebDriverCommandMsg::MouseButtonAction( session.webview_id, MouseButtonAction::Up, @@ -496,9 +500,15 @@ impl Handler { let current_y = pointer_input_state.y; // Step 7 - if x != current_x || y != current_y { + // Actually "last" should not be checked here based on spec. + // However, we need to send the webdriver id at the final perform. + if x != current_x || y != current_y || last { // Step 7.2 - let msg_id = self.current_action_id.get().unwrap(); + let msg_id = if last { + self.current_action_id.get() + } else { + None + }; let cmd_msg = WebDriverCommandMsg::MouseMoveAction( session.webview_id, x as f32, @@ -627,14 +637,23 @@ impl Handler { }; // Step 5 - if delta_x != 0 || delta_y != 0 { + // 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 || delta_y != 0 || last { // Perform implementation-specific action dispatch steps + let msg_id = if last { + self.current_action_id.get() + } else { + None + }; let cmd_msg = WebDriverCommandMsg::WheelScrollAction( session.webview_id, x as f32, y as f32, delta_x as f64, delta_y as f64, + msg_id, + self.constellation_sender.clone(), ); self.constellation_chan .send(EmbedderToConstellationMessage::WebDriverCommand(cmd_msg)) diff --git a/ports/servoshell/desktop/headed_window.rs b/ports/servoshell/desktop/headed_window.rs index a1c7576a8c9..93bcc86acb5 100644 --- a/ports/servoshell/desktop/headed_window.rs +++ b/ports/servoshell/desktop/headed_window.rs @@ -603,7 +603,7 @@ impl WindowPortsMethods for Window { let phase = winit_phase_to_touch_event_type(phase); // Send events - webview.notify_input_event(InputEvent::Wheel(WheelEvent { delta, point })); + webview.notify_input_event(InputEvent::Wheel(WheelEvent::new(delta, point))); webview.notify_scroll_event( scroll_location, self.webview_relative_mouse_point.get().to_i32(), diff --git a/tests/wpt/meta/webdriver/tests/classic/perform_actions/wheel.py.ini b/tests/wpt/meta/webdriver/tests/classic/perform_actions/wheel.py.ini index 0eec2e693d6..4d4fe99691e 100644 --- a/tests/wpt/meta/webdriver/tests/classic/perform_actions/wheel.py.ini +++ b/tests/wpt/meta/webdriver/tests/classic/perform_actions/wheel.py.ini @@ -1,13 +1,4 @@ [wheel.py] - [test_scroll_not_scrollable] - expected: FAIL - - [test_scroll_scrollable_overflow] - expected: FAIL - - [test_scroll_iframe] - expected: FAIL - [test_scroll_shadow_tree[outer-open\]] expected: FAIL