From ebf8a35c84b0c8996cf3d659dd6305430b1b7df7 Mon Sep 17 00:00:00 2001 From: Jonathan Schwender <55576758+jschwe@users.noreply.github.com> Date: Mon, 25 Aug 2025 20:11:03 +0200 Subject: [PATCH] webdriver: Port WebDriverLoadStatus to Generic Channel (#38915) Ports the channel for WebDriverLoadStatus to GenericChannel. Testing: No functional changes - Covered by existing webdriver tests Part of #38912 Signed-off-by: Jonathan Schwender --- components/constellation/constellation.rs | 2 +- components/script/dom/window.rs | 4 ++-- components/script/webdriver_handlers.rs | 3 ++- components/shared/embedder/webdriver.rs | 20 +++++++++------- components/webdriver_server/lib.rs | 29 +++++++++++++---------- ports/servoshell/desktop/app_state.rs | 5 ++-- 6 files changed, 37 insertions(+), 26 deletions(-) diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index 8ec22e25a55..b4bb8de3a4b 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -418,7 +418,7 @@ pub struct Constellation { next_pipeline_namespace_id: PipelineNamespaceId, /// An [`IpcSender`] to notify navigation events to webdriver. - webdriver_load_status_sender: Option<(IpcSender, PipelineId)>, + webdriver_load_status_sender: Option<(GenericSender, PipelineId)>, /// An [`IpcSender`] to forward responses from the `ScriptThread` to the WebDriver server. webdriver_input_command_reponse_sender: Option>, diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index db82b4ec0d4..baa67ead879 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -326,7 +326,7 @@ pub(crate) struct Window { /// A channel to notify webdriver if there is a navigation #[no_trace] - webdriver_load_status_sender: RefCell>>, + webdriver_load_status_sender: RefCell>>, /// The current state of the window object current_state: Cell, @@ -2863,7 +2863,7 @@ impl Window { pub(crate) fn set_webdriver_load_status_sender( &self, - sender: Option>, + sender: Option>, ) { *self.webdriver_load_status_sender.borrow_mut() = sender; } diff --git a/components/script/webdriver_handlers.rs b/components/script/webdriver_handlers.rs index b8e85eec675..55dac357dc3 100644 --- a/components/script/webdriver_handlers.rs +++ b/components/script/webdriver_handlers.rs @@ -6,6 +6,7 @@ use std::collections::{HashMap, HashSet}; use std::ffi::CString; use std::ptr::NonNull; +use base::generic_channel::GenericSender; use base::id::{BrowsingContextId, PipelineId}; use cookie::Cookie; use embedder_traits::{ @@ -2066,7 +2067,7 @@ pub(crate) fn handle_is_selected( pub(crate) fn handle_add_load_status_sender( documents: &DocumentCollection, pipeline: PipelineId, - reply: IpcSender, + reply: GenericSender, ) { if let Some(document) = documents.find_document(pipeline) { let window = document.window(); diff --git a/components/shared/embedder/webdriver.rs b/components/shared/embedder/webdriver.rs index 13c426ac4f4..f40af46d039 100644 --- a/components/shared/embedder/webdriver.rs +++ b/components/shared/embedder/webdriver.rs @@ -6,6 +6,7 @@ use std::collections::HashMap; +use base::generic_channel::GenericSender; use base::id::{BrowsingContextId, WebViewId}; use cookie::Cookie; use euclid::default::Rect as UntypedRect; @@ -81,13 +82,13 @@ pub enum WebDriverCommandMsg { /// Get the viewport size. GetViewportSize(WebViewId, IpcSender>), /// Load a URL in the top-level browsing context with the given ID. - LoadUrl(WebViewId, ServoUrl, IpcSender), + LoadUrl(WebViewId, ServoUrl, GenericSender), /// Refresh the top-level browsing context with the given ID. - Refresh(WebViewId, IpcSender), + Refresh(WebViewId, GenericSender), /// Navigate the webview with the given ID to the previous page in the browsing context's history. - GoBack(WebViewId, IpcSender), + GoBack(WebViewId, GenericSender), /// Navigate the webview with the given ID to the next page in the browsing context's history. - GoForward(WebViewId, IpcSender), + GoForward(WebViewId, GenericSender), /// Pass a webdriver command to the script thread of the current pipeline /// of a browsing context. ScriptCommand(BrowsingContextId, WebDriverScriptCommand), @@ -147,7 +148,10 @@ pub enum WebDriverCommandMsg { /// Create a new webview that loads about:blank. The embedder will use /// the provided channels to return the top level browsing context id /// associated with the new webview, and sets a "load status sender" if provided. - NewWebView(IpcSender, Option>), + NewWebView( + IpcSender, + Option>, + ), /// Close the webview associated with the provided id. CloseWebView(WebViewId, IpcSender<()>), /// Focus the webview associated with the provided id. @@ -243,7 +247,7 @@ pub enum WebDriverScriptCommand { GetTitle(IpcSender), /// Deal with the case of input element for Element Send Keys, which does not send keys. WillSendKeys(String, String, bool, IpcSender>), - AddLoadStatusSender(WebViewId, IpcSender), + AddLoadStatusSender(WebViewId, GenericSender), RemoveLoadStatusSender(WebViewId), } @@ -289,8 +293,8 @@ pub enum WebDriverLoadStatus { /// to a WebDriver server with information about application state. #[derive(Clone, Default)] pub struct WebDriverSenders { - pub load_status_senders: HashMap>, + pub load_status_senders: HashMap>, pub script_evaluation_interrupt_sender: Option>, - pub pending_traversals: HashMap>, + pub pending_traversals: HashMap>, pub pending_focus: HashMap>, } diff --git a/components/webdriver_server/lib.rs b/components/webdriver_server/lib.rs index d1b0505bab8..9e35ff0a884 100644 --- a/components/webdriver_server/lib.rs +++ b/components/webdriver_server/lib.rs @@ -21,11 +21,12 @@ use std::net::{SocketAddr, SocketAddrV4}; use std::time::Duration; use std::{env, fmt, process, thread}; +use base::generic_channel::{self, GenericSender, RoutedReceiver}; use base::id::{BrowsingContextId, WebViewId}; use base64::Engine; use capabilities::ServoCapabilities; use cookie::{CookieBuilder, Expiration, SameSite}; -use crossbeam_channel::{Receiver, Sender, after, select, unbounded}; +use crossbeam_channel::{Sender, after, select}; use embedder_traits::{ EventLoopWaker, JSValue, MouseButton, WebDriverCommandMsg, WebDriverCommandResponse, WebDriverFrameId, WebDriverJSError, WebDriverJSResult, WebDriverLoadStatus, WebDriverMessageId, @@ -34,8 +35,7 @@ use embedder_traits::{ use euclid::{Point2D, Rect, Size2D}; use http::method::Method; use image::{DynamicImage, ImageFormat, RgbaImage}; -use ipc_channel::ipc::{self, IpcReceiver, IpcSender}; -use ipc_channel::router::ROUTER; +use ipc_channel::ipc::{self, IpcReceiver}; use keyboard_types::webdriver::{Event as DispatchStringEvent, KeyInputState, send_keys}; use keyboard_types::{Code, Key, KeyState, KeyboardEvent, Location, NamedKey}; use log::{debug, error, info}; @@ -166,11 +166,11 @@ struct Handler { /// The threaded receiver on which we can block for a load-status. /// It will receive messages sent on the load_status_sender, /// and forwarded by the IPC router. - load_status_receiver: Receiver, + load_status_receiver: RoutedReceiver, /// The IPC sender which we can clone and pass along to the constellation, /// for it to send us a load-status. Messages sent on it /// will be forwarded to the load_status_receiver. - load_status_sender: IpcSender, + load_status_sender: GenericSender, session: Option, @@ -415,9 +415,8 @@ impl Handler { // and keep a threaded receiver to block on an incoming load-status. // Pass the others to the IPC router so that IPC messages are forwarded to the threaded receiver. // We need to use the router because IPC does not come with a timeout on receive/select. - let (load_status_sender, receiver) = ipc::channel().unwrap(); - let (sender, load_status_receiver) = unbounded(); - ROUTER.route_ipc_receiver_to_crossbeam_sender(receiver, sender); + let (load_status_sender, receiver) = generic_channel::channel().unwrap(); + let load_status_receiver = receiver.route_preserving_errors(); Handler { load_status_sender, @@ -708,7 +707,7 @@ impl Handler { recv(self.load_status_receiver) -> res => { match res { // If the navigation is navigation to IFrame, no document state event is fired. - Ok(WebDriverLoadStatus::Blocked) => { + Ok(Ok(WebDriverLoadStatus::Blocked)) => { // TODO: evaluate the correctness later // Load status is block means an user prompt is shown. // Alot of tests expect this to return success @@ -717,8 +716,8 @@ impl Handler { // an error anyway. Ok(WebDriverResponse::Void) }, - Ok(WebDriverLoadStatus::Complete) | - Ok(WebDriverLoadStatus::NavigationStop) => + Ok(Ok(WebDriverLoadStatus::Complete)) | + Ok(Ok(WebDriverLoadStatus::NavigationStop)) => Ok(WebDriverResponse::Void) , _ => Err(WebDriverError::new( @@ -738,7 +737,7 @@ impl Handler { /// fn wait_for_navigation(&self) -> WebDriverResult { let navigation_status = match self.load_status_receiver.try_recv() { - Ok(status) => status, + Ok(Ok(status)) => status, // Empty channel means no navigation started. Nothing to wait for. Err(crossbeam_channel::TryRecvError::Empty) => { return Ok(WebDriverResponse::Void); @@ -749,6 +748,12 @@ impl Handler { "Load status channel disconnected", )); }, + Ok(Err(ipc_error)) => { + return Err(WebDriverError::new( + ErrorStatus::UnknownError, + format!("Load status channel ipc error: {ipc_error}"), + )); + }, }; match navigation_status { diff --git a/ports/servoshell/desktop/app_state.rs b/ports/servoshell/desktop/app_state.rs index 99baf6ea377..57c25a6036a 100644 --- a/ports/servoshell/desktop/app_state.rs +++ b/ports/servoshell/desktop/app_state.rs @@ -13,6 +13,7 @@ use embedder_traits::webdriver::WebDriverSenders; use euclid::Vector2D; use keyboard_types::{Key, Modifiers, NamedKey, ShortcutMatcher}; use log::{error, info}; +use servo::base::generic_channel::GenericSender; use servo::base::id::WebViewId; use servo::config::pref; use servo::ipc_channel::ipc::IpcSender; @@ -456,7 +457,7 @@ impl RunningAppState { pub(crate) fn set_pending_traversal( &self, traversal_id: TraversalId, - sender: IpcSender, + sender: GenericSender, ) { self.webdriver_senders .borrow_mut() @@ -467,7 +468,7 @@ impl RunningAppState { pub(crate) fn set_load_status_sender( &self, webview_id: WebViewId, - sender: IpcSender, + sender: GenericSender, ) { self.webdriver_senders .borrow_mut()