diff --git a/crates/ely_app/src/shell/web_surface.rs b/crates/ely_app/src/shell/web_surface.rs index 1301817..77c31af 100644 --- a/crates/ely_app/src/shell/web_surface.rs +++ b/crates/ely_app/src/shell/web_surface.rs @@ -1,9 +1,9 @@ use std::collections::BTreeMap; use std::time::{Duration, Instant}; -use ely_domain::{BrowserTab, TabId}; +use ely_domain::{BrowserTab, ProfileId, TabId}; -use crate::services::ProfileDataMode; +use crate::services::{ProfileDataMode, servo_live::ServoLivePermissionGrant}; use super::{ web_surface_cadence::{ACTIVE_POLL_INTERVAL, IDLE_POLL_INTERVAL}, @@ -250,7 +250,7 @@ impl WebSurfaceStore { .min(IDLE_POLL_INTERVAL) } - pub(super) fn retain_tabs(&mut self, open_tab_ids: &[TabId]) { + pub(super) fn retain_tabs(&mut self, open_tab_ids: &[TabId]) -> Vec { let stale_tab_ids = self .surfaces .keys() @@ -260,6 +260,16 @@ impl WebSurfaceStore { for tab_id in stale_tab_ids { self.close_surface(&tab_id); } + self.runtime.take_retired_permission_consumptions() + } + + pub(super) fn reconcile_tab_scope( + &mut self, + tab_id: &TabId, + profile_id: &ProfileId, + profile_data_mode: ProfileDataMode, + ) -> Vec { + self.runtime.reconcile_tab_scope(tab_id, profile_id, profile_data_mode) } fn take_pending_input( diff --git a/crates/ely_app/src/shell/web_surface_controller.rs b/crates/ely_app/src/shell/web_surface_controller.rs index 682d9bf..53336e5 100644 --- a/crates/ely_app/src/shell/web_surface_controller.rs +++ b/crates/ely_app/src/shell/web_surface_controller.rs @@ -1,7 +1,7 @@ use std::{collections::HashMap, sync::Arc}; use ely_browser_core::{BrowserCore, BrowserSnapshot}; -use ely_domain::{BrowserTab, ProfileKind, TabId, UrlText}; +use ely_domain::{BrowserTab, ProfileId, ProfileKind, TabId, UrlText}; use gpui::{AnyElement, Bounds, Context, Pixels, Point}; use crate::services::{ProfileDataMode, servo_live::ServoLivePermissionGrant}; @@ -49,15 +49,34 @@ impl ElyShell { } pub(super) fn tick_external_web_surfaces(&mut self, cx: &mut Context) -> bool { - let (visible_tab_ids, open_tab_ids, visible_tabs) = match &self.state { + let (visible_tab_ids, external_tab_ids, external_scopes, visible_tabs) = match &self.state { super::ShellState::Ready(core) => { let visible_tabs = core.visible_content_tabs().unwrap_or_else(|_| Vec::new()); - let visible_tab_ids = visible_tabs.iter().map(|tab| tab.id().clone()).collect(); - (visible_tab_ids, core.open_tab_ids(), visible_web_surface_tabs(core, visible_tabs)) + let visible_tab_ids = external_web_surface_tab_ids(&visible_tabs); + let external_tab_ids = external_web_surface_tab_ids(core.open_tabs()); + let external_scopes = external_web_surface_scopes(core); + (visible_tab_ids, external_tab_ids, external_scopes, visible_tabs) } - super::ShellState::StartupError(_) => (Vec::new(), Vec::new(), Vec::new()), + super::ShellState::StartupError(_) => (Vec::new(), Vec::new(), Vec::new(), Vec::new()), + }; + let mut retired_permissions = self.web_surfaces.retain_tabs(&external_tab_ids); + for (tab_id, profile_id, profile_data_mode) in external_scopes { + retired_permissions.extend(self.web_surfaces.reconcile_tab_scope( + &tab_id, + &profile_id, + profile_data_mode, + )); + } + let mut permission_changed = false; + if let super::ShellState::Ready(core) = &mut self.state { + for consumed in retired_permissions { + permission_changed |= apply_permission_consumption(core, &consumed); + } + } + let visible_tabs = match &self.state { + super::ShellState::Ready(core) => visible_web_surface_tabs(core, visible_tabs), + super::ShellState::StartupError(_) => Vec::new(), }; - self.web_surfaces.retain_tabs(&open_tab_ids); let mut url_changed = self.ensure_visible_web_surfaces(visible_tabs); let result = self.web_surfaces.tick(&visible_tab_ids); for url_change in result.url_changes { @@ -67,7 +86,6 @@ 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 @@ -288,6 +306,25 @@ fn visible_web_surface_tabs( visible } +fn external_web_surface_tab_ids(tabs: &[BrowserTab]) -> Vec { + tabs.iter() + .filter(|tab| super::web_surface::is_external_web_url(tab.url().as_str())) + .map(|tab| tab.id().clone()) + .collect() +} + +fn external_web_surface_scopes(core: &BrowserCore) -> Vec<(TabId, ProfileId, ProfileDataMode)> { + core.open_tabs() + .iter() + .filter(|tab| super::web_surface::is_external_web_url(tab.url().as_str())) + .filter_map(|tab| { + core.profile_kind_for(tab.profile_id()).ok().map(|kind| { + (tab.id().clone(), tab.profile_id().clone(), profile_data_mode_from_kind(kind)) + }) + }) + .collect() +} + fn profile_data_mode_for(tab: &BrowserTab, snapshot: &BrowserSnapshot) -> Option { snapshot .profiles @@ -327,34 +364,5 @@ fn apply_permission_consumption( } #[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(()) - } -} +#[path = "web_surface_controller_tests.rs"] +mod tests; diff --git a/crates/ely_app/src/shell/web_surface_controller_tests.rs b/crates/ely_app/src/shell/web_surface_controller_tests.rs new file mode 100644 index 0000000..a1bc3f2 --- /dev/null +++ b/crates/ely_app/src/shell/web_surface_controller_tests.rs @@ -0,0 +1,141 @@ +use ely_browser_core::{BrowserCore, InitialBrowserConfig}; +use ely_domain::{SiteOrigin, SitePermissionDecision, SitePermissionFeature, UrlText}; + +use crate::services::ProfileDataMode; + +use super::{ + ServoLivePermissionGrant, apply_permission_consumption, external_web_surface_scopes, + external_web_surface_tab_ids, visible_web_surface_tabs, +}; + +#[test] +fn consumption_receipt_finishes_a_pending_allow_once_grant() +-> Result<(), Box> { + let mut core = BrowserCore::new(InitialBrowserConfig::private_window()?)?; + 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(()) +} + +#[test] +fn stale_consumption_keeps_a_newer_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 stale_revision = + core.site_permission_revision(&profile_id, &origin, SitePermissionFeature::Camera); + let stale = ServoLivePermissionGrant::new( + profile_id.clone(), + origin.clone(), + SitePermissionFeature::Camera, + stale_revision, + ); + core.set_site_permission( + origin.clone(), + SitePermissionFeature::Camera, + SitePermissionDecision::DenyAlways, + )?; + core.set_site_permission( + origin, + SitePermissionFeature::Camera, + SitePermissionDecision::AllowOnce, + )?; + + assert!(!apply_permission_consumption(&mut core, &stale)); + assert_eq!( + core.site_permissions_for_profile(&profile_id)[0].decision(), + SitePermissionDecision::AllowOnce, + ); + Ok(()) +} + +#[test] +fn retired_permissions_are_settled_before_the_next_external_snapshot() +-> Result<(), Box> { + let mut core = BrowserCore::new(InitialBrowserConfig::private_window()?)?; + core.navigate_active_tab(UrlText::parse("https://example.com/a")?)?; + let profile_id = core.snapshot()?.active_profile_id; + let retiring_tab_id = core.snapshot()?.active_tab_id; + let origin = SiteOrigin::parse("https://example.com")?; + let mut retired = Vec::new(); + for feature in [SitePermissionFeature::Camera, SitePermissionFeature::Microphone] { + core.set_site_permission(origin.clone(), feature, SitePermissionDecision::AllowOnce)?; + let revision = core.site_permission_revision(&profile_id, &origin, feature); + retired.push(ServoLivePermissionGrant::new( + profile_id.clone(), + origin.clone(), + feature, + revision, + )); + } + assert!(core.transfer_site_permission_once( + &profile_id, + &origin, + SitePermissionFeature::Microphone, + retired[1].grant_revision(), + )?); + let visible_tab_id = core.open_tab(UrlText::parse("https://example.com/b")?); + core.navigate_tab_to_loaded_url(&retiring_tab_id, UrlText::parse("ely://settings/general")?)?; + let raw_visible_tabs = core.visible_content_tabs()?; + + for consumed in &retired { + assert!(apply_permission_consumption(&mut core, consumed)); + } + let visible_tabs = visible_web_surface_tabs(&core, raw_visible_tabs); + + assert_eq!(visible_tabs.len(), 1); + assert_eq!(visible_tabs[0].tab.id(), &visible_tab_id); + assert!(visible_tabs[0].permissions.is_empty()); + for consumed in &retired { + assert!(!apply_permission_consumption(&mut core, consumed)); + } + Ok(()) +} + +#[test] +fn external_tab_ids_exclude_internal_routes() -> Result<(), Box> { + let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + let tab_id = core.snapshot()?.active_tab_id; + core.navigate_active_tab(UrlText::parse("https://example.com")?)?; + assert_eq!(external_web_surface_tab_ids(core.open_tabs()), vec![tab_id.clone()]); + + core.navigate_active_tab(UrlText::parse("ely://settings/general")?)?; + let snapshot = core.snapshot()?; + assert_eq!(snapshot.active_tab_id, tab_id); + assert!(external_web_surface_tab_ids(core.open_tabs()).is_empty()); + Ok(()) +} + +#[test] +fn external_tab_ids_include_inactive_spaces() -> Result<(), Box> { + let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + let profile_id = core.snapshot()?.active_profile_id; + let external_tab_id = core.snapshot()?.active_tab_id; + core.navigate_active_tab(UrlText::parse("https://example.com")?)?; + core.create_space("Second", "circle", 0x807d72)?; + + assert!(external_web_surface_tab_ids(&core.snapshot()?.tabs).is_empty()); + assert_eq!(external_web_surface_tab_ids(core.open_tabs()), vec![external_tab_id.clone()]); + assert_eq!( + external_web_surface_scopes(&core), + vec![(external_tab_id, profile_id, ProfileDataMode::Persistent,)] + ); + 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 index d79177b..e063197 100644 --- a/crates/ely_app/src/shell/web_surface_permission_lifecycle_tests.rs +++ b/crates/ely_app/src/shell/web_surface_permission_lifecycle_tests.rs @@ -71,15 +71,14 @@ fn successful_worker_ensure_confirms_allow_once_transfer() -> Result<(), String> 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(), - )], - ); + let transferred = vec![crate::services::servo_live::ServoLivePermissionGrant::new( + profile_id, + permission.origin().clone(), + permission.feature(), + permission.revision(), + )]; + assert_eq!(result.permission_transfers, transferred); + assert_eq!(store.retain_tabs(&[]), transferred); Ok(()) } @@ -108,6 +107,42 @@ fn rejected_worker_ensure_keeps_allow_once_untransferred() -> Result<(), String> Ok(()) } +#[test] +fn retiring_transient_worker_returns_only_submitted_allow_once_grants() -> 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 second = BrowserTab::new( + TabId::new(), + SpaceId::new(), + tab.profile_id().clone(), + "Second", + UrlText::parse("https://example.com/second").map_err(|error| error.to_string())?, + ); + let submitted = crate::services::servo_live::ServoLivePermissionGrant::new( + tab.profile_id().clone(), + permission.origin().clone(), + permission.feature(), + permission.revision(), + ); + record_viewport(&mut store, &tab); + assert!(store.ensure_surface( + &tab, + ProfileDataMode::Transient, + std::slice::from_ref(&permission), + )); + record_viewport(&mut store, &second); + assert!(store.ensure_surface( + &second, + ProfileDataMode::Transient, + std::slice::from_ref(&permission), + )); + + assert!(store.retain_tabs(std::slice::from_ref(second.id())).is_empty()); + assert_eq!(store.retain_tabs(&[]), vec![submitted]); + Ok(()) +} + fn tab_and_permission() -> Result<(BrowserTab, WebSurfaceSitePermission), String> { let profile_id = ProfileId::new(); let tab = BrowserTab::new( diff --git a/crates/ely_app/src/shell/web_surface_runtime.rs b/crates/ely_app/src/shell/web_surface_runtime.rs index a3a3087..5132417 100644 --- a/crates/ely_app/src/shell/web_surface_runtime.rs +++ b/crates/ely_app/src/shell/web_surface_runtime.rs @@ -8,7 +8,7 @@ use ely_domain::{BrowserTab, TabId}; use crate::services::{ ProfileDataMode, - servo_live::{ServoLiveEnsureRequest, ServoLiveSitePermission}, + servo_live::{ServoLiveEnsureRequest, ServoLivePermissionGrant, ServoLiveSitePermission}, servo_profile_data::cleanup_stale_transient_profile_data_dirs, }; @@ -39,6 +39,7 @@ pub(super) struct WebSurfaceRuntime { sessions: BTreeMap, retry_state: BTreeMap, retired_workers: Vec>>, + retired_permission_consumptions: Vec, transient_cleanup_error: Option, client_factory: LiveRuntimeClientFactory, last_generation: u64, @@ -55,6 +56,7 @@ impl WebSurfaceRuntime { sessions: BTreeMap::new(), retry_state: BTreeMap::new(), retired_workers: Vec::new(), + retired_permission_consumptions: Vec::new(), transient_cleanup_error, client_factory: new_servo_live_client, last_generation: 0, @@ -68,6 +70,7 @@ impl WebSurfaceRuntime { sessions: BTreeMap::new(), retry_state: BTreeMap::new(), retired_workers: Vec::new(), + retired_permission_consumptions: Vec::new(), transient_cleanup_error: None, client_factory, last_generation: 0, @@ -136,9 +139,10 @@ impl WebSurfaceRuntime { allow_once_grants: allow_once_grants(tab.profile_id(), permissions), }; - let Some(scoped) = self.workers.get(&scope) else { + let Some(scoped) = self.workers.get_mut(&scope) else { return Err("Servo sidecar worker was created but is no longer registered".to_string()); }; + scoped.track_allow_once_grants(&request.allow_once_grants); scoped.worker.submit_ensure(generation, request); if let Some(session) = self.sessions.get_mut(tab.id()) { session.cadence.note_poll_submitted(submitted_at); @@ -150,7 +154,7 @@ impl WebSurfaceRuntime { pub(super) fn tick(&mut self, visible_tab_ids: &[TabId]) -> Vec { self.reap_retired_workers(false); - let mut frames = Vec::new(); + let mut frames = self.take_retired_permission_frames(); let now = Instant::now(); let scopes = self.workers.keys().cloned().collect::>(); let mut unavailable_scopes = Vec::new(); @@ -168,6 +172,7 @@ impl WebSurfaceRuntime { for scope in unavailable_scopes { self.invalidate_scope(&scope, now); } + frames.extend(self.take_retired_permission_frames()); let poll_now = Instant::now(); for tab_id in visible_tab_ids { @@ -209,19 +214,6 @@ impl WebSurfaceRuntime { .min() } - pub(super) fn close_tab(&mut self, tab_id: &TabId) { - let Some(session) = self.sessions.remove(tab_id) else { - return; - }; - let has_remaining_session = - self.sessions.values().any(|candidate| candidate.scope == session.scope); - if session.scope.is_transient() && !has_remaining_session { - self.remove_worker(&session.scope); - } else if let Some(scoped) = self.workers.get(&session.scope) { - scoped.worker.submit_close(tab_id.as_str().to_string()); - } - } - pub(super) fn has_session( &self, tab_id: &TabId, @@ -277,28 +269,10 @@ impl WebSurfaceRuntime { return Err(error); } }; - self.workers.insert(scope, ScopedWorker { worker, transient_profile_data_dir }); + self.workers.insert(scope, ScopedWorker::new(worker, transient_profile_data_dir)); Ok(()) } - fn detach_tab_from_previous_scope(&mut self, tab_id: &TabId, scope: &WebSurfaceRuntimeScope) { - let Some(previous_scope) = self.sessions.get(tab_id).map(|session| session.scope.clone()) - else { - return; - }; - if &previous_scope == scope { - return; - } - self.sessions.remove(tab_id); - let has_remaining_session = - self.sessions.values().any(|candidate| candidate.scope == previous_scope); - if previous_scope.is_transient() && !has_remaining_session { - self.remove_worker(&previous_scope); - } else if let Some(scoped) = self.workers.get(&previous_scope) { - scoped.worker.submit_close(tab_id.as_str().to_string()); - } - } - fn collect_responses( &mut self, scope: &WebSurfaceRuntimeScope, @@ -362,6 +336,9 @@ impl WebSurfaceRuntime { frames.push(WebSurfaceRuntimeFrame::PermissionSnapshotAccepted(grant)); } WorkerResponse::PermissionConsumed(consumed) => { + if let Some(scoped) = self.workers.get_mut(scope) { + scoped.mark_permission_consumed(&consumed); + } frames.push(WebSurfaceRuntimeFrame::PermissionConsumed(consumed)); } } @@ -398,9 +375,10 @@ impl WebSurfaceRuntime { } fn remove_worker(&mut self, scope: &WebSurfaceRuntimeScope) { - let Some(scoped) = self.workers.remove(scope) else { + let Some(mut scoped) = self.workers.remove(scope) else { return; }; + self.retire_scoped_worker_permissions(&mut scoped); if scoped.transient_profile_data_dir.is_some() { match std::thread::Builder::new() .name("ely-servo-profile-cleanup".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 c74f859..01089a7 100644 --- a/crates/ely_app/src/shell/web_surface_runtime_cleanup.rs +++ b/crates/ely_app/src/shell/web_surface_runtime_cleanup.rs @@ -1,8 +1,17 @@ use std::path::PathBuf; -use crate::services::{servo_live::ServoLiveClient, servo_profile_data::TransientProfileDataDir}; +use ely_domain::{ProfileId, TabId}; -use super::{LiveRuntimeClient, LiveRuntimeWorker}; +use crate::services::{ + ProfileDataMode, + servo_live::{ServoLiveClient, ServoLivePermissionGrant}, + servo_profile_data::TransientProfileDataDir, +}; + +use super::{ + LiveRuntimeClient, LiveRuntimeWorker, WebSurfaceRuntime, WebSurfaceRuntimeFrame, + WebSurfaceRuntimeScope, +}; pub(super) type LiveRuntimeClientFactory = fn(PathBuf) -> Result, String>; @@ -18,10 +27,36 @@ pub(super) fn new_servo_live_client( pub(super) struct ScopedWorker { pub(super) worker: LiveRuntimeWorker, pub(super) transient_profile_data_dir: Option, + allow_once_grants: Vec, +} + +impl ScopedWorker { + pub(super) fn new( + worker: LiveRuntimeWorker, + transient_profile_data_dir: Option, + ) -> Self { + Self { worker, transient_profile_data_dir, allow_once_grants: Vec::new() } + } + + pub(super) fn track_allow_once_grants(&mut self, grants: &[ServoLivePermissionGrant]) { + for grant in grants { + if !self.allow_once_grants.contains(grant) { + self.allow_once_grants.push(grant.clone()); + } + } + } + + pub(super) fn mark_permission_consumed(&mut self, consumed: &ServoLivePermissionGrant) { + self.allow_once_grants.retain(|grant| grant != consumed); + } + + fn take_allow_once_grants(&mut self) -> Vec { + std::mem::take(&mut self.allow_once_grants) + } } pub(super) fn shutdown_scoped_worker(scoped: ScopedWorker) -> Result<(), String> { - let ScopedWorker { worker, transient_profile_data_dir } = scoped; + let ScopedWorker { worker, transient_profile_data_dir, .. } = scoped; drop(worker); let Some(directory) = transient_profile_data_dir else { return Ok(()); @@ -30,3 +65,72 @@ pub(super) fn shutdown_scoped_worker(scoped: ScopedWorker) -> Result<(), String> .close() .map_err(|error| format!("failed to remove transient Servo profile data: {error}")) } + +impl WebSurfaceRuntime { + pub(in crate::shell) fn reconcile_tab_scope( + &mut self, + tab_id: &TabId, + profile_id: &ProfileId, + profile_data_mode: ProfileDataMode, + ) -> Vec { + let scope = WebSurfaceRuntimeScope::new(profile_id.clone(), profile_data_mode); + self.detach_tab_from_previous_scope(tab_id, &scope); + self.take_retired_permission_consumptions() + } + + pub(in crate::shell) fn close_tab(&mut self, tab_id: &TabId) { + let Some(session) = self.sessions.remove(tab_id) else { + return; + }; + let has_remaining_session = + self.sessions.values().any(|candidate| candidate.scope == session.scope); + if session.scope.is_transient() && !has_remaining_session { + self.remove_worker(&session.scope); + } else if let Some(scoped) = self.workers.get(&session.scope) { + scoped.worker.submit_close(tab_id.as_str().to_string()); + } + } + + pub(super) fn retire_scoped_worker_permissions(&mut self, scoped: &mut ScopedWorker) { + for grant in scoped.take_allow_once_grants() { + if !self.retired_permission_consumptions.contains(&grant) { + self.retired_permission_consumptions.push(grant); + } + } + } + + pub(in crate::shell) fn take_retired_permission_consumptions( + &mut self, + ) -> Vec { + std::mem::take(&mut self.retired_permission_consumptions) + } + + pub(super) fn take_retired_permission_frames(&mut self) -> Vec { + self.take_retired_permission_consumptions() + .into_iter() + .map(WebSurfaceRuntimeFrame::PermissionConsumed) + .collect() + } + + pub(super) fn detach_tab_from_previous_scope( + &mut self, + tab_id: &TabId, + scope: &WebSurfaceRuntimeScope, + ) { + let Some(previous_scope) = self.sessions.get(tab_id).map(|session| session.scope.clone()) + else { + return; + }; + if &previous_scope == scope { + return; + } + self.sessions.remove(tab_id); + let has_remaining_session = + self.sessions.values().any(|candidate| candidate.scope == previous_scope); + if previous_scope.is_transient() && !has_remaining_session { + self.remove_worker(&previous_scope); + } else if let Some(scoped) = self.workers.get(&previous_scope) { + scoped.worker.submit_close(tab_id.as_str().to_string()); + } + } +} diff --git a/crates/ely_app/src/shell/web_surface_runtime_generation_tests.rs b/crates/ely_app/src/shell/web_surface_runtime_generation_tests.rs index a4dfa8b..a1f1cc9 100644 --- a/crates/ely_app/src/shell/web_surface_runtime_generation_tests.rs +++ b/crates/ely_app/src/shell/web_surface_runtime_generation_tests.rs @@ -4,16 +4,20 @@ use std::{ time::Instant, }; -use ely_domain::{BrowserTab, ProfileId, SpaceId, TabId, UrlText}; +use ely_domain::{ + BrowserTab, ProfileId, SiteOrigin, SitePermissionDecision, SitePermissionFeature, SpaceId, + TabId, UrlText, +}; use crate::services::{ ProfileDataMode, - servo_live::{ServoLiveEnsureRequest, ServoLiveFrame}, + servo_live::{ServoLiveEnsureRequest, ServoLiveFrame, ServoLivePermissionGrant}, }; use super::{ super::{ web_surface_geometry::{WebSurfaceScrollOffset, WebSurfaceSize}, + web_surface_permissions::{WebSurfaceSitePermission, WebSurfaceSitePermissionState}, web_surface_state::WebSurfacePendingInput, web_surface_worker::{LiveRuntimeClient, LiveRuntimeClientError, WorkerResponse}, }, @@ -107,6 +111,123 @@ fn late_frame_from_a_is_discarded_after_a_to_b() -> Result<(), String> { Ok(()) } +#[test] +fn late_frame_is_discarded_after_tab_session_closes() -> Result<(), String> { + let mut runtime = WebSurfaceRuntime::new_with_client_factory(empty_client_factory); + let tab_id = TabId::new(); + let profile_id = ProfileId::new(); + let tab = web_tab(tab_id.clone(), profile_id.clone(), "https://example.com/external")?; + runtime.ensure_tab(&tab, surface_size(), ProfileDataMode::Transient, &[], pending_input())?; + let generation = current_generation(&runtime, &tab_id)?; + + runtime.close_tab(&tab_id); + let mut frames = Vec::new(); + runtime.collect_responses( + &scope(&profile_id), + vec![ + WorkerResponse::Frame { + generation, + tab_id: tab_id.as_str().to_string(), + frame: live_frame(), + }, + WorkerResponse::Failed { + generation, + tab_id: tab_id.as_str().to_string(), + message: "late failure".to_string(), + }, + ], + Instant::now(), + &mut frames, + ); + + assert!(!runtime.sessions.contains_key(&tab_id)); + assert!(frames.is_empty()); + Ok(()) +} + +#[test] +fn scope_change_returns_submitted_permission_grants_on_the_next_tick() -> Result<(), String> { + let mut runtime = WebSurfaceRuntime::new_with_client_factory(empty_client_factory); + let tab_id = TabId::new(); + let profile_a = ProfileId::new(); + let (permission, grant) = allow_once_permission(&profile_a)?; + let tab_a = web_tab(tab_id.clone(), profile_a, "https://example.com/a")?; + let tab_b = web_tab(tab_id, ProfileId::new(), "https://example.com/b")?; + runtime.ensure_tab( + &tab_a, + surface_size(), + ProfileDataMode::Transient, + std::slice::from_ref(&permission), + pending_input(), + )?; + runtime.ensure_tab(&tab_b, surface_size(), ProfileDataMode::Transient, &[], pending_input())?; + + let frames = runtime.tick(&[]); + assert!(matches!( + frames.as_slice(), + [WebSurfaceRuntimeFrame::PermissionConsumed(consumed)] if consumed == &grant + )); + Ok(()) +} + +#[test] +fn reconciliation_retires_same_profile_mode_before_replacement_ensure() -> Result<(), String> { + let mut runtime = WebSurfaceRuntime::new_with_client_factory(empty_client_factory); + let tab_id = TabId::new(); + let profile_id = ProfileId::new(); + let (permission, grant) = allow_once_permission(&profile_id)?; + let transient = web_tab(tab_id.clone(), profile_id.clone(), "https://example.com/private")?; + let persistent = web_tab(tab_id.clone(), profile_id.clone(), "https://example.com/standard")?; + runtime.ensure_tab( + &transient, + surface_size(), + ProfileDataMode::Transient, + std::slice::from_ref(&permission), + pending_input(), + )?; + + let retired = runtime.reconcile_tab_scope(&tab_id, &profile_id, ProfileDataMode::Persistent); + assert_eq!(retired, vec![grant]); + runtime.ensure_tab( + &persistent, + surface_size(), + ProfileDataMode::Persistent, + &[], + pending_input(), + )?; + assert!(runtime.has_session(&tab_id, &profile_id, ProfileDataMode::Persistent)); + Ok(()) +} + +#[test] +fn consumed_permission_is_removed_from_worker_tracking() -> Result<(), String> { + let mut runtime = WebSurfaceRuntime::new_with_client_factory(empty_client_factory); + let tab_id = TabId::new(); + let profile_id = ProfileId::new(); + let scope = scope(&profile_id); + let (permission, grant) = allow_once_permission(&profile_id)?; + let tab = web_tab(tab_id.clone(), profile_id, "https://example.com")?; + runtime.ensure_tab( + &tab, + surface_size(), + ProfileDataMode::Transient, + std::slice::from_ref(&permission), + pending_input(), + )?; + let mut frames = Vec::new(); + runtime.collect_responses( + &scope, + vec![WorkerResponse::PermissionConsumed(grant)], + Instant::now(), + &mut frames, + ); + + runtime.close_tab(&tab_id); + assert!(runtime.take_retired_permission_consumptions().is_empty()); + assert!(matches!(frames.as_slice(), [WebSurfaceRuntimeFrame::PermissionConsumed(_)])); + Ok(()) +} + #[test] fn generation_blocks_aba_frames_and_failures() -> Result<(), String> { let mut runtime = WebSurfaceRuntime::new_with_client_factory(empty_client_factory); @@ -321,3 +442,20 @@ fn pending_input() -> WebSurfacePendingInput { fn live_frame() -> ServoLiveFrame { ServoLiveFrame::for_test(1, 1, vec![16, 32, 64, 255]) } + +fn allow_once_permission( + profile_id: &ProfileId, +) -> Result<(WebSurfaceSitePermission, ServoLivePermissionGrant), String> { + let origin = SiteOrigin::parse("https://example.com").map_err(|error| error.to_string())?; + let feature = SitePermissionFeature::Camera; + let revision = 7; + Ok(( + WebSurfaceSitePermission::new( + origin.clone(), + feature, + WebSurfaceSitePermissionState::Decision(SitePermissionDecision::AllowOnce), + revision, + ), + ServoLivePermissionGrant::new(profile_id.clone(), origin, feature, revision), + )) +} diff --git a/crates/ely_app/src/shell/web_surface_scope_tests.rs b/crates/ely_app/src/shell/web_surface_scope_tests.rs index 26e2bb1..a4a33e6 100644 --- a/crates/ely_app/src/shell/web_surface_scope_tests.rs +++ b/crates/ely_app/src/shell/web_surface_scope_tests.rs @@ -20,6 +20,7 @@ use super::super::{ use super::WebSurfaceStore; static ENSURES: Mutex> = Mutex::new(Vec::new()); +static CLOSES: Mutex> = Mutex::new(Vec::new()); #[derive(Debug)] struct RecordedEnsure { @@ -28,6 +29,7 @@ struct RecordedEnsure { } struct RecordingClient; +struct ClosingClient; impl LiveRuntimeClient for RecordingClient { fn ensure( @@ -58,12 +60,36 @@ impl LiveRuntimeClient for RecordingClient { } } +impl LiveRuntimeClient for ClosingClient { + 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> { + CLOSES.lock().map_err(|_| "close recorder lock was poisoned".to_string())?.push(tab_id); + Ok(()) + } +} + fn recording_client_factory( _config_dir: std::path::PathBuf, ) -> Result, String> { Ok(Box::new(RecordingClient)) } +fn closing_client_factory( + _config_dir: std::path::PathBuf, +) -> Result, String> { + Ok(Box::new(ClosingClient)) +} + #[test] fn profile_scope_change_clears_pixels_input_and_focus_before_ensure() -> Result<(), Box> { @@ -121,6 +147,44 @@ fn profile_scope_change_clears_pixels_input_and_focus_before_ensure() -> Result< Ok(()) } +#[test] +fn retaining_only_external_tabs_closes_an_internal_navigation_surface() -> Result<(), Box> +{ + CLOSES.lock().map_err(|_| "close recorder lock was poisoned")?.clear(); + let runtime = WebSurfaceRuntime::new_with_client_factory(closing_client_factory); + let mut store = WebSurfaceStore::new_with_runtime(runtime); + let tab = BrowserTab::new( + TabId::new(), + SpaceId::new(), + ProfileId::new(), + "Web", + UrlText::parse("https://example.com")?, + ); + + assert_eq!( + store.record_viewport_size(tab.id(), viewport_bounds(), 1.0), + WebSurfaceInputOutcome::Applied, + ); + assert!(store.ensure_surface(&tab, ProfileDataMode::Persistent, &[])); + store.flush_runtime_for_test(); + assert_eq!( + store.record_click_point(tab.id(), tab.url().as_str(), point(px(20.0), px(20.0)), 1.0,), + WebSurfaceInputOutcome::Applied, + ); + + store.retain_tabs(&[]); + store.flush_runtime_for_test(); + + assert!(store.surface_for_test(tab.id()).is_none()); + assert!(store.runtime.session_scope_for_test(tab.id()).is_none()); + assert!(store.keyboard_focus.is_none()); + assert_eq!( + CLOSES.lock().map_err(|_| "close recorder lock was poisoned")?.as_slice(), + [tab.id().as_str()], + ); + Ok(()) +} + fn viewport_bounds() -> Bounds { Bounds::new(point(px(0.0), px(0.0)), size(px(640.0), px(480.0))) } diff --git a/crates/ely_browser_core/src/state/visible_content.rs b/crates/ely_browser_core/src/state/visible_content.rs index b89f2fb..c87eda1 100644 --- a/crates/ely_browser_core/src/state/visible_content.rs +++ b/crates/ely_browser_core/src/state/visible_content.rs @@ -5,8 +5,8 @@ use super::BrowserCore; use crate::CoreError; impl BrowserCore { - pub fn open_tab_ids(&self) -> Vec { - self.tabs.iter().map(|tab| tab.id().clone()).collect() + pub fn open_tabs(&self) -> &[BrowserTab] { + &self.tabs } pub fn visible_content_tab_ids(&self) -> Result, CoreError> {