diff --git a/crates/ely_app/src/services/mod.rs b/crates/ely_app/src/services/mod.rs index ac69174..1977cd5 100644 --- a/crates/ely_app/src/services/mod.rs +++ b/crates/ely_app/src/services/mod.rs @@ -6,6 +6,9 @@ pub mod plugin_signatures; mod servo_profile_data; pub mod servo_sidecar; mod servo_sidecar_command; +mod servo_sidecar_request; + +pub(crate) use servo_profile_data::ProfileDataMode; #[cfg(all(test, feature = "live-site-smoke"))] pub(crate) mod prd_live_sites; diff --git a/crates/ely_app/src/services/servo_profile_data.rs b/crates/ely_app/src/services/servo_profile_data.rs index beed14e..3f1c50f 100644 --- a/crates/ely_app/src/services/servo_profile_data.rs +++ b/crates/ely_app/src/services/servo_profile_data.rs @@ -1,4 +1,8 @@ -use std::path::{Path, PathBuf}; +use std::{ + env, + path::{Path, PathBuf}, + time::{SystemTime, SystemTimeError, UNIX_EPOCH}, +}; use directories::ProjectDirs; use ely_domain::ProfileId; @@ -15,3 +19,20 @@ pub(super) fn default_profile_data_root() -> Option { pub(super) fn profile_data_dir(profile_data_root: &Path, profile_id: &ProfileId) -> PathBuf { profile_data_root.join(profile_id.as_str()).join("servo") } + +pub(super) fn transient_profile_data_dir( + profile_id: &ProfileId, +) -> Result { + let timestamp = SystemTime::now().duration_since(UNIX_EPOCH)?.as_nanos(); + Ok(env::temp_dir().join("ely-browser-servo-profiles").join(format!( + "{}-{}-{timestamp}", + std::process::id(), + profile_id.as_str() + ))) +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum ProfileDataMode { + Persistent, + Transient, +} diff --git a/crates/ely_app/src/services/servo_sidecar.rs b/crates/ely_app/src/services/servo_sidecar.rs index 5604b0d..a68a385 100644 --- a/crates/ely_app/src/services/servo_sidecar.rs +++ b/crates/ely_app/src/services/servo_sidecar.rs @@ -6,12 +6,16 @@ use std::{ time::{Duration, Instant, SystemTime, SystemTimeError, UNIX_EPOCH}, }; -use ely_domain::{ProfileId, UrlText}; +use ely_domain::ProfileId; use serde::Deserialize; use thiserror::Error; +pub use super::servo_sidecar_request::SidecarSnapshotRequest; + use super::{ - servo_profile_data::{default_profile_data_root, profile_data_dir}, + servo_profile_data::{ + ProfileDataMode, default_profile_data_root, profile_data_dir, transient_profile_data_dir, + }, servo_sidecar_command::{SidecarCommandTarget, default_sidecar_command}, }; @@ -63,29 +67,30 @@ impl ServoSidecarClient { request: &SidecarSnapshotRequest, ) -> Result { let rgba_path = temporary_rgba_path()?; - let profile_data_dir = profile_data_dir(&self.profile_data_root, &request.profile_id); + let profile_data_dir = request.profile_data_dir(&self.profile_data_root)?; let output = match self.run_snapshot_command(request, &rgba_path, &profile_data_dir) { Ok(output) => output, Err(error) => { - remove_temporary_file(&rgba_path)?; - return Err(error); + return cleanup_failed_snapshot(request, &rgba_path, &profile_data_dir, error); } }; if !output.status.success() { - remove_temporary_file(&rgba_path)?; - return Err(ServoSidecarError::SidecarFailed { + let error = ServoSidecarError::SidecarFailed { status: output.status.to_string(), stderr: String::from_utf8_lossy(&output.stderr).trim().to_string(), - }); + }; + return cleanup_failed_snapshot(request, &rgba_path, &profile_data_dir, error); } let snapshot = read_sidecar_snapshot(&output.stdout, &rgba_path, &request.profile_id); let cleanup = remove_temporary_file(&rgba_path); - match (snapshot, cleanup) { - (Ok(snapshot), Ok(())) => Ok(snapshot), - (Err(error), Ok(())) => Err(error), - (Ok(_), Err(error)) | (Err(_), Err(error)) => Err(error), + let profile_cleanup = request.cleanup_profile_data_dir(&profile_data_dir); + match (snapshot, cleanup, profile_cleanup) { + (Ok(snapshot), Ok(()), Ok(())) => Ok(snapshot), + (Err(error), Ok(()), Ok(())) => Err(error), + (Ok(_), Err(error), _) | (Err(_), Err(error), _) => Err(error), + (Ok(_), Ok(()), Err(error)) | (Err(_), Ok(()), Err(error)) => Err(error), } } @@ -160,60 +165,24 @@ fn append_snapshot_args( .arg(request.scroll_y.to_string()); } -#[derive(Clone, Debug)] -pub struct SidecarSnapshotRequest { - url: UrlText, - profile_id: ProfileId, - width: u32, - height: u32, - scroll_x: i32, - scroll_y: i32, - click_point: Option, - typed_text: Option, -} - impl SidecarSnapshotRequest { - #[must_use] - pub fn new(url: UrlText, profile_id: ProfileId, width: u32, height: u32) -> Self { - Self { - url, - profile_id, - width, - height, - scroll_x: 0, - scroll_y: 0, - click_point: None, - typed_text: None, + fn profile_data_dir(&self, profile_data_root: &Path) -> Result { + match self.profile_data_mode { + ProfileDataMode::Persistent => { + Ok(profile_data_dir(profile_data_root, &self.profile_id)) + } + ProfileDataMode::Transient => { + transient_profile_data_dir(&self.profile_id).map_err(ServoSidecarError::SystemClock) + } } } - #[must_use] - pub fn with_scroll_offset(mut self, scroll_x: i32, scroll_y: i32) -> Self { - self.scroll_x = scroll_x; - self.scroll_y = scroll_y; - self - } + fn cleanup_profile_data_dir(&self, profile_data_dir: &Path) -> Result<(), ServoSidecarError> { + if self.profile_data_mode == ProfileDataMode::Persistent { + return Ok(()); + } - #[must_use] - pub fn with_click_point(mut self, x: u32, y: u32) -> Self { - self.click_point = Some(SidecarClickPoint { x, y }); - self - } - - #[must_use] - pub fn with_typed_text(mut self, typed_text: String) -> Self { - self.typed_text = Some(typed_text); - self - } - - #[cfg(test)] - pub(crate) fn typed_text_for_test(&self) -> Option<&str> { - self.typed_text.as_deref() - } - - #[cfg(test)] - pub(crate) fn profile_id_for_test(&self) -> &ProfileId { - &self.profile_id + remove_temporary_directory(profile_data_dir) } fn max_attempts(&self) -> usize { @@ -225,12 +194,6 @@ impl SidecarSnapshotRequest { } } -#[derive(Clone, Copy, Debug)] -struct SidecarClickPoint { - x: u32, - y: u32, -} - #[derive(Clone, Debug)] pub struct SidecarSnapshot { loaded_url: Option, @@ -360,6 +323,9 @@ pub enum ServoSidecarError { #[error("failed to remove servo frame file: {0}")] FrameCleanup(#[source] io::Error), + #[error("failed to remove transient servo profile data at {path}: {source}")] + ProfileDataCleanup { path: PathBuf, source: io::Error }, + #[error("servo sidecar returned incomplete render state: {state}")] IncompleteRender { state: String }, @@ -416,6 +382,31 @@ fn remove_temporary_file(path: &Path) -> Result<(), ServoSidecarError> { } } +fn remove_temporary_directory(path: &Path) -> Result<(), ServoSidecarError> { + match fs::remove_dir_all(path) { + Ok(()) => Ok(()), + Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(()), + Err(error) => { + Err(ServoSidecarError::ProfileDataCleanup { path: path.to_path_buf(), source: error }) + } + } +} + +fn cleanup_failed_snapshot( + request: &SidecarSnapshotRequest, + rgba_path: &Path, + profile_data_dir: &Path, + snapshot_error: ServoSidecarError, +) -> Result { + let frame_cleanup = remove_temporary_file(rgba_path); + let profile_cleanup = request.cleanup_profile_data_dir(profile_data_dir); + match (frame_cleanup, profile_cleanup) { + (Ok(()), Ok(())) => Err(snapshot_error), + (Err(error), _) => Err(error), + (Ok(()), Err(error)) => Err(error), + } +} + fn terminate_child(mut child: std::process::Child) -> Result<(), ServoSidecarError> { match child.kill() { Ok(()) => { diff --git a/crates/ely_app/src/services/servo_sidecar_request.rs b/crates/ely_app/src/services/servo_sidecar_request.rs new file mode 100644 index 0000000..d7a2e6b --- /dev/null +++ b/crates/ely_app/src/services/servo_sidecar_request.rs @@ -0,0 +1,79 @@ +use ely_domain::{ProfileId, UrlText}; + +use super::ProfileDataMode; + +#[derive(Clone, Debug)] +pub struct SidecarSnapshotRequest { + pub(in crate::services) url: UrlText, + pub(in crate::services) profile_id: ProfileId, + pub(in crate::services) profile_data_mode: ProfileDataMode, + pub(in crate::services) width: u32, + pub(in crate::services) height: u32, + pub(in crate::services) scroll_x: i32, + pub(in crate::services) scroll_y: i32, + pub(in crate::services) click_point: Option, + pub(in crate::services) typed_text: Option, +} + +impl SidecarSnapshotRequest { + #[must_use] + pub fn new(url: UrlText, profile_id: ProfileId, width: u32, height: u32) -> Self { + Self { + url, + profile_id, + profile_data_mode: ProfileDataMode::Persistent, + width, + height, + scroll_x: 0, + scroll_y: 0, + click_point: None, + typed_text: None, + } + } + + #[must_use] + pub fn with_profile_data_mode(mut self, profile_data_mode: ProfileDataMode) -> Self { + self.profile_data_mode = profile_data_mode; + self + } + + #[must_use] + pub fn with_scroll_offset(mut self, scroll_x: i32, scroll_y: i32) -> Self { + self.scroll_x = scroll_x; + self.scroll_y = scroll_y; + self + } + + #[must_use] + pub fn with_click_point(mut self, x: u32, y: u32) -> Self { + self.click_point = Some(SidecarClickPoint { x, y }); + self + } + + #[must_use] + pub fn with_typed_text(mut self, typed_text: String) -> Self { + self.typed_text = Some(typed_text); + self + } + + #[cfg(test)] + pub(crate) fn typed_text_for_test(&self) -> Option<&str> { + self.typed_text.as_deref() + } + + #[cfg(test)] + pub(crate) fn profile_id_for_test(&self) -> &ProfileId { + &self.profile_id + } + + #[cfg(test)] + pub(crate) fn profile_data_mode_for_test(&self) -> ProfileDataMode { + self.profile_data_mode + } +} + +#[derive(Clone, Copy, Debug)] +pub(in crate::services) struct SidecarClickPoint { + pub(in crate::services) x: u32, + pub(in crate::services) y: u32, +} diff --git a/crates/ely_app/src/services/servo_sidecar_tests.rs b/crates/ely_app/src/services/servo_sidecar_tests.rs index 64ac083..2c68691 100644 --- a/crates/ely_app/src/services/servo_sidecar_tests.rs +++ b/crates/ely_app/src/services/servo_sidecar_tests.rs @@ -1,5 +1,7 @@ use std::error::Error; +use ely_domain::UrlText; + use super::*; #[cfg(feature = "live-site-smoke")] @@ -82,6 +84,46 @@ fn keeps_page_interactions_single_attempt() -> Result<(), Box> { Ok(()) } +#[test] +fn persistent_profile_data_uses_profile_root() -> Result<(), Box> { + let profile_id = ProfileId::new(); + let request = SidecarSnapshotRequest::new( + UrlText::parse("https://example.com")?, + profile_id.clone(), + 2, + 1, + ); + let root = std::env::temp_dir().join("ely-browser-profile-root-test"); + + assert_eq!(request.profile_data_mode_for_test(), ProfileDataMode::Persistent); + assert_eq!(request.profile_data_dir(&root)?, root.join(profile_id.as_str()).join("servo")); + Ok(()) +} + +#[test] +fn transient_profile_data_uses_temporary_directory_and_cleans_up() -> Result<(), Box> { + let profile_id = ProfileId::new(); + let request = SidecarSnapshotRequest::new( + UrlText::parse("https://example.com")?, + profile_id.clone(), + 2, + 1, + ) + .with_profile_data_mode(ProfileDataMode::Transient); + let root = std::env::temp_dir().join("ely-browser-profile-root-test"); + let profile_data_dir = request.profile_data_dir(&root)?; + + assert!(profile_data_dir.starts_with(std::env::temp_dir().join("ely-browser-servo-profiles"))); + assert!(profile_data_dir.to_string_lossy().contains(profile_id.as_str())); + std::fs::create_dir_all(&profile_data_dir)?; + std::fs::write(profile_data_dir.join("probe"), b"private")?; + + request.cleanup_profile_data_dir(&profile_data_dir)?; + + assert!(!profile_data_dir.exists()); + Ok(()) +} + #[cfg(feature = "live-site-smoke")] #[test] fn desktop_sidecar_opens_prd_top_sites() -> Result<(), Box> { diff --git a/crates/ely_app/src/shell/internal_pages.rs b/crates/ely_app/src/shell/internal_pages.rs index fb37e0a..5789d1b 100644 --- a/crates/ely_app/src/shell/internal_pages.rs +++ b/crates/ely_app/src/shell/internal_pages.rs @@ -94,7 +94,7 @@ impl ElyShell { "ely://settings/updates" => self.render_updates_page(snapshot), "ely://sync/status" => self.render_sync_page(snapshot, cx), url if super::web_surface::is_external_web_url(url) => { - self.render_external_web_canvas(tab, cx) + self.render_external_web_canvas(tab, snapshot, cx) } _ => render_default_page(tab), } diff --git a/crates/ely_app/src/shell/web_surface.rs b/crates/ely_app/src/shell/web_surface.rs index 3398768..7d730d5 100644 --- a/crates/ely_app/src/shell/web_surface.rs +++ b/crates/ely_app/src/shell/web_surface.rs @@ -3,7 +3,7 @@ use std::collections::BTreeMap; use ely_domain::{BrowserTab, TabId}; use gpui::{Bounds, Pixels, Point}; -use crate::services::servo_sidecar::SidecarSnapshotRequest; +use crate::services::{ProfileDataMode, servo_sidecar::SidecarSnapshotRequest}; use super::{ web_surface_frame::WebSurfaceFrame, @@ -47,7 +47,11 @@ impl WebSurfaceStore { self.states.get(tab_id) } - pub(super) fn prepare_request(&mut self, tab: &BrowserTab) -> Option { + pub(super) fn prepare_request( + &mut self, + tab: &BrowserTab, + profile_data_mode: ProfileDataMode, + ) -> Option { if !is_external_web_url(tab.url().as_str()) { return None; } @@ -108,6 +112,7 @@ impl WebSurfaceStore { size.width, size.height, ) + .with_profile_data_mode(profile_data_mode) .with_scroll_offset(scroll_offset.x(), scroll_offset.y()); if let Some(click_point) = click_point { snapshot_request = snapshot_request.with_click_point(click_point.x(), click_point.y()); @@ -410,7 +415,7 @@ mod tests { use ely_domain::{BrowserTab, ProfileId, SpaceId, TabId, UrlText}; use gpui::{Bounds, point, px, size}; - use super::WebSurfaceStore; + use super::{ProfileDataMode, WebSurfaceStore}; #[test] fn typed_text_enters_snapshot_request_after_clicked_viewport() -> Result<(), Box> { @@ -427,7 +432,9 @@ mod tests { assert!(store.record_typed_text(tab.id(), tab.url().as_str(), "e")); assert!(store.record_typed_text(tab.id(), tab.url().as_str(), "l")); - let request = store.prepare_request(&tab).ok_or("missing web surface request")?; + let request = store + .prepare_request(&tab, ProfileDataMode::Persistent) + .ok_or("missing web surface request")?; assert_eq!(request.typed_text.as_deref(), Some("el")); assert_eq!(request.snapshot_request.typed_text_for_test(), Some("el")); @@ -435,6 +442,25 @@ mod tests { Ok(()) } + #[test] + fn private_profile_enters_transient_snapshot_request() -> Result<(), Box> { + let mut store = WebSurfaceStore::new(); + let tab = web_tab("https://example.com/private")?; + + let bounds = Bounds::new(point(px(0.0), px(0.0)), size(px(640.0), px(480.0))); + assert!(store.record_viewport_size(tab.id(), bounds)); + + let request = store + .prepare_request(&tab, ProfileDataMode::Transient) + .ok_or("missing web surface request")?; + + assert_eq!( + request.snapshot_request.profile_data_mode_for_test(), + ProfileDataMode::Transient + ); + Ok(()) + } + fn web_tab(url: &str) -> Result> { Ok(BrowserTab::new( TabId::new(), diff --git a/crates/ely_app/src/shell/web_surface_controller.rs b/crates/ely_app/src/shell/web_surface_controller.rs index 6792e6c..30dcc0c 100644 --- a/crates/ely_app/src/shell/web_surface_controller.rs +++ b/crates/ely_app/src/shell/web_surface_controller.rs @@ -1,7 +1,11 @@ -use ely_domain::{BrowserTab, TabId}; +use ely_browser_core::BrowserSnapshot; +use ely_domain::{BrowserTab, ProfileKind, TabId}; use gpui::{AnyElement, Bounds, Context, Pixels, Point}; -use crate::services::servo_sidecar::{ServoSidecarError, SidecarSnapshot}; +use crate::services::{ + ProfileDataMode, + servo_sidecar::{ServoSidecarError, SidecarSnapshot}, +}; use super::{ ElyShell, @@ -26,11 +30,16 @@ impl ElyShell { pub(super) fn render_external_web_canvas( &mut self, tab: &BrowserTab, + snapshot: &BrowserSnapshot, cx: &mut Context, ) -> AnyElement { - self.ensure_external_web_frame(tab, cx); - let state_entity = cx.entity().clone(); + let Some(profile_data_mode) = profile_data_mode_for(tab, snapshot) else { + return render_failed_web_surface(tab, "Profile context is unavailable.", state_entity); + }; + + self.ensure_external_web_frame(tab, profile_data_mode, cx); + match self.web_surfaces.state(tab.id()) { Some(WebSurfaceState::Ready(frame)) => { render_ready_web_surface(frame, tab, state_entity) @@ -47,8 +56,13 @@ impl ElyShell { } } - fn ensure_external_web_frame(&mut self, tab: &BrowserTab, cx: &mut Context) { - let Some(request) = self.web_surfaces.prepare_request(tab) else { + fn ensure_external_web_frame( + &mut self, + tab: &BrowserTab, + profile_data_mode: ProfileDataMode, + cx: &mut Context, + ) { + let Some(request) = self.web_surfaces.prepare_request(tab, profile_data_mode) else { return; }; @@ -190,3 +204,12 @@ impl ElyShell { false } } + +fn profile_data_mode_for(tab: &BrowserTab, snapshot: &BrowserSnapshot) -> Option { + snapshot.profiles.iter().find(|profile| profile.id() == tab.profile_id()).map(|profile| { + match profile.kind() { + ProfileKind::Standard => ProfileDataMode::Persistent, + ProfileKind::Private => ProfileDataMode::Transient, + } + }) +} diff --git a/crates/ely_app/src/shell/web_surface_live_site_tests.rs b/crates/ely_app/src/shell/web_surface_live_site_tests.rs index dd026d9..8282bdc 100644 --- a/crates/ely_app/src/shell/web_surface_live_site_tests.rs +++ b/crates/ely_app/src/shell/web_surface_live_site_tests.rs @@ -4,6 +4,7 @@ use ely_domain::{BrowserTab, ProfileId, SpaceId, TabId, UrlText}; use gpui::{Bounds, point, px, size}; use crate::{ + services::ProfileDataMode, services::prd_live_sites::{ LiveSiteCase, PRD_REFERENCE_SITE_CASES, PRD_TOP_SITE_CASES, assert_prd_reference_urls_are_covered, @@ -42,7 +43,7 @@ fn assert_web_surfaces_render(cases: &[LiveSiteCase]) -> Result<(), Box