From d970584332a3761009f672f975bfffa917513b85 Mon Sep 17 00:00:00 2001 From: batu_hoang <55729155+longvatrong111@users.noreply.github.com> Date: Wed, 25 Jun 2025 18:29:34 +0800 Subject: [PATCH] webdriver: Move `NewWebView`, `FocusWebView`, `GetWindowSize`, and `SetWindowSize` to servoshell (#37555) Follow up to: https://github.com/servo/servo/pull/36714 Moving webdriver [context commands](https://www.w3.org/TR/webdriver2/#contexts) to be handled in embedder: - [x] New Window command - `WebdriverCommandMsg::NewWebView` - [x] Switch To Window command - `WebdriverCommandMsg::FocusWebView` - [x] Resizing commands cc: @xiaochengh --------- Signed-off-by: batu_hoang --- components/constellation/constellation.rs | 52 +++++------------- components/shared/embedder/webdriver.rs | 20 ++++--- components/webdriver_server/lib.rs | 51 +++++++----------- ports/servoshell/desktop/app.rs | 65 ++++++++++++++++++++--- ports/servoshell/desktop/app_state.rs | 49 ++++++++++++++--- ports/servoshell/desktop/headed_window.rs | 2 +- ports/servoshell/egl/app_state.rs | 4 +- ports/servoshell/egl/ohos.rs | 2 +- 8 files changed, 148 insertions(+), 97 deletions(-) diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index 5cb25646afd..b741079f2b5 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -4542,41 +4542,11 @@ where WebDriverCommandMsg::CloseWebView(..) => { unreachable!("This command should be send directly to the embedder."); }, - WebDriverCommandMsg::NewWebView( - originating_webview_id, - response_sender, - load_status_sender, - ) => { - let (embedder_sender, receiver) = match ipc::channel() { - Ok(result) => result, - Err(error) => return warn!("Failed to create channel: {error:?}"), - }; - self.embedder_proxy.send(EmbedderMsg::AllowOpeningWebView( - originating_webview_id, - embedder_sender, - )); - let (new_webview_id, viewport_details) = match receiver.recv() { - Ok(Some((new_webview_id, viewport_details))) => { - (new_webview_id, viewport_details) - }, - Ok(None) => return warn!("Embedder refused to allow opening webview"), - Err(error) => return warn!("Failed to receive webview id: {error:?}"), - }; - self.handle_new_top_level_browsing_context( - ServoUrl::parse_with_base(None, "about:blank").expect("Infallible parse"), - new_webview_id, - viewport_details, - Some(load_status_sender), - ); - if let Err(error) = response_sender.send(new_webview_id) { - error!( - "WebDriverCommandMsg::NewWebView: IPC error when sending new_webview_id \ - to webdriver server: {error}" - ); - } + WebDriverCommandMsg::NewWebView(..) => { + unreachable!("This command should be send directly to the embedder."); }, - WebDriverCommandMsg::FocusWebView(webview_id) => { - self.handle_focus_web_view(webview_id); + WebDriverCommandMsg::FocusWebView(..) => { + unreachable!("This command should be send directly to the embedder."); }, WebDriverCommandMsg::IsWebViewOpen(..) => { unreachable!("This command should be send directly to the embedder."); @@ -4585,7 +4555,10 @@ where let is_open = self.browsing_contexts.contains_key(&browsing_context_id); let _ = response_sender.send(is_open); }, - WebDriverCommandMsg::GetWindowSize(webview_id, response_sender) => { + WebDriverCommandMsg::GetWindowSize(..) => { + unreachable!("This command should be send directly to the embedder."); + }, + WebDriverCommandMsg::GetViewportSize(webview_id, response_sender) => { let browsing_context_id = BrowsingContextId::from(webview_id); let size = self .browsing_contexts @@ -4594,10 +4567,8 @@ where .unwrap_or_default(); let _ = response_sender.send(size); }, - WebDriverCommandMsg::SetWindowSize(webview_id, size, response_sender) => { - self.webdriver.resize_channel = Some(response_sender); - self.embedder_proxy - .send(EmbedderMsg::ResizeTo(webview_id, size)); + WebDriverCommandMsg::SetWindowSize(..) => { + unreachable!("This command should be send directly to the embedder."); }, WebDriverCommandMsg::LoadUrl(webview_id, url, response_sender) => { let load_data = LoadData::new( @@ -4767,6 +4738,9 @@ where response_sender, )); }, + _ => { + warn!("Unhandled WebDriver command: {:?}", msg); + }, } } diff --git a/components/shared/embedder/webdriver.rs b/components/shared/embedder/webdriver.rs index 225403a2cc7..b141c892dbd 100644 --- a/components/shared/embedder/webdriver.rs +++ b/components/shared/embedder/webdriver.rs @@ -20,7 +20,7 @@ use servo_url::ServoUrl; use style_traits::CSSPixel; use webdriver::common::{WebElement, WebFrame, WebWindow}; use webdriver::error::ErrorStatus; -use webrender_api::units::DeviceIntSize; +use webrender_api::units::{DeviceIntSize, DevicePixel}; use crate::{MouseButton, MouseButtonAction}; @@ -31,7 +31,9 @@ pub struct WebDriverMessageId(pub usize); #[derive(Debug, Deserialize, Serialize)] pub enum WebDriverCommandMsg { /// Get the window size. - GetWindowSize(WebViewId, IpcSender>), + GetWindowSize(WebViewId, IpcSender>), + /// Get the viewport size. + GetViewportSize(WebViewId, IpcSender>), /// Load a URL in the top-level browsing context with the given ID. LoadUrl(WebViewId, ServoUrl, IpcSender), /// Refresh the top-level browsing context with the given ID. @@ -83,7 +85,11 @@ pub enum WebDriverCommandMsg { IpcSender, ), /// Set the window size. - SetWindowSize(WebViewId, DeviceIntSize, IpcSender>), + SetWindowSize( + WebViewId, + DeviceIntSize, + IpcSender>, + ), /// Take a screenshot of the window. TakeScreenshot( WebViewId, @@ -94,15 +100,13 @@ pub enum WebDriverCommandMsg { /// the provided channels to return the top level browsing context id /// associated with the new webview, and a notification when the initial /// load is complete. - NewWebView( - WebViewId, - IpcSender, - IpcSender, - ), + NewWebView(IpcSender, IpcSender), /// Close the webview associated with the provided id. CloseWebView(WebViewId), /// Focus the webview associated with the provided id. FocusWebView(WebViewId), + /// Get focused webview. + GetFocusedWebView(IpcSender>), /// Check whether top-level browsing context is open. IsWebViewOpen(WebViewId, IpcSender), /// Check whether browsing context is open. diff --git a/components/webdriver_server/lib.rs b/components/webdriver_server/lib.rs index 41a47d1e74f..b1beecd91b5 100644 --- a/components/webdriver_server/lib.rs +++ b/components/webdriver_server/lib.rs @@ -517,11 +517,8 @@ impl Handler { } fn focus_webview_id(&self) -> WebDriverResult { - debug!("Getting focused context."); let (sender, receiver) = ipc::channel().unwrap(); - - let msg = EmbedderToConstellationMessage::GetFocusTopLevelBrowsingContext(sender.clone()); - self.constellation_chan.send(msg).unwrap(); + self.send_message_to_embedder(WebDriverCommandMsg::GetFocusedWebView(sender.clone()))?; // Wait until the document is ready before returning the top-level browsing context id. match wait_for_script_response(receiver)? { Some(webview_id) => Ok(webview_id), @@ -807,18 +804,14 @@ impl Handler { self.verify_top_level_browsing_context_is_open(webview_id)?; - let cmd_msg = WebDriverCommandMsg::GetWindowSize(webview_id, sender); - - self.constellation_chan - .send(EmbedderToConstellationMessage::WebDriverCommand(cmd_msg)) - .unwrap(); + self.send_message_to_embedder(WebDriverCommandMsg::GetWindowSize(webview_id, sender))?; let window_size = wait_for_script_response(receiver)?; let window_size_response = WindowRectResponse { x: 0, y: 0, - width: window_size.width as i32, - height: window_size.height as i32, + width: window_size.width, + height: window_size.height, }; Ok(WebDriverResponse::WindowRect(window_size_response)) } @@ -844,30 +837,30 @@ impl Handler { // Step 12. If session's current top-level browsing context is no longer open, // return error with error code no such window. self.verify_top_level_browsing_context_is_open(webview_id)?; - let cmd_msg = WebDriverCommandMsg::SetWindowSize(webview_id, size.to_i32(), sender.clone()); - self.constellation_chan - .send(EmbedderToConstellationMessage::WebDriverCommand(cmd_msg)) - .unwrap(); + self.send_message_to_embedder(WebDriverCommandMsg::SetWindowSize( + webview_id, + size.to_i32(), + sender.clone(), + ))?; let timeout = self.resize_timeout; - let constellation_chan = self.constellation_chan.clone(); + let embedder_sender = self.embedder_sender.clone(); + let waker = self.event_loop_waker.clone(); thread::spawn(move || { // On timeout, we send a GetWindowSize message to the constellation, // which will give the current window size. thread::sleep(Duration::from_millis(timeout as u64)); - let cmd_msg = WebDriverCommandMsg::GetWindowSize(webview_id, sender); - constellation_chan - .send(EmbedderToConstellationMessage::WebDriverCommand(cmd_msg)) - .unwrap(); + let _ = embedder_sender.send(WebDriverCommandMsg::GetWindowSize(webview_id, sender)); + waker.wake(); }); let window_size = wait_for_script_response(receiver)?; let window_size_response = WindowRectResponse { x: 0, y: 0, - width: window_size.width as i32, - height: window_size.height as i32, + width: window_size.width, + height: window_size.height, }; Ok(WebDriverResponse::WindowRect(window_size_response)) } @@ -1071,16 +1064,10 @@ impl Handler { let session = self.session().unwrap(); self.verify_top_level_browsing_context_is_open(session.webview_id)?; - let cmd_msg = WebDriverCommandMsg::NewWebView( - session.webview_id, - sender, - self.load_status_sender.clone(), - ); + let cmd_msg = WebDriverCommandMsg::NewWebView(sender, self.load_status_sender.clone()); // Step 5. Create a new top-level browsing context by running the window open steps. // This MUST be done without invoking the focusing steps. - self.constellation_chan - .send(EmbedderToConstellationMessage::WebDriverCommand(cmd_msg)) - .unwrap(); + self.send_message_to_embedder(cmd_msg)?; let mut handle = self.session.as_ref().unwrap().id.to_string(); if let Ok(new_webview_id) = receiver.recv() { @@ -1145,8 +1132,8 @@ impl Handler { session.webview_id = webview_id; session.browsing_context_id = BrowsingContextId::from(webview_id); - let msg = EmbedderToConstellationMessage::FocusWebView(webview_id); - self.constellation_chan.send(msg).unwrap(); + let msg = WebDriverCommandMsg::FocusWebView(webview_id); + self.send_message_to_embedder(msg)?; Ok(WebDriverResponse::Void) } else { Err(WebDriverError::new( diff --git a/ports/servoshell/desktop/app.rs b/ports/servoshell/desktop/app.rs index a7d48c4cca9..b31f3f06a18 100644 --- a/ports/servoshell/desktop/app.rs +++ b/ports/servoshell/desktop/app.rs @@ -182,7 +182,7 @@ impl App { self.servoshell_preferences.clone(), webdriver_receiver, )); - running_state.new_toplevel_webview(self.initial_url.clone().into_url()); + running_state.create_and_focus_toplevel_webview(self.initial_url.clone().into_url()); if let Some(ref mut minibrowser) = self.minibrowser { minibrowser.update(window.winit_window().unwrap(), &running_state, "init"); @@ -310,7 +310,7 @@ impl App { }, MinibrowserEvent::NewWebView => { minibrowser.update_location_dirty(false); - state.new_toplevel_webview(Url::parse("servo:newtab").unwrap()); + state.create_and_focus_toplevel_webview(Url::parse("servo:newtab").unwrap()); }, MinibrowserEvent::CloseWebView(id) => { minibrowser.update_location_dirty(false); @@ -333,16 +333,69 @@ impl App { match msg { WebDriverCommandMsg::IsWebViewOpen(webview_id, sender) => { let context = running_state.webview_by_id(webview_id); - sender.send(context.is_some()).unwrap(); + + if let Err(error) = sender.send(context.is_some()) { + warn!("Failed to send response of IsWebViewOpein: {error}"); + } }, webdriver_msg @ WebDriverCommandMsg::IsBrowsingContextOpen(..) => { running_state.forward_webdriver_command(webdriver_msg); }, + WebDriverCommandMsg::NewWebView(response_sender, load_status_sender) => { + let new_webview = + running_state.create_toplevel_webview(Url::parse("auto:blank").unwrap()); + + if let Err(error) = response_sender.send(new_webview.id()) { + warn!("Failed to send response of NewWebview: {error}"); + } + + running_state.set_load_status_sender(new_webview.id(), load_status_sender); + }, WebDriverCommandMsg::CloseWebView(webview_id) => { running_state.close_webview(webview_id); }, - WebDriverCommandMsg::GetWindowSize(..) | - WebDriverCommandMsg::FocusWebView(..) | + WebDriverCommandMsg::FocusWebView(webview_id) => { + if let Some(webview) = running_state.webview_by_id(webview_id) { + webview.focus(); + } + + // TODO: send a response to the WebDriver + // so it knows when the focus has finished. + }, + WebDriverCommandMsg::GetWindowSize(_webview_id, response_sender) => { + let window = self + .windows + .values() + .next() + .expect("Should have at least one window in servoshell"); + + if let Err(error) = response_sender.send(window.screen_geometry().size) { + warn!("Failed to send response of GetWindowSize: {error}"); + } + }, + WebDriverCommandMsg::SetWindowSize(webview_id, size, size_sender) => { + let Some(webview) = running_state.webview_by_id(webview_id) else { + continue; + }; + + let window = self + .windows + .values() + .next() + .expect("Should have at least one window in servoshell"); + + let size = window.request_resize(&webview, size); + if let Err(error) = size_sender.send(size.unwrap_or_default()) { + warn!("Failed to send window size: {error}"); + } + }, + WebDriverCommandMsg::GetFocusedWebView(sender) => { + let focused_webview = running_state.focused_webview(); + if let Err(error) = sender.send(focused_webview.map(|w| w.id())) { + warn!("Failed to send response of GetFocusedWebView: {error}"); + }; + }, + WebDriverCommandMsg::GetViewportSize(..) | WebDriverCommandMsg::LoadUrl(..) | WebDriverCommandMsg::ScriptCommand(..) | WebDriverCommandMsg::SendKeys(..) | @@ -350,9 +403,7 @@ impl App { WebDriverCommandMsg::MouseButtonAction(..) | WebDriverCommandMsg::MouseMoveAction(..) | WebDriverCommandMsg::WheelScrollAction(..) | - WebDriverCommandMsg::SetWindowSize(..) | WebDriverCommandMsg::TakeScreenshot(..) | - WebDriverCommandMsg::NewWebView(..) | WebDriverCommandMsg::Refresh(..) => { warn!( "WebDriverCommand {:?} is still not moved from constellation to embedder", diff --git a/ports/servoshell/desktop/app_state.rs b/ports/servoshell/desktop/app_state.rs index 61ea93a48bc..bad4ea4eaa8 100644 --- a/ports/servoshell/desktop/app_state.rs +++ b/ports/servoshell/desktop/app_state.rs @@ -19,7 +19,7 @@ use servo::webrender_api::units::{DeviceIntPoint, DeviceIntSize}; use servo::{ AllowOrDenyRequest, AuthenticationRequest, FilterPattern, FormControl, GamepadHapticEffectType, KeyboardEvent, LoadStatus, PermissionRequest, Servo, ServoDelegate, ServoError, SimpleDialog, - WebDriverCommandMsg, WebView, WebViewBuilder, WebViewDelegate, + WebDriverCommandMsg, WebDriverLoadStatus, WebView, WebViewBuilder, WebViewDelegate, }; use url::Url; @@ -37,6 +37,13 @@ pub(crate) enum AppState { ShuttingDown, } +/// A collection of [`IpcSender`]s that are used to asynchronously communicate +/// to a WebDriver server with information about application state. +#[derive(Clone, Default)] +struct WebDriverSenders { + pub load_status_senders: HashMap>, +} + pub(crate) struct RunningAppState { /// A handle to the Servo instance of the [`RunningAppState`]. This is not stored inside /// `inner` so that we can keep a reference to Servo in order to spin the event loop, @@ -48,6 +55,7 @@ pub(crate) struct RunningAppState { /// A [`Receiver`] for receiving commands from a running WebDriver server, if WebDriver /// was enabled. webdriver_receiver: Option>, + webdriver_senders: RefCell, inner: RefCell, } @@ -99,6 +107,7 @@ impl RunningAppState { servo, servoshell_preferences, webdriver_receiver, + webdriver_senders: RefCell::default(), inner: RefCell::new(RunningAppStateInner { webviews: HashMap::default(), creation_order: Default::default(), @@ -112,7 +121,13 @@ impl RunningAppState { } } - pub(crate) fn new_toplevel_webview(self: &Rc, url: Url) { + pub(crate) fn create_and_focus_toplevel_webview(self: &Rc, url: Url) { + let webview = self.create_toplevel_webview(url); + webview.focus(); + webview.raise_to_top(true); + } + + pub(crate) fn create_toplevel_webview(self: &Rc, url: Url) -> WebView { let webview = WebViewBuilder::new(self.servo()) .url(url) .hidpi_scale_factor(self.inner().window.hidpi_scale_factor()) @@ -120,10 +135,8 @@ impl RunningAppState { .build(); webview.notify_theme_change(self.inner().window.theme()); - webview.focus(); - webview.raise_to_top(true); - - self.add(webview); + self.add(webview.clone()); + webview } pub(crate) fn inner(&self) -> Ref { @@ -382,6 +395,17 @@ impl RunningAppState { webview.notify_scroll_event(location, origin); }); } + + pub(crate) fn set_load_status_sender( + &self, + webview_id: WebViewId, + sender: IpcSender, + ) { + self.webdriver_senders + .borrow_mut() + .load_status_senders + .insert(webview_id, sender); + } } struct ServoShellServoDelegate; @@ -507,8 +531,19 @@ impl WebViewDelegate for RunningAppState { self.inner().window.set_cursor(cursor); } - fn notify_load_status_changed(&self, _webview: servo::WebView, _status: LoadStatus) { + fn notify_load_status_changed(&self, webview: servo::WebView, status: LoadStatus) { self.inner_mut().need_update = true; + + if status == LoadStatus::Complete { + if let Some(sender) = self + .webdriver_senders + .borrow_mut() + .load_status_senders + .remove(&webview.id()) + { + let _ = sender.send(WebDriverLoadStatus::Complete); + } + } } fn notify_fullscreen_state_changed(&self, _webview: servo::WebView, fullscreen_state: bool) { diff --git a/ports/servoshell/desktop/headed_window.rs b/ports/servoshell/desktop/headed_window.rs index fb2275f57da..a4ebb4463e7 100644 --- a/ports/servoshell/desktop/headed_window.rs +++ b/ports/servoshell/desktop/headed_window.rs @@ -394,7 +394,7 @@ impl Window { } }) .shortcut(CMD_OR_CONTROL, 'T', || { - state.new_toplevel_webview(Url::parse("servo:newtab").unwrap()); + state.create_and_focus_toplevel_webview(Url::parse("servo:newtab").unwrap()); }) .shortcut(CMD_OR_CONTROL, 'Q', || state.servo().start_shutting_down()) .otherwise(|| handled = false); diff --git a/ports/servoshell/egl/app_state.rs b/ports/servoshell/egl/app_state.rs index 6746aa05866..e767e1a8c2b 100644 --- a/ports/servoshell/egl/app_state.rs +++ b/ports/servoshell/egl/app_state.rs @@ -336,11 +336,11 @@ impl RunningAppState { }), }); - app_state.new_toplevel_webview(initial_url); + app_state.create_and_focus_toplevel_webview(initial_url); app_state } - pub(crate) fn new_toplevel_webview(self: &Rc, url: Url) { + pub(crate) fn create_and_focus_toplevel_webview(self: &Rc, url: Url) { let webview = WebViewBuilder::new(&self.servo) .url(url) .hidpi_scale_factor(self.inner().hidpi_scale_factor) diff --git a/ports/servoshell/egl/ohos.rs b/ports/servoshell/egl/ohos.rs index abafb8e97cb..1e0cfe16f97 100644 --- a/ports/servoshell/egl/ohos.rs +++ b/ports/servoshell/egl/ohos.rs @@ -228,7 +228,7 @@ impl ServoAction { }, NewWebview(xcomponent, window) => { servo.pause_compositor(); - servo.new_toplevel_webview("about:blank".parse().unwrap()); + servo.create_and_focus_toplevel_webview("about:blank".parse().unwrap()); let (window_handle, _, coordinates) = simpleservo::get_raw_window_handle(xcomponent.0, window.0);