From ac84a80d9e51b8a3f2d46d8f34d314d60f5af426 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Wed, 13 May 2026 02:41:54 -0400 Subject: [PATCH] Close Servo live sessions for removed tabs --- crates/ely_app/src/services/servo_live.rs | 4 +++ .../ely_app/src/services/servo_live_wire.rs | 19 ++++++++++++ crates/ely_app/src/shell/web_surface.rs | 24 +++++++++++++++ .../src/shell/web_surface_controller.rs | 7 +++-- .../src/shell/web_surface_live_site_tests.rs | 5 +++- .../ely_app/src/shell/web_surface_runtime.rs | 15 ++++++++++ .../src/shell/web_surface_runtime_tests.rs | 29 ++++++++++++++++++- .../src/state/visible_content.rs | 4 +++ .../src/bin/ely_servo_sidecar/live.rs | 7 +++++ .../bin/ely_servo_sidecar/live_protocol.rs | 20 +++++++++++++ crates/ely_servo_host/src/runtime.rs | 4 +++ 11 files changed, 133 insertions(+), 5 deletions(-) diff --git a/crates/ely_app/src/services/servo_live.rs b/crates/ely_app/src/services/servo_live.rs index 666351d..254c422 100644 --- a/crates/ely_app/src/services/servo_live.rs +++ b/crates/ely_app/src/services/servo_live.rs @@ -99,6 +99,10 @@ impl ServoLiveClient { self.request(LiveRequest::Poll { tab_id }) } + pub fn close(&mut self, tab_id: String) -> Result<(), ServoLiveError> { + self.request(LiveRequest::Close { tab_id }).map(|_| ()) + } + fn request(&mut self, request: LiveRequest) -> Result, ServoLiveError> { serde_json::to_writer(&mut self.stdin, &request)?; self.stdin.write_all(b"\n").map_err(ServoLiveError::Command)?; diff --git a/crates/ely_app/src/services/servo_live_wire.rs b/crates/ely_app/src/services/servo_live_wire.rs index 2a1fd20..1cfbee5 100644 --- a/crates/ely_app/src/services/servo_live_wire.rs +++ b/crates/ely_app/src/services/servo_live_wire.rs @@ -27,6 +27,9 @@ pub(super) enum LiveRequest { Poll { tab_id: String, }, + Close { + tab_id: String, + }, } #[derive(Deserialize)] @@ -156,3 +159,19 @@ pub(super) fn log_frame_perf(summary: &LiveFramePerfSummary) { "frame_perf", ); } + +#[cfg(test)] +mod tests { + use serde_json::json; + + use super::*; + + #[test] + fn close_request_serializes_to_wire() -> Result<(), serde_json::Error> { + let value = + serde_json::to_value(LiveRequest::Close { tab_id: "tab-live-close".to_string() })?; + + assert_eq!(value, json!({"type": "close", "tab_id": "tab-live-close"})); + Ok(()) + } +} diff --git a/crates/ely_app/src/shell/web_surface.rs b/crates/ely_app/src/shell/web_surface.rs index 12d4299..009bba8 100644 --- a/crates/ely_app/src/shell/web_surface.rs +++ b/crates/ely_app/src/shell/web_surface.rs @@ -97,6 +97,22 @@ impl WebSurfaceStore { result } + pub(super) fn retain_tabs(&mut self, open_tab_ids: &[TabId]) { + let stale_tab_ids = self + .surfaces + .keys() + .filter(|tab_id| !open_tab_ids.iter().any(|open_tab_id| open_tab_id == *tab_id)) + .cloned() + .collect::>(); + for tab_id in stale_tab_ids { + self.close_surface(&tab_id); + } + } + + pub(super) fn close_surface(&mut self, tab_id: &TabId) { + self.close_surface_for_tab(tab_id); + } + pub(super) fn record_scroll_delta( &mut self, tab_id: &TabId, @@ -334,6 +350,14 @@ impl WebSurfaceStore { self.surfaces.entry(tab_id.clone()).or_insert_with(PerTabSurface::new) } + fn close_surface_for_tab(&mut self, tab_id: &TabId) { + self.runtime.close_tab(tab_id); + self.surfaces.remove(tab_id); + if self.keyboard_focus.as_ref().is_some_and(|focus| focus.tab_id == *tab_id) { + self.keyboard_focus = None; + } + } + #[cfg(test)] pub(super) fn surface_for_test(&self, tab_id: &TabId) -> Option<&PerTabSurface> { self.surfaces.get(tab_id) diff --git a/crates/ely_app/src/shell/web_surface_controller.rs b/crates/ely_app/src/shell/web_surface_controller.rs index 3b7e075..4554e01 100644 --- a/crates/ely_app/src/shell/web_surface_controller.rs +++ b/crates/ely_app/src/shell/web_surface_controller.rs @@ -51,12 +51,13 @@ impl ElyShell { } pub(super) fn tick_external_web_surfaces(&mut self) -> bool { - let visible_tab_ids = match &self.state { + let (visible_tab_ids, open_tab_ids) = match &self.state { super::ShellState::Ready(core) => { - core.visible_content_tab_ids().unwrap_or_else(|_| Vec::new()) + (core.visible_content_tab_ids().unwrap_or_else(|_| Vec::new()), core.open_tab_ids()) } - super::ShellState::StartupError(_) => Vec::new(), + super::ShellState::StartupError(_) => (Vec::new(), Vec::new()), }; + self.web_surfaces.retain_tabs(&open_tab_ids); let result = self.web_surfaces.tick(&visible_tab_ids); let mut url_changed = false; for url_change in result.url_changes { diff --git a/crates/ely_app/src/shell/web_surface_live_site_tests.rs b/crates/ely_app/src/shell/web_surface_live_site_tests.rs index 87e71c9..0a45e49 100644 --- a/crates/ely_app/src/shell/web_surface_live_site_tests.rs +++ b/crates/ely_app/src/shell/web_surface_live_site_tests.rs @@ -109,7 +109,10 @@ fn render_web_surface_frame( ); store.ensure_surface(&tab, ProfileDataMode::Transient, &[]); - match wait_for_ready_frame(store, tab.id(), case) { + let result = wait_for_ready_frame(store, tab.id(), case); + store.close_surface(tab.id()); + + match result { Ok(frame) => return Ok(frame), Err(error) => last_error = error, } diff --git a/crates/ely_app/src/shell/web_surface_runtime.rs b/crates/ely_app/src/shell/web_surface_runtime.rs index 934bd09..5f010cc 100644 --- a/crates/ely_app/src/shell/web_surface_runtime.rs +++ b/crates/ely_app/src/shell/web_surface_runtime.rs @@ -149,6 +149,15 @@ impl WebSurfaceRuntime { frames } + pub(super) fn close_tab(&mut self, tab_id: &TabId) { + let Some(session) = self.sessions.remove(tab_id) else { + return; + }; + if let Some(client) = self.clients.get_mut(&session.scope) { + let _ = client.client.close(tab_id.as_str().to_string()); + } + } + fn ensure_runtime(&mut self, scope: WebSurfaceRuntimeScope) -> Result<(), String> { if self.clients.contains_key(&scope) { return Ok(()); @@ -200,6 +209,8 @@ trait LiveRuntimeClient { fn ensure(&mut self, request: ServoLiveEnsureRequest) -> Result, String>; fn poll(&mut self, tab_id: String) -> Result, String>; + + fn close(&mut self, tab_id: String) -> Result<(), String>; } type WebLiveFrame = crate::services::servo_live::ServoLiveFrame; @@ -212,6 +223,10 @@ impl LiveRuntimeClient for ServoLiveClient { fn poll(&mut self, tab_id: String) -> Result, String> { ServoLiveClient::poll(self, tab_id).map_err(|error| error.to_string()) } + + fn close(&mut self, tab_id: String) -> Result<(), String> { + ServoLiveClient::close(self, tab_id).map_err(|error| error.to_string()) + } } fn new_servo_live_client(config_dir: PathBuf) -> Result, String> { diff --git a/crates/ely_app/src/shell/web_surface_runtime_tests.rs b/crates/ely_app/src/shell/web_surface_runtime_tests.rs index dce7bbe..45eb547 100644 --- a/crates/ely_app/src/shell/web_surface_runtime_tests.rs +++ b/crates/ely_app/src/shell/web_surface_runtime_tests.rs @@ -1,4 +1,7 @@ -use std::collections::BTreeMap; +use std::{ + collections::BTreeMap, + sync::atomic::{AtomicUsize, Ordering}, +}; use ely_domain::{BrowserTab, ProfileId, SpaceId, TabId, UrlText}; @@ -12,6 +15,8 @@ use crate::{ use super::*; +static FAKE_CLOSE_COUNT: AtomicUsize = AtomicUsize::new(0); + #[test] fn runtime_keeps_independent_clients_for_profile_scopes() -> Result<(), String> { let mut runtime = WebSurfaceRuntime::new_with_client_factory(fake_client_factory); @@ -54,6 +59,23 @@ fn runtime_keeps_independent_clients_for_profile_scopes() -> Result<(), String> Ok(()) } +#[test] +fn close_tab_removes_session_and_closes_client() -> Result<(), String> { + let before = FAKE_CLOSE_COUNT.load(Ordering::SeqCst); + let mut runtime = WebSurfaceRuntime::new_with_client_factory(fake_client_factory); + let tab = web_tab(TabId::new(), ProfileId::new(), "https://example.com/close")?; + + runtime.ensure_tab(&tab, surface_size(), ProfileDataMode::Transient, &[], pending_input())?; + runtime.close_tab(tab.id()); + + assert_eq!(runtime.session_scope_for_test(tab.id()), None); + assert_eq!(FAKE_CLOSE_COUNT.load(Ordering::SeqCst), before + 1); + + runtime.close_tab(tab.id()); + assert_eq!(FAKE_CLOSE_COUNT.load(Ordering::SeqCst), before + 1); + Ok(()) +} + #[test] fn session_scope_change_resets_tab_state() { let tab_id = TabId::new(); @@ -88,6 +110,11 @@ impl LiveRuntimeClient for FakeLiveRuntimeClient { fn poll(&mut self, _tab_id: String) -> Result, String> { Ok(None) } + + fn close(&mut self, _tab_id: String) -> Result<(), String> { + FAKE_CLOSE_COUNT.fetch_add(1, Ordering::SeqCst); + Ok(()) + } } fn fake_client_factory( diff --git a/crates/ely_browser_core/src/state/visible_content.rs b/crates/ely_browser_core/src/state/visible_content.rs index 4e5ddb2..904c351 100644 --- a/crates/ely_browser_core/src/state/visible_content.rs +++ b/crates/ely_browser_core/src/state/visible_content.rs @@ -4,6 +4,10 @@ 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 visible_content_tab_ids(&self) -> Result, CoreError> { let active_tab = self.active_tab()?; let Some(split_id) = active_tab.split_id() else { 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 37d6f86..19a5353 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 @@ -177,6 +177,13 @@ fn handle_request( ); Ok(outcome) } + LiveRequest::Close { tab_id } => { + if let Some(session) = sessions.remove(&tab_id) { + host.close_webview(&session.webview_id); + } + published_surface_ids.remove(&tab_id); + Ok(LiveOutcome::empty()) + } } } 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 eaef074..b756103 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 @@ -45,6 +45,9 @@ pub(super) enum LiveRequest { Poll { tab_id: String, }, + Close { + tab_id: String, + }, } fn default_device_pixel_ratio() -> f32 { @@ -240,3 +243,20 @@ pub(super) enum LiveSidecarError { #[error(transparent)] Json(#[from] serde_json::Error), } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn close_request_deserializes_from_wire() -> Result<(), serde_json::Error> { + let request = + serde_json::from_str::(r#"{"type":"close","tab_id":"tab-live-close"}"#)?; + + assert!(matches!( + request, + LiveRequest::Close { tab_id } if tab_id == "tab-live-close" + )); + Ok(()) + } +} diff --git a/crates/ely_servo_host/src/runtime.rs b/crates/ely_servo_host/src/runtime.rs index 4d1a34f..84cb232 100644 --- a/crates/ely_servo_host/src/runtime.rs +++ b/crates/ely_servo_host/src/runtime.rs @@ -114,6 +114,10 @@ impl SoftwareServoHost { self.paint_webview(webview_id, false).map(|_| ()) } + pub fn close_webview(&mut self, webview_id: &WebViewId) -> bool { + self.webviews.remove(webview_id).is_some() + } + fn new_started( size: ServoSurfaceSize, config_dir: Option,