From e442e6def48d38881713086a9112f4f85664f41a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Sat, 16 May 2026 01:41:42 -0400 Subject: [PATCH] perf(web-surface): avoid snapshot clones on live ticks --- .../src/shell/web_surface_controller.rs | 113 +++++++----------- .../src/shell/web_surface_permissions.rs | 19 +++ crates/ely_browser_core/src/state/profiles.rs | 8 ++ .../src/state/site_permissions.rs | 12 ++ .../src/state/visible_content.rs | 10 ++ 5 files changed, 94 insertions(+), 68 deletions(-) diff --git a/crates/ely_app/src/shell/web_surface_controller.rs b/crates/ely_app/src/shell/web_surface_controller.rs index 1afd1c5..1cfd385 100644 --- a/crates/ely_app/src/shell/web_surface_controller.rs +++ b/crates/ely_app/src/shell/web_surface_controller.rs @@ -1,4 +1,4 @@ -use ely_browser_core::BrowserSnapshot; +use ely_browser_core::{BrowserCore, BrowserSnapshot}; use ely_domain::{BrowserTab, ProfileKind, TabId, UrlText}; use gpui::{AnyElement, Bounds, Context, Pixels, Point}; @@ -7,7 +7,9 @@ use crate::services::ProfileDataMode; use super::{ ElyShell, web_surface_metadata::WebSurfacePageMetadata, - web_surface_permissions::web_surface_site_permissions_for_tab, + web_surface_permissions::{ + WebSurfaceSitePermission, web_surface_site_permissions_for_core_tab, + }, web_surface_runtime::{WebSurfaceUrlChange, WebSurfaceUrlChangeKind}, web_surface_state::{WebSurfaceInputOutcome, WebSurfaceState}, web_surface_view::{ @@ -44,19 +46,16 @@ impl ElyShell { } pub(super) fn tick_external_web_surfaces(&mut self) -> bool { - let (visible_tab_ids, open_tab_ids, snapshot) = match &self.state { - super::ShellState::Ready(core) => ( - core.visible_content_tab_ids().unwrap_or_else(|_| Vec::new()), - core.open_tab_ids(), - core.snapshot().ok(), - ), - super::ShellState::StartupError(_) => (Vec::new(), Vec::new(), None), + let (visible_tab_ids, open_tab_ids, 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)) + } + super::ShellState::StartupError(_) => (Vec::new(), Vec::new(), Vec::new()), }; self.web_surfaces.retain_tabs(&open_tab_ids); - let mut url_changed = snapshot - .as_ref() - .map(|snapshot| self.ensure_visible_web_surfaces(snapshot, &visible_tab_ids)) - .unwrap_or(false); + 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 { url_changed |= self.apply_web_surface_url_change(url_change); @@ -182,20 +181,15 @@ impl ElyShell { } impl ElyShell { - fn ensure_visible_web_surfaces( - &mut self, - snapshot: &BrowserSnapshot, - visible_tab_ids: &[TabId], - ) -> bool { + fn ensure_visible_web_surfaces(&mut self, visible_tabs: Vec) -> bool { let mut url_changed = false; let mut changed = false; - let visible_tabs = visible_external_web_tabs(&snapshot.tabs, visible_tab_ids); - for tab in visible_tabs { - let Some(profile_data_mode) = profile_data_mode_for(tab, snapshot) else { - continue; - }; - let permissions = web_surface_site_permissions_for_tab(tab, snapshot); - let outcome = self.web_surfaces.ensure_surface(tab, profile_data_mode, &permissions); + for visible in visible_tabs { + let outcome = self.web_surfaces.ensure_surface( + &visible.tab, + visible.profile_data_mode, + &visible.permissions, + ); changed |= outcome.changed; if let Some(url_change) = outcome.url_change { url_changed |= self.apply_web_surface_url_change(url_change); @@ -241,55 +235,38 @@ impl ElyShell { } } -fn visible_external_web_tabs<'a>( - tabs: &'a [BrowserTab], - visible_tab_ids: &[TabId], -) -> Vec<&'a BrowserTab> { - visible_tab_ids - .iter() - .filter_map(|tab_id| tabs.iter().find(|tab| tab.id() == tab_id)) +struct VisibleWebSurfaceTab { + tab: BrowserTab, + profile_data_mode: ProfileDataMode, + permissions: Vec, +} + +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() } fn profile_data_mode_for(tab: &BrowserTab, snapshot: &BrowserSnapshot) -> Option { - snapshot.profiles.iter().find(|profile| profile.id() == tab.profile_id()).map(|profile| { - match profile.kind() { - ProfileKind::Standard => ProfileDataMode::Persistent, - ProfileKind::Private => ProfileDataMode::Transient, - } - }) + snapshot + .profiles + .iter() + .find(|profile| profile.id() == tab.profile_id()) + .map(|profile| profile_data_mode_from_kind(profile.kind().clone())) } -#[cfg(test)] -mod tests { - use ely_domain::{BrowserTab, ProfileId, SpaceId, TabId, UrlText}; - - use super::visible_external_web_tabs; - - #[test] - fn visible_external_web_tabs_follow_visible_order() -> Result<(), String> { - let first_id = TabId::new(); - let second_id = TabId::new(); - let internal_id = TabId::new(); - let tabs = vec![ - web_tab(first_id.clone(), "https://example.com/first")?, - web_tab(internal_id.clone(), "ely://settings")?, - web_tab(second_id.clone(), "http://example.com/second")?, - ]; - - let visible = - visible_external_web_tabs(&tabs, &[internal_id, second_id.clone(), first_id.clone()]) - .into_iter() - .map(|tab| tab.id().clone()) - .collect::>(); - - assert_eq!(visible, vec![second_id, first_id]); - Ok(()) - } - - fn web_tab(tab_id: TabId, url: &str) -> Result { - let url = UrlText::parse(url).map_err(|error| error.to_string())?; - Ok(BrowserTab::new(tab_id, SpaceId::new(), ProfileId::new(), "Web", url)) +fn profile_data_mode_from_kind(kind: ProfileKind) -> ProfileDataMode { + match kind { + ProfileKind::Standard => ProfileDataMode::Persistent, + ProfileKind::Private => ProfileDataMode::Transient, } } diff --git a/crates/ely_app/src/shell/web_surface_permissions.rs b/crates/ely_app/src/shell/web_surface_permissions.rs index 603d141..5e715d7 100644 --- a/crates/ely_app/src/shell/web_surface_permissions.rs +++ b/crates/ely_app/src/shell/web_surface_permissions.rs @@ -1,3 +1,5 @@ +use ely_browser_core::BrowserCore; +#[cfg(test)] use ely_browser_core::BrowserSnapshot; use ely_domain::{BrowserTab, SiteOrigin, SitePermissionDecision, SitePermissionFeature}; @@ -30,6 +32,7 @@ impl WebSurfaceSitePermission { } } +#[cfg(test)] pub(super) fn web_surface_site_permissions_for_tab( tab: &BrowserTab, snapshot: &BrowserSnapshot, @@ -49,6 +52,22 @@ pub(super) fn web_surface_site_permissions_for_tab( .collect() } +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(); + }; + + core.site_permissions_for_profile_origin(tab.profile_id(), &origin) + .into_iter() + .map(|entry| { + WebSurfaceSitePermission::new(entry.origin().clone(), entry.feature(), entry.decision()) + }) + .collect() +} + #[cfg(test)] mod tests { use std::error::Error; diff --git a/crates/ely_browser_core/src/state/profiles.rs b/crates/ely_browser_core/src/state/profiles.rs index 8d61c6f..57204c6 100644 --- a/crates/ely_browser_core/src/state/profiles.rs +++ b/crates/ely_browser_core/src/state/profiles.rs @@ -94,6 +94,14 @@ impl BrowserCore { self.set_active_profile_download_policy(DownloadPolicy::ask_every_time()) } + pub fn profile_kind_for(&self, profile_id: &ProfileId) -> Result { + self.profiles + .iter() + .find(|profile| profile.id() == profile_id) + .map(|profile| profile.kind().clone()) + .ok_or_else(|| CoreError::ProfileNotFound { id: profile_id.clone() }) + } + pub fn set_profile_sync_policy( &mut self, profile_id: &ProfileId, diff --git a/crates/ely_browser_core/src/state/site_permissions.rs b/crates/ely_browser_core/src/state/site_permissions.rs index 66c2566..24f0f90 100644 --- a/crates/ely_browser_core/src/state/site_permissions.rs +++ b/crates/ely_browser_core/src/state/site_permissions.rs @@ -50,6 +50,18 @@ impl BrowserCore { .collect() } + pub fn site_permissions_for_profile_origin( + &self, + profile_id: &ProfileId, + origin: &SiteOrigin, + ) -> Vec { + self.site_permissions + .iter() + .filter(|entry| entry.profile_id() == profile_id && entry.origin() == origin) + .cloned() + .collect() + } + pub(super) fn visible_site_permission_audit_events(&self) -> Vec { self.site_permission_audit_events .iter() diff --git a/crates/ely_browser_core/src/state/visible_content.rs b/crates/ely_browser_core/src/state/visible_content.rs index 904c351..b89f2fb 100644 --- a/crates/ely_browser_core/src/state/visible_content.rs +++ b/crates/ely_browser_core/src/state/visible_content.rs @@ -1,3 +1,4 @@ +use ely_domain::BrowserTab; use ely_domain::TabId; use super::BrowserCore; @@ -23,4 +24,13 @@ impl BrowserCore { } Ok(layout.panes().iter().map(|pane| pane.tab_id().clone()).collect()) } + + pub fn visible_content_tabs(&self) -> Result, CoreError> { + let ids = self.visible_content_tab_ids()?; + Ok(ids + .iter() + .filter_map(|id| self.tabs.iter().find(|tab| tab.id() == id)) + .cloned() + .collect()) + } }