diff --git a/crates/ely_app/src/services/servo_live.rs b/crates/ely_app/src/services/servo_live.rs index 27ea938..656a22b 100644 --- a/crates/ely_app/src/services/servo_live.rs +++ b/crates/ely_app/src/services/servo_live.rs @@ -13,11 +13,14 @@ use std::collections::BTreeSet; mod iosurface_importer; #[path = "servo_live_ipc.rs"] mod ipc; +#[path = "servo_live_permission_grant.rs"] +mod permission_grant; #[path = "servo_live_types.rs"] mod types; #[path = "servo_live_wire.rs"] mod wire; +pub(crate) use permission_grant::ServoLivePermissionGrant; pub(crate) use types::{ ServoLiveEnsureRequest, ServoLiveError, ServoLiveFrame, ServoLiveSitePermission, }; @@ -53,6 +56,7 @@ pub(crate) struct ServoLiveClient { ipc: ServoLiveIpc, timeouts: SidecarTimeouts, active: bool, + pending_permission_consumptions: Vec, #[cfg(target_os = "macos")] iosurface_cache: IOSurfaceCache, #[cfg(target_os = "macos")] @@ -115,6 +119,7 @@ impl ServoLiveClient { ipc: ServoLiveIpc::spawn(stdin, stdout), timeouts, active: true, + pending_permission_consumptions: Vec::new(), #[cfg(target_os = "macos")] iosurface_cache: IOSurfaceCache::new(), #[cfg(target_os = "macos")] @@ -155,6 +160,7 @@ impl ServoLiveClient { hover_x: request.hover_x, hover_y: request.hover_y, typed_text: request.typed_text, + site_permission_generation: request.site_permission_generation, site_permissions: request.site_permissions, ready_surface_ids, pending_surface_ids, @@ -184,6 +190,7 @@ impl ServoLiveClient { if reply.frame.is_some() || reply.surface_handle.is_some() || reply.current_surface_id.is_some() + || !reply.permission_consumptions.is_empty() { return Err(ServoLiveError::InvalidResponse { message: "handshake response contains a frame", @@ -197,6 +204,7 @@ impl ServoLiveClient { fn request(&mut self, request: LiveRequest) -> Result, ServoLiveError> { let reply = self.exchange(request, self.timeouts.request, "request")?; + self.pending_permission_consumptions.extend(reply.permission_consumptions); if let Some(message) = reply.error { return Err(ServoLiveError::SidecarFailed { message }); } @@ -224,6 +232,10 @@ impl ServoLiveClient { Ok(frame) } + pub(crate) fn take_permission_consumptions(&mut self) -> Vec { + std::mem::take(&mut self.pending_permission_consumptions) + } + fn exchange( &mut self, request: LiveRequest, diff --git a/crates/ely_app/src/services/servo_live_ipc.rs b/crates/ely_app/src/services/servo_live_ipc.rs index 411df31..462ecfb 100644 --- a/crates/ely_app/src/services/servo_live_ipc.rs +++ b/crates/ely_app/src/services/servo_live_ipc.rs @@ -7,7 +7,7 @@ use std::{ }; use super::{ - ServoLiveError, ServoLiveFrame, + ServoLiveError, ServoLiveFrame, ServoLivePermissionGrant, wire::{ LiveRequest, LiveResponse, LiveSurfaceHandle, MAX_FRAME_BYTE_COUNT, MAX_FRAME_DIMENSION, }, @@ -26,6 +26,7 @@ pub(super) struct IpcReply { pub(super) frame: Option, pub(super) surface_handle: Option, pub(super) current_surface_id: Option, + pub(super) permission_consumptions: Vec, } struct IpcRequest { @@ -194,6 +195,11 @@ fn read_reply(stdout: &mut impl BufRead) -> Result { frame: frame.transpose()?, surface_handle: response.surface_handle, current_surface_id: response.current_surface_id, + permission_consumptions: response + .permission_consumptions + .into_iter() + .map(ServoLivePermissionGrant::try_from) + .collect::, _>>()?, }) } @@ -240,7 +246,7 @@ mod tests { #[test] fn reply_rejects_oversized_frame_before_readback_allocation() { let header = format!( - "{{\"protocol_version\":2,\"error\":null,\"frame\":{{\"loaded_url\":null,\"title\":null,\"state\":\"complete\",\"width\":{0},\"height\":{0},\"device_pixel_ratio\":1.0,\"css_viewport_width\":{0},\"css_viewport_height\":{0},\"rgba_byte_count\":1073741824,\"pixels_changed\":true}}}}\n", + "{{\"protocol_version\":3,\"error\":null,\"frame\":{{\"loaded_url\":null,\"title\":null,\"state\":\"complete\",\"width\":{0},\"height\":{0},\"device_pixel_ratio\":1.0,\"css_viewport_width\":{0},\"css_viewport_height\":{0},\"rgba_byte_count\":1073741824,\"pixels_changed\":true}}}}\n", MAX_FRAME_DIMENSION ); let mut input = Cursor::new(header.into_bytes()); @@ -251,11 +257,34 @@ mod tests { )); } + #[test] + fn reply_parses_permission_consumption_without_a_frame() -> Result<(), ServoLiveError> { + let profile_id = ely_domain::ProfileId::new(); + let header = format!( + "{{\"protocol_version\":3,\"error\":null,\"frame\":null,\"permission_consumptions\":[{{\"profile_id\":\"{}\",\"origin\":\"https://example.com\",\"feature\":\"camera\",\"grant_revision\":7}}]}}\n", + profile_id.as_str(), + ); + let mut input = Cursor::new(header.into_bytes()); + + let reply = read_reply(&mut input)?; + + let [consumed] = reply.permission_consumptions.as_slice() else { + return Err(ServoLiveError::InvalidResponse { + message: "permission consumption was missing", + }); + }; + assert_eq!(consumed.profile_id(), &profile_id); + assert_eq!(consumed.origin().as_str(), "https://example.com"); + assert_eq!(consumed.feature(), ely_domain::SitePermissionFeature::Camera); + assert_eq!(consumed.grant_revision(), 7); + Ok(()) + } + #[cfg(target_os = "macos")] #[test] fn hardware_reply_uses_surface_without_rgba_allocation() -> Result<(), ServoLiveError> { let header = concat!( - "{\"protocol_version\":2,\"error\":null,", + "{\"protocol_version\":3,\"error\":null,", "\"surface_handle\":{\"mach_port_name\":91,\"surface_id\":7,\"width\":64,\"height\":48},", "\"current_surface_id\":7,", "\"frame\":{\"loaded_url\":null,\"title\":null,\"state\":\"complete\",", @@ -307,7 +336,7 @@ mod tests { #[cfg(target_os = "macos")] fn hardware_header(current_surface_id: u64, handle_width: u32, handle_height: u32) -> String { format!( - "{{\"protocol_version\":2,\"error\":null,\"surface_handle\":{{\"mach_port_name\":91,\"surface_id\":7,\"width\":{handle_width},\"height\":{handle_height}}},\"current_surface_id\":{current_surface_id},\"frame\":{{\"loaded_url\":null,\"title\":null,\"state\":\"complete\",\"width\":64,\"height\":48,\"device_pixel_ratio\":1.0,\"css_viewport_width\":64,\"css_viewport_height\":48,\"rgba_byte_count\":0,\"pixels_changed\":true}}}}\n" + "{{\"protocol_version\":3,\"error\":null,\"surface_handle\":{{\"mach_port_name\":91,\"surface_id\":7,\"width\":{handle_width},\"height\":{handle_height}}},\"current_surface_id\":{current_surface_id},\"frame\":{{\"loaded_url\":null,\"title\":null,\"state\":\"complete\",\"width\":64,\"height\":48,\"device_pixel_ratio\":1.0,\"css_viewport_width\":64,\"css_viewport_height\":48,\"rgba_byte_count\":0,\"pixels_changed\":true}}}}\n" ) } } diff --git a/crates/ely_app/src/services/servo_live_permission_grant.rs b/crates/ely_app/src/services/servo_live_permission_grant.rs new file mode 100644 index 0000000..e8463be --- /dev/null +++ b/crates/ely_app/src/services/servo_live_permission_grant.rs @@ -0,0 +1,51 @@ +use ely_domain::{ProfileId, SiteOrigin, SitePermissionFeature}; + +use super::wire::LivePermissionConsumption; + +#[derive(Clone, Debug, Eq, PartialEq)] +pub(crate) struct ServoLivePermissionGrant { + profile_id: ProfileId, + origin: SiteOrigin, + feature: SitePermissionFeature, + grant_revision: u64, +} + +impl ServoLivePermissionGrant { + pub(crate) fn new( + profile_id: ProfileId, + origin: SiteOrigin, + feature: SitePermissionFeature, + grant_revision: u64, + ) -> Self { + Self { profile_id, origin, feature, grant_revision } + } + + pub(crate) fn profile_id(&self) -> &ProfileId { + &self.profile_id + } + + pub(crate) fn origin(&self) -> &SiteOrigin { + &self.origin + } + + pub(crate) fn feature(&self) -> SitePermissionFeature { + self.feature + } + + pub(crate) fn grant_revision(&self) -> u64 { + self.grant_revision + } +} + +impl TryFrom for ServoLivePermissionGrant { + type Error = ely_domain::DomainError; + + fn try_from(consumed: LivePermissionConsumption) -> Result { + Ok(Self { + profile_id: ProfileId::parse(consumed.profile_id)?, + origin: SiteOrigin::parse(consumed.origin)?, + feature: SitePermissionFeature::parse(&consumed.feature)?, + grant_revision: consumed.grant_revision, + }) + } +} diff --git a/crates/ely_app/src/services/servo_live_types.rs b/crates/ely_app/src/services/servo_live_types.rs index da908cb..b45b952 100644 --- a/crates/ely_app/src/services/servo_live_types.rs +++ b/crates/ely_app/src/services/servo_live_types.rs @@ -2,11 +2,10 @@ use std::sync::Arc; use std::{collections::TryReserveError, io, path::PathBuf}; -use ely_domain::SitePermissionDecision; use serde::Serialize; use thiserror::Error; -use super::wire::LiveFrameReport; +use super::{ServoLivePermissionGrant, wire::LiveFrameReport}; use crate::services::servo_sidecar_command::SidecarCommandError; #[cfg(target_os = "macos")] @@ -34,23 +33,27 @@ pub(crate) struct ServoLiveEnsureRequest { pub(crate) hover_x: Option, pub(crate) hover_y: Option, pub(crate) typed_text: Option, + pub(crate) site_permission_generation: u64, pub(crate) site_permissions: Vec, + pub(crate) allow_once_grants: Vec, } #[derive(Clone, Debug, Serialize)] pub(crate) struct ServoLiveSitePermission { pub(crate) origin: String, pub(crate) feature: String, - pub(crate) decision: String, + pub(crate) state: String, + pub(crate) revision: u64, } impl ServoLiveSitePermission { pub fn new( origin: impl Into, feature: impl Into, - decision: SitePermissionDecision, + state: impl Into, + revision: u64, ) -> Self { - Self { origin: origin.into(), feature: feature.into(), decision: decision.as_str().into() } + Self { origin: origin.into(), feature: feature.into(), state: state.into(), revision } } } @@ -364,6 +367,9 @@ pub(crate) enum ServoLiveError { #[error(transparent)] Json(#[from] serde_json::Error), + #[error(transparent)] + Domain(#[from] ely_domain::DomainError), + #[error(transparent)] SidecarCommand(#[from] SidecarCommandError), } @@ -380,7 +386,8 @@ impl ServoLiveError { | Self::InvalidFrameByteCount { .. } | Self::FrameAllocation { .. } | Self::InvalidResponse { .. } - | Self::Json(_) => true, + | Self::Json(_) + | Self::Domain(_) => true, #[cfg(target_os = "macos")] Self::IOSurfaceImportFailed { .. } | Self::IOSurfaceBackingFailed { .. } diff --git a/crates/ely_app/src/services/servo_live_wire.rs b/crates/ely_app/src/services/servo_live_wire.rs index 06cd210..0c176b6 100644 --- a/crates/ely_app/src/services/servo_live_wire.rs +++ b/crates/ely_app/src/services/servo_live_wire.rs @@ -2,7 +2,7 @@ use serde::{Deserialize, Serialize}; use super::ServoLiveSitePermission; -pub(super) const LIVE_PROTOCOL_VERSION: u32 = 2; +pub(super) const LIVE_PROTOCOL_VERSION: u32 = 3; pub(super) const MAX_FRAME_DIMENSION: u32 = 16_384; pub(super) const MAX_FRAME_BYTE_COUNT: usize = 256 * 1024 * 1024; @@ -29,6 +29,7 @@ pub(super) enum LiveRequest { hover_x: Option, hover_y: Option, typed_text: Option, + site_permission_generation: u64, site_permissions: Vec, ready_surface_ids: Vec, pending_surface_ids: Vec, @@ -54,6 +55,16 @@ pub(super) struct LiveResponse { pub(super) surface_handle: Option, #[serde(default)] pub(super) current_surface_id: Option, + #[serde(default)] + pub(super) permission_consumptions: Vec, +} + +#[derive(Deserialize)] +pub(super) struct LivePermissionConsumption { + pub(super) profile_id: String, + pub(super) origin: String, + pub(super) feature: String, + pub(super) grant_revision: u64, } #[derive(Clone, Copy, Debug, Deserialize)] @@ -98,7 +109,7 @@ fn default_device_pixel_ratio() -> f32 { mod tests { use serde_json::json; - use super::{LIVE_PROTOCOL_VERSION, LiveRequest}; + use super::{LIVE_PROTOCOL_VERSION, LiveRequest, ServoLiveSitePermission}; #[test] fn handshake_request_serializes_protocol_version() -> Result<(), serde_json::Error> { @@ -138,4 +149,21 @@ mod tests { ); Ok(()) } + + #[test] + fn site_permission_serializes_revision() -> Result<(), serde_json::Error> { + let permission = + ServoLiveSitePermission::new("https://example.com", "camera", "allow-once", 7); + + assert_eq!( + serde_json::to_value(permission)?, + json!({ + "origin": "https://example.com", + "feature": "camera", + "state": "allow-once", + "revision": 7, + }), + ); + Ok(()) + } } diff --git a/crates/ely_app/src/shell/web_surface.rs b/crates/ely_app/src/shell/web_surface.rs index 99a6f44..1301817 100644 --- a/crates/ely_app/src/shell/web_surface.rs +++ b/crates/ely_app/src/shell/web_surface.rs @@ -97,14 +97,11 @@ impl WebSurfaceStore { // which destroys and reallocates the framebuffer — the source of // the per-frame blank flash. The first ensure (when no prior // `last_ensure_key` is set) always fires so the page can load. - let already_ensured = - self.surfaces.get(tab.id()).is_some_and(|surface| surface.last_ensure_key.is_some()); - if already_ensured - && self - .surfaces - .get(tab.id()) - .is_some_and(|surface| surface.viewport_size_is_settling(Instant::now())) - { + let defer_resize = self + .surfaces + .get(tab.id()) + .is_some_and(|surface| surface.should_defer_resize(&ensure_key, Instant::now())); + if defer_resize { return false; } let previous_frame = @@ -205,6 +202,7 @@ impl WebSurfaceStore { } } WebSurfaceRuntimeFrame::Failed { tab_id, message } => { + self.surface_mut(&tab_id).last_ensure_key = None; let had_ready = matches!( self.surfaces.get(&tab_id).and_then(|surface| surface.state.as_ref()), Some(WebSurfaceState::Ready(_)) @@ -221,6 +219,12 @@ impl WebSurfaceStore { self.surface_mut(&tab_id).state = Some(WebSurfaceState::Failed { message }); result.changed = true; } + WebSurfaceRuntimeFrame::PermissionSnapshotAccepted(grant) => { + result.permission_transfers.push(grant); + } + WebSurfaceRuntimeFrame::PermissionConsumed(consumed) => { + result.permission_consumptions.push(consumed); + } } } @@ -381,3 +385,7 @@ mod web_surface_hardware_import_tests; #[cfg(test)] #[path = "web_surface_scope_tests.rs"] mod web_surface_scope_tests; + +#[cfg(test)] +#[path = "web_surface_permission_lifecycle_tests.rs"] +mod web_surface_permission_lifecycle_tests; diff --git a/crates/ely_app/src/shell/web_surface_controller.rs b/crates/ely_app/src/shell/web_surface_controller.rs index da94dd8..682d9bf 100644 --- a/crates/ely_app/src/shell/web_surface_controller.rs +++ b/crates/ely_app/src/shell/web_surface_controller.rs @@ -1,8 +1,10 @@ +use std::{collections::HashMap, sync::Arc}; + use ely_browser_core::{BrowserCore, BrowserSnapshot}; use ely_domain::{BrowserTab, ProfileKind, TabId, UrlText}; use gpui::{AnyElement, Bounds, Context, Pixels, Point}; -use crate::services::ProfileDataMode; +use crate::services::{ProfileDataMode, servo_live::ServoLivePermissionGrant}; use super::{ ElyShell, @@ -65,11 +67,27 @@ impl ElyShell { for metadata in result.page_metadata { metadata_changed |= self.apply_web_surface_page_metadata(metadata); } + let mut permission_changed = false; + if let super::ShellState::Ready(core) = &mut self.state { + for grant in result.permission_transfers { + permission_changed |= core + .transfer_site_permission_once( + grant.profile_id(), + grant.origin(), + grant.feature(), + grant.grant_revision(), + ) + .unwrap_or(false); + } + for consumed in result.permission_consumptions { + permission_changed |= apply_permission_consumption(core, &consumed); + } + } let sync_changed = self.drain_sync_updates(); if url_changed || metadata_changed { self.schedule_cloud_sync_upload(cx); } - result.changed || url_changed || metadata_changed || sync_changed + result.changed || url_changed || metadata_changed || permission_changed || sync_changed } pub(super) fn external_web_surface_tick_delay(&self) -> std::time::Duration { @@ -191,7 +209,7 @@ impl ElyShell { changed |= self.web_surfaces.ensure_surface( &visible.tab, visible.profile_data_mode, - &visible.permissions, + visible.permissions.as_ref(), ); } changed @@ -237,22 +255,37 @@ impl ElyShell { struct VisibleWebSurfaceTab { tab: BrowserTab, profile_data_mode: ProfileDataMode, - permissions: Vec, + permissions: Arc<[WebSurfaceSitePermission]>, } fn visible_web_surface_tabs( core: &BrowserCore, tabs: Vec, ) -> Vec { - tabs.into_iter() - .filter(|tab| super::web_surface::is_external_web_url(tab.url().as_str())) - .filter_map(|tab| { - let profile_data_mode = - core.profile_kind_for(tab.profile_id()).ok().map(profile_data_mode_from_kind)?; - let permissions = web_surface_site_permissions_for_core_tab(core, &tab); - Some(VisibleWebSurfaceTab { tab, profile_data_mode, permissions }) - }) - .collect() + let mut permission_cache = HashMap::new(); + let mut visible = Vec::new(); + for tab in tabs { + if !super::web_surface::is_external_web_url(tab.url().as_str()) { + continue; + } + let Ok(kind) = core.profile_kind_for(tab.profile_id()) else { + continue; + }; + let permissions = permission_cache + .entry(tab.profile_id().clone()) + .or_insert_with(|| { + Arc::<[WebSurfaceSitePermission]>::from(web_surface_site_permissions_for_core_tab( + core, &tab, + )) + }) + .clone(); + visible.push(VisibleWebSurfaceTab { + tab, + profile_data_mode: profile_data_mode_from_kind(kind), + permissions, + }); + } + visible } fn profile_data_mode_for(tab: &BrowserTab, snapshot: &BrowserSnapshot) -> Option { @@ -269,3 +302,59 @@ fn profile_data_mode_from_kind(kind: ProfileKind) -> ProfileDataMode { ProfileKind::Private => ProfileDataMode::Transient, } } + +fn apply_permission_consumption( + core: &mut BrowserCore, + consumed: &ServoLivePermissionGrant, +) -> bool { + let transferred = core + .transfer_site_permission_once( + consumed.profile_id(), + consumed.origin(), + consumed.feature(), + consumed.grant_revision(), + ) + .unwrap_or(false); + let finished = core + .finish_site_permission_once( + consumed.profile_id(), + consumed.origin(), + consumed.feature(), + consumed.grant_revision(), + ) + .unwrap_or(false); + transferred || finished +} + +#[cfg(test)] +mod tests { + use ely_browser_core::{BrowserCore, InitialBrowserConfig}; + use ely_domain::{SiteOrigin, SitePermissionDecision, SitePermissionFeature}; + + use super::{ServoLivePermissionGrant, apply_permission_consumption}; + + #[test] + fn consumption_receipt_finishes_a_pending_allow_once_grant() + -> Result<(), Box> { + let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + let profile_id = core.snapshot()?.active_profile_id; + let origin = SiteOrigin::parse("https://example.com")?; + core.set_site_permission( + origin.clone(), + SitePermissionFeature::Camera, + SitePermissionDecision::AllowOnce, + )?; + let revision = + core.site_permission_revision(&profile_id, &origin, SitePermissionFeature::Camera); + let consumed = ServoLivePermissionGrant::new( + profile_id, + origin, + SitePermissionFeature::Camera, + revision, + ); + + assert!(apply_permission_consumption(&mut core, &consumed)); + assert!(core.snapshot()?.site_permissions.is_empty()); + Ok(()) + } +} diff --git a/crates/ely_app/src/shell/web_surface_permission_lifecycle_tests.rs b/crates/ely_app/src/shell/web_surface_permission_lifecycle_tests.rs new file mode 100644 index 0000000..d79177b --- /dev/null +++ b/crates/ely_app/src/shell/web_surface_permission_lifecycle_tests.rs @@ -0,0 +1,136 @@ +use ely_domain::{ + BrowserTab, ProfileId, SiteOrigin, SitePermissionDecision, SitePermissionFeature, SpaceId, + TabId, UrlText, +}; + +use crate::services::{ + ProfileDataMode, + servo_live::{ServoLiveEnsureRequest, ServoLiveFrame}, +}; + +use super::{WebSurfaceSitePermission, WebSurfaceStore}; +use crate::shell::{ + web_surface_permissions::WebSurfaceSitePermissionState, + web_surface_runtime::WebSurfaceRuntime, + web_surface_state::WebSurfaceInputOutcome, + web_surface_worker::{LiveRuntimeClient, LiveRuntimeClientError}, +}; + +struct AcceptingClient; +struct RejectingClient; +static REJECTED_ENSURE_COUNT: AtomicUsize = AtomicUsize::new(0); + +impl LiveRuntimeClient for AcceptingClient { + fn ensure( + &mut self, + _request: ServoLiveEnsureRequest, + ) -> Result, LiveRuntimeClientError> { + Ok(None) + } + + fn poll(&mut self, _tab_id: String) -> Result, LiveRuntimeClientError> { + Ok(None) + } + + fn close(&mut self, _tab_id: String) -> Result<(), LiveRuntimeClientError> { + Ok(()) + } +} + +impl LiveRuntimeClient for RejectingClient { + fn ensure( + &mut self, + _request: ServoLiveEnsureRequest, + ) -> Result, LiveRuntimeClientError> { + REJECTED_ENSURE_COUNT.fetch_add(1, Ordering::SeqCst); + Err(LiveRuntimeClientError::Message("ensure rejected".to_string())) + } + + fn poll(&mut self, _tab_id: String) -> Result, LiveRuntimeClientError> { + Ok(None) + } + + fn close(&mut self, _tab_id: String) -> Result<(), LiveRuntimeClientError> { + Ok(()) + } +} + +#[test] +fn successful_worker_ensure_confirms_allow_once_transfer() -> Result<(), String> { + let runtime = WebSurfaceRuntime::new_with_client_factory(|_| Ok(Box::new(AcceptingClient))); + let mut store = WebSurfaceStore::new_with_runtime(runtime); + let (tab, permission) = tab_and_permission()?; + let profile_id = tab.profile_id().clone(); + + record_viewport(&mut store, &tab); + assert!(store.ensure_surface( + &tab, + ProfileDataMode::Transient, + std::slice::from_ref(&permission), + )); + store.flush_runtime_for_test(); + let result = store.tick(std::slice::from_ref(tab.id())); + + assert_eq!( + result.permission_transfers, + vec![crate::services::servo_live::ServoLivePermissionGrant::new( + profile_id, + permission.origin().clone(), + permission.feature(), + permission.revision(), + )], + ); + Ok(()) +} + +#[test] +fn rejected_worker_ensure_keeps_allow_once_untransferred() -> Result<(), String> { + REJECTED_ENSURE_COUNT.store(0, Ordering::SeqCst); + let runtime = WebSurfaceRuntime::new_with_client_factory(|_| Ok(Box::new(RejectingClient))); + let mut store = WebSurfaceStore::new_with_runtime(runtime); + let (tab, permission) = tab_and_permission()?; + record_viewport(&mut store, &tab); + + assert!(store.ensure_surface( + &tab, + ProfileDataMode::Transient, + std::slice::from_ref(&permission), + )); + store.flush_runtime_for_test(); + let result = store.tick(std::slice::from_ref(tab.id())); + + assert!(result.permission_transfers.is_empty()); + let _ = + store.ensure_surface(&tab, ProfileDataMode::Transient, std::slice::from_ref(&permission)); + store.flush_runtime_for_test(); + let _ = store.tick(std::slice::from_ref(tab.id())); + assert_eq!(REJECTED_ENSURE_COUNT.load(Ordering::SeqCst), 2); + Ok(()) +} + +fn tab_and_permission() -> Result<(BrowserTab, WebSurfaceSitePermission), String> { + let profile_id = ProfileId::new(); + let tab = BrowserTab::new( + TabId::new(), + SpaceId::new(), + profile_id, + "Web", + UrlText::parse("https://example.com/").map_err(|error| error.to_string())?, + ); + let permission = WebSurfaceSitePermission::new( + SiteOrigin::parse("https://example.com").map_err(|error| error.to_string())?, + SitePermissionFeature::Camera, + WebSurfaceSitePermissionState::Decision(SitePermissionDecision::AllowOnce), + 7, + ); + Ok((tab, permission)) +} + +fn record_viewport(store: &mut WebSurfaceStore, tab: &BrowserTab) { + let bounds = gpui::Bounds::new( + gpui::point(gpui::px(0.0), gpui::px(0.0)), + gpui::size(gpui::px(640.0), gpui::px(480.0)), + ); + assert_eq!(store.record_viewport_size(tab.id(), bounds, 1.0), WebSurfaceInputOutcome::Applied); +} +use std::sync::atomic::{AtomicUsize, Ordering}; diff --git a/crates/ely_app/src/shell/web_surface_permissions.rs b/crates/ely_app/src/shell/web_surface_permissions.rs index ab16362..fab9a41 100644 --- a/crates/ely_app/src/shell/web_surface_permissions.rs +++ b/crates/ely_app/src/shell/web_surface_permissions.rs @@ -1,22 +1,39 @@ +use std::collections::HashMap; + use ely_browser_core::BrowserCore; -#[cfg(test)] -use ely_browser_core::BrowserSnapshot; use ely_domain::{BrowserTab, SiteOrigin, SitePermissionDecision, SitePermissionFeature}; +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(super) enum WebSurfaceSitePermissionState { + Decision(SitePermissionDecision), + TransferredAllowOnce, +} + +impl WebSurfaceSitePermissionState { + pub(super) fn as_str(self) -> &'static str { + match self { + Self::Decision(decision) => decision.as_str(), + Self::TransferredAllowOnce => "transferred-allow-once", + } + } +} + #[derive(Clone, Debug, Eq, PartialEq)] pub(super) struct WebSurfaceSitePermission { origin: SiteOrigin, feature: SitePermissionFeature, - decision: SitePermissionDecision, + state: WebSurfaceSitePermissionState, + revision: u64, } impl WebSurfaceSitePermission { pub(super) fn new( origin: SiteOrigin, feature: SitePermissionFeature, - decision: SitePermissionDecision, + state: WebSurfaceSitePermissionState, + revision: u64, ) -> Self { - Self { origin, feature, decision } + Self { origin, feature, state, revision } } pub(super) fn origin(&self) -> &SiteOrigin { @@ -27,45 +44,52 @@ impl WebSurfaceSitePermission { self.feature } - pub(super) fn decision(&self) -> SitePermissionDecision { - self.decision + pub(super) fn state(&self) -> WebSurfaceSitePermissionState { + self.state } -} -#[cfg(test)] -pub(super) fn web_surface_site_permissions_for_tab( - tab: &BrowserTab, - snapshot: &BrowserSnapshot, -) -> Vec { - let Ok(Some(origin)) = SiteOrigin::from_url(tab.url()) else { - return Vec::new(); - }; - - snapshot - .site_permissions - .iter() - .filter(|entry| entry.profile_id() == tab.profile_id()) - .filter(|entry| entry.origin() == &origin) - .map(|entry| { - WebSurfaceSitePermission::new(entry.origin().clone(), entry.feature(), entry.decision()) - }) - .collect() + pub(super) fn revision(&self) -> u64 { + self.revision + } } pub(super) fn web_surface_site_permissions_for_core_tab( core: &BrowserCore, tab: &BrowserTab, ) -> Vec { - let Ok(Some(origin)) = SiteOrigin::from_url(tab.url()) else { - return Vec::new(); - }; + let mut latest = HashMap::new(); + for event in core.site_permission_audit_events_for_profile(tab.profile_id()) { + let key = (event.origin().clone(), event.feature()); + let revision = latest.entry(key).or_insert(0_u64); + *revision = revision.saturating_add(1); + } - core.site_permissions_for_profile_origin(tab.profile_id(), &origin) - .into_iter() - .map(|entry| { - WebSurfaceSitePermission::new(entry.origin().clone(), entry.feature(), entry.decision()) - }) - .collect() + let mut permissions = Vec::new(); + for entry in core.site_permissions_for_profile(tab.profile_id()) { + let key = (entry.origin().clone(), entry.feature()); + let revision = latest.remove(&key).unwrap_or(0); + permissions.push(WebSurfaceSitePermission::new( + entry.origin().clone(), + entry.feature(), + WebSurfaceSitePermissionState::Decision(entry.decision()), + revision, + )); + } + for (entry, grant_revision) in core.transferred_site_permissions_for_profile(tab.profile_id()) { + let key = (entry.origin().clone(), entry.feature()); + latest.remove(&key); + permissions.push(WebSurfaceSitePermission::new( + entry.origin().clone(), + entry.feature(), + WebSurfaceSitePermissionState::TransferredAllowOnce, + grant_revision, + )); + } + permissions.sort_by(|left, right| { + (left.origin().as_str(), left.feature().as_str()) + .cmp(&(right.origin().as_str(), right.feature().as_str())) + }); + permissions } #[cfg(test)] @@ -77,7 +101,7 @@ mod tests { BrowserTab, SiteOrigin, SitePermissionDecision, SitePermissionFeature, UrlText, }; - use super::web_surface_site_permissions_for_tab; + use super::{WebSurfaceSitePermissionState, web_surface_site_permissions_for_core_tab}; #[test] fn includes_matching_profile_and_origin_permissions() -> Result<(), Box> { @@ -91,17 +115,20 @@ mod tests { let snapshot = core.snapshot()?; let tab = active_tab(&snapshot)?; - let permissions = web_surface_site_permissions_for_tab(tab, &snapshot); + let permissions = web_surface_site_permissions_for_core_tab(&core, tab); assert_eq!(permissions.len(), 1); assert_eq!(permissions[0].origin().as_str(), "https://example.com"); assert_eq!(permissions[0].feature(), SitePermissionFeature::Camera); - assert_eq!(permissions[0].decision(), SitePermissionDecision::AllowAlways); + assert_eq!( + permissions[0].state(), + WebSurfaceSitePermissionState::Decision(SitePermissionDecision::AllowAlways), + ); Ok(()) } #[test] - fn filters_other_origins() -> Result<(), Box> { + fn includes_the_complete_profile_snapshot() -> Result<(), Box> { let mut core = browser_core_for("https://example.com/page")?; core.set_site_permission( SiteOrigin::parse("https://other.test")?, @@ -112,7 +139,30 @@ mod tests { let snapshot = core.snapshot()?; let tab = active_tab(&snapshot)?; - assert!(web_surface_site_permissions_for_tab(tab, &snapshot).is_empty()); + let permissions = web_surface_site_permissions_for_core_tab(&core, tab); + assert_eq!(permissions.len(), 1); + assert_eq!(permissions[0].origin().as_str(), "https://other.test"); + assert_eq!(permissions[0].revision(), 1); + assert_eq!( + permissions[0].state(), + WebSurfaceSitePermissionState::Decision(SitePermissionDecision::AllowOnce), + ); + + let profile_id = tab.profile_id().clone(); + let origin = SiteOrigin::parse("https://other.test")?; + assert!(core.transfer_site_permission_once( + &profile_id, + &origin, + SitePermissionFeature::Location, + 1, + )?); + let transferred = web_surface_site_permissions_for_core_tab(&core, tab); + assert_eq!(transferred[0].state(), WebSurfaceSitePermissionState::TransferredAllowOnce); + assert_eq!(transferred[0].revision(), 1); + + core.revoke_site_permission(&origin, SitePermissionFeature::Location)?; + let revoked = web_surface_site_permissions_for_core_tab(&core, tab); + assert!(revoked.is_empty()); Ok(()) } diff --git a/crates/ely_app/src/shell/web_surface_runtime.rs b/crates/ely_app/src/shell/web_surface_runtime.rs index ee162d8..a3a3087 100644 --- a/crates/ely_app/src/shell/web_surface_runtime.rs +++ b/crates/ely_app/src/shell/web_surface_runtime.rs @@ -1,6 +1,5 @@ use std::{ collections::BTreeMap, - path::PathBuf, thread::JoinHandle, time::{Duration, Instant}, }; @@ -9,7 +8,7 @@ use ely_domain::{BrowserTab, TabId}; use crate::services::{ ProfileDataMode, - servo_live::{ServoLiveClient, ServoLiveEnsureRequest, ServoLiveSitePermission}, + servo_live::{ServoLiveEnsureRequest, ServoLiveSitePermission}, servo_profile_data::cleanup_stale_transient_profile_data_dirs, }; @@ -21,13 +20,15 @@ use super::{ WebSurfaceRuntimeScope, WebSurfaceSession, config_dir_for_scope, session_for_scope, }, web_surface_runtime_wire::{ - input_requests_history_navigation, log_ensure_submitted, pending_input_kind, - scroll_wire_fields, + allow_once_grants, input_requests_history_navigation, log_ensure_submitted, + pending_input_kind, scroll_wire_fields, }, web_surface_state::WebSurfacePendingInput, web_surface_worker::{LiveRuntimeClient, LiveRuntimeWorker, RequestGeneration, WorkerResponse}, }; -use cleanup::{ScopedWorker, shutdown_scoped_worker}; +use cleanup::{ + LiveRuntimeClientFactory, ScopedWorker, new_servo_live_client, shutdown_scoped_worker, +}; pub(super) use super::web_surface_runtime_session::{ WebSurfaceEnsureResult, WebSurfaceRuntimeFrame, WebSurfaceUrlChange, WebSurfaceUrlChangeKind, @@ -130,7 +131,9 @@ impl WebSurfaceRuntime { hover_x: input.hover_point.map(|point| point.x()), hover_y: input.hover_point.map(|point| point.y()), typed_text: input.typed_text, + site_permission_generation: generation.value(), site_permissions: permissions.iter().map(ServoLiveSitePermission::from).collect(), + allow_once_grants: allow_once_grants(tab.profile_id(), permissions), }; let Some(scoped) = self.workers.get(&scope) else { @@ -168,22 +171,24 @@ impl WebSurfaceRuntime { let poll_now = Instant::now(); for tab_id in visible_tab_ids { - let Some(scope) = self.sessions.get(tab_id).and_then(|session| { - session.cadence.should_poll(poll_now).then(|| session.scope.clone()) + let Some((scope, generation)) = self.sessions.get(tab_id).and_then(|session| { + session + .cadence + .should_poll(poll_now) + .then(|| (session.scope.clone(), session.generation)) }) else { continue; }; + let Some(generation) = generation else { + continue; + }; if !self.workers.contains_key(&scope) { continue; } - let generation = self.next_request_generation(); - let submitted = self.workers.get(&scope).is_some_and(|scoped| { + let _ = self.workers.get(&scope).is_some_and(|scoped| { scoped.worker.submit_poll(generation, tab_id.as_str().to_string()) }); if let Some(session) = self.sessions.get_mut(tab_id) { - if submitted { - session.generation = Some(generation); - } session.cadence.note_poll_submitted(poll_now); } } @@ -353,6 +358,12 @@ impl WebSurfaceRuntime { } } WorkerResponse::RuntimeUnavailable => runtime_unavailable = true, + WorkerResponse::PermissionSnapshotAccepted(grant) => { + frames.push(WebSurfaceRuntimeFrame::PermissionSnapshotAccepted(grant)); + } + WorkerResponse::PermissionConsumed(consumed) => { + frames.push(WebSurfaceRuntimeFrame::PermissionConsumed(consumed)); + } } } runtime_unavailable @@ -448,15 +459,6 @@ impl Drop for WebSurfaceRuntime { } } -pub(super) type LiveRuntimeClientFactory = - fn(PathBuf) -> Result, String>; - -fn new_servo_live_client(config_dir: PathBuf) -> Result, String> { - ServoLiveClient::new(config_dir) - .map(|client| Box::new(client) as Box) - .map_err(|error| error.to_string()) -} - #[derive(Clone, Copy, Debug)] struct ScopeRetryState { failure_count: u32, diff --git a/crates/ely_app/src/shell/web_surface_runtime_backpressure_tests.rs b/crates/ely_app/src/shell/web_surface_runtime_backpressure_tests.rs index 02ddf4f..97fe10d 100644 --- a/crates/ely_app/src/shell/web_surface_runtime_backpressure_tests.rs +++ b/crates/ely_app/src/shell/web_surface_runtime_backpressure_tests.rs @@ -121,12 +121,22 @@ fn pending_poll_advances_deadline_under_worker_backpressure() -> Result<(), Stri let tab = web_tab("Backpressure")?; runtime.ensure_tab(&tab, surface_size(), ProfileDataMode::Transient, &[], pending_input())?; runtime.flush_for_test(); + let ensure_generation = runtime + .sessions + .get(tab.id()) + .and_then(|session| session.generation) + .ok_or_else(|| "ensure generation was missing".to_string())?; thread::sleep(Duration::from_millis(10)); runtime.tick(std::slice::from_ref(tab.id())); thread::sleep(Duration::from_millis(10)); runtime.tick(std::slice::from_ref(tab.id())); + assert_eq!( + runtime.sessions.get(tab.id()).and_then(|session| session.generation), + Some(ensure_generation), + ); + let delay = runtime .next_poll_delay(std::slice::from_ref(tab.id()), std::time::Instant::now()) .ok_or_else(|| "visible tab lost its poll deadline".to_string())?; diff --git a/crates/ely_app/src/shell/web_surface_runtime_cleanup.rs b/crates/ely_app/src/shell/web_surface_runtime_cleanup.rs index 6fe724b..c74f859 100644 --- a/crates/ely_app/src/shell/web_surface_runtime_cleanup.rs +++ b/crates/ely_app/src/shell/web_surface_runtime_cleanup.rs @@ -1,6 +1,19 @@ -use crate::services::servo_profile_data::TransientProfileDataDir; +use std::path::PathBuf; -use super::LiveRuntimeWorker; +use crate::services::{servo_live::ServoLiveClient, servo_profile_data::TransientProfileDataDir}; + +use super::{LiveRuntimeClient, LiveRuntimeWorker}; + +pub(super) type LiveRuntimeClientFactory = + fn(PathBuf) -> Result, String>; + +pub(super) fn new_servo_live_client( + config_dir: PathBuf, +) -> Result, String> { + ServoLiveClient::new(config_dir) + .map(|client| Box::new(client) as Box) + .map_err(|error| error.to_string()) +} pub(super) struct ScopedWorker { pub(super) worker: LiveRuntimeWorker, diff --git a/crates/ely_app/src/shell/web_surface_runtime_session.rs b/crates/ely_app/src/shell/web_surface_runtime_session.rs index 50e4f8d..71be9a6 100644 --- a/crates/ely_app/src/shell/web_surface_runtime_session.rs +++ b/crates/ely_app/src/shell/web_surface_runtime_session.rs @@ -2,6 +2,7 @@ use std::{collections::BTreeMap, path::PathBuf}; use crate::services::{ ProfileDataMode, + servo_live::ServoLivePermissionGrant, servo_profile_data::{ TransientProfileDataDir, create_profile_data_dir, default_profile_data_root, transient_profile_data_dir, @@ -102,6 +103,8 @@ pub(super) struct WebSurfaceEnsureResult { pub(super) enum WebSurfaceRuntimeFrame { Ready { tab_id: TabId, frame: Box, url_change: Option }, Failed { tab_id: TabId, message: String }, + PermissionSnapshotAccepted(ServoLivePermissionGrant), + PermissionConsumed(ServoLivePermissionGrant), } #[derive(Clone, Debug, Eq, PartialEq)] diff --git a/crates/ely_app/src/shell/web_surface_runtime_wire.rs b/crates/ely_app/src/shell/web_surface_runtime_wire.rs index 013b3fc..e6bd063 100644 --- a/crates/ely_app/src/shell/web_surface_runtime_wire.rs +++ b/crates/ely_app/src/shell/web_surface_runtime_wire.rs @@ -1,13 +1,13 @@ use std::time::Instant; -use ely_domain::BrowserTab; +use ely_domain::{BrowserTab, ProfileId, SitePermissionDecision}; -use crate::services::servo_live::ServoLiveSitePermission; +use crate::services::servo_live::{ServoLivePermissionGrant, ServoLiveSitePermission}; use super::{ web_surface_cadence::WebSurfaceInputKind, web_surface_geometry::{WebSurfaceClickPoint, WebSurfaceScrollDelta, WebSurfaceSize}, - web_surface_permissions::WebSurfaceSitePermission, + web_surface_permissions::{WebSurfaceSitePermission, WebSurfaceSitePermissionState}, web_surface_state::WebSurfacePendingInput, }; @@ -30,6 +30,27 @@ pub(super) fn input_requests_history_navigation(input: &WebSurfacePendingInput) || input.typed_text.as_deref().is_some_and(|text| text.contains('\n')) } +pub(super) fn allow_once_grants( + profile_id: &ProfileId, + permissions: &[WebSurfaceSitePermission], +) -> Vec { + permissions + .iter() + .filter(|permission| { + permission.state() + == WebSurfaceSitePermissionState::Decision(SitePermissionDecision::AllowOnce) + }) + .map(|permission| { + ServoLivePermissionGrant::new( + profile_id.clone(), + permission.origin().clone(), + permission.feature(), + permission.revision(), + ) + }) + .collect() +} + pub(super) fn pending_input_kind(input: &WebSurfacePendingInput) -> WebSurfaceInputKind { if input.scroll_delta.is_some() { WebSurfaceInputKind::Scroll @@ -74,7 +95,8 @@ impl From<&WebSurfaceSitePermission> for ServoLiveSitePermission { Self::new( permission.origin().as_str(), permission.feature().as_str(), - permission.decision(), + permission.state().as_str(), + permission.revision(), ) } } diff --git a/crates/ely_app/src/shell/web_surface_state.rs b/crates/ely_app/src/shell/web_surface_state.rs index bf7d1ab..8838f3a 100644 --- a/crates/ely_app/src/shell/web_surface_state.rs +++ b/crates/ely_app/src/shell/web_surface_state.rs @@ -4,6 +4,7 @@ use ely_domain::{ProfileId, TabId}; use gpui::{Bounds, Pixels}; use crate::services::ProfileDataMode; +use crate::services::servo_live::ServoLivePermissionGrant; use super::{ web_surface_cadence::ACTIVE_POLL_INTERVAL, @@ -110,6 +111,8 @@ pub(super) struct WebSurfaceTickResult { pub(super) changed: bool, pub(super) url_changes: Vec, pub(super) page_metadata: Vec, + pub(super) permission_transfers: Vec, + pub(super) permission_consumptions: Vec, } /// All per-tab surface invariants in one owner. @@ -196,6 +199,11 @@ impl PerTabSurface { self.last_ensure_key.as_ref() != Some(key) || self.has_pending_input() } + pub(super) fn should_defer_resize(&self, key: &WebSurfaceEnsureKey, now: Instant) -> bool { + self.last_ensure_key.as_ref().is_some_and(|last| last.permissions == key.permissions) + && self.viewport_size_is_settling(now) + } + pub(super) fn has_scope( &self, profile_id: &ProfileId, @@ -368,6 +376,31 @@ mod tests { assert!(surface.should_ensure(&new_key)); } + #[test] + fn permission_change_bypasses_viewport_resize_debounce() + -> Result<(), Box> { + let now = Instant::now(); + let profile_id = ProfileId::new(); + let old_key = ensure_key("https://example.com/", 800, 600, &profile_id); + let resized_key = ensure_key("https://example.com/", 1024, 768, &profile_id); + let mut new_key = ensure_key("https://example.com/", 1024, 768, &profile_id); + new_key.permissions.push(WebSurfaceSitePermission::new( + ely_domain::SiteOrigin::parse("https://example.com")?, + ely_domain::SitePermissionFeature::Camera, + crate::shell::web_surface_permissions::WebSurfaceSitePermissionState::Decision( + ely_domain::SitePermissionDecision::DenyAlways, + ), + 1, + )); + let mut surface = PerTabSurface::new(); + surface.mark_ensured(old_key); + surface.mark_viewport_size_changed(now); + + assert!(surface.should_defer_resize(&resized_key, now)); + assert!(!surface.should_defer_resize(&new_key, now)); + Ok(()) + } + #[test] fn profile_change_forces_ensure() { let old_key = ensure_key("https://example.com/", 800, 600, &ProfileId::new()); diff --git a/crates/ely_app/src/shell/web_surface_worker.rs b/crates/ely_app/src/shell/web_surface_worker.rs index 8f1fa86..a994ea1 100644 --- a/crates/ely_app/src/shell/web_surface_worker.rs +++ b/crates/ely_app/src/shell/web_surface_worker.rs @@ -7,6 +7,14 @@ use std::{ use crate::services::servo_live::{ ServoLiveClient, ServoLiveEnsureRequest, ServoLiveError, ServoLiveFrame, + ServoLivePermissionGrant, +}; + +#[path = "web_surface_worker_dispatch.rs"] +mod dispatch; +use dispatch::{ + dispatch_result, forward_permission_consumptions, preserve_latest_hover, + request_has_ordered_input, }; /// Blocking transport for one profile-scoped Servo sidecar. @@ -22,6 +30,10 @@ pub(super) trait LiveRuntimeClient { fn poll(&mut self, tab_id: String) -> Result, LiveRuntimeClientError>; fn close(&mut self, tab_id: String) -> Result<(), LiveRuntimeClientError>; + + fn take_permission_consumptions(&mut self) -> Vec { + Vec::new() + } } impl LiveRuntimeClient for ServoLiveClient { @@ -39,6 +51,10 @@ impl LiveRuntimeClient for ServoLiveClient { fn close(&mut self, tab_id: String) -> Result<(), LiveRuntimeClientError> { ServoLiveClient::close(self, tab_id).map_err(LiveRuntimeClientError::from) } + + fn take_permission_consumptions(&mut self) -> Vec { + ServoLiveClient::take_permission_consumptions(self) + } } #[derive(Debug)] @@ -88,6 +104,8 @@ pub(super) enum WorkerResponse { Frame { generation: RequestGeneration, tab_id: String, frame: ServoLiveFrame }, Failed { generation: RequestGeneration, tab_id: String, message: String }, RuntimeUnavailable, + PermissionSnapshotAccepted(ServoLivePermissionGrant), + PermissionConsumed(ServoLivePermissionGrant), } #[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd)] @@ -97,10 +115,14 @@ impl RequestGeneration { pub(super) const fn new(value: u64) -> Self { Self(value) } + + pub(super) const fn value(self) -> u64 { + self.0 + } } enum WorkerRequest { - Ensure { generation: RequestGeneration, request: ServoLiveEnsureRequest }, + Ensure { generation: RequestGeneration, request: Box }, Poll { generation: RequestGeneration, tab_id: String }, } @@ -190,7 +212,7 @@ impl LiveRuntimeWorker { let _ = self.response_tx.send(WorkerResponse::Failed { generation, tab_id, message }); return; } - let mut request = WorkerRequest::Ensure { generation, request }; + let mut request = WorkerRequest::Ensure { generation, request: Box::new(request) }; if let Some(pending) = q.pending.get_mut(&tab_id) { let replace_tail = pending.back().is_some_and(|tail| { matches!(tail, WorkerRequest::Poll { .. }) @@ -394,15 +416,26 @@ fn run_worker( let exit_after_dispatch = match work { Work::Close(tab_id) => { let _ = client.close(tab_id); + forward_permission_consumptions(&mut *client, &response_tx); false } - Work::Request(WorkerRequest::Ensure { generation, request }) => { + Work::Request(WorkerRequest::Ensure { generation, mut request }) => { let tab_id = request.tab_id.clone(); - dispatch_result(&response_tx, generation, tab_id, client.ensure(request)) + let allow_once_grants = std::mem::take(&mut request.allow_once_grants); + let result = client.ensure(*request); + if result.is_ok() { + for grant in allow_once_grants { + let _ = response_tx.send(WorkerResponse::PermissionSnapshotAccepted(grant)); + } + } + forward_permission_consumptions(&mut *client, &response_tx); + dispatch_result(&response_tx, generation, tab_id, result) } Work::Request(WorkerRequest::Poll { generation, tab_id }) => { let request_tab_id = tab_id.clone(); - dispatch_result(&response_tx, generation, request_tab_id, client.poll(tab_id)) + let result = client.poll(tab_id); + forward_permission_consumptions(&mut *client, &response_tx); + dispatch_result(&response_tx, generation, request_tab_id, result) } }; @@ -424,60 +457,6 @@ enum Work { Request(WorkerRequest), } -/// Forward a single client result to the response channel. Returns -/// `true` when the worker should exit. -fn dispatch_result( - response_tx: &mpsc::Sender, - generation: RequestGeneration, - tab_id: String, - result: Result, LiveRuntimeClientError>, -) -> bool { - match result { - Ok(Some(frame)) => { - let _ = response_tx.send(WorkerResponse::Frame { generation, tab_id, frame }); - false - } - Ok(None) => false, - Err(error) => { - let unavailable = error.is_runtime_unavailable(); - let message = error.to_string(); - let _ = response_tx.send(WorkerResponse::Failed { generation, tab_id, message }); - if unavailable { - let _ = response_tx.send(WorkerResponse::RuntimeUnavailable); - return true; - } - false - } - } -} - -fn request_has_ordered_input(request: &WorkerRequest) -> bool { - let WorkerRequest::Ensure { request, .. } = request else { - return false; - }; - request.scroll_delta_x != 0 - || request.scroll_delta_y != 0 - || request.scroll_point_x.is_some() - || request.scroll_point_y.is_some() - || request.click_x.is_some() - || request.click_y.is_some() - || request.typed_text.is_some() -} - -fn preserve_latest_hover(latest: &mut WorkerRequest, previous: &WorkerRequest) { - let ( - WorkerRequest::Ensure { request: latest, .. }, - WorkerRequest::Ensure { request: previous, .. }, - ) = (latest, previous) - else { - return; - }; - if latest.hover_x.is_none() && latest.hover_y.is_none() { - latest.hover_x = previous.hover_x; - latest.hover_y = previous.hover_y; - } -} - #[cfg(test)] #[path = "web_surface_worker_tests.rs"] mod tests; diff --git a/crates/ely_app/src/shell/web_surface_worker_dispatch.rs b/crates/ely_app/src/shell/web_surface_worker_dispatch.rs new file mode 100644 index 0000000..92c2cda --- /dev/null +++ b/crates/ely_app/src/shell/web_surface_worker_dispatch.rs @@ -0,0 +1,69 @@ +use std::sync::mpsc; + +use crate::services::servo_live::ServoLiveFrame; + +use super::{ + LiveRuntimeClient, LiveRuntimeClientError, RequestGeneration, WorkerRequest, WorkerResponse, +}; + +pub(super) fn forward_permission_consumptions( + client: &mut dyn LiveRuntimeClient, + response_tx: &mpsc::Sender, +) { + for consumption in client.take_permission_consumptions() { + let _ = response_tx.send(WorkerResponse::PermissionConsumed(consumption)); + } +} + +pub(super) fn dispatch_result( + response_tx: &mpsc::Sender, + generation: RequestGeneration, + tab_id: String, + result: Result, LiveRuntimeClientError>, +) -> bool { + match result { + Ok(Some(frame)) => { + let _ = response_tx.send(WorkerResponse::Frame { generation, tab_id, frame }); + false + } + Ok(None) => false, + Err(error) => { + let unavailable = error.is_runtime_unavailable(); + let message = error.to_string(); + let _ = response_tx.send(WorkerResponse::Failed { generation, tab_id, message }); + if unavailable { + let _ = response_tx.send(WorkerResponse::RuntimeUnavailable); + return true; + } + false + } + } +} + +pub(super) fn request_has_ordered_input(request: &WorkerRequest) -> bool { + let WorkerRequest::Ensure { request, .. } = request else { + return false; + }; + request.scroll_delta_x != 0 + || request.scroll_delta_y != 0 + || request.scroll_point_x.is_some() + || request.scroll_point_y.is_some() + || request.click_x.is_some() + || request.click_y.is_some() + || request.typed_text.is_some() + || request.site_permissions.iter().any(|permission| permission.state == "allow-once") +} + +pub(super) fn preserve_latest_hover(latest: &mut WorkerRequest, previous: &WorkerRequest) { + let ( + WorkerRequest::Ensure { request: latest, .. }, + WorkerRequest::Ensure { request: previous, .. }, + ) = (latest, previous) + else { + return; + }; + if latest.hover_x.is_none() && latest.hover_y.is_none() { + latest.hover_x = previous.hover_x; + latest.hover_y = previous.hover_y; + } +} diff --git a/crates/ely_app/src/shell/web_surface_worker_tests.rs b/crates/ely_app/src/shell/web_surface_worker_tests.rs index 48650db..f5d73a4 100644 --- a/crates/ely_app/src/shell/web_surface_worker_tests.rs +++ b/crates/ely_app/src/shell/web_surface_worker_tests.rs @@ -3,7 +3,10 @@ use std::{ time::Duration, }; -use crate::services::servo_live::{ServoLiveEnsureRequest, ServoLiveFrame}; +use crate::services::servo_live::{ + ServoLiveEnsureRequest, ServoLiveFrame, ServoLivePermissionGrant, ServoLiveSitePermission, +}; +use ely_domain::{ProfileId, SiteOrigin, SitePermissionFeature}; use super::{ LiveRuntimeClient, LiveRuntimeClientError, LiveRuntimeWorker, RequestGeneration, WorkerResponse, @@ -16,6 +19,7 @@ enum RecordedInput { Click(String), Text(String), Hover(String), + Permission(String), } struct SlowRecordingClient { @@ -27,6 +31,31 @@ struct SlowRecordingClient { struct GenerationClient; +struct ConsumptionClient { + pending: Vec, +} + +impl LiveRuntimeClient for ConsumptionClient { + fn ensure( + &mut self, + _request: ServoLiveEnsureRequest, + ) -> Result, LiveRuntimeClientError> { + Err(LiveRuntimeClientError::Message("ensure failed".to_string())) + } + + fn poll(&mut self, _tab_id: String) -> Result, LiveRuntimeClientError> { + Ok(None) + } + + fn close(&mut self, _tab_id: String) -> Result<(), LiveRuntimeClientError> { + Ok(()) + } + + fn take_permission_consumptions(&mut self) -> Vec { + std::mem::take(&mut self.pending) + } +} + impl LiveRuntimeClient for GenerationClient { fn ensure( &mut self, @@ -117,6 +146,52 @@ fn queued_edge_inputs_for_one_tab_are_preserved() -> Result<(), String> { Ok(()) } +#[test] +fn allow_once_transfer_is_preserved_ahead_of_idle_updates() -> Result<(), String> { + let calls = Arc::new(Mutex::new(Vec::new())); + let (first_started_tx, first_started_rx) = mpsc::channel(); + let (release_first_tx, release_first_rx) = mpsc::channel(); + let client_calls = calls.clone(); + let worker = LiveRuntimeWorker::new(move || { + Ok(Box::new(SlowRecordingClient { + calls: client_calls, + first_started_tx: Some(first_started_tx), + release_first_rx, + return_frame: false, + })) + })?; + + worker.submit_ensure( + RequestGeneration::new(1), + ensure_request("tab-a", RecordedInput::Idle("tab-a".to_string())), + ); + first_started_rx.recv_timeout(Duration::from_secs(1)).map_err(|error| error.to_string())?; + let mut transfer = ensure_request("tab-a", RecordedInput::Idle("tab-a".to_string())); + transfer.site_permissions.push(ServoLiveSitePermission::new( + "https://example.com", + "camera", + "allow-once", + 7, + )); + worker.submit_ensure(RequestGeneration::new(2), transfer); + worker.submit_ensure( + RequestGeneration::new(3), + ensure_request("tab-a", RecordedInput::Idle("tab-a".to_string())), + ); + release_first_tx.send(()).map_err(|error| error.to_string())?; + worker.wait_until_idle(); + + assert_eq!( + *calls.lock().map_err(|_| "call recorder lock was poisoned".to_string())?, + vec![ + RecordedInput::Idle("tab-a".to_string()), + RecordedInput::Permission("tab-a".to_string()), + RecordedInput::Idle("tab-a".to_string()), + ], + ); + Ok(()) +} + #[test] fn queued_tabs_are_dispatched_round_robin() -> Result<(), String> { let calls = Arc::new(Mutex::new(Vec::new())); @@ -191,6 +266,32 @@ fn responses_keep_their_request_generation() -> Result<(), String> { Ok(()) } +#[test] +fn permission_consumption_is_forwarded_after_ensure_error() -> Result<(), String> { + let profile_id = ProfileId::new(); + let origin = SiteOrigin::parse("https://example.com").map_err(|error| error.to_string())?; + let grant = ServoLivePermissionGrant::new(profile_id, origin, SitePermissionFeature::Camera, 7); + let expected = grant.clone(); + let consumed = grant.clone(); + let worker = LiveRuntimeWorker::new(move || { + Ok(Box::new(ConsumptionClient { pending: vec![consumed] })) + })?; + + let mut request = ensure_request("tab-a", RecordedInput::Idle("tab-a".to_string())); + request.allow_once_grants.push(grant); + worker.submit_ensure(RequestGeneration::new(1), request); + worker.wait_until_idle(); + + assert!(matches!( + worker.drain_responses().as_slice(), + [ + WorkerResponse::PermissionConsumed(consumed), + WorkerResponse::Failed { message, .. }, + ] if consumed == &expected && message == "ensure failed" + )); + Ok(()) +} + #[test] fn hover_updates_coalesce_to_latest_state() -> Result<(), String> { let calls = Arc::new(Mutex::new(Vec::new())); @@ -321,7 +422,9 @@ fn poisoned_queue_still_shuts_down_worker() -> Result<(), String> { fn recorded_input(request: &ServoLiveEnsureRequest) -> RecordedInput { let tab_id = request.tab_id.clone(); - if request.scroll_delta_x != 0 || request.scroll_delta_y != 0 { + if request.site_permissions.iter().any(|permission| permission.state == "allow-once") { + RecordedInput::Permission(tab_id) + } else if request.scroll_delta_x != 0 || request.scroll_delta_y != 0 { RecordedInput::Scroll(tab_id) } else if request.click_x.is_some() { RecordedInput::Click(tab_id) @@ -356,6 +459,8 @@ fn ensure_request(tab_id: &str, input: RecordedInput) -> ServoLiveEnsureRequest hover_x: hover.then_some(1), hover_y: hover.then_some(1), typed_text: text.then(|| "text".to_string()), + site_permission_generation: 1, site_permissions: Vec::new(), + allow_once_grants: Vec::new(), } } diff --git a/crates/ely_browser_core/src/state.rs b/crates/ely_browser_core/src/state.rs index e9d2692..11659d1 100644 --- a/crates/ely_browser_core/src/state.rs +++ b/crates/ely_browser_core/src/state.rs @@ -143,6 +143,12 @@ pub struct BrowserSnapshot { pub command_query: String, } +#[derive(Debug)] +struct TransferredSitePermission { + entry: SitePermissionEntry, + grant_revision: u64, +} + #[derive(Debug)] pub struct BrowserCore { spaces: Vec, @@ -153,6 +159,7 @@ pub struct BrowserCore { notes: Vec, reading_list: Vec, site_permissions: Vec, + transferred_site_permissions: Vec, site_permission_audit_events: Vec, download_entries: Vec, history_entries: Vec, @@ -238,6 +245,7 @@ impl BrowserCore { notes: Vec::new(), reading_list: Vec::new(), site_permissions: Vec::new(), + transferred_site_permissions: Vec::new(), site_permission_audit_events: Vec::new(), download_entries: Vec::new(), history_entries: Vec::new(), diff --git a/crates/ely_browser_core/src/state/local_data_export_records.rs b/crates/ely_browser_core/src/state/local_data_export_records.rs index edddccd..b4c039d 100644 --- a/crates/ely_browser_core/src/state/local_data_export_records.rs +++ b/crates/ely_browser_core/src/state/local_data_export_records.rs @@ -224,6 +224,8 @@ pub(super) struct ElyLocalSitePermissionAuditRecord { enum ElyLocalSitePermissionAuditActionRecord { Set { decision: String }, Revoked, + Transferred, + Consumed, } impl ElyLocalProfileRecord { @@ -436,6 +438,8 @@ impl ElyLocalSitePermissionAuditActionRecord { Self::Set { decision: decision.as_str().to_string() } } SitePermissionAuditAction::Revoked => Self::Revoked, + SitePermissionAuditAction::Transferred => Self::Transferred, + SitePermissionAuditAction::Consumed => Self::Consumed, } } } diff --git a/crates/ely_browser_core/src/state/local_data_exports.rs b/crates/ely_browser_core/src/state/local_data_exports.rs index f73f840..2f331bc 100644 --- a/crates/ely_browser_core/src/state/local_data_exports.rs +++ b/crates/ely_browser_core/src/state/local_data_exports.rs @@ -11,7 +11,7 @@ use super::local_data_export_records::{ }; use crate::CoreError; -pub const ELYDATA_SCHEMA_VERSION: u16 = 1; +pub const ELYDATA_SCHEMA_VERSION: u16 = 2; pub const ELYDATA_FILE_EXTENSION: &str = "elydata"; impl BrowserCore { @@ -79,6 +79,7 @@ impl BrowserCore { .site_permissions .iter() .filter(|entry| entry.profile_id() == profile_id) + .filter(|entry| entry.decision() != ely_domain::SitePermissionDecision::AllowOnce) .map(ElyLocalSitePermissionRecord::from_site_permission) .collect(), site_permission_audit_events: self diff --git a/crates/ely_browser_core/src/state/privacy.rs b/crates/ely_browser_core/src/state/privacy.rs index 05a0527..e249506 100644 --- a/crates/ely_browser_core/src/state/privacy.rs +++ b/crates/ely_browser_core/src/state/privacy.rs @@ -132,6 +132,7 @@ impl BrowserCore { .site_permissions .iter() .filter(|entry| entry.profile_id() == profile_id) + .filter(|entry| entry.decision() != ely_domain::SitePermissionDecision::AllowOnce) .count(), site_permission_audit_events: self .site_permission_audit_events diff --git a/crates/ely_browser_core/src/state/site_data.rs b/crates/ely_browser_core/src/state/site_data.rs index 1ecdfc6..23e2e5e 100644 --- a/crates/ely_browser_core/src/state/site_data.rs +++ b/crates/ely_browser_core/src/state/site_data.rs @@ -75,6 +75,15 @@ impl BrowserCore { true } }); + self.transferred_site_permissions.retain(|permission| { + if permission.entry.profile_id() == profile_id && permission.entry.origin() == origin { + revoked_permissions + .push((permission.entry.origin().clone(), permission.entry.feature())); + false + } else { + true + } + }); let revoked_count = revoked_permissions.len(); for (origin, feature) in revoked_permissions { diff --git a/crates/ely_browser_core/src/state/site_permissions.rs b/crates/ely_browser_core/src/state/site_permissions.rs index 24f0f90..20d1494 100644 --- a/crates/ely_browser_core/src/state/site_permissions.rs +++ b/crates/ely_browser_core/src/state/site_permissions.rs @@ -7,7 +7,7 @@ use ely_domain::{ use crate::CoreError; -use super::BrowserCore; +use super::{BrowserCore, TransferredSitePermission}; impl BrowserCore { pub fn set_site_permission( @@ -45,21 +45,112 @@ impl BrowserCore { pub(super) fn visible_site_permissions(&self) -> Vec { self.site_permissions .iter() + .chain(self.transferred_site_permissions.iter().map(|permission| &permission.entry)) .filter(|entry| entry.profile_id() == &self.active_profile_id) .cloned() .collect() } - pub fn site_permissions_for_profile_origin( + pub fn site_permissions_for_profile(&self, profile_id: &ProfileId) -> Vec { + self.site_permissions + .iter() + .filter(|entry| entry.profile_id() == profile_id) + .cloned() + .collect() + } + + pub fn transferred_site_permissions_for_profile( + &self, + profile_id: &ProfileId, + ) -> Vec<(SitePermissionEntry, u64)> { + self.transferred_site_permissions + .iter() + .filter(|permission| permission.entry.profile_id() == profile_id) + .map(|permission| (permission.entry.clone(), permission.grant_revision)) + .collect() + } + + pub fn site_permission_audit_events_for_profile( + &self, + profile_id: &ProfileId, + ) -> Vec { + self.site_permission_audit_events + .iter() + .filter(|event| event.profile_id() == profile_id) + .cloned() + .collect() + } + + pub fn site_permission_revision( &self, profile_id: &ProfileId, origin: &SiteOrigin, - ) -> Vec { - self.site_permissions + feature: SitePermissionFeature, + ) -> u64 { + self.site_permission_audit_events .iter() - .filter(|entry| entry.profile_id() == profile_id && entry.origin() == origin) - .cloned() - .collect() + .filter(|event| { + event.profile_id() == profile_id + && event.origin() == origin + && event.feature() == feature + }) + .fold(0_u64, |revision, _| revision.saturating_add(1)) + } + + pub fn transfer_site_permission_once( + &mut self, + profile_id: &ProfileId, + origin: &SiteOrigin, + feature: SitePermissionFeature, + revision: u64, + ) -> Result { + self.require_profile(profile_id)?; + if self.site_permission_revision(profile_id, origin, feature) != revision { + return Ok(false); + } + let Some(index) = self.site_permission_entry_index(profile_id, origin, feature) else { + return Ok(false); + }; + if self.site_permissions[index].decision() != SitePermissionDecision::AllowOnce { + return Ok(false); + } + + let entry = self.site_permissions.remove(index); + self.record_site_permission_audit_event( + profile_id.clone(), + entry.origin().clone(), + feature, + SitePermissionAuditAction::Transferred, + ); + self.transferred_site_permissions + .push(TransferredSitePermission { entry, grant_revision: revision }); + Ok(true) + } + + pub fn finish_site_permission_once( + &mut self, + profile_id: &ProfileId, + origin: &SiteOrigin, + feature: SitePermissionFeature, + grant_revision: u64, + ) -> Result { + self.require_profile(profile_id)?; + let Some(index) = self.transferred_site_permissions.iter().position(|permission| { + permission.entry.profile_id() == profile_id + && permission.entry.origin() == origin + && permission.entry.feature() == feature + && permission.grant_revision == grant_revision + }) else { + return Ok(false); + }; + let permission = self.transferred_site_permissions.remove(index); + self.record_site_permission_audit_event( + profile_id.clone(), + permission.entry.origin().clone(), + feature, + SitePermissionAuditAction::Consumed, + ); + Ok(true) } pub(super) fn visible_site_permission_audit_events(&self) -> Vec { @@ -79,6 +170,12 @@ impl BrowserCore { ) -> Result<(), CoreError> { self.require_profile(profile_id)?; + self.transferred_site_permissions.retain(|permission| { + permission.entry.profile_id() != profile_id + || permission.entry.origin() != &origin + || permission.entry.feature() != feature + }); + if let Some(entry) = self.site_permission_entry_mut(profile_id, &origin, feature) { if entry.decision() == decision { return Ok(()); @@ -109,11 +206,20 @@ impl BrowserCore { feature: SitePermissionFeature, ) -> Result<(), CoreError> { self.require_profile(profile_id)?; - let Some(index) = self.site_permission_entry_index(profile_id, origin, feature) else { - return Ok(()); - }; - - let entry = self.site_permissions.remove(index); + let entry = + if let Some(index) = self.site_permission_entry_index(profile_id, origin, feature) { + self.site_permissions.remove(index) + } else if let Some(index) = + self.transferred_site_permissions.iter().position(|permission| { + permission.entry.profile_id() == profile_id + && permission.entry.origin() == origin + && permission.entry.feature() == feature + }) + { + self.transferred_site_permissions.remove(index).entry + } else { + return Ok(()); + }; self.record_site_permission_audit_event( profile_id.clone(), entry.origin().clone(), @@ -138,6 +244,15 @@ impl BrowserCore { true } }); + self.transferred_site_permissions.retain(|permission| { + if permission.entry.profile_id() == profile_id { + revoked_permissions + .push((permission.entry.origin().clone(), permission.entry.feature())); + false + } else { + true + } + }); let revoked_count = revoked_permissions.len(); for (origin, feature) in revoked_permissions { diff --git a/crates/ely_browser_core/src/state/sync_site_permissions.rs b/crates/ely_browser_core/src/state/sync_site_permissions.rs index 169e9bf..2df0e7e 100644 --- a/crates/ely_browser_core/src/state/sync_site_permissions.rs +++ b/crates/ely_browser_core/src/state/sync_site_permissions.rs @@ -15,6 +15,7 @@ impl BrowserCore { self.site_permissions .iter() .filter(|entry| self.profile_allows_cloud_sync(entry.profile_id())) + .filter(|entry| entry.decision() != SitePermissionDecision::AllowOnce) .collect() } @@ -30,6 +31,15 @@ impl BrowserCore { SitePermissionFeature::parse(&record.feature).map_err(snapshot_schema_error)?; let decision = SitePermissionDecision::parse(&record.decision).map_err(snapshot_schema_error)?; + if decision == SitePermissionDecision::AllowOnce { + summary.record_skipped(); + return Ok(()); + } + self.transferred_site_permissions.retain(|permission| { + permission.entry.profile_id() != &profile_id + || permission.entry.origin() != &origin + || permission.entry.feature() != feature + }); let existing_index = self.site_permissions.iter().position(|entry| { entry.profile_id() == &profile_id && entry.origin() == &origin @@ -51,3 +61,32 @@ impl BrowserCore { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::{InitialBrowserConfig, sync_engine::SyncSnapshotApplySummary}; + + #[test] + fn legacy_allow_once_sync_record_is_skipped() -> Result<(), Box> { + let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + let profile_id = core.snapshot()?.active_profile_id; + let record = SitePermissionSyncRecord { + profile_id: profile_id.as_str().to_string(), + origin: "https://example.com".to_string(), + feature: "camera".to_string(), + decision: "allow-once".to_string(), + }; + let mut summary = SyncSnapshotApplySummary::default(); + + core.apply_site_permission_sync_record( + record, + &mut summary, + &SyncSnapshotApplyContext::default(), + )?; + + assert_eq!(summary.skipped(), 1); + assert!(core.snapshot()?.site_permissions.is_empty()); + Ok(()) + } +} diff --git a/crates/ely_browser_core/tests/privacy.rs b/crates/ely_browser_core/tests/privacy.rs index 9eb27f6..2ca01bc 100644 --- a/crates/ely_browser_core/tests/privacy.rs +++ b/crates/ely_browser_core/tests/privacy.rs @@ -136,6 +136,32 @@ fn export_local_data_command_opens_privacy_security_page() -> Result<(), Box Result<(), Box> { + let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + let profile_id = core.snapshot()?.active_profile_id; + let origin = SiteOrigin::parse("https://example.com")?; + let feature = SitePermissionFeature::Camera; + core.set_site_permission(origin.clone(), feature, SitePermissionDecision::AllowOnce)?; + + let document: serde_json::Value = + serde_json::from_str(&core.export_local_data_package_json()?)?; + let inventory = core.active_profile_local_data_inventory(); + + assert_eq!(inventory.site_permissions(), 0); + assert_eq!(array_len(&document, "site_permissions"), 0); + assert_eq!(array_len(&document, "site_permission_audit_events"), 1); + + let revision = core.site_permission_revision(&profile_id, &origin, feature); + assert!(core.transfer_site_permission_once(&profile_id, &origin, feature, revision)?); + assert!(core.finish_site_permission_once(&profile_id, &origin, feature, revision)?); + let consumed: serde_json::Value = + serde_json::from_str(&core.export_local_data_package_json()?)?; + assert_eq!(array_len(&consumed, "site_permission_audit_events"), 3); + assert_eq!(consumed["site_permission_audit_events"][2]["action"]["kind"], "consumed"); + Ok(()) +} + fn array_len(document: &serde_json::Value, field: &str) -> usize { document[field].as_array().map_or(0, Vec::len) } diff --git a/crates/ely_browser_core/tests/site_data.rs b/crates/ely_browser_core/tests/site_data.rs index 0c3a2b6..10c9c5a 100644 --- a/crates/ely_browser_core/tests/site_data.rs +++ b/crates/ely_browser_core/tests/site_data.rs @@ -91,8 +91,17 @@ fn clear_active_profile_site_data_reports_removed_counts() -> Result<(), Box Result<(), Box Result<(), Box> { + let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + let snapshot = core.snapshot()?; + let profile_id = snapshot.active_profile_id; + let origin = SiteOrigin::parse("https://example.com")?; + let feature = SitePermissionFeature::Camera; + core.set_site_permission(origin.clone(), feature, SitePermissionDecision::AllowOnce)?; + let revision = core.site_permission_revision(&profile_id, &origin, feature); + + assert!(!core.transfer_site_permission_once(&profile_id, &origin, feature, revision + 1,)?); + assert!(core.transfer_site_permission_once(&profile_id, &origin, feature, revision)?); + + let snapshot = core.snapshot()?; + assert_eq!(snapshot.site_permissions.len(), 1); + assert_eq!( + snapshot.site_permission_audit_events.last().map(|event| event.action()), + Some(&SitePermissionAuditAction::Transferred), + ); + assert!(!core.finish_site_permission_once(&profile_id, &origin, feature, revision + 1)?); + assert!(core.finish_site_permission_once(&profile_id, &origin, feature, revision)?); + let snapshot = core.snapshot()?; + assert!(snapshot.site_permissions.is_empty()); + assert_eq!( + snapshot.site_permission_audit_events.last().map(|event| event.action()), + Some(&SitePermissionAuditAction::Consumed), + ); + Ok(()) +} + +#[test] +fn clear_site_permissions_revokes_transferred_allow_once() -> Result<(), Box> { + let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + let profile_id = core.snapshot()?.active_profile_id; + let origin = SiteOrigin::parse("https://example.com")?; + let feature = SitePermissionFeature::Camera; + core.set_site_permission(origin.clone(), feature, SitePermissionDecision::AllowOnce)?; + let revision = core.site_permission_revision(&profile_id, &origin, feature); + assert!(core.transfer_site_permission_once(&profile_id, &origin, feature, revision)?); + + assert_eq!(core.clear_active_profile_site_permissions()?, 1); + + let snapshot = core.snapshot()?; + assert!(snapshot.site_permissions.is_empty()); + assert_eq!( + snapshot.site_permission_audit_events.last().map(|event| event.action()), + Some(&SitePermissionAuditAction::Revoked), + ); + Ok(()) +} + +#[test] +fn revoke_then_readd_advances_permission_revision() -> Result<(), Box> { + let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + let profile_id = core.snapshot()?.active_profile_id; + let origin = SiteOrigin::parse("https://example.com")?; + let feature = SitePermissionFeature::Location; + core.set_site_permission(origin.clone(), feature, SitePermissionDecision::AllowOnce)?; + let first_revision = core.site_permission_revision(&profile_id, &origin, feature); + + core.revoke_site_permission(&origin, feature)?; + core.set_site_permission(origin.clone(), feature, SitePermissionDecision::AllowOnce)?; + + assert!(core.site_permission_revision(&profile_id, &origin, feature) > first_revision); + Ok(()) +} + #[test] fn site_settings_command_opens_active_origin() -> Result<(), Box> { let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; diff --git a/crates/ely_browser_core/tests/sync_site_permissions.rs b/crates/ely_browser_core/tests/sync_site_permissions.rs index 5179e4f..067acb0 100644 --- a/crates/ely_browser_core/tests/sync_site_permissions.rs +++ b/crates/ely_browser_core/tests/sync_site_permissions.rs @@ -98,3 +98,25 @@ fn sync_snapshot_omits_paused_site_permissions() -> Result<(), Box> { assert!(snapshot.site_permissions.is_empty()); Ok(()) } + +#[test] +fn sync_snapshot_omits_allow_once_permissions() -> Result<(), Box> { + let mut source = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + let source_home_tab_id = source.snapshot()?.active_tab_id; + source.set_tab_sync_enabled(&source_home_tab_id, false)?; + source.set_site_permission( + SiteOrigin::parse("https://example.com")?, + SitePermissionFeature::Camera, + SitePermissionDecision::AllowOnce, + )?; + let bytes = source.build_sync_snapshot_bytes()?; + + let mut target = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + let summary = target.apply_sync_snapshot_bytes(&bytes)?; + + assert_eq!(summary.imported(), 0); + assert_eq!(summary.updated(), 0); + assert_eq!(summary.skipped(), 0); + assert!(target.snapshot()?.site_permissions.is_empty()); + Ok(()) +} diff --git a/crates/ely_domain/src/site_permission.rs b/crates/ely_domain/src/site_permission.rs index 06266f9..1d50d40 100644 --- a/crates/ely_domain/src/site_permission.rs +++ b/crates/ely_domain/src/site_permission.rs @@ -46,6 +46,8 @@ pub struct SitePermissionEntry { pub enum SitePermissionAuditAction { Set(SitePermissionDecision), Revoked, + Transferred, + Consumed, } #[derive(Clone, Debug, Eq, PartialEq)] diff --git a/crates/ely_servo_host/src/bin/ely_servo_sidecar/live.rs b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live.rs index 848e2b3..561bb5c 100644 --- a/crates/ely_servo_host/src/bin/ely_servo_sidecar/live.rs +++ b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live.rs @@ -74,6 +74,8 @@ pub(super) fn run(args: LiveArgs) -> Result<(), LiveSidecarError> { request, ) }); + let outcome = outcome + .map(|outcome| outcome.with_permission_consumptions(host.take_consumed_permissions())); write_outcome(&mut stdout, outcome)?; if should_shutdown { break; @@ -126,6 +128,7 @@ fn handle_request( hover_x, hover_y, typed_text, + site_permission_generation, site_permissions, ready_surface_ids, pending_surface_ids, @@ -138,7 +141,13 @@ fn handle_request( let session = ensure_session(host, sessions, tab_id.clone(), &tab, &profile, width, height)?; apply_layout(host, session, width, height, page_zoom_percent, device_pixel_ratio)?; - apply_permissions(host, session, &profile, site_permissions)?; + apply_permissions( + host, + session, + &profile, + site_permission_generation, + site_permissions, + )?; if session.requested_url != url.as_str() { let servo_current_url = host.snapshot(&session.webview_id)?.url().map(str::to_string); diff --git a/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_output.rs b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_output.rs index 4720c13..cacd09a 100644 --- a/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_output.rs +++ b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_output.rs @@ -13,7 +13,9 @@ pub(super) fn write_outcome( if let Some(frame) = outcome.frame.as_ref() && let Err(error) = validate_frame(frame.width(), frame.height(), frame.rgba_bytes().len()) { + let consumptions = std::mem::take(&mut outcome.response.permission_consumptions); outcome = LiveOutcome::error(error.to_string()); + outcome.response.permission_consumptions = consumptions; } serde_json::to_writer(&mut *stdout, &outcome.response)?; @@ -39,11 +41,12 @@ mod tests { #[test] fn mismatched_frame_becomes_header_only_error() -> Result<(), LiveSidecarError> { + let profile_id = ely_domain::ProfileId::new(); let frame = ely_servo_host::RenderedFrame::from_rgba_bytes(2, 2, vec![0; 4]); let snapshot = ely_servo_host::WebViewSnapshot::new( ely_domain::WebViewId::new(), ely_domain::TabId::new(), - ely_domain::ProfileId::new(), + profile_id.clone(), ely_servo_host::WebViewState::Complete, None, None, @@ -52,13 +55,23 @@ mod tests { let report = super::super::live_protocol::LiveFrameReport::new(&snapshot, &frame, 1.0, true); let mut output = Vec::new(); + let outcome = LiveOutcome::frame(report, frame).with_permission_consumptions(vec![ + ely_servo_host::ConsumedPermission { + profile_id: profile_id.clone(), + origin: ely_domain::SiteOrigin::parse("https://example.com")?, + feature: ely_domain::SitePermissionFeature::Camera, + grant_revision: 7, + }, + ]); - write_outcome(&mut output, Ok(LiveOutcome::frame(report, frame)))?; + write_outcome(&mut output, Ok(outcome))?; assert!(output.ends_with(b"\n")); let response: serde_json::Value = serde_json::from_slice(&output)?; assert!(response["error"].as_str().is_some()); assert!(response["frame"].is_null()); + assert_eq!(response["permission_consumptions"][0]["profile_id"], profile_id.as_str()); + assert_eq!(response["permission_consumptions"][0]["grant_revision"], 7); Ok(()) } } diff --git a/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_protocol.rs b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_protocol.rs index b1ec4ef..ab51db1 100644 --- a/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_protocol.rs +++ b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_protocol.rs @@ -1,7 +1,8 @@ use std::io; use ely_servo_host::{ - IOSurfaceHandle, RenderedFrame, ServoHostError, WebViewSnapshot, WebViewState, + ConsumedPermission, IOSurfaceHandle, RenderedFrame, ServoHostError, WebViewSnapshot, + WebViewState, }; use serde::{Deserialize, Serialize}; use thiserror::Error; @@ -9,7 +10,7 @@ use thiserror::Error; #[cfg(all(feature = "hardware-render", target_os = "macos"))] use super::iosurface_mach::IOSurfaceMachError; -pub(super) const LIVE_PROTOCOL_VERSION: u32 = 2; +pub(super) const LIVE_PROTOCOL_VERSION: u32 = 3; pub(super) const MAX_FRAME_DIMENSION: u32 = 16_384; pub(super) const MAX_FRAME_BYTE_COUNT: usize = 256 * 1024 * 1024; @@ -47,6 +48,7 @@ pub(super) enum LiveRequest { hover_y: Option, #[serde(default)] typed_text: Option, + site_permission_generation: u64, #[serde(default)] site_permissions: Vec, #[serde(default)] @@ -79,7 +81,8 @@ const fn default_device_pixel_ratio() -> f32 { pub(super) struct LiveSitePermission { pub(super) origin: String, pub(super) feature: String, - pub(super) decision: String, + pub(super) state: String, + pub(super) revision: u64, } pub(super) struct LiveOutcome { @@ -100,6 +103,15 @@ impl LiveOutcome { Self { response: LiveResponse::frame(report), frame: Some(frame) } } + pub(super) fn with_permission_consumptions( + mut self, + consumptions: Vec, + ) -> Self { + self.response.permission_consumptions = + consumptions.into_iter().map(LivePermissionConsumption::from).collect(); + self + } + #[cfg(all(feature = "hardware-render", target_os = "macos"))] pub(super) fn surface(report: LiveFrameReport) -> Self { Self { response: LiveResponse::frame(report), frame: None } @@ -115,6 +127,27 @@ pub(super) struct LiveResponse { pub(super) surface_handle: Option, #[serde(skip_serializing_if = "Option::is_none")] pub(super) current_surface_id: Option, + #[serde(skip_serializing_if = "Vec::is_empty")] + pub(super) permission_consumptions: Vec, +} + +#[derive(Debug, Serialize)] +pub(super) struct LivePermissionConsumption { + pub(super) profile_id: String, + pub(super) origin: String, + pub(super) feature: String, + pub(super) grant_revision: u64, +} + +impl From for LivePermissionConsumption { + fn from(consumed: ConsumedPermission) -> Self { + Self { + profile_id: consumed.profile_id.as_str().to_string(), + origin: consumed.origin.as_str().to_string(), + feature: consumed.feature.as_str().to_string(), + grant_revision: consumed.grant_revision, + } + } } impl LiveResponse { @@ -125,6 +158,7 @@ impl LiveResponse { frame: None, surface_handle: None, current_surface_id: None, + permission_consumptions: Vec::new(), } } @@ -135,6 +169,7 @@ impl LiveResponse { frame: None, surface_handle: None, current_surface_id: None, + permission_consumptions: Vec::new(), } } @@ -145,6 +180,7 @@ impl LiveResponse { frame: Some(frame), surface_handle: None, current_surface_id: None, + permission_consumptions: Vec::new(), } } } @@ -332,7 +368,7 @@ mod tests { #[test] fn ensure_defaults_optional_input_fields() -> Result<(), serde_json::Error> { let request = serde_json::from_str::( - r#"{"type":"ensure","tab_id":"tab","profile_id":"profile","url":"https://example.com","width":800,"height":600}"#, + r#"{"type":"ensure","tab_id":"tab","profile_id":"profile","url":"https://example.com","width":800,"height":600,"site_permission_generation":0}"#, )?; assert!(matches!( @@ -353,12 +389,21 @@ mod tests { #[test] fn handshake_deserializes_protocol_version() -> Result<(), serde_json::Error> { let request = - serde_json::from_str::(r#"{"type":"handshake","protocol_version":2}"#)?; + serde_json::from_str::(r#"{"type":"handshake","protocol_version":3}"#)?; - assert!(matches!(request, LiveRequest::Handshake { protocol_version: 2 })); + assert!(matches!(request, LiveRequest::Handshake { protocol_version: 3 })); Ok(()) } + #[test] + fn site_permission_requires_revision() { + let request = serde_json::from_str::( + r#"{"type":"ensure","tab_id":"tab","profile_id":"profile","url":"https://example.com","width":800,"height":600,"site_permission_generation":0,"site_permissions":[{"origin":"https://example.com","feature":"camera","state":"allow-once"}]}"#, + ); + + assert!(request.is_err()); + } + #[test] fn frame_layout_enforces_dimension_and_byte_limits() { assert!(matches!( diff --git a/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_session.rs b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_session.rs index 2f126bf..9505a12 100644 --- a/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_session.rs +++ b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_session.rs @@ -3,7 +3,8 @@ use std::collections::{HashMap, hash_map::Entry}; use ely_domain::{ProfileId, TabId, validate_zoom_percent}; use ely_servo_host::{ HidpiScaleRequest, KeyboardTextRequest, MouseClickRequest, MouseHoverRequest, PageZoomRequest, - PermissionDecision, PermissionRequest, RenderedFrame, ResizeRequest, ScrollRequest, ServoHost, + PermissionDecision, PermissionSnapshotEntry, PermissionSnapshotRequest, + PermissionSnapshotState, RenderedFrame, ResizeRequest, ScrollRequest, ServoHost, ServoSurfaceSize, SoftwareServoHost, }; @@ -132,22 +133,41 @@ pub(super) fn apply_permissions( host: &mut SoftwareServoHost, session: &LiveSession, profile_id: &ProfileId, + generation: u64, permissions: Vec, ) -> Result<(), LiveSidecarError> { - for permission in permissions { - host.set_permission( - PermissionRequest { - webview_id: session.webview_id.clone(), - profile_id: profile_id.clone(), + let entries = permissions + .into_iter() + .map(|permission| { + let state = match permission.state.as_str() { + "allow-once" => PermissionSnapshotState::Decision(PermissionDecision::AllowOnce), + "allow-always" => { + PermissionSnapshotState::Decision(PermissionDecision::AllowAlways) + } + "deny-always" => PermissionSnapshotState::Decision(PermissionDecision::DenyAlways), + "transferred-allow-once" => PermissionSnapshotState::TransferredAllowOnce, + value => { + return Err(ely_domain::DomainError::InvalidSitePermissionDecision { + value: value.to_string(), + } + .into()); + } + }; + Ok(PermissionSnapshotEntry { origin: ely_domain::SiteOrigin::parse(permission.origin)?, feature: ely_domain::SitePermissionFeature::parse(&permission.feature)?, - }, - PermissionDecision::from(ely_domain::SitePermissionDecision::parse( - &permission.decision, - )?), - )?; - } - Ok(()) + state, + revision: permission.revision, + }) + }) + .collect::, LiveSidecarError>>()?; + host.replace_permissions(PermissionSnapshotRequest { + webview_id: session.webview_id.clone(), + profile_id: profile_id.clone(), + generation, + entries, + }) + .map_err(LiveSidecarError::from) } pub(super) struct LiveInput { diff --git a/crates/ely_servo_host/src/error.rs b/crates/ely_servo_host/src/error.rs index a1abd72..9bd0325 100644 --- a/crates/ely_servo_host/src/error.rs +++ b/crates/ely_servo_host/src/error.rs @@ -1,4 +1,4 @@ -use ely_domain::{ProfileId, WebViewId}; +use ely_domain::{ProfileId, SiteOrigin, SitePermissionFeature, WebViewId}; use thiserror::Error; #[derive(Clone, Debug, Error, Eq, PartialEq)] @@ -15,6 +15,13 @@ pub enum ServoHostError { #[error("permission profile mismatch for {webview_id}: expected {expected}, got {actual}")] PermissionProfileMismatch { webview_id: WebViewId, expected: ProfileId, actual: ProfileId }, + #[error("duplicate permission snapshot entry for {profile_id} {origin:?} {feature:?}")] + DuplicatePermissionSnapshotEntry { + profile_id: ProfileId, + origin: SiteOrigin, + feature: SitePermissionFeature, + }, + #[error("servo runtime is already started in this process")] RuntimeAlreadyStarted, diff --git a/crates/ely_servo_host/src/host.rs b/crates/ely_servo_host/src/host.rs index 330f85e..e3fd441 100644 --- a/crates/ely_servo_host/src/host.rs +++ b/crates/ely_servo_host/src/host.rs @@ -314,21 +314,43 @@ pub struct KeyboardTextRequest { pub text: String, } -#[derive(Clone, Debug, Eq, PartialEq)] -pub struct PermissionRequest { - pub webview_id: WebViewId, - pub profile_id: ProfileId, - pub origin: SiteOrigin, - pub feature: SitePermissionFeature, -} - -#[derive(Clone, Debug, Eq, PartialEq)] +#[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum PermissionDecision { AllowOnce, AllowAlways, DenyAlways, } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum PermissionSnapshotState { + Decision(PermissionDecision), + TransferredAllowOnce, +} + +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct PermissionSnapshotEntry { + pub origin: SiteOrigin, + pub feature: SitePermissionFeature, + pub state: PermissionSnapshotState, + pub revision: u64, +} + +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct PermissionSnapshotRequest { + pub webview_id: WebViewId, + pub profile_id: ProfileId, + pub generation: u64, + pub entries: Vec, +} + +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct ConsumedPermission { + pub profile_id: ProfileId, + pub origin: SiteOrigin, + pub feature: SitePermissionFeature, + pub grant_revision: u64, +} + impl From for PermissionDecision { fn from(decision: SitePermissionDecision) -> Self { match decision { @@ -366,12 +388,13 @@ pub trait ServoHost { fn type_text(&mut self, request: KeyboardTextRequest) -> Result<(), ServoHostError>; - fn set_permission( + fn replace_permissions( &mut self, - request: PermissionRequest, - decision: PermissionDecision, + request: PermissionSnapshotRequest, ) -> Result<(), ServoHostError>; + fn take_consumed_permissions(&mut self) -> Vec; + fn state(&self, webview_id: &WebViewId) -> Result; fn snapshot(&self, webview_id: &WebViewId) -> Result; diff --git a/crates/ely_servo_host/src/lib.rs b/crates/ely_servo_host/src/lib.rs index b7041ae..2a22f8d 100644 --- a/crates/ely_servo_host/src/lib.rs +++ b/crates/ely_servo_host/src/lib.rs @@ -20,8 +20,9 @@ pub use error::ServoHostError; #[cfg(feature = "hardware-render")] pub use hardware_rendering_context::HardwareOffscreenContext; pub use host::{ - HidpiScaleRequest, KeyboardTextRequest, MouseClickRequest, MouseDragRequest, MouseHoverRequest, - NavigationRequest, PageZoomRequest, PermissionDecision, PermissionRequest, RenderedFrame, + ConsumedPermission, HidpiScaleRequest, KeyboardTextRequest, MouseClickRequest, + MouseDragRequest, MouseHoverRequest, NavigationRequest, PageZoomRequest, PermissionDecision, + PermissionSnapshotEntry, PermissionSnapshotRequest, PermissionSnapshotState, RenderedFrame, RenderedFrameSummary, ResizeRequest, ScrollRequest, ServoHost, TouchTapRequest, WebViewSnapshot, WebViewSnapshotPending, WebViewState, }; diff --git a/crates/ely_servo_host/src/runtime.rs b/crates/ely_servo_host/src/runtime.rs index b89d588..cc0d6cc 100644 --- a/crates/ely_servo_host/src/runtime.rs +++ b/crates/ely_servo_host/src/runtime.rs @@ -1,6 +1,5 @@ use std::{ - cell::RefCell, - collections::HashMap, + collections::{HashMap, HashSet}, path::PathBuf, rc::Rc, sync::{ @@ -13,8 +12,8 @@ use dpi::PhysicalSize; use ely_domain::{ProfileId, TabId, WebViewId}; use raw_window_handle::{HasDisplayHandle, HasWindowHandle}; use servo::{ - DevicePoint, DeviceVector2D, Opts, Preferences, Scroll, Servo, ServoBuilder, WebViewBuilder, - WebViewPoint, WebViewVector, + DevicePoint, DeviceVector2D, Opts, Scroll, Servo, ServoBuilder, WebViewBuilder, WebViewPoint, + WebViewVector, }; #[path = "runtime_context.rs"] @@ -24,20 +23,25 @@ mod runtime_context; mod runtime_hardware; #[path = "runtime_paint.rs"] mod runtime_paint; +#[path = "runtime_preferences.rs"] +mod runtime_preferences; use runtime_context::hidpi_scale_from_factor; pub use runtime_context::{RenderingContextKind, ServoSurfaceSize}; +use runtime_preferences::ely_servo_preferences; use url::Url; use crate::{ - HidpiScaleRequest, KeyboardTextRequest, MouseClickRequest, MouseDragRequest, MouseHoverRequest, - NavigationRequest, PageZoomRequest, PermissionDecision, PermissionRequest, RenderedFrame, - ResizeRequest, ScrollRequest, ServoHost, ServoHostError, TouchTapRequest, WebViewSnapshot, - WebViewState, + ConsumedPermission, HidpiScaleRequest, KeyboardTextRequest, MouseClickRequest, + MouseDragRequest, MouseHoverRequest, NavigationRequest, PageZoomRequest, + PermissionSnapshotRequest, RenderedFrame, ResizeRequest, ScrollRequest, ServoHost, + ServoHostError, TouchTapRequest, WebViewSnapshot, WebViewState, runtime_input::{ send_keyboard_text, send_mouse_click, send_mouse_drag, send_mouse_hover, send_touch_tap, }, - runtime_permissions::{PermissionStore, set_permission_decision}, + runtime_permissions::{ + PermissionStore, drain_consumed_permissions, replace_permission_decisions, + }, runtime_waker::ServoWakeFlag, runtime_webview::{HostWebView, HostWebViewDelegate}, }; @@ -135,7 +139,7 @@ impl SoftwareServoHost { default_surface_size: size, rendering_context_kind, webviews: HashMap::new(), - permissions: Rc::new(RefCell::new(HashMap::new())), + permissions: PermissionStore::default(), wake_requested, last_rendered_frame: None, }) @@ -166,25 +170,6 @@ fn install_rustls_provider() { }); } -fn ely_servo_preferences() -> Preferences { - // `Preferences::default()` is Servo's conservative *library* default: it ships - // modern-layout features off even though servo-layout/Stylo implement them and - // Servo's own servoshell browser enables them. `Servo::new` forwards these to - // Stylo via `prefs::set`, so an embedder building a browser must turn them on or - // pages render wrong: - // - `layout.grid.enabled` off => Stylo blockifies `display: grid`, collapsing - // grid layouts into one stacked column (the "broken" modern-site render). - // - `layout.variable_fonts.enabled` off => `font-variation-settings` and - // variable weight/width axes are ignored, so a variable font only ever - // renders its default instance (every requested weight looks identical). - Preferences { - dom_intersection_observer_enabled: true, - layout_grid_enabled: true, - layout_variable_fonts_enabled: true, - ..Preferences::default() - } -} - impl ServoHost for SoftwareServoHost { fn create_webview( &mut self, @@ -332,10 +317,9 @@ impl ServoHost for SoftwareServoHost { Ok(()) } - fn set_permission( + fn replace_permissions( &mut self, - request: PermissionRequest, - decision: PermissionDecision, + request: PermissionSnapshotRequest, ) -> Result<(), ServoHostError> { let webview = self .webviews @@ -348,11 +332,29 @@ impl ServoHost for SoftwareServoHost { actual: request.profile_id, }); } - - set_permission_decision(&self.permissions, request, decision); + let mut keys = HashSet::new(); + for entry in &request.entries { + if !keys.insert((entry.origin.clone(), entry.feature)) { + return Err(ServoHostError::DuplicatePermissionSnapshotEntry { + profile_id: request.profile_id, + origin: entry.origin.clone(), + feature: entry.feature, + }); + } + } + replace_permission_decisions( + &self.permissions, + &request.profile_id, + request.generation, + request.entries, + ); Ok(()) } + fn take_consumed_permissions(&mut self) -> Vec { + drain_consumed_permissions(&self.permissions) + } + fn state(&self, webview_id: &WebViewId) -> Result { Ok(self.webview(webview_id)?.state()) } diff --git a/crates/ely_servo_host/src/runtime_paint.rs b/crates/ely_servo_host/src/runtime_paint.rs index aad2441..c10957f 100644 --- a/crates/ely_servo_host/src/runtime_paint.rs +++ b/crates/ely_servo_host/src/runtime_paint.rs @@ -53,16 +53,14 @@ fn consume_pending_then_paint(delegate: &HostWebViewDelegate, paint: impl FnOnce #[cfg(test)] mod tests { - use std::{cell::RefCell, collections::HashMap, rc::Rc}; - use ely_domain::ProfileId; use super::{HostWebViewDelegate, consume_pending_then_paint}; + use crate::runtime_permissions::PermissionStore; #[test] fn frame_arriving_during_paint_remains_pending() { - let delegate = - HostWebViewDelegate::new(ProfileId::new(), Rc::new(RefCell::new(HashMap::new()))); + let delegate = HostWebViewDelegate::new(ProfileId::new(), PermissionStore::default()); delegate.mark_frame_ready(); consume_pending_then_paint(&delegate, || delegate.mark_frame_ready()); diff --git a/crates/ely_servo_host/src/runtime_permissions.rs b/crates/ely_servo_host/src/runtime_permissions.rs index 4d7867c..788800d 100644 --- a/crates/ely_servo_host/src/runtime_permissions.rs +++ b/crates/ely_servo_host/src/runtime_permissions.rs @@ -3,9 +3,25 @@ use std::{cell::RefCell, collections::HashMap, rc::Rc}; use ely_domain::{ProfileId, SiteOrigin, SitePermissionFeature}; use servo::WebView; -use crate::{PermissionDecision, PermissionRequest}; +use crate::{ + ConsumedPermission, PermissionDecision, PermissionSnapshotEntry, PermissionSnapshotState, +}; -pub(super) type PermissionStore = Rc>>; +pub(super) type PermissionStore = Rc>; + +#[derive(Default)] +pub(super) struct PermissionState { + entries: HashMap, + generations: HashMap, + consumed: Vec, +} + +#[derive(Clone, Debug, Eq, PartialEq)] +struct StoredPermission { + decision: PermissionDecision, + grant_revision: u64, + consumed: bool, +} #[derive(Clone, Debug, Eq, Hash, PartialEq)] pub(super) struct PermissionKey { @@ -20,14 +36,64 @@ impl PermissionKey { } } -pub(super) fn set_permission_decision( +pub(super) fn replace_permission_decisions( permissions: &PermissionStore, - request: PermissionRequest, - decision: PermissionDecision, + profile_id: &ProfileId, + generation: u64, + entries: Vec, ) { - permissions - .borrow_mut() - .insert(PermissionKey::new(request.profile_id, request.origin, request.feature), decision); + let mut state = permissions.borrow_mut(); + if state.generations.get(profile_id).is_some_and(|current| generation < *current) { + return; + } + let mut freshly_consumed = Vec::new(); + let replacements = entries + .into_iter() + .map(|entry| { + let key = PermissionKey::new(profile_id.clone(), entry.origin, entry.feature); + let stored = match entry.state { + PermissionSnapshotState::Decision(decision) => { + let consumed = decision == PermissionDecision::AllowOnce + && state.entries.get(&key).is_some_and(|current| { + current.decision == PermissionDecision::AllowOnce + && current.grant_revision == entry.revision + && current.consumed + }); + StoredPermission { decision, grant_revision: entry.revision, consumed } + } + PermissionSnapshotState::TransferredAllowOnce => match state.entries.get(&key) { + Some(current) + if current.decision == PermissionDecision::AllowOnce + && current.grant_revision == entry.revision => + { + StoredPermission { + decision: PermissionDecision::AllowOnce, + grant_revision: current.grant_revision, + consumed: current.consumed, + } + } + _ => { + freshly_consumed.push(ConsumedPermission { + profile_id: profile_id.clone(), + origin: key.origin.clone(), + feature: key.feature, + grant_revision: entry.revision, + }); + StoredPermission { + decision: PermissionDecision::AllowOnce, + grant_revision: entry.revision, + consumed: true, + } + } + }, + }; + (key, stored) + }) + .collect::>(); + state.entries.retain(|key, _| &key.profile_id != profile_id); + state.entries.extend(replacements); + state.consumed.extend(freshly_consumed); + state.generations.insert(profile_id.clone(), generation); } pub(super) fn permission_decision_for_webview( @@ -49,11 +115,31 @@ fn take_permission_decision( feature: SitePermissionFeature, ) -> Option { let key = PermissionKey::new(profile_id.clone(), origin, feature); - let mut permissions = permissions.borrow_mut(); - match permissions.get(&key).cloned() { - Some(PermissionDecision::AllowOnce) => permissions.remove(&key), - decision => decision, + let mut state = permissions.borrow_mut(); + let (decision, grant_revision) = { + let stored = state.entries.get_mut(&key)?; + match stored.decision { + PermissionDecision::AllowOnce if stored.consumed => return None, + PermissionDecision::AllowOnce => { + stored.consumed = true; + (PermissionDecision::AllowOnce, Some(stored.grant_revision)) + } + decision => (decision, None), + } + }; + if let Some(grant_revision) = grant_revision { + state.consumed.push(ConsumedPermission { + profile_id: key.profile_id, + origin: key.origin, + feature: key.feature, + grant_revision, + }); } + Some(decision) +} + +pub(super) fn drain_consumed_permissions(permissions: &PermissionStore) -> Vec { + std::mem::take(&mut permissions.borrow_mut().consumed) } fn site_origin_for_webview(webview: &WebView, fallback_url: Option) -> Option { @@ -87,121 +173,5 @@ fn site_permission_feature_for_servo( } #[cfg(test)] -mod tests { - use ely_domain::{ProfileId, SiteOrigin, SitePermissionFeature}; - - use crate::{PermissionDecision, PermissionRequest}; - - use super::{ - PermissionStore, set_permission_decision, site_permission_feature_for_servo, - take_permission_decision, - }; - - #[test] - fn keeps_disabled_servo_permissions_out_of_site_settings() { - for feature in [ - servo::PermissionFeature::ScreenWakeLock(servo::WakeLockType::Screen), - servo::PermissionFeature::Gamepad, - ] { - assert_eq!(site_permission_feature_for_servo(feature), None); - } - } - - #[test] - fn allow_once_is_consumed_after_one_matching_origin_request() - -> Result<(), Box> { - let permissions = PermissionStore::default(); - let profile_id = ProfileId::new(); - let origin = SiteOrigin::parse("https://example.com/path")?; - - set_permission_decision( - &permissions, - PermissionRequest { - webview_id: ely_domain::WebViewId::new(), - profile_id: profile_id.clone(), - origin: origin.clone(), - feature: SitePermissionFeature::Camera, - }, - PermissionDecision::AllowOnce, - ); - - assert_eq!( - take_permission_decision( - &permissions, - &profile_id, - origin.clone(), - SitePermissionFeature::Camera, - ), - Some(PermissionDecision::AllowOnce) - ); - assert_eq!( - take_permission_decision( - &permissions, - &profile_id, - origin, - SitePermissionFeature::Camera - ), - None - ); - Ok(()) - } - - #[test] - fn allow_always_stays_scoped_to_profile_origin_and_feature() - -> Result<(), Box> { - let permissions = PermissionStore::default(); - let profile_id = ProfileId::new(); - let other_profile_id = ProfileId::new(); - let origin = SiteOrigin::parse("https://example.com")?; - let other_origin = SiteOrigin::parse("https://example.org")?; - - set_permission_decision( - &permissions, - PermissionRequest { - webview_id: ely_domain::WebViewId::new(), - profile_id: profile_id.clone(), - origin: origin.clone(), - feature: SitePermissionFeature::Notifications, - }, - PermissionDecision::AllowAlways, - ); - - assert_eq!( - take_permission_decision( - &permissions, - &profile_id, - origin.clone(), - SitePermissionFeature::Notifications, - ), - Some(PermissionDecision::AllowAlways) - ); - assert_eq!( - take_permission_decision( - &permissions, - &other_profile_id, - origin, - SitePermissionFeature::Notifications, - ), - None - ); - assert_eq!( - take_permission_decision( - &permissions, - &profile_id, - other_origin, - SitePermissionFeature::Notifications, - ), - None - ); - assert_eq!( - take_permission_decision( - &permissions, - &profile_id, - SiteOrigin::parse("https://example.com")?, - SitePermissionFeature::Camera, - ), - None - ); - Ok(()) - } -} +#[path = "runtime_permissions_tests.rs"] +mod tests; diff --git a/crates/ely_servo_host/src/runtime_permissions_tests.rs b/crates/ely_servo_host/src/runtime_permissions_tests.rs new file mode 100644 index 0000000..85cd181 --- /dev/null +++ b/crates/ely_servo_host/src/runtime_permissions_tests.rs @@ -0,0 +1,349 @@ +use ely_domain::{ProfileId, SiteOrigin, SitePermissionFeature}; + +use crate::{ + ConsumedPermission, PermissionDecision, PermissionSnapshotEntry, PermissionSnapshotState, + runtime_permissions::{ + PermissionStore, drain_consumed_permissions, replace_permission_decisions, + site_permission_feature_for_servo, take_permission_decision, + }, +}; + +#[test] +fn keeps_disabled_servo_permissions_out_of_site_settings() { + for feature in [ + servo::PermissionFeature::ScreenWakeLock(servo::WakeLockType::Screen), + servo::PermissionFeature::Gamepad, + ] { + assert_eq!(site_permission_feature_for_servo(feature), None); + } +} + +#[test] +fn allow_once_is_consumed_after_one_matching_request() -> Result<(), Box> { + let permissions = PermissionStore::default(); + let profile_id = ProfileId::new(); + let origin = SiteOrigin::parse("https://example.com")?; + replace( + &permissions, + &profile_id, + 1, + vec![decision( + origin.clone(), + SitePermissionFeature::Camera, + PermissionDecision::AllowOnce, + 1, + )], + ); + + assert_eq!(take(&permissions, &profile_id, &origin), Some(PermissionDecision::AllowOnce)); + assert_eq!(take(&permissions, &profile_id, &origin), None); + assert_eq!( + drain_consumed_permissions(&permissions), + vec![ConsumedPermission { + profile_id, + origin, + feature: SitePermissionFeature::Camera, + grant_revision: 1, + }], + ); + Ok(()) +} + +#[test] +fn durable_permissions_stay_scoped_to_profile_origin_and_feature() +-> Result<(), Box> { + let permissions = PermissionStore::default(); + let profile_id = ProfileId::new(); + let other_profile = ProfileId::new(); + let origin = SiteOrigin::parse("https://example.com")?; + replace( + &permissions, + &profile_id, + 1, + vec![decision( + origin.clone(), + SitePermissionFeature::Camera, + PermissionDecision::AllowAlways, + 1, + )], + ); + + assert_eq!(take(&permissions, &profile_id, &origin), Some(PermissionDecision::AllowAlways)); + assert_eq!(take(&permissions, &other_profile, &origin), None); + assert_eq!( + take_permission_decision( + &permissions, + &profile_id, + SiteOrigin::parse("https://other.test")?, + SitePermissionFeature::Camera, + ), + None, + ); + Ok(()) +} + +#[test] +fn authoritative_snapshot_removes_missing_durable_entries() -> Result<(), Box> +{ + let permissions = PermissionStore::default(); + let profile_id = ProfileId::new(); + let other_profile = ProfileId::new(); + let origin = SiteOrigin::parse("https://example.com")?; + let other_origin = SiteOrigin::parse("https://other.test")?; + replace( + &permissions, + &profile_id, + 1, + vec![decision( + origin.clone(), + SitePermissionFeature::Camera, + PermissionDecision::AllowAlways, + 1, + )], + ); + replace( + &permissions, + &other_profile, + 1, + vec![decision( + other_origin.clone(), + SitePermissionFeature::Camera, + PermissionDecision::DenyAlways, + 1, + )], + ); + + replace(&permissions, &profile_id, 2, Vec::new()); + + assert_eq!(take(&permissions, &profile_id, &origin), None); + assert_eq!( + take(&permissions, &other_profile, &other_origin), + Some(PermissionDecision::DenyAlways) + ); + Ok(()) +} + +#[test] +fn transferred_marker_preserves_existing_token_and_fails_closed_for_fresh_host() +-> Result<(), Box> { + let profile_id = ProfileId::new(); + let origin = SiteOrigin::parse("https://example.com")?; + let marker = state( + origin.clone(), + SitePermissionFeature::Camera, + PermissionSnapshotState::TransferredAllowOnce, + 1, + ); + let live = PermissionStore::default(); + replace( + &live, + &profile_id, + 1, + vec![decision( + origin.clone(), + SitePermissionFeature::Camera, + PermissionDecision::AllowOnce, + 1, + )], + ); + replace(&live, &profile_id, 2, vec![marker.clone()]); + + let restarted = PermissionStore::default(); + replace(&restarted, &profile_id, 2, vec![marker]); + + assert!(drain_consumed_permissions(&live).is_empty()); + assert_eq!( + drain_consumed_permissions(&restarted), + vec![ConsumedPermission { + profile_id: profile_id.clone(), + origin: origin.clone(), + feature: SitePermissionFeature::Camera, + grant_revision: 1, + }], + ); + assert_eq!(take(&live, &profile_id, &origin), Some(PermissionDecision::AllowOnce)); + assert_eq!(take(&restarted, &profile_id, &origin), None); + assert_eq!( + drain_consumed_permissions(&live), + vec![ConsumedPermission { + profile_id, + origin, + feature: SitePermissionFeature::Camera, + grant_revision: 1, + }], + ); + Ok(()) +} + +#[test] +fn empty_snapshot_removes_transferred_allow_once() -> Result<(), Box> { + let permissions = PermissionStore::default(); + let profile_id = ProfileId::new(); + let origin = SiteOrigin::parse("https://example.com")?; + replace( + &permissions, + &profile_id, + 1, + vec![decision( + origin.clone(), + SitePermissionFeature::Camera, + PermissionDecision::AllowOnce, + 1, + )], + ); + replace( + &permissions, + &profile_id, + 2, + vec![state( + origin.clone(), + SitePermissionFeature::Camera, + PermissionSnapshotState::TransferredAllowOnce, + 1, + )], + ); + replace(&permissions, &profile_id, 3, Vec::new()); + + assert_eq!(take(&permissions, &profile_id, &origin), None); + Ok(()) +} + +#[test] +fn mismatched_transferred_revision_consumes_the_replacement_token() +-> Result<(), Box> { + let permissions = PermissionStore::default(); + let profile_id = ProfileId::new(); + let origin = SiteOrigin::parse("https://example.com")?; + replace( + &permissions, + &profile_id, + 1, + vec![decision( + origin.clone(), + SitePermissionFeature::Camera, + PermissionDecision::AllowOnce, + 1, + )], + ); + replace( + &permissions, + &profile_id, + 2, + vec![state( + origin.clone(), + SitePermissionFeature::Camera, + PermissionSnapshotState::TransferredAllowOnce, + 2, + )], + ); + + assert_eq!(take(&permissions, &profile_id, &origin), None); + assert_eq!( + drain_consumed_permissions(&permissions), + vec![ConsumedPermission { + profile_id, + origin, + feature: SitePermissionFeature::Camera, + grant_revision: 2, + }], + ); + Ok(()) +} + +#[test] +fn stale_profile_generation_cannot_restore_revoked_permission() +-> Result<(), Box> { + let permissions = PermissionStore::default(); + let profile_id = ProfileId::new(); + let origin = SiteOrigin::parse("https://example.com")?; + let old = + decision(origin.clone(), SitePermissionFeature::Camera, PermissionDecision::AllowAlways, 1); + replace(&permissions, &profile_id, 1, vec![old.clone()]); + replace(&permissions, &profile_id, 2, Vec::new()); + + replace(&permissions, &profile_id, 1, vec![old]); + + assert_eq!(take(&permissions, &profile_id, &origin), None); + Ok(()) +} + +#[test] +fn same_revision_stays_consumed_and_new_revision_rearms_token() +-> Result<(), Box> { + let permissions = PermissionStore::default(); + let profile_id = ProfileId::new(); + let origin = SiteOrigin::parse("https://example.com")?; + replace( + &permissions, + &profile_id, + 1, + vec![decision( + origin.clone(), + SitePermissionFeature::Camera, + PermissionDecision::AllowOnce, + 1, + )], + ); + assert_eq!(take(&permissions, &profile_id, &origin), Some(PermissionDecision::AllowOnce)); + replace( + &permissions, + &profile_id, + 2, + vec![decision( + origin.clone(), + SitePermissionFeature::Camera, + PermissionDecision::AllowOnce, + 1, + )], + ); + assert_eq!(take(&permissions, &profile_id, &origin), None); + replace( + &permissions, + &profile_id, + 3, + vec![decision( + origin.clone(), + SitePermissionFeature::Camera, + PermissionDecision::AllowOnce, + 3, + )], + ); + + assert_eq!(take(&permissions, &profile_id, &origin), Some(PermissionDecision::AllowOnce)); + Ok(()) +} + +fn replace( + permissions: &PermissionStore, + profile_id: &ProfileId, + generation: u64, + entries: Vec, +) { + replace_permission_decisions(permissions, profile_id, generation, entries); +} + +fn decision( + origin: SiteOrigin, + feature: SitePermissionFeature, + decision: PermissionDecision, + revision: u64, +) -> PermissionSnapshotEntry { + state(origin, feature, PermissionSnapshotState::Decision(decision), revision) +} + +fn state( + origin: SiteOrigin, + feature: SitePermissionFeature, + state: PermissionSnapshotState, + revision: u64, +) -> PermissionSnapshotEntry { + PermissionSnapshotEntry { origin, feature, state, revision } +} + +fn take( + permissions: &PermissionStore, + profile_id: &ProfileId, + origin: &SiteOrigin, +) -> Option { + take_permission_decision(permissions, profile_id, origin.clone(), SitePermissionFeature::Camera) +} diff --git a/crates/ely_servo_host/src/runtime_preferences.rs b/crates/ely_servo_host/src/runtime_preferences.rs new file mode 100644 index 0000000..f4ff2fb --- /dev/null +++ b/crates/ely_servo_host/src/runtime_preferences.rs @@ -0,0 +1,49 @@ +use servo::Preferences; + +pub(super) fn ely_servo_preferences() -> Preferences { + // Servo's current permission request omits the requesting principal. WebRTC, + // Clipboard, and Bluetooth also contain paths that bypass the embedder broker. + // Keep every affected API explicitly gated until those contracts are complete. + Preferences { + dom_async_clipboard_enabled: false, + dom_bluetooth_enabled: false, + dom_geolocation_enabled: false, + dom_notification_enabled: false, + dom_permissions_enabled: false, + dom_storage_manager_api_enabled: false, + dom_wakelock_enabled: false, + dom_webrtc_enabled: false, + dom_intersection_observer_enabled: true, + layout_grid_enabled: true, + layout_variable_fonts_enabled: true, + ..Preferences::default() + } +} + +#[cfg(test)] +mod tests { + use super::ely_servo_preferences; + + #[test] + fn unsafe_permission_surfaces_stay_gated() { + let preferences = ely_servo_preferences(); + + assert!(!preferences.dom_async_clipboard_enabled); + assert!(!preferences.dom_bluetooth_enabled); + assert!(!preferences.dom_geolocation_enabled); + assert!(!preferences.dom_notification_enabled); + assert!(!preferences.dom_permissions_enabled); + assert!(!preferences.dom_storage_manager_api_enabled); + assert!(!preferences.dom_wakelock_enabled); + assert!(!preferences.dom_webrtc_enabled); + } + + #[test] + fn required_rendering_features_stay_enabled() { + let preferences = ely_servo_preferences(); + + assert!(preferences.dom_intersection_observer_enabled); + assert!(preferences.layout_grid_enabled); + assert!(preferences.layout_variable_fonts_enabled); + } +} diff --git a/crates/ely_servo_host/src/runtime_webview.rs b/crates/ely_servo_host/src/runtime_webview.rs index 519a560..45349bc 100644 --- a/crates/ely_servo_host/src/runtime_webview.rs +++ b/crates/ely_servo_host/src/runtime_webview.rs @@ -184,16 +184,13 @@ impl WebViewDelegate for HostWebViewDelegate { #[cfg(test)] mod tests { - use std::{cell::RefCell, collections::HashMap, rc::Rc}; - use ely_domain::ProfileId; - use super::HostWebViewDelegate; + use super::{HostWebViewDelegate, PermissionStore}; #[test] fn metadata_changes_are_separate_from_pending_frame() { - let delegate = - HostWebViewDelegate::new(ProfileId::new(), Rc::new(RefCell::new(HashMap::new()))); + let delegate = HostWebViewDelegate::new(ProfileId::new(), PermissionStore::default()); assert!(!delegate.has_pending_frame()); assert!(!delegate.has_pending_metadata()); diff --git a/crates/ely_servo_host/tests/sidecar.rs b/crates/ely_servo_host/tests/sidecar.rs index 8879bca..5d57fc2 100644 --- a/crates/ely_servo_host/tests/sidecar.rs +++ b/crates/ely_servo_host/tests/sidecar.rs @@ -156,6 +156,74 @@ fn live_sidecar_rejects_oversized_frame_dimensions() -> Result<(), Box Result<(), Box> { + let root = TestDirectory::new()?; + let profile_id = ProfileId::new(); + let tab_id = TabId::new(); + let mut sidecar = Sidecar::spawn(root.path())?; + let mut ensure = ensure_request(&tab_id, &profile_id, "about:blank"); + ensure["site_permissions"] = json!([ + { + "origin": "https://example.com", + "feature": "camera", + "state": "allow-always", + "revision": 1, + }, + { + "origin": "https://example.com", + "feature": "camera", + "state": "deny-always", + "revision": 2, + }, + ]); + + let response = sidecar.exchange(&ensure)?; + + assert!(response.error.as_deref().is_some_and(|error| error.contains("duplicate permission"))); + sidecar.shutdown()?; + Ok(()) +} + +#[test] +fn live_sidecar_accepts_permission_snapshot_lifecycle() -> Result<(), Box> { + let root = TestDirectory::new()?; + let profile_id = ProfileId::new(); + let tab_id = TabId::new(); + let mut sidecar = Sidecar::spawn(root.path())?; + let mut ensure = ensure_request(&tab_id, &profile_id, "about:blank"); + + for (generation, state, revision) in [(1, "allow-once", 1), (2, "transferred-allow-once", 2)] { + ensure["site_permission_generation"] = json!(generation); + ensure["site_permissions"] = json!([{ + "origin": "https://example.com", + "feature": "camera", + "state": state, + "revision": revision, + }]); + let response = sidecar.exchange(&ensure)?; + assert!(response.error.is_none(), "state={state} error={:?}", response.error); + } + + ensure["site_permission_generation"] = json!(3); + ensure["site_permissions"] = json!([]); + let response = sidecar.exchange(&ensure)?; + assert!(response.error.is_none(), "empty snapshot error={:?}", response.error); + + ensure["site_permission_generation"] = json!(1); + ensure["site_permissions"] = json!([{ + "origin": "https://example.com", + "feature": "camera", + "state": "allow-once", + "revision": 1, + }]); + let response = sidecar.exchange(&ensure)?; + assert!(response.error.is_none(), "stale snapshot error={:?}", response.error); + + sidecar.shutdown()?; + Ok(()) +} + #[cfg(all(feature = "hardware-render", target_os = "macos"))] #[test] fn hardware_sidecar_transfers_a_real_iosurface_mach_descriptor() -> Result<(), Box> { diff --git a/crates/ely_servo_host/tests/sidecar/support.rs b/crates/ely_servo_host/tests/sidecar/support.rs index 650ce39..070a236 100644 --- a/crates/ely_servo_host/tests/sidecar/support.rs +++ b/crates/ely_servo_host/tests/sidecar/support.rs @@ -19,7 +19,7 @@ use serde_json::{Value, json}; pub(super) const WIDTH: u32 = 360; pub(super) const HEIGHT: u32 = 240; pub(super) const RESPONSE_TIMEOUT: Duration = Duration::from_secs(20); -pub(super) const LIVE_PROTOCOL_VERSION: u32 = 2; +pub(super) const LIVE_PROTOCOL_VERSION: u32 = 3; pub(super) const MAX_FRAME_DIMENSION: u32 = 16_384; const MAX_FRAME_BYTE_COUNT: usize = 256 * 1024 * 1024; @@ -33,6 +33,7 @@ pub(super) fn ensure_request(tab_id: &TabId, profile_id: &ProfileId, url: &str) "height": HEIGHT, "page_zoom_percent": 100, "device_pixel_ratio": 1.0, + "site_permission_generation": 0, "site_permissions": [], }) } diff --git a/crates/ely_servo_host/tests/software_host.rs b/crates/ely_servo_host/tests/software_host.rs index bce4448..a41bc2c 100644 --- a/crates/ely_servo_host/tests/software_host.rs +++ b/crates/ely_servo_host/tests/software_host.rs @@ -9,8 +9,9 @@ use std::{ use ely_domain::{ProfileId, SiteOrigin, SitePermissionFeature, TabId, UrlText}; use ely_servo_host::{ HidpiScaleRequest, KeyboardTextRequest, MouseClickRequest, MouseDragRequest, NavigationRequest, - PageZoomRequest, PermissionDecision, PermissionRequest, ResizeRequest, ScrollRequest, - ServoHost, ServoHostError, ServoSurfaceSize, SoftwareServoHost, TouchTapRequest, WebViewState, + PageZoomRequest, PermissionDecision, PermissionSnapshotEntry, PermissionSnapshotRequest, + PermissionSnapshotState, ResizeRequest, ScrollRequest, ServoHost, ServoHostError, + ServoSurfaceSize, SoftwareServoHost, TouchTapRequest, WebViewState, }; const MINIMUM_CONTENT_PIXELS: u64 = 1_000; @@ -202,25 +203,29 @@ fn exercise_real_servo_webview_lifecycle() -> Result<(), Box> { assert_eq!(snapshot.profile_id(), &profile_id); assert_eq!(snapshot.state(), &WebViewState::Created); - host.set_permission( - PermissionRequest { - webview_id: webview_id.clone(), - profile_id: profile_id.clone(), + host.replace_permissions(PermissionSnapshotRequest { + webview_id: webview_id.clone(), + profile_id: profile_id.clone(), + generation: 1, + entries: vec![PermissionSnapshotEntry { origin: SiteOrigin::parse("https://example.com")?, feature: SitePermissionFeature::Camera, - }, - PermissionDecision::AllowOnce, - )?; + state: PermissionSnapshotState::Decision(PermissionDecision::AllowOnce), + revision: 1, + }], + })?; let other_profile_id = ProfileId::new(); - let mismatch = host.set_permission( - PermissionRequest { - webview_id: webview_id.clone(), - profile_id: other_profile_id.clone(), + let mismatch = host.replace_permissions(PermissionSnapshotRequest { + webview_id: webview_id.clone(), + profile_id: other_profile_id.clone(), + generation: 2, + entries: vec![PermissionSnapshotEntry { origin: SiteOrigin::parse("https://example.com")?, feature: SitePermissionFeature::Camera, - }, - PermissionDecision::AllowAlways, - ); + state: PermissionSnapshotState::Decision(PermissionDecision::AllowAlways), + revision: 1, + }], + }); assert!( matches!( mismatch, diff --git a/docs/servo-embedding-architecture.md b/docs/servo-embedding-architecture.md index 3dea012..cc175eb 100644 --- a/docs/servo-embedding-architecture.md +++ b/docs/servo-embedding-architecture.md @@ -50,12 +50,21 @@ transfers the IOSurface send right; the JSON port number is diagnostic metadata. The protocol supports: -- `handshake`: verify protocol version `2` before accepting browser commands. +- `handshake`: verify protocol version `3` before accepting browser commands. - `ensure`: create or update a WebView, navigation, viewport, zoom, permissions, and input. - `poll`: advance Servo and return pending frame or metadata state. - `close`: destroy one tab's WebView. - `shutdown`: acknowledge graceful process shutdown so Servo flushes profile storage. +Each `ensure` carries the complete permission snapshot for its Profile. The sidecar +atomically replaces that Profile at the newest request generation, so cross-tab round-robin cannot +restore an older grant. Entries carry per-key revisions plus explicit `transferred-allow-once` +state and full-snapshot removal semantics. BrowserCore moves an accepted one-time grant into +revocable transferred state, the worker preserves the grant request from idle +coalescing, and the sidecar returns a consumption receipt after the first matching request. A fresh +worker interprets transferred state as consumed and returns the same completion receipt, which +keeps process-restart behavior fail closed. + `ensure` and `poll` carry `ready_surface_ids` and `pending_surface_ids`. The app distinguishes completed imports from handles queued behind bounded importer backpressure. Cache entries retain the IOSurface reference and keep only a weak reference to an active `CVPixelBuffer` backing. The