From d0100797e871e3267383a489ce064c8c7373c8f4 Mon Sep 17 00:00:00 2001 From: batu_hoang <55729155+longvatrong111@users.noreply.github.com> Date: Thu, 19 Jun 2025 17:52:01 +0800 Subject: [PATCH] [webdriver] Move Webdriver to ServoShell (#36714) Moving `webdriver` to `servoshell`: - Let `servoshell` manage lifecycle of `webdriver` - One by one, move the handling of webdriver commands from `constellation` to `embedder` Partially fix: https://github.com/servo/servo/issues/37370 Partially fix webdriver test timeout with `no_top_browsing_context` cc: @xiaochengh --------- Signed-off-by: batu_hoang --- Cargo.lock | 3 + components/constellation/constellation.rs | 9 ++- components/constellation/tracing.rs | 1 + components/servo/lib.rs | 19 ++++--- components/shared/embedder/lib.rs | 1 + components/webdriver_server/lib.rs | 66 ++++++++++++++++------ ports/servoshell/Cargo.toml | 5 +- ports/servoshell/desktop/app.rs | 67 ++++++++++++++++++++++- ports/servoshell/desktop/app_state.rs | 20 ++++++- 9 files changed, 156 insertions(+), 35 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a114365378d..616031a9f30 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7191,6 +7191,8 @@ dependencies = [ "backtrace", "cc", "cfg-if", + "constellation_traits", + "crossbeam-channel", "dirs", "dpi", "egui", @@ -7237,6 +7239,7 @@ dependencies = [ "tracing-perfetto", "tracing-subscriber", "url", + "webdriver_server", "windows-sys 0.59.0", "winit", "winres", diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index d320e5c958d..6a07bbcb60e 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -4536,8 +4536,8 @@ where // Find the script channel for the given parent pipeline, // and pass the event to that script thread. match msg { - WebDriverCommandMsg::CloseWebView(webview_id) => { - self.handle_close_top_level_browsing_context(webview_id); + WebDriverCommandMsg::CloseWebView(..) => { + unreachable!("This command should be send directly to the embedder."); }, WebDriverCommandMsg::NewWebView( originating_webview_id, @@ -4575,9 +4575,8 @@ where WebDriverCommandMsg::FocusWebView(webview_id) => { self.handle_focus_web_view(webview_id); }, - WebDriverCommandMsg::IsWebViewOpen(webview_id, response_sender) => { - let is_open = self.webviews.get(webview_id).is_some(); - let _ = response_sender.send(is_open); + WebDriverCommandMsg::IsWebViewOpen(..) => { + unreachable!("This command should be send directly to the embedder."); }, WebDriverCommandMsg::IsBrowsingContextOpen(browsing_context_id, response_sender) => { let is_open = self.browsing_contexts.contains_key(&browsing_context_id); diff --git a/components/constellation/tracing.rs b/components/constellation/tracing.rs index c97b8722b6e..16027a23cec 100644 --- a/components/constellation/tracing.rs +++ b/components/constellation/tracing.rs @@ -247,6 +247,7 @@ mod from_script { Self::FinishJavaScriptEvaluation(..) => { target_variant!("FinishJavaScriptEvaluation") }, + Self::WebDriverCommand(..) => target_variant!("WebDriverCommand"), } } } diff --git a/components/servo/lib.rs b/components/servo/lib.rs index 846f240eabe..ce552eda963 100644 --- a/components/servo/lib.rs +++ b/components/servo/lib.rs @@ -138,9 +138,6 @@ fn webdriver(port: u16, constellation: Sender) { webdriver_server::start_server(port, constellation); } -#[cfg(not(feature = "webdriver"))] -fn webdriver(_port: u16, _constellation: Sender) {} - #[cfg(feature = "media-gstreamer")] mod media_platform { #[cfg(any(windows, target_os = "macos"))] @@ -468,12 +465,6 @@ impl Servo { builder.user_content_manager, ); - if cfg!(feature = "webdriver") { - if let Some(port) = opts.webdriver_port { - webdriver(port, constellation_chan.clone()); - } - } - // The compositor coordinates with the client window to create the final // rendered page and display it somewhere. let shutdown_state = Rc::new(Cell::new(ShutdownState::NotShuttingDown)); @@ -1008,8 +999,18 @@ impl Servo { webview.delegate().show_form_control(webview, form_control); } }, + _ => {}, } } + + pub fn constellation_sender(&self) -> Sender { + self.constellation_proxy.sender() + } + + pub fn execute_webdriver_command(&self, command: WebDriverCommandMsg) { + self.constellation_proxy + .send(EmbedderToConstellationMessage::WebDriverCommand(command)); + } } fn create_embedder_channel( diff --git a/components/shared/embedder/lib.rs b/components/shared/embedder/lib.rs index 4485ccb9006..adbb2237e23 100644 --- a/components/shared/embedder/lib.rs +++ b/components/shared/embedder/lib.rs @@ -372,6 +372,7 @@ pub enum EmbedderMsg { JavaScriptEvaluationId, Result, ), + WebDriverCommand(WebDriverCommandMsg), } impl Debug for EmbedderMsg { diff --git a/components/webdriver_server/lib.rs b/components/webdriver_server/lib.rs index 1512bdb4305..168a5e4ac1f 100644 --- a/components/webdriver_server/lib.rs +++ b/components/webdriver_server/lib.rs @@ -24,8 +24,8 @@ use constellation_traits::{EmbedderToConstellationMessage, TraversalDirection}; use cookie::{CookieBuilder, Expiration}; use crossbeam_channel::{Receiver, Sender, after, select, unbounded}; use embedder_traits::{ - MouseButton, WebDriverCommandMsg, WebDriverCommandResponse, WebDriverFrameId, WebDriverJSError, - WebDriverJSResult, WebDriverJSValue, WebDriverLoadStatus, WebDriverMessageId, + EventLoopWaker, MouseButton, WebDriverCommandMsg, WebDriverCommandResponse, WebDriverFrameId, + WebDriverJSError, WebDriverJSResult, WebDriverJSValue, WebDriverLoadStatus, WebDriverMessageId, WebDriverScriptCommand, }; use euclid::{Rect, Size2D}; @@ -122,8 +122,17 @@ fn cookie_msg_to_cookie(cookie: cookie::Cookie) -> Cookie { } } -pub fn start_server(port: u16, constellation_chan: Sender) { - let handler = Handler::new(constellation_chan); +pub fn start_server( + port: u16, + constellation_chan_deprecated: Sender, + embedder_sender: Sender, + event_loop_waker: Box, +) { + let handler = Handler::new( + constellation_chan_deprecated, + embedder_sender, + event_loop_waker, + ); thread::Builder::new() .name("WebDriverHttpServer".to_owned()) .spawn(move || { @@ -223,10 +232,20 @@ struct Handler { session: Option, + /// A [`Sender`] that sends messages to the embedder that this `WebDriver instance controls. + /// In addition to sending a message, we must always wake up the embedder's event loop so it + /// knows that more messages are available for processing. + embedder_sender: Sender, + + /// An [`EventLoopWaker`] which is used to wake up the embedder event loop. + event_loop_waker: Box, + /// The channel for sending Webdriver messages to the constellation. + /// TODO: change name to constellation_sender constellation_chan: Sender, /// The IPC sender which we can clone and pass along to the constellation + /// TODO: change name to webdriver_response_sender constellation_sender: IpcSender, /// Receiver notification from the constellation when a command is completed @@ -450,7 +469,11 @@ impl<'de> Visitor<'de> for TupleVecMapVisitor { } impl Handler { - pub fn new(constellation_chan: Sender) -> Handler { + pub fn new( + constellation_chan: Sender, + embedder_sender: Sender, + event_loop_waker: Box, + ) -> Handler { // Create a pair of both an IPC and a threaded channel, // keep the IPC sender to clone and pass to the constellation for each load, // and keep a threaded receiver to block on an incoming load-status. @@ -466,6 +489,8 @@ impl Handler { load_status_sender, load_status_receiver, session: None, + embedder_sender, + event_loop_waker, constellation_chan, constellation_sender, constellation_receiver, @@ -482,6 +507,17 @@ impl Handler { .set(self.num_pending_actions.get() + 1); } + fn send_message_to_embedder(&self, msg: WebDriverCommandMsg) -> WebDriverResult<()> { + self.embedder_sender.send(msg).map_err(|_| { + WebDriverError::new( + ErrorStatus::UnknownError, + "Failed to send message to embedder", + ) + })?; + self.event_loop_waker.wake(); + Ok(()) + } + fn focus_webview_id(&self) -> WebDriverResult { debug!("Getting focused context."); let (sender, receiver) = ipc::channel().unwrap(); @@ -1001,9 +1037,7 @@ impl Handler { // Step 3. Close session's current top-level browsing context. session.window_handles.remove(&webview_id); let cmd_msg = WebDriverCommandMsg::CloseWebView(session.webview_id); - self.constellation_chan - .send(EmbedderToConstellationMessage::WebDriverCommand(cmd_msg)) - .unwrap(); + self.send_message_to_embedder(cmd_msg)?; let window_handles: Vec = self .session() .unwrap() @@ -1015,6 +1049,7 @@ impl Handler { if window_handles.is_empty() { self.session = None; } + // Step 5. Return the result of running the remote end steps for the Get Window Handles command Ok(WebDriverResponse::CloseWindow(CloseWindowResponse( window_handles, @@ -2052,11 +2087,7 @@ impl Handler { webview_id: WebViewId, ) -> Result<(), WebDriverError> { let (sender, receiver) = ipc::channel().unwrap(); - self.constellation_chan - .send(EmbedderToConstellationMessage::WebDriverCommand( - WebDriverCommandMsg::IsWebViewOpen(webview_id, sender), - )) - .unwrap(); + self.send_message_to_embedder(WebDriverCommandMsg::IsWebViewOpen(webview_id, sender))?; if !receiver.recv().unwrap_or(false) { Err(WebDriverError::new( ErrorStatus::NoSuchWindow, @@ -2072,11 +2103,10 @@ impl Handler { browsing_context_id: BrowsingContextId, ) -> Result<(), WebDriverError> { let (sender, receiver) = ipc::channel().unwrap(); - self.constellation_chan - .send(EmbedderToConstellationMessage::WebDriverCommand( - WebDriverCommandMsg::IsBrowsingContextOpen(browsing_context_id, sender), - )) - .unwrap(); + self.send_message_to_embedder(WebDriverCommandMsg::IsBrowsingContextOpen( + browsing_context_id, + sender, + ))?; if !receiver.recv().unwrap_or(false) { Err(WebDriverError::new( ErrorStatus::NoSuchWindow, diff --git a/ports/servoshell/Cargo.toml b/ports/servoshell/Cargo.toml index 885d6e958d9..0f6afd7722a 100644 --- a/ports/servoshell/Cargo.toml +++ b/ports/servoshell/Cargo.toml @@ -37,7 +37,7 @@ ProductName = "Servo" [features] crown = ["libservo/crown"] debugmozjs = ["libservo/debugmozjs"] -default = ["max_log_level", "webdriver", "webgpu", "webxr"] +default = ["max_log_level", "webgpu", "webxr"] jitspew = ["libservo/jitspew"] js_backtrace = ["libservo/js_backtrace"] max_log_level = ["log/release_max_level_info"] @@ -55,6 +55,8 @@ webxr = ["libservo/webxr"] [dependencies] cfg-if = { workspace = true } +constellation_traits = { workspace = true } +crossbeam-channel = { workspace = true } dpi = { workspace = true } euclid = { workspace = true } getopts = { workspace = true } @@ -72,6 +74,7 @@ tracing = { workspace = true, optional = true } tracing-perfetto = { workspace = true, optional = true } tracing-subscriber = { workspace = true, optional = true, features = ["env-filter"] } url = { workspace = true } +webdriver_server = { path = "../../components/webdriver_server" } [target.'cfg(target_os = "android")'.dependencies] android_logger = "0.15" diff --git a/ports/servoshell/desktop/app.rs b/ports/servoshell/desktop/app.rs index 4f3ce2266c5..95407a09f1e 100644 --- a/ports/servoshell/desktop/app.rs +++ b/ports/servoshell/desktop/app.rs @@ -12,13 +12,14 @@ use std::time::Instant; use std::{env, fs}; use ::servo::ServoBuilder; +use crossbeam_channel::unbounded; use log::{info, trace, warn}; use net::protocols::ProtocolRegistry; -use servo::EventLoopWaker; use servo::config::opts::Opts; use servo::config::prefs::Preferences; use servo::servo_url::ServoUrl; use servo::user_content_manager::{UserContentManager, UserScript}; +use servo::{EventLoopWaker, WebDriverCommandMsg}; use url::Url; use winit::application::ApplicationHandler; use winit::event::WindowEvent; @@ -154,10 +155,28 @@ impl App { let servo = servo_builder.build(); servo.setup_logging(); + // Initialize WebDriver server here before `servo` is moved. + let webdriver_receiver = self.opts.webdriver_port.map(|port| { + let (embedder_sender, embedder_receiver) = unbounded(); + + // TODO: WebDriver will no longer need this channel once all WebDriver + // commands are executed via the Servo API. + let constellation_sender_deprecated = servo.constellation_sender(); + webdriver_server::start_server( + port, + constellation_sender_deprecated, + embedder_sender, + self.waker.clone(), + ); + + embedder_receiver + }); + let running_state = Rc::new(RunningAppState::new( servo, window.clone(), self.servoshell_preferences.clone(), + webdriver_receiver, )); running_state.new_toplevel_webview(self.initial_url.clone().into_url()); @@ -296,6 +315,49 @@ impl App { } } } + + fn handle_webdriver_messages(&mut self) { + let AppState::Running(running_state) = &self.state else { + return; + }; + + let Some(webdriver_receiver) = running_state.webdriver_receiver() else { + return; + }; + + while let Ok(msg) = webdriver_receiver.try_recv() { + match msg { + WebDriverCommandMsg::IsWebViewOpen(webview_id, sender) => { + let context = running_state.webview_by_id(webview_id); + sender.send(context.is_some()).unwrap(); + }, + webdriver_msg @ WebDriverCommandMsg::IsBrowsingContextOpen(..) => { + running_state.forward_webdriver_command(webdriver_msg); + }, + WebDriverCommandMsg::CloseWebView(webview_id) => { + running_state.close_webview(webview_id); + }, + WebDriverCommandMsg::GetWindowSize(..) | + WebDriverCommandMsg::FocusWebView(..) | + WebDriverCommandMsg::LoadUrl(..) | + WebDriverCommandMsg::ScriptCommand(..) | + WebDriverCommandMsg::SendKeys(..) | + WebDriverCommandMsg::KeyboardAction(..) | + WebDriverCommandMsg::MouseButtonAction(..) | + WebDriverCommandMsg::MouseMoveAction(..) | + WebDriverCommandMsg::WheelScrollAction(..) | + WebDriverCommandMsg::SetWindowSize(..) | + WebDriverCommandMsg::TakeScreenshot(..) | + WebDriverCommandMsg::NewWebView(..) | + WebDriverCommandMsg::Refresh(..) => { + warn!( + "WebDriverCommand {:?} is still not moved from constellation to embedder", + msg + ); + }, + }; + } + } } impl ApplicationHandler for App { @@ -435,6 +497,9 @@ impl ApplicationHandler for App { // Consume and handle any events from the Minibrowser. self.handle_servoshell_ui_events(); + // Consume and handle any events from the WebDriver. + self.handle_webdriver_messages(); + self.handle_events_with_winit(event_loop, window); } diff --git a/ports/servoshell/desktop/app_state.rs b/ports/servoshell/desktop/app_state.rs index b62f00388f9..6e3813733cb 100644 --- a/ports/servoshell/desktop/app_state.rs +++ b/ports/servoshell/desktop/app_state.rs @@ -7,6 +7,7 @@ use std::collections::HashMap; use std::path::PathBuf; use std::rc::Rc; +use crossbeam_channel::Receiver; use euclid::Vector2D; use keyboard_types::{Key, Modifiers, ShortcutMatcher}; use log::{error, info}; @@ -18,7 +19,7 @@ use servo::webrender_api::units::{DeviceIntPoint, DeviceIntSize}; use servo::{ AllowOrDenyRequest, AuthenticationRequest, FilterPattern, FormControl, GamepadHapticEffectType, KeyboardEvent, LoadStatus, PermissionRequest, Servo, ServoDelegate, ServoError, SimpleDialog, - TouchEventType, WebView, WebViewBuilder, WebViewDelegate, + TouchEventType, WebDriverCommandMsg, WebView, WebViewBuilder, WebViewDelegate, }; use url::Url; @@ -44,6 +45,9 @@ pub(crate) struct RunningAppState { /// The preferences for this run of servoshell. This is not mutable, so doesn't need to /// be stored inside the [`RunningAppStateInner`]. servoshell_preferences: ServoShellPreferences, + /// A [`Receiver`] for receiving commands from a running WebDriver server, if WebDriver + /// was enabled. + webdriver_receiver: Option>, inner: RefCell, } @@ -88,11 +92,13 @@ impl RunningAppState { servo: Servo, window: Rc, servoshell_preferences: ServoShellPreferences, + webdriver_receiver: Option>, ) -> RunningAppState { servo.set_delegate(Rc::new(ServoShellServoDelegate)); RunningAppState { servo, servoshell_preferences, + webdriver_receiver, inner: RefCell::new(RunningAppStateInner { webviews: HashMap::default(), creation_order: Default::default(), @@ -132,6 +138,14 @@ impl RunningAppState { &self.servo } + pub(crate) fn webdriver_receiver(&self) -> Option<&Receiver> { + self.webdriver_receiver.as_ref() + } + + pub(crate) fn forward_webdriver_command(&self, command: WebDriverCommandMsg) { + self.servo().execute_webdriver_command(command); + } + pub(crate) fn hidpi_scale_factor_changed(&self) { let inner = self.inner(); let new_scale_factor = inner.window.hidpi_scale_factor(); @@ -261,6 +275,10 @@ impl RunningAppState { .collect() } + pub fn webview_by_id(&self, id: WebViewId) -> Option { + self.inner().webviews.get(&id).cloned() + } + pub fn handle_gamepad_events(&self) { let Some(active_webview) = self.focused_webview() else { return;