diff --git a/crates/ely_app/src/main.rs b/crates/ely_app/src/main.rs index ab430d1..4df6347 100644 --- a/crates/ely_app/src/main.rs +++ b/crates/ely_app/src/main.rs @@ -3,8 +3,8 @@ mod shell; mod shortcuts; use gpui::{ - App, AppContext, Application, Bounds, Focusable, Menu, MenuItem, SystemMenuType, WindowBounds, - WindowOptions, actions, px, size, + App, AppContext, Application, Bounds, Focusable, Menu, MenuItem, SystemMenuType, + TitlebarOptions, WindowBounds, WindowOptions, actions, point, px, size, }; use gpui_component_assets::Assets; use shell::ElyShell; @@ -81,7 +81,11 @@ fn main() { let bounds = Bounds::centered(None, size(px(1240.0), px(780.0)), cx); let opened = cx.open_window( WindowOptions { - titlebar: None, + titlebar: Some(TitlebarOptions { + title: Some("ELY Browser".into()), + appears_transparent: true, + traffic_light_position: Some(point(px(18.0), px(24.0))), + }), window_bounds: Some(WindowBounds::Windowed(bounds)), ..WindowOptions::default() }, diff --git a/crates/ely_app/src/services/servo_sidecar.rs b/crates/ely_app/src/services/servo_sidecar.rs index e9e303c..1302f6b 100644 --- a/crates/ely_app/src/services/servo_sidecar.rs +++ b/crates/ely_app/src/services/servo_sidecar.rs @@ -1,14 +1,18 @@ use std::{ env, fs, io, path::{Path, PathBuf}, - process::Command, - time::{SystemTime, SystemTimeError, UNIX_EPOCH}, + process::{Command, Output, Stdio}, + thread, + time::{Duration, Instant, SystemTime, SystemTimeError, UNIX_EPOCH}, }; use ely_domain::UrlText; use serde::Deserialize; use thiserror::Error; +const SIDECAR_COMMAND_TIMEOUT: Duration = Duration::from_secs(20); +const SIDECAR_POLL_INTERVAL: Duration = Duration::from_millis(20); + #[derive(Clone, Debug)] pub struct ServoSidecarClient { binary_path: PathBuf, @@ -30,7 +34,13 @@ impl ServoSidecarClient { } let rgba_path = temporary_rgba_path()?; - let output = self.run_snapshot_command(&request, &rgba_path)?; + let output = match self.run_snapshot_command(&request, &rgba_path) { + Ok(output) => output, + Err(error) => { + remove_temporary_file(&rgba_path)?; + return Err(error); + } + }; if !output.status.success() { remove_temporary_file(&rgba_path)?; @@ -53,8 +63,8 @@ impl ServoSidecarClient { &self, request: &SidecarSnapshotRequest, rgba_path: &Path, - ) -> Result { - Command::new(&self.binary_path) + ) -> Result { + let mut child = Command::new(&self.binary_path) .arg("snapshot") .arg("--url") .arg(request.url.as_str()) @@ -64,8 +74,27 @@ impl ServoSidecarClient { .arg(request.width.to_string()) .arg("--height") .arg(request.height.to_string()) - .output() - .map_err(ServoSidecarError::Command) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .map_err(ServoSidecarError::Command)?; + + let started_at = Instant::now(); + loop { + if child.try_wait().map_err(ServoSidecarError::Command)?.is_some() { + return child.wait_with_output().map_err(ServoSidecarError::Command); + } + + if started_at.elapsed() >= SIDECAR_COMMAND_TIMEOUT { + terminate_child(child)?; + return Err(ServoSidecarError::SidecarTimedOut { + url: request.url.as_str().to_string(), + seconds: SIDECAR_COMMAND_TIMEOUT.as_secs(), + }); + } + + thread::sleep(SIDECAR_POLL_INTERVAL); + } } } @@ -111,6 +140,11 @@ impl SidecarSnapshot { requested_url: report.requested_url, }); } + if report.content_pixel_count == 0 { + return Err(ServoSidecarError::ContentlessRenderedFrame { + requested_url: report.requested_url, + }); + } Ok(Self { loaded_url: report.loaded_url, @@ -170,6 +204,9 @@ pub enum ServoSidecarError { #[error("servo sidecar exited with {status}: {stderr}")] SidecarFailed { status: String, stderr: String }, + #[error("servo sidecar timed out after {seconds}s while rendering {url}")] + SidecarTimedOut { url: String, seconds: u64 }, + #[error("failed to parse servo sidecar report: {0}")] Report(#[from] serde_json::Error), @@ -192,6 +229,9 @@ pub enum ServoSidecarError { #[error("servo rendered a blank frame for {requested_url}")] BlankRenderedFrame { requested_url: String }, + + #[error("servo rendered a frame without visible content for {requested_url}")] + ContentlessRenderedFrame { requested_url: String }, } #[derive(Deserialize)] @@ -204,6 +244,7 @@ struct SidecarReport { height: u32, rgba_byte_count: usize, non_white_pixel_count: u64, + content_pixel_count: u64, } fn default_sidecar_path() -> Result { @@ -234,6 +275,17 @@ fn remove_temporary_file(path: &Path) -> Result<(), ServoSidecarError> { } } +fn terminate_child(mut child: std::process::Child) -> Result<(), ServoSidecarError> { + match child.kill() { + Ok(()) => { + let _output = child.wait_with_output().map_err(ServoSidecarError::Command)?; + Ok(()) + } + Err(error) if error.kind() == io::ErrorKind::InvalidInput => Ok(()), + Err(error) => Err(ServoSidecarError::Command(error)), + } +} + fn read_sidecar_snapshot( stdout: &[u8], rgba_path: &Path, diff --git a/crates/ely_app/src/shell/render.rs b/crates/ely_app/src/shell/render.rs index 9ff9653..f285d0e 100644 --- a/crates/ely_app/src/shell/render.rs +++ b/crates/ely_app/src/shell/render.rs @@ -97,7 +97,8 @@ impl ElyShell { div() .h(px(spacing::COMMAND_BAR_HEIGHT)) - .px_4() + .pl(px(96.0)) + .pr_4() .gap_3() .flex() .items_center() diff --git a/crates/ely_app/src/shell/web_surface.rs b/crates/ely_app/src/shell/web_surface.rs index 07a7ccb..77686da 100644 --- a/crates/ely_app/src/shell/web_surface.rs +++ b/crates/ely_app/src/shell/web_surface.rs @@ -2,11 +2,12 @@ use std::{collections::BTreeMap, sync::Arc}; use ely_domain::{BrowserTab, TabId}; use gpui::{ - AnyElement, Context, ImageSource, IntoElement, ObjectFit, ParentElement, RenderImage, Styled, - StyledImage, div, img, px, rgb, + AnyElement, App, Bounds, Context, Entity, ImageSource, IntoElement, ObjectFit, ParentElement, + Pixels, RenderImage, Styled, StyledImage, Window, canvas, div, img, prelude::FluentBuilder, px, + rgb, }; use gpui_component::StyledExt; -use image::{ImageBuffer, Rgba}; +use image::{ImageBuffer, Rgba, imageops::FilterType}; use thiserror::Error; use crate::services::servo_sidecar::{ @@ -16,17 +17,21 @@ use crate::services::servo_sidecar::{ use super::ElyShell; use ely_design_system::{colors, spacing}; -const WEB_SURFACE_WIDTH: u32 = 1024; -const WEB_SURFACE_HEIGHT: u32 = 768; +const WEB_SURFACE_IMAGE_MAX_EDGE: u32 = 1024; pub(super) struct WebSurfaceStore { client: WebSurfaceClient, + viewport_sizes: BTreeMap, states: BTreeMap, } impl WebSurfaceStore { pub(super) fn new() -> Self { - Self { client: WebSurfaceClient::new(), states: BTreeMap::new() } + Self { + client: WebSurfaceClient::new(), + viewport_sizes: BTreeMap::new(), + states: BTreeMap::new(), + } } fn state(&self, tab_id: &TabId) -> Option<&WebSurfaceState> { @@ -38,8 +43,12 @@ impl WebSurfaceStore { return None; } + let size = self.viewport_sizes.get(tab.id()).copied()?; let requested_url = tab.url().as_str().to_string(); - if self.has_current_state(tab.id(), &requested_url) { + if self.is_loading_requested_url(tab.id(), requested_url.as_str()) { + return None; + } + if self.has_current_state(tab.id(), &requested_url, size) { return None; } @@ -48,7 +57,7 @@ impl WebSurfaceStore { WebSurfaceClient::Unavailable(message) => { self.states.insert( tab.id().clone(), - WebSurfaceState::Failed { requested_url, message: message.clone() }, + WebSurfaceState::Failed { requested_url, size, message: message.clone() }, ); return None; } @@ -56,38 +65,51 @@ impl WebSurfaceStore { self.states.insert( tab.id().clone(), - WebSurfaceState::Loading { requested_url: requested_url.clone() }, + WebSurfaceState::Loading { requested_url: requested_url.clone(), size }, ); Some(WebSurfaceRequest { tab_id: tab.id().clone(), requested_url, + size, client, snapshot_request: SidecarSnapshotRequest::new( tab.url().clone(), - WEB_SURFACE_WIDTH, - WEB_SURFACE_HEIGHT, + size.width, + size.height, ), }) } - fn has_current_state(&self, tab_id: &TabId, requested_url: &str) -> bool { + fn has_current_state(&self, tab_id: &TabId, requested_url: &str, size: WebSurfaceSize) -> bool { match self.states.get(tab_id) { - Some(WebSurfaceState::Loading { requested_url: current_url }) => { - current_url == requested_url + Some(WebSurfaceState::Loading { requested_url: current_url, size: current_size }) => { + current_url == requested_url && *current_size == size } - Some(WebSurfaceState::Ready(frame)) => frame.requested_url == requested_url, - Some(WebSurfaceState::Failed { requested_url: current_url, .. }) => { - current_url == requested_url + Some(WebSurfaceState::Ready(frame)) => { + frame.requested_url == requested_url && frame.size() == size } + Some(WebSurfaceState::Failed { + requested_url: current_url, + size: current_size, + .. + }) => current_url == requested_url && *current_size == size, None => false, } } - fn is_loading(&self, tab_id: &TabId, requested_url: &str) -> bool { + fn is_loading(&self, tab_id: &TabId, requested_url: &str, size: WebSurfaceSize) -> bool { matches!( self.states.get(tab_id), - Some(WebSurfaceState::Loading { requested_url: current_url }) + Some(WebSurfaceState::Loading { requested_url: current_url, size: current_size }) + if current_url == requested_url && *current_size == size + ) + } + + fn is_loading_requested_url(&self, tab_id: &TabId, requested_url: &str) -> bool { + matches!( + self.states.get(tab_id), + Some(WebSurfaceState::Loading { requested_url: current_url, .. }) if current_url == requested_url ) } @@ -95,6 +117,19 @@ impl WebSurfaceStore { fn finish(&mut self, tab_id: TabId, state: WebSurfaceState) { self.states.insert(tab_id, state); } + + fn record_viewport_size(&mut self, tab_id: &TabId, bounds: Bounds) -> bool { + let Some(size) = WebSurfaceSize::from_bounds(bounds) else { + return false; + }; + + if self.viewport_sizes.get(tab_id) == Some(&size) { + return false; + } + + self.viewport_sizes.insert(tab_id.clone(), size); + true + } } enum WebSurfaceClient { @@ -114,14 +149,30 @@ impl WebSurfaceClient { struct WebSurfaceRequest { tab_id: TabId, requested_url: String, + size: WebSurfaceSize, client: ServoSidecarClient, snapshot_request: SidecarSnapshotRequest, } enum WebSurfaceState { - Loading { requested_url: String }, + Loading { requested_url: String, size: WebSurfaceSize }, Ready(WebSurfaceFrame), - Failed { requested_url: String, message: String }, + Failed { requested_url: String, size: WebSurfaceSize, message: String }, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +struct WebSurfaceSize { + width: u32, + height: u32, +} + +impl WebSurfaceSize { + fn from_bounds(bounds: Bounds) -> Option { + Some(Self { + width: viewport_dimension(bounds.size.width)?, + height: viewport_dimension(bounds.size.height)?, + }) + } } struct WebSurfaceFrame { @@ -148,13 +199,15 @@ impl WebSurfaceFrame { return Err(WebSurfaceError::InvalidFrameBuffer { width, height }); }; + let image_buffer = renderable_image_buffer(buffer); + Ok(Self { requested_url, loaded_url, title, width, height, - image: Arc::new(RenderImage::new([image::Frame::new(buffer)])), + image: Arc::new(RenderImage::new([image::Frame::new(image_buffer)])), }) } @@ -165,6 +218,10 @@ impl WebSurfaceFrame { fn url_label(&self) -> &str { self.loaded_url.as_deref().unwrap_or(self.requested_url.as_str()) } + + fn size(&self) -> WebSurfaceSize { + WebSurfaceSize { width: self.width, height: self.height } + } } #[derive(Debug, Error)] @@ -181,12 +238,17 @@ impl ElyShell { ) -> AnyElement { self.ensure_external_web_frame(tab, cx); + let state_entity = cx.entity().clone(); match self.web_surfaces.state(tab.id()) { - Some(WebSurfaceState::Ready(frame)) => render_ready_web_surface(frame), + Some(WebSurfaceState::Ready(frame)) => { + render_ready_web_surface(frame, tab, state_entity) + } Some(WebSurfaceState::Failed { message, .. }) => { - render_failed_web_surface(tab, message.as_str()) + render_failed_web_surface(tab, message.as_str(), state_entity) + } + Some(WebSurfaceState::Loading { .. }) | None => { + render_loading_web_surface(tab, state_entity) } - Some(WebSurfaceState::Loading { .. }) | None => render_loading_web_surface(tab), } } @@ -195,7 +257,7 @@ impl ElyShell { return; }; - let WebSurfaceRequest { tab_id, requested_url, client, snapshot_request } = request; + let WebSurfaceRequest { tab_id, requested_url, size, client, snapshot_request } = request; cx.spawn(async move |shell, cx| { let result = cx .background_executor() @@ -203,7 +265,7 @@ impl ElyShell { .await; _ = shell.update(cx, |shell, cx| { - shell.handle_external_web_frame_result(tab_id, requested_url, result); + shell.handle_external_web_frame_result(tab_id, requested_url, size, result); cx.notify(); }); }) @@ -214,42 +276,55 @@ impl ElyShell { &mut self, tab_id: TabId, requested_url: String, + size: WebSurfaceSize, result: Result, ) { - if !self.web_surfaces.is_loading(&tab_id, requested_url.as_str()) { + if !self.web_surfaces.is_loading(&tab_id, requested_url.as_str(), size) { return; } let state = match result { Ok(snapshot) => match WebSurfaceFrame::from_snapshot(requested_url.clone(), snapshot) { Ok(frame) => WebSurfaceState::Ready(frame), - Err(error) => WebSurfaceState::Failed { requested_url, message: error.to_string() }, + Err(error) => { + WebSurfaceState::Failed { requested_url, size, message: error.to_string() } + } }, - Err(error) => WebSurfaceState::Failed { requested_url, message: error.to_string() }, + Err(error) => { + WebSurfaceState::Failed { requested_url, size, message: error.to_string() } + } }; self.web_surfaces.finish(tab_id, state); } + + fn record_external_web_viewport( + &mut self, + tab_id: TabId, + bounds: Bounds, + cx: &mut Context, + ) { + if self.web_surfaces.record_viewport_size(&tab_id, bounds) { + cx.notify(); + } + } } -fn render_ready_web_surface(frame: &WebSurfaceFrame) -> AnyElement { +fn render_ready_web_surface( + frame: &WebSurfaceFrame, + tab: &BrowserTab, + state_entity: Entity, +) -> AnyElement { render_web_surface( - div() - .size_full() - .flex() - .flex_col() - .bg(rgb(colors::SURFACE_CARD)) - .child(render_web_surface_header(frame)) - .child( - div().flex_1().min_h_0().overflow_hidden().bg(rgb(colors::SURFACE_CARD)).child( - img(ImageSource::Render(frame.image.clone())) - .size_full() - .object_fit(ObjectFit::Contain), - ), - ), + tab, + state_entity, + frame.title_label(), + frame.url_label().to_string(), + Some(format!("{}x{}", frame.width, frame.height)), + img(ImageSource::Render(frame.image.clone())).size_full().object_fit(ObjectFit::Contain), ) } -fn render_web_surface_header(frame: &WebSurfaceFrame) -> AnyElement { +fn render_web_surface_header(title: String, url: String, detail: Option) -> AnyElement { div() .h(px(34.0)) .px_3() @@ -267,36 +342,41 @@ fn render_web_surface_header(frame: &WebSurfaceFrame) -> AnyElement { .text_sm() .font_semibold() .text_color(rgb(colors::INK)) - .child(frame.title_label()), + .child(title), ) + .when_some(detail, |this, detail| { + this.child(div().text_xs().text_color(rgb(colors::MUTED)).child(detail)) + }) .child( - div() - .text_xs() - .text_color(rgb(colors::MUTED)) - .child(format!("{}x{}", frame.width, frame.height)), - ) - .child( - div() - .max_w(px(420.0)) - .truncate() - .text_xs() - .text_color(rgb(colors::MUTED)) - .child(frame.url_label().to_string()), + div().max_w(px(420.0)).truncate().text_xs().text_color(rgb(colors::MUTED)).child(url), ) .into_any_element() } -fn render_loading_web_surface(tab: &BrowserTab) -> AnyElement { - render_web_surface(centered_status( - tab.title(), - tab.url().as_str(), - "Rendering page with Servo", - colors::BODY, - )) +fn render_loading_web_surface(tab: &BrowserTab, state_entity: Entity) -> AnyElement { + render_web_surface( + tab, + state_entity, + tab.title().to_string(), + tab.url().as_str().to_string(), + Some("Rendering".to_string()), + centered_status(tab.title(), tab.url().as_str(), "Rendering page with Servo", colors::BODY), + ) } -fn render_failed_web_surface(tab: &BrowserTab, message: &str) -> AnyElement { - render_web_surface(centered_status(tab.title(), tab.url().as_str(), message, colors::ERROR)) +fn render_failed_web_surface( + tab: &BrowserTab, + message: &str, + state_entity: Entity, +) -> AnyElement { + render_web_surface( + tab, + state_entity, + tab.title().to_string(), + tab.url().as_str().to_string(), + Some("Render failed".to_string()), + centered_status(tab.title(), tab.url().as_str(), message, colors::ERROR), + ) } fn centered_status(title: &str, url: &str, detail: &str, detail_color: u32) -> impl IntoElement { @@ -314,7 +394,14 @@ fn centered_status(title: &str, url: &str, detail: &str, detail_color: u32) -> i .child(div().text_sm().text_color(rgb(detail_color)).child(detail.to_string())) } -fn render_web_surface(content: impl IntoElement) -> AnyElement { +fn render_web_surface( + tab: &BrowserTab, + state_entity: Entity, + title: String, + url: String, + detail: Option, + content: impl IntoElement, +) -> AnyElement { div() .flex_1() .h_full() @@ -328,11 +415,70 @@ fn render_web_surface(content: impl IntoElement) -> AnyElement { .border_1() .border_color(rgb(colors::HAIRLINE)) .bg(rgb(colors::SURFACE_CARD)) - .child(content), + .flex() + .flex_col() + .child(render_web_surface_header(title, url, detail)) + .child( + div() + .relative() + .flex_1() + .min_h_0() + .overflow_hidden() + .bg(rgb(colors::SURFACE_CARD)) + .child(content) + .child(render_viewport_tracker(tab.id().clone(), state_entity)), + ), ) .into_any_element() } +fn render_viewport_tracker(tab_id: TabId, state_entity: Entity) -> impl IntoElement { + canvas( + move |bounds, _window: &mut Window, cx: &mut App| { + state_entity.update(cx, |shell, cx| { + shell.record_external_web_viewport(tab_id, bounds, cx); + }); + }, + |_, _, _, _| {}, + ) + .absolute() + .size_full() +} + +fn viewport_dimension(pixels: Pixels) -> Option { + let value = f32::from(pixels.round()); + if !value.is_finite() || value < 1.0 || value > u32::MAX as f32 { + return None; + } + + Some(value as u32) +} + +fn renderable_image_buffer( + buffer: ImageBuffer, Vec>, +) -> ImageBuffer, Vec> { + let largest_edge = buffer.width().max(buffer.height()); + if largest_edge <= WEB_SURFACE_IMAGE_MAX_EDGE { + return buffer; + } + + image::imageops::resize( + &buffer, + scaled_image_dimension(buffer.width(), largest_edge), + scaled_image_dimension(buffer.height(), largest_edge), + FilterType::Triangle, + ) +} + +fn scaled_image_dimension(dimension: u32, largest_edge: u32) -> u32 { + let numerator = u64::from(dimension) * u64::from(WEB_SURFACE_IMAGE_MAX_EDGE); + let rounded = (numerator + u64::from(largest_edge / 2)) / u64::from(largest_edge); + match u32::try_from(rounded.max(1)) { + Ok(value) => value, + Err(_) => WEB_SURFACE_IMAGE_MAX_EDGE, + } +} + pub(super) fn is_external_web_url(url: &str) -> bool { url.starts_with("https://") || url.starts_with("http://") } diff --git a/crates/ely_servo_host/src/bin/ely_servo_sidecar.rs b/crates/ely_servo_host/src/bin/ely_servo_sidecar.rs index 3c19dfa..447e547 100644 --- a/crates/ely_servo_host/src/bin/ely_servo_sidecar.rs +++ b/crates/ely_servo_host/src/bin/ely_servo_sidecar.rs @@ -1,4 +1,10 @@ -use std::{env, num::ParseIntError, path::PathBuf, thread, time::Duration}; +use std::{ + env, + num::ParseIntError, + path::PathBuf, + thread, + time::{Duration, Instant}, +}; use ely_domain::{ProfileId, TabId, UrlText}; use ely_servo_host::{ @@ -10,6 +16,7 @@ use thiserror::Error; const WAIT_ITERATIONS: usize = 5_000; const WAIT_INTERVAL: Duration = Duration::from_millis(2); +const RENDER_TIMEOUT: Duration = Duration::from_secs(20); fn main() -> Result<(), SidecarError> { match parse_command(env::args())? { @@ -175,7 +182,12 @@ fn wait_for_frame( webview_id: &ely_domain::WebViewId, url: &str, ) -> Result { + let started_at = Instant::now(); for _ in 0..WAIT_ITERATIONS { + if started_at.elapsed() >= RENDER_TIMEOUT { + break; + } + host.tick(); let snapshot = host.snapshot(webview_id)?; if snapshot.has_pending_frame() { @@ -210,6 +222,7 @@ struct SnapshotReport { rgba_byte_count: usize, opaque_pixel_count: u64, non_white_pixel_count: u64, + content_pixel_count: u64, sample_hash: u64, } @@ -231,6 +244,7 @@ impl SnapshotReport { rgba_byte_count: frame.rgba_bytes().len(), opaque_pixel_count: frame.opaque_pixel_count(), non_white_pixel_count: frame.non_white_pixel_count(), + content_pixel_count: frame.content_pixel_count(), sample_hash: frame.sample_hash(), } } diff --git a/crates/ely_servo_host/src/host.rs b/crates/ely_servo_host/src/host.rs index b7e4d76..65abaf6 100644 --- a/crates/ely_servo_host/src/host.rs +++ b/crates/ely_servo_host/src/host.rs @@ -17,6 +17,7 @@ pub struct RenderedFrameSummary { height: u32, opaque_pixel_count: u64, non_white_pixel_count: u64, + content_pixel_count: u64, sample_hash: u64, } @@ -25,6 +26,7 @@ impl RenderedFrameSummary { pub fn from_rgba_bytes(width: u32, height: u32, rgba_bytes: &[u8]) -> Self { let mut opaque_pixel_count = 0; let mut non_white_pixel_count = 0; + let mut content_pixel_count = 0; let mut sample_hash = 0xcbf29ce484222325_u64; for (index, pixel) in rgba_bytes.chunks_exact(4).enumerate() { @@ -35,6 +37,9 @@ impl RenderedFrameSummary { if alpha > 0 && (red < 245 || green < 245 || blue < 245) { non_white_pixel_count += 1; } + if alpha > 0 && (red < 220 || green < 220 || blue < 220) { + content_pixel_count += 1; + } if index % 97 == 0 { for byte in pixel { sample_hash ^= u64::from(*byte); @@ -43,7 +48,14 @@ impl RenderedFrameSummary { } } - Self { width, height, opaque_pixel_count, non_white_pixel_count, sample_hash } + Self { + width, + height, + opaque_pixel_count, + non_white_pixel_count, + content_pixel_count, + sample_hash, + } } #[must_use] @@ -66,6 +78,11 @@ impl RenderedFrameSummary { self.non_white_pixel_count } + #[must_use] + pub fn content_pixel_count(&self) -> u64 { + self.content_pixel_count + } + #[must_use] pub fn sample_hash(&self) -> u64 { self.sample_hash @@ -117,6 +134,11 @@ impl RenderedFrame { self.summary.non_white_pixel_count() } + #[must_use] + pub fn content_pixel_count(&self) -> u64 { + self.summary.content_pixel_count() + } + #[must_use] pub fn sample_hash(&self) -> u64 { self.summary.sample_hash() diff --git a/crates/ely_servo_host/tests/sidecar.rs b/crates/ely_servo_host/tests/sidecar.rs index e7130f7..61fcd11 100644 --- a/crates/ely_servo_host/tests/sidecar.rs +++ b/crates/ely_servo_host/tests/sidecar.rs @@ -1,64 +1,167 @@ #![cfg(feature = "servo-engine")] -use std::{error::Error, process::Command}; +use std::{ + error::Error, + io, + process::{Child, Command, Output, Stdio}, + thread, + time::{Duration, Instant}, +}; -const WIDTH: u64 = 640; -const HEIGHT: u64 = 480; -const PRD_SITE_COMPATIBILITY_URLS: &[&str] = &["https://example.com", "https://servo.org"]; +const MINIMUM_CONTENT_PIXELS: u64 = 1_000; +const SIDECAR_TIMEOUT: Duration = Duration::from_secs(25); +const SIDECAR_POLL_INTERVAL: Duration = Duration::from_millis(20); +const PRD_SITE_COMPATIBILITY_CASES: &[PrdSiteCompatibilityCase] = &[ + PrdSiteCompatibilityCase { url: "https://example.com", title_fragment: "Example Domain" }, + PrdSiteCompatibilityCase { url: "https://servo.org", title_fragment: "Servo" }, +]; +const PRD_SITE_COMPATIBILITY_SIZES: &[FrameSize] = &[ + FrameSize { width: 640, height: 480 }, + FrameSize { width: 934, height: 657 }, + FrameSize { width: 1614, height: 980 }, +]; + +struct PrdSiteCompatibilityCase { + url: &'static str, + title_fragment: &'static str, +} + +#[derive(Clone, Copy)] +struct FrameSize { + width: u64, + height: u64, +} #[test] fn sidecar_snapshots_prd_sites_to_rgba_files() -> Result<(), Box> { - for site_url in PRD_SITE_COMPATIBILITY_URLS { - snapshot_prd_site(site_url)?; + for case in PRD_SITE_COMPATIBILITY_CASES { + for size in PRD_SITE_COMPATIBILITY_SIZES { + snapshot_prd_site(case, *size)?; + } } Ok(()) } -fn snapshot_prd_site(site_url: &str) -> Result<(), Box> { - let site_name = site_url +fn snapshot_prd_site( + case: &PrdSiteCompatibilityCase, + size: FrameSize, +) -> Result<(), Box> { + let site_name = case + .url .chars() .map(|character| if character.is_ascii_alphanumeric() { character } else { '-' }) .collect::(); - let output_path = std::env::temp_dir() - .join(format!("ely-servo-sidecar-{}-{site_name}.rgba", std::process::id())); + let output_path = std::env::temp_dir().join(format!( + "ely-servo-sidecar-{}-{site_name}-{}x{}.rgba", + std::process::id(), + size.width, + size.height + )); if output_path.exists() { std::fs::remove_file(&output_path)?; } - let output = Command::new(env!("CARGO_BIN_EXE_ely_servo_sidecar")) - .arg("snapshot") - .arg("--url") - .arg(site_url) - .arg("--rgba-out") - .arg(&output_path) - .arg("--width") - .arg(WIDTH.to_string()) - .arg("--height") - .arg(HEIGHT.to_string()) - .output()?; + let output = run_sidecar_snapshot(case.url, &output_path, size)?; assert!( output.status.success(), - "{site_url}\nstatus: {:?}\nstdout: {}\nstderr: {}", + "{} {}x{}\nstatus: {:?}\nstdout: {}\nstderr: {}", + case.url, + size.width, + size.height, output.status.code(), String::from_utf8_lossy(&output.stdout), String::from_utf8_lossy(&output.stderr) ); let report: serde_json::Value = serde_json::from_slice(&output.stdout)?; - assert_eq!(report_field_as_u64(&report, "width")?, WIDTH, "{site_url}"); - assert_eq!(report_field_as_u64(&report, "height")?, HEIGHT, "{site_url}"); - assert_eq!(report_field_as_u64(&report, "rgba_byte_count")?, WIDTH * HEIGHT * 4, "{site_url}"); - assert!(report_field_as_u64(&report, "non_white_pixel_count")? > 0, "{site_url}"); - assert!(report_field_as_u64(&report, "sample_hash")? > 0, "{site_url}"); - assert_eq!(std::fs::metadata(&output_path)?.len(), WIDTH * HEIGHT * 4); + assert_eq!(report_field_as_u64(&report, "width")?, size.width, "{}", case.url); + assert_eq!(report_field_as_u64(&report, "height")?, size.height, "{}", case.url); + assert_eq!( + report_field_as_u64(&report, "rgba_byte_count")?, + size.width * size.height * 4, + "{}", + case.url + ); + assert_report_text_contains(&report, "loaded_url", case.url)?; + assert_report_text_contains(&report, "title", case.title_fragment)?; + assert!(report_field_as_u64(&report, "non_white_pixel_count")? > 0, "{}", case.url); + assert!( + report_field_as_u64(&report, "content_pixel_count")? >= MINIMUM_CONTENT_PIXELS, + "{}", + case.url + ); + assert!(report_field_as_u64(&report, "sample_hash")? > 0, "{}", case.url); + assert_eq!(std::fs::metadata(&output_path)?.len(), size.width * size.height * 4); std::fs::remove_file(&output_path)?; Ok(()) } +fn run_sidecar_snapshot( + site_url: &str, + output_path: &std::path::Path, + size: FrameSize, +) -> Result> { + let mut child = Command::new(env!("CARGO_BIN_EXE_ely_servo_sidecar")) + .arg("snapshot") + .arg("--url") + .arg(site_url) + .arg("--rgba-out") + .arg(output_path) + .arg("--width") + .arg(size.width.to_string()) + .arg("--height") + .arg(size.height.to_string()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn()?; + + let started_at = Instant::now(); + loop { + if child.try_wait()?.is_some() { + return child.wait_with_output().map_err(Into::into); + } + + if started_at.elapsed() >= SIDECAR_TIMEOUT { + terminate_child(child)?; + return Err(format!( + "timed out rendering {site_url} at {}x{}", + size.width, size.height + ) + .into()); + } + + thread::sleep(SIDECAR_POLL_INTERVAL); + } +} + +fn terminate_child(mut child: Child) -> Result<(), Box> { + match child.kill() { + Ok(()) => { + let _output = child.wait_with_output()?; + Ok(()) + } + Err(error) if error.kind() == io::ErrorKind::InvalidInput => Ok(()), + Err(error) => Err(error.into()), + } +} + +fn assert_report_text_contains( + report: &serde_json::Value, + field: &'static str, + fragment: &str, +) -> Result<(), Box> { + let value = report + .get(field) + .and_then(serde_json::Value::as_str) + .ok_or_else(|| format!("missing text report field: {field}"))?; + assert!(value.contains(fragment), "{field}: {value}"); + Ok(()) +} + fn report_field_as_u64( report: &serde_json::Value, field: &'static str, diff --git a/crates/ely_servo_host/tests/software_host.rs b/crates/ely_servo_host/tests/software_host.rs index 0f1782e..4db3065 100644 --- a/crates/ely_servo_host/tests/software_host.rs +++ b/crates/ely_servo_host/tests/software_host.rs @@ -7,7 +7,16 @@ use ely_servo_host::{ NavigationRequest, ServoHost, ServoHostError, ServoSurfaceSize, SoftwareServoHost, WebViewState, }; -const PRD_SITE_COMPATIBILITY_URLS: &[&str] = &["https://example.com", "https://servo.org"]; +const MINIMUM_CONTENT_PIXELS: u64 = 1_000; +const PRD_SITE_COMPATIBILITY_CASES: &[PrdSiteCompatibilityCase] = &[ + PrdSiteCompatibilityCase { url: "https://example.com", title_fragment: "Example Domain" }, + PrdSiteCompatibilityCase { url: "https://servo.org", title_fragment: "Servo" }, +]; + +struct PrdSiteCompatibilityCase { + url: &'static str, + title_fragment: &'static str, +} #[test] fn manages_real_servo_webview_lifecycle() -> Result<(), Box> { @@ -35,22 +44,28 @@ fn manages_real_servo_webview_lifecycle() -> Result<(), Box> { snapshot.url().is_some_and(|value| value.starts_with("data:text/html,")), "snapshot: {snapshot:?}" ); - assert_rendered_frame_has_content(&host, "data:text/html")?; + assert_rendered_frame_has_content(&host, "data:text/html", 1)?; let mut previous_frame_hash = Some(host.last_rendered_frame()?.sample_hash()); - for site_url in PRD_SITE_COMPATIBILITY_URLS { + for site in PRD_SITE_COMPATIBILITY_CASES { let tab_id = TabId::new(); - let url = UrlText::parse(*site_url)?; + let url = UrlText::parse(site.url)?; host.navigate(NavigationRequest { webview_id: webview_id.clone(), tab_id, url })?; let snapshot = wait_for_rendered_webview(&mut host, &webview_id, previous_frame_hash)?; - assert_eq!(snapshot.state(), &WebViewState::Complete, "{site_url}: {snapshot:?}"); + assert_eq!(snapshot.state(), &WebViewState::Complete, "{}: {snapshot:?}", site.url); assert!( - snapshot.url().is_some_and(|value| value.starts_with(site_url)), - "{site_url}: {snapshot:?}" + snapshot.url().is_some_and(|value| value.starts_with(site.url)), + "{}: {snapshot:?}", + site.url ); - assert_rendered_frame_has_content(&host, site_url)?; + assert!( + snapshot.title().is_some_and(|value| value.contains(site.title_fragment)), + "{}: {snapshot:?}", + site.url + ); + assert_rendered_frame_has_content(&host, site.url, MINIMUM_CONTENT_PIXELS)?; previous_frame_hash = Some(host.last_rendered_frame()?.sample_hash()); } @@ -96,6 +111,7 @@ fn wait_for_rendered_webview( fn assert_rendered_frame_has_content( host: &SoftwareServoHost, label: &str, + minimum_content_pixels: u64, ) -> Result<(), Box> { let frame = host.last_rendered_frame()?; @@ -103,6 +119,7 @@ fn assert_rendered_frame_has_content( assert_eq!(frame.height(), 480, "{label}: {frame:?}"); assert!(frame.opaque_pixel_count() > 0, "{label}: {frame:?}"); assert!(frame.non_white_pixel_count() > 0, "{label}: {frame:?}"); + assert!(frame.content_pixel_count() >= minimum_content_pixels, "{label}: {frame:?}"); assert_ne!(frame.sample_hash(), 0, "{label}: {frame:?}"); Ok(()) }