From 57fc56981888918b125341b3bbff143aeb31993d Mon Sep 17 00:00:00 2001 From: Euclid Ye Date: Tue, 22 Jul 2025 18:17:19 +0800 Subject: [PATCH] Remove Webdriver Server's access to Constellation (#38212) `WebDriverCommandMsg::TakeScreenshot` was the last thing blocking us from removing constellation access in webdriver server. Now we can happily remove it. Surprisingly, this reduces binary size by 185KB for release profile in Windows. #37737 removes a net total of 300 lines, but only reduced 98KB. Testing: No regression after testing. Signed-off-by: Euclid Ye --- components/compositing/compositor.rs | 18 +++--------------- components/compositing/tracing.rs | 1 - components/constellation/constellation.rs | 10 ++-------- components/servo/lib.rs | 21 ++++++++++++++++++--- components/shared/compositing/lib.rs | 9 --------- components/shared/embedder/webdriver.rs | 2 +- components/webdriver_server/lib.rs | 22 ++++++---------------- ports/servoshell/desktop/app.rs | 6 +----- 8 files changed, 31 insertions(+), 58 deletions(-) diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs index 5a422444493..765dcce08cb 100644 --- a/components/compositing/compositor.rs +++ b/components/compositing/compositor.rs @@ -60,12 +60,12 @@ use crate::webview_manager::WebViewManager; use crate::webview_renderer::{PinchZoomResult, UnknownWebView, WebViewRenderer}; #[derive(Debug, PartialEq)] -enum UnableToComposite { +pub enum UnableToComposite { NotReadyToPaintImage(NotReadyToPaint), } #[derive(Debug, PartialEq)] -enum NotReadyToPaint { +pub enum NotReadyToPaint { JustNotifiedConstellation, WaitingOnConstellation, } @@ -591,18 +591,6 @@ impl IOCompositor { }; webview_renderer.on_touch_event_processed(result); }, - - CompositorMsg::CreatePng(webview_id, page_rect, reply) => { - let res = self.render_to_shared_memory(webview_id, page_rect); - if let Err(ref e) = res { - info!("Error retrieving PNG: {:?}", e); - } - let img = res.unwrap_or(None); - if let Err(e) = reply.send(img) { - warn!("Sending reply to create png failed ({:?}).", e); - } - }, - CompositorMsg::IsReadyToSaveImageReply(is_ready) => { assert_eq!( self.ready_to_save_state, @@ -1351,7 +1339,7 @@ impl IOCompositor { /// Render the WebRender scene to the shared memory, without updating other state of this /// [`IOCompositor`]. If succesful return the output image in shared memory. - fn render_to_shared_memory( + pub fn render_to_shared_memory( &mut self, webview_id: WebViewId, page_rect: Option>, diff --git a/components/compositing/tracing.rs b/components/compositing/tracing.rs index 7de9ebc0f9e..1c46205f5ec 100644 --- a/components/compositing/tracing.rs +++ b/components/compositing/tracing.rs @@ -34,7 +34,6 @@ mod from_constellation { Self::CreateOrUpdateWebView(..) => target!("CreateOrUpdateWebView"), Self::RemoveWebView(..) => target!("RemoveWebView"), Self::TouchEventProcessed(..) => target!("TouchEventProcessed"), - Self::CreatePng(..) => target!("CreatePng"), Self::IsReadyToSaveImageReply(..) => target!("IsReadyToSaveImageReply"), Self::SetThrottled(..) => target!("SetThrottled"), Self::NewWebRenderFrameReady(..) => target!("NewWebRenderFrameReady"), diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index bbc57bac88f..235710fbeb7 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -4455,13 +4455,6 @@ where self.handle_send_error(pipeline_id, e); } }, - WebDriverCommandMsg::TakeScreenshot(webview_id, rect, response_sender) => { - self.compositor_proxy.send(CompositorMsg::CreatePng( - webview_id, - rect, - response_sender, - )); - }, WebDriverCommandMsg::CloseWebView(..) | WebDriverCommandMsg::NewWebView(..) | WebDriverCommandMsg::FocusWebView(..) | @@ -4475,7 +4468,8 @@ where WebDriverCommandMsg::KeyboardAction(..) | WebDriverCommandMsg::MouseButtonAction(..) | WebDriverCommandMsg::MouseMoveAction(..) | - WebDriverCommandMsg::WheelScrollAction(..) => { + WebDriverCommandMsg::WheelScrollAction(..) | + WebDriverCommandMsg::TakeScreenshot(..) => { unreachable!("This command should be send directly to the embedder."); }, _ => { diff --git a/components/servo/lib.rs b/components/servo/lib.rs index cbb8800a5d7..cae4dd3f9f1 100644 --- a/components/servo/lib.rs +++ b/components/servo/lib.rs @@ -88,7 +88,7 @@ pub use keyboard_types::{ Code, CompositionEvent, CompositionState, Key, KeyState, Location, Modifiers, }; use layout::LayoutFactoryImpl; -use log::{Log, Metadata, Record, debug, warn}; +use log::{Log, Metadata, Record, debug, error, warn}; use media::{GlApi, NativeDisplay, WindowGLContext}; use net::protocols::ProtocolRegistry; use net::resource_thread::new_resource_threads; @@ -1061,8 +1061,23 @@ impl Servo { } pub fn execute_webdriver_command(&self, command: WebDriverCommandMsg) { - self.constellation_proxy - .send(EmbedderToConstellationMessage::WebDriverCommand(command)); + if let WebDriverCommandMsg::TakeScreenshot(webview_id, page_rect, response_sender) = command + { + let res = self + .compositor + .borrow_mut() + .render_to_shared_memory(webview_id, page_rect); + if let Err(ref e) = res { + error!("Error retrieving PNG: {:?}", e); + } + let img = res.unwrap_or(None); + if let Err(e) = response_sender.send(img) { + error!("Sending reply to create png failed ({:?}).", e); + } + } else { + self.constellation_proxy + .send(EmbedderToConstellationMessage::WebDriverCommand(command)); + } } } diff --git a/components/shared/compositing/lib.rs b/components/shared/compositing/lib.rs index 654137a6771..2c6ab5fd225 100644 --- a/components/shared/compositing/lib.rs +++ b/components/shared/compositing/lib.rs @@ -9,14 +9,11 @@ use std::fmt::{Debug, Error, Formatter}; use base::id::{PipelineId, WebViewId}; use crossbeam_channel::Sender; use embedder_traits::{AnimationState, EventLoopWaker, TouchEventResult}; -use euclid::Rect; use ipc_channel::ipc::IpcSender; use log::warn; use malloc_size_of_derive::MallocSizeOf; -use pixels::RasterImage; use smallvec::SmallVec; use strum_macros::IntoStaticStr; -use style_traits::CSSPixel; use webrender_api::DocumentId; pub mod display_list; @@ -80,12 +77,6 @@ pub enum CompositorMsg { RemoveWebView(WebViewId), /// Script has handled a touch event, and either prevented or allowed default actions. TouchEventProcessed(WebViewId, TouchEventResult), - /// Composite to a PNG file and return the Image over a passed channel. - CreatePng( - WebViewId, - Option>, - IpcSender>, - ), /// A reply to the compositor asking if the output image is stable. IsReadyToSaveImageReply(bool), /// Set whether to use less resources by stopping animations. diff --git a/components/shared/embedder/webdriver.rs b/components/shared/embedder/webdriver.rs index 89a26d2a5bd..2b69f0cf656 100644 --- a/components/shared/embedder/webdriver.rs +++ b/components/shared/embedder/webdriver.rs @@ -138,7 +138,7 @@ pub enum WebDriverCommandMsg { DeviceIndependentIntSize, IpcSender>, ), - /// Take a screenshot of the window. + /// Take a screenshot of the viewport. TakeScreenshot( WebViewId, Option>, diff --git a/components/webdriver_server/lib.rs b/components/webdriver_server/lib.rs index e862d7d0264..831b13e0890 100644 --- a/components/webdriver_server/lib.rs +++ b/components/webdriver_server/lib.rs @@ -21,7 +21,6 @@ use std::{env, fmt, process, thread}; use base::id::{BrowsingContextId, WebViewId}; use base64::Engine; use capabilities::ServoCapabilities; -use constellation_traits::EmbedderToConstellationMessage; use cookie::{CookieBuilder, Expiration}; use crossbeam_channel::{Receiver, Sender, after, select, unbounded}; use embedder_traits::{ @@ -128,13 +127,11 @@ fn cookie_msg_to_cookie(cookie: cookie::Cookie) -> Cookie { pub fn start_server( port: u16, - constellation_chan_deprecated: Sender, embedder_sender: Sender, event_loop_waker: Box, webdriver_response_receiver: IpcReceiver, ) { let handler = Handler::new( - constellation_chan_deprecated, embedder_sender, event_loop_waker, webdriver_response_receiver, @@ -245,10 +242,6 @@ struct Handler { /// 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, - /// Receiver notification from the constellation when a command is completed webdriver_response_receiver: IpcReceiver, @@ -474,7 +467,6 @@ enum VerifyBrowsingContextIsOpen { impl Handler { pub fn new( - constellation_chan: Sender, embedder_sender: Sender, event_loop_waker: Box, webdriver_response_receiver: IpcReceiver, @@ -494,7 +486,6 @@ impl Handler { session: None, embedder_sender, event_loop_waker, - constellation_chan, webdriver_response_receiver, id_generator: WebDriverMessageIdGenerator::new(), current_action_id: Cell::new(None), @@ -2247,9 +2238,10 @@ impl Handler { fn take_screenshot(&self, rect: Option>) -> WebDriverResult { // Step 1. 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(self.session()?.webview_id)?; + let webview_id = self.session()?.webview_id; + self.verify_top_level_browsing_context_is_open(webview_id)?; - self.handle_any_user_prompts(self.session()?.webview_id)?; + self.handle_any_user_prompts(webview_id)?; let mut img = None; @@ -2259,11 +2251,9 @@ impl Handler { for _ in 0..iterations { let (sender, receiver) = ipc::channel().unwrap(); - let cmd_msg = - WebDriverCommandMsg::TakeScreenshot(self.session()?.webview_id, rect, sender); - self.constellation_chan - .send(EmbedderToConstellationMessage::WebDriverCommand(cmd_msg)) - .unwrap(); + self.send_message_to_embedder(WebDriverCommandMsg::TakeScreenshot( + webview_id, rect, sender, + ))?; if let Some(x) = wait_for_script_response(receiver)? { img = Some(x); diff --git a/ports/servoshell/desktop/app.rs b/ports/servoshell/desktop/app.rs index d799efece05..97f1403af8f 100644 --- a/ports/servoshell/desktop/app.rs +++ b/ports/servoshell/desktop/app.rs @@ -187,7 +187,6 @@ impl App { webdriver_server::start_server( port, - servo.constellation_sender(), embedder_sender, self.waker.clone(), webdriver_response_receiver, @@ -621,10 +620,7 @@ impl App { running_state.set_alert_text_of_newest_dialog(webview_id, text); }, WebDriverCommandMsg::TakeScreenshot(..) => { - warn!( - "WebDriverCommand {:?} is still not moved from constellation to embedder", - msg - ); + running_state.forward_webdriver_command(msg); }, }; }