From 59b90c60aef17825a5c0a2cfcc54da6a63c6e774 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Fri, 8 May 2026 18:31:52 -0400 Subject: [PATCH] Ensure PRD sites render through app sidecar --- crates/ely_app/build.rs | 7 + crates/ely_app/src/services/servo_sidecar.rs | 129 ++++++++++++++---- crates/ely_servo_host/tests/sidecar.rs | 4 +- .../ely_servo_host/tests/sidecar/support.rs | 1 + 4 files changed, 113 insertions(+), 28 deletions(-) diff --git a/crates/ely_app/build.rs b/crates/ely_app/build.rs index 4c1b27c..398c213 100644 --- a/crates/ely_app/build.rs +++ b/crates/ely_app/build.rs @@ -28,6 +28,7 @@ fn main() -> Result<(), Box> { emit_env("ELY_BUILD_REVISION", &git_revision(workspace_root)?)?; emit_env("ELY_WORKSPACE_LICENSE", string_value(package, "license")?)?; + emit_env("ELY_WORKSPACE_MANIFEST", path_value(&workspace_manifest_path)?)?; emit_env("ELY_GPUI_VERSION", dependency_version(dependencies, "gpui")?)?; emit_env("ELY_GPUI_COMPONENT_VERSION", dependency_version(dependencies, "gpui-component")?)?; emit_env("ELY_SERVO_VERSION", dependency_version(dependencies, "servo")?)?; @@ -65,6 +66,12 @@ fn string_value<'a>(value: &'a toml::Table, key: &str) -> Result<&'a str, Box Result<&str, Box> { + path.to_str() + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "workspace path is not UTF-8")) + .map_err(Into::into) +} + fn dependency_version<'a>( dependencies: &'a toml::Table, name: &str, diff --git a/crates/ely_app/src/services/servo_sidecar.rs b/crates/ely_app/src/services/servo_sidecar.rs index fe406ff..371bc21 100644 --- a/crates/ely_app/src/services/servo_sidecar.rs +++ b/crates/ely_app/src/services/servo_sidecar.rs @@ -10,27 +10,27 @@ use ely_domain::UrlText; use serde::Deserialize; use thiserror::Error; -const SIDECAR_COMMAND_TIMEOUT: Duration = Duration::from_secs(35); +const SIDECAR_BINARY_TIMEOUT: Duration = Duration::from_secs(35); +const SIDECAR_CARGO_TIMEOUT: Duration = Duration::from_secs(180); const SIDECAR_POLL_INTERVAL: Duration = Duration::from_millis(20); +const SIDECAR_PATH_ENV: &str = "ELY_SERVO_SIDECAR"; #[derive(Clone, Debug)] pub struct ServoSidecarClient { - binary_path: PathBuf, + command_target: SidecarCommandTarget, } impl ServoSidecarClient { pub fn new() -> Result { - Ok(Self { binary_path: default_sidecar_path()? }) + Ok(Self { command_target: default_sidecar_command()? }) } pub fn snapshot( &self, request: SidecarSnapshotRequest, ) -> Result { - if !self.binary_path.is_file() { - return Err(ServoSidecarError::SidecarBinaryUnavailable { - path: self.binary_path.clone(), - }); + if let Some(path) = self.command_target.missing_binary_path() { + return Err(ServoSidecarError::SidecarBinaryUnavailable { path: path.to_path_buf() }); } let rgba_path = temporary_rgba_path()?; @@ -64,21 +64,8 @@ impl ServoSidecarClient { request: &SidecarSnapshotRequest, rgba_path: &Path, ) -> Result { - let mut command = Command::new(&self.binary_path); - command - .arg("snapshot") - .arg("--url") - .arg(request.url.as_str()) - .arg("--rgba-out") - .arg(rgba_path) - .arg("--width") - .arg(request.width.to_string()) - .arg("--height") - .arg(request.height.to_string()) - .arg("--scroll-x") - .arg(request.scroll_x.to_string()) - .arg("--scroll-y") - .arg(request.scroll_y.to_string()); + let mut command = self.command_target.command(); + append_snapshot_args(&mut command, request, rgba_path); if let Some(click_point) = request.click_point { command .arg("--click-x") @@ -102,11 +89,12 @@ impl ServoSidecarClient { return child.wait_with_output().map_err(ServoSidecarError::Command); } - if started_at.elapsed() >= SIDECAR_COMMAND_TIMEOUT { + let timeout = self.command_target.timeout(); + if started_at.elapsed() >= timeout { terminate_child(child)?; return Err(ServoSidecarError::SidecarTimedOut { url: request.url.as_str().to_string(), - seconds: SIDECAR_COMMAND_TIMEOUT.as_secs(), + seconds: timeout.as_secs(), }); } @@ -115,6 +103,67 @@ impl ServoSidecarClient { } } +#[derive(Clone, Debug)] +enum SidecarCommandTarget { + Binary(PathBuf), + Cargo { manifest_path: PathBuf }, +} + +impl SidecarCommandTarget { + fn command(&self) -> Command { + match self { + Self::Binary(path) => Command::new(path), + Self::Cargo { manifest_path } => { + let mut command = Command::new("cargo"); + command + .arg("run") + .arg("--quiet") + .arg("--manifest-path") + .arg(manifest_path) + .arg("-p") + .arg("ely_servo_host") + .arg("--features") + .arg("servo-engine") + .arg("--bin") + .arg("ely_servo_sidecar") + .arg("--"); + command + } + } + } + + fn timeout(&self) -> Duration { + match self { + Self::Binary(_) => SIDECAR_BINARY_TIMEOUT, + Self::Cargo { .. } => SIDECAR_CARGO_TIMEOUT, + } + } + + fn missing_binary_path(&self) -> Option<&Path> { + match self { + Self::Binary(path) if !path.is_file() => Some(path.as_path()), + Self::Binary(_) | Self::Cargo { .. } => None, + } + } +} + +fn append_snapshot_args(command: &mut Command, request: &SidecarSnapshotRequest, rgba_path: &Path) { + command + .arg("snapshot") + .arg("--url") + .arg(request.url.as_str()) + .arg("--rgba-out") + .arg(rgba_path) + .arg("--width") + .arg(request.width.to_string()) + .arg("--height") + .arg(request.height.to_string()) + .arg("--scroll-x") + .arg(request.scroll_x.to_string()) + .arg("--scroll-y") + .arg(request.scroll_y.to_string()); +} + #[derive(Clone, Debug)] pub struct SidecarSnapshotRequest { url: UrlText, @@ -302,13 +351,41 @@ struct SidecarReport { content_pixel_count: u64, } -fn default_sidecar_path() -> Result { +fn default_sidecar_command() -> Result { + if let Some(path) = env::var_os(SIDECAR_PATH_ENV) { + return Ok(SidecarCommandTarget::Binary(PathBuf::from(path))); + } + let current_exe = env::current_exe().map_err(ServoSidecarError::CurrentExecutable)?; let exe_dir = current_exe.parent().ok_or_else(|| { ServoSidecarError::CurrentExecutableDirectoryUnavailable { path: current_exe.clone() } })?; + let adjacent_sidecar = exe_dir.join(sidecar_binary_name()); + if adjacent_sidecar.is_file() { + return Ok(SidecarCommandTarget::Binary(adjacent_sidecar)); + } - Ok(exe_dir.join(sidecar_binary_name())) + if let Some(manifest_path) = workspace_manifest_path() { + if let Some(target_sidecar) = workspace_target_sidecar_path(&manifest_path) + && target_sidecar.is_file() + { + return Ok(SidecarCommandTarget::Binary(target_sidecar)); + } + if manifest_path.is_file() { + return Ok(SidecarCommandTarget::Cargo { manifest_path }); + } + } + + Ok(SidecarCommandTarget::Binary(adjacent_sidecar)) +} + +fn workspace_manifest_path() -> Option { + option_env!("ELY_WORKSPACE_MANIFEST").map(PathBuf::from) +} + +fn workspace_target_sidecar_path(manifest_path: &Path) -> Option { + let profile = if cfg!(debug_assertions) { "debug" } else { "release" }; + Some(manifest_path.parent()?.join("target").join(profile).join(sidecar_binary_name())) } fn sidecar_binary_name() -> String { diff --git a/crates/ely_servo_host/tests/sidecar.rs b/crates/ely_servo_host/tests/sidecar.rs index b493af4..8f67bcb 100644 --- a/crates/ely_servo_host/tests/sidecar.rs +++ b/crates/ely_servo_host/tests/sidecar.rs @@ -8,7 +8,7 @@ mod support; use support::*; #[test] -fn sidecar_snapshots_prd_sites_to_rgba_files() -> Result<(), Box> { +fn sidecar_opens_and_renders_prd_sites_to_rgba_files() -> Result<(), Box> { for case in PRD_SITE_COMPATIBILITY_CASES { for size in PRD_SITE_COMPATIBILITY_SIZES { snapshot_prd_site(case, *size, ScrollOffset::ZERO)?; @@ -19,7 +19,7 @@ fn sidecar_snapshots_prd_sites_to_rgba_files() -> Result<(), Box> { } #[test] -fn sidecar_snapshots_prd_reference_sites_to_rgba_files() -> Result<(), Box> { +fn sidecar_opens_and_renders_prd_reference_sites_to_rgba_files() -> Result<(), Box> { for case in PRD_REFERENCE_SITE_COMPATIBILITY_CASES { snapshot_prd_site(case, PRD_REFERENCE_SITE_SIZE, ScrollOffset::ZERO)?; } diff --git a/crates/ely_servo_host/tests/sidecar/support.rs b/crates/ely_servo_host/tests/sidecar/support.rs index 7094726..d3f2477 100644 --- a/crates/ely_servo_host/tests/sidecar/support.rs +++ b/crates/ely_servo_host/tests/sidecar/support.rs @@ -14,6 +14,7 @@ const SIDECAR_COMMAND_COOLDOWN: Duration = Duration::from_millis(750); const SIDECAR_RETRY_INTERVAL: Duration = Duration::from_millis(250); static SIDECAR_COMMAND_LOCK: Mutex<()> = Mutex::new(()); pub(super) const PRD_SITE_COMPATIBILITY_CASES: &[PrdSiteCompatibilityCase] = &[ + PrdSiteCompatibilityCase { url: "https://github.com", title_fragment: "GitHub" }, PrdSiteCompatibilityCase { url: "https://example.com", title_fragment: "Example Domain" }, PrdSiteCompatibilityCase { url: "https://servo.org/", title_fragment: "Servo" }, ];