From df78e226500cd63a2bc267c2cbc558e34dbe98f6 Mon Sep 17 00:00:00 2001 From: zhom <2717306+zhom@users.noreply.github.com> Date: Sat, 26 Jul 2025 18:27:38 +0400 Subject: [PATCH] refactor: properly cleanup unused binaries and simplify downloaded browser registry --- src-tauri/src/auto_updater.rs | 9 +- src-tauri/src/browser_runner.rs | 28 ++++-- src-tauri/src/downloaded_browsers.rs | 139 +++++++++------------------ src-tauri/src/lib.rs | 19 ++++ 4 files changed, 94 insertions(+), 101 deletions(-) diff --git a/src-tauri/src/auto_updater.rs b/src-tauri/src/auto_updater.rs index de47fe1..cae80c9 100644 --- a/src-tauri/src/auto_updater.rs +++ b/src-tauri/src/auto_updater.rs @@ -370,12 +370,15 @@ impl AutoUpdater { let mut registry = crate::downloaded_browsers::DownloadedBrowsersRegistry::load() .map_err(|e| format!("Failed to load browser registry: {e}"))?; - // Get active browser versions + // Get active browser versions (all profiles) let active_versions = registry.get_active_browser_versions(&profiles); - // Cleanup unused binaries + // Get running browser versions (only running profiles) + let running_versions = registry.get_running_browser_versions(&profiles); + + // Cleanup unused binaries (but keep running ones) let cleaned_up = registry - .cleanup_unused_binaries(&active_versions) + .cleanup_unused_binaries(&active_versions, &running_versions) .map_err(|e| format!("Failed to cleanup unused binaries: {e}"))?; // Save updated registry diff --git a/src-tauri/src/browser_runner.rs b/src-tauri/src/browser_runner.rs index 4d8a9fc..9b08778 100644 --- a/src-tauri/src/browser_runner.rs +++ b/src-tauri/src/browser_runner.rs @@ -1434,6 +1434,11 @@ impl BrowserRunner { self.disable_proxy_settings_in_profile(&profile_data_dir)?; } + // Perform cleanup after profile creation to remove any unused binaries + if let Err(e) = self.cleanup_unused_binaries_internal() { + println!("Warning: Failed to cleanup unused binaries after profile creation: {e}"); + } + Ok(profile) } @@ -1616,7 +1621,7 @@ impl BrowserRunner { } /// Internal method to cleanup unused binaries (used by auto-cleanup) - fn cleanup_unused_binaries_internal( + pub fn cleanup_unused_binaries_internal( &self, ) -> Result, Box> { // Load current profiles @@ -1627,11 +1632,14 @@ impl BrowserRunner { // Load registry let mut registry = crate::downloaded_browsers::DownloadedBrowsersRegistry::load()?; - // Get active browser versions + // Get active browser versions (all profiles) let active_versions = registry.get_active_browser_versions(&profiles); - // Cleanup unused binaries - let cleaned_up = registry.cleanup_unused_binaries(&active_versions)?; + // Get running browser versions (only running profiles) + let running_versions = registry.get_running_browser_versions(&profiles); + + // Cleanup unused binaries (but keep running ones) + let cleaned_up = registry.cleanup_unused_binaries(&active_versions, &running_versions)?; // Save updated registry registry.save()?; @@ -3190,14 +3198,14 @@ impl BrowserRunner { } // Mark download as completed in registry - let actual_version = if browser_str == "chromium" { + let _actual_version = if browser_str == "chromium" { Some(version.clone()) } else { None }; registry - .mark_download_completed_with_actual_version(&browser_str, &version, actual_version) + .mark_download_completed(&browser_str, &version) .map_err(|e| format!("Failed to mark download as completed: {e}"))?; registry .save() @@ -3651,6 +3659,14 @@ pub async fn ensure_all_binaries_exist( .map_err(|e| format!("Failed to ensure all binaries exist: {e}")) } +#[tauri::command] +pub async fn cleanup_unused_binaries() -> Result, String> { + let browser_runner = BrowserRunner::new(); + browser_runner + .cleanup_unused_binaries_internal() + .map_err(|e| format!("Failed to cleanup unused binaries: {e}")) +} + #[cfg(test)] mod tests { use super::*; diff --git a/src-tauri/src/downloaded_browsers.rs b/src-tauri/src/downloaded_browsers.rs index 837bbe8..a587e4d 100644 --- a/src-tauri/src/downloaded_browsers.rs +++ b/src-tauri/src/downloaded_browsers.rs @@ -8,13 +8,7 @@ use std::path::PathBuf; pub struct DownloadedBrowserInfo { pub browser: String, pub version: String, - pub download_date: u64, pub file_path: PathBuf, - pub verified: bool, - pub actual_version: Option, // For browsers like Chromium where we track the actual version - pub file_size: Option, // For tracking file size changes (useful for rolling releases) - #[serde(default)] // Add default value (false) for backwards compatibility - pub is_rolling_release: bool, // True for Zen's twilight releases and other rolling releases } #[derive(Debug, Serialize, Deserialize, Default)] @@ -82,66 +76,39 @@ impl DownloadedBrowsersRegistry { .browsers .get(browser) .and_then(|versions| versions.get(version)) - .map(|info| info.verified) - .unwrap_or(false) + .is_some() } pub fn get_downloaded_versions(&self, browser: &str) -> Vec { self .browsers .get(browser) - .map(|versions| { - versions - .iter() - .filter(|(_, info)| info.verified) - .map(|(version, _)| version.clone()) - .collect() - }) + .map(|versions| versions.keys().cloned().collect()) .unwrap_or_default() } pub fn mark_download_started(&mut self, browser: &str, version: &str, file_path: PathBuf) { - let is_rolling = Self::is_rolling_release(browser, version); let info = DownloadedBrowserInfo { browser: browser.to_string(), version: version.to_string(), - download_date: std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap_or_default() - .as_secs(), file_path, - verified: false, - actual_version: None, - file_size: None, - is_rolling_release: is_rolling, }; self.add_browser(info); } - pub fn mark_download_completed_with_actual_version( - &mut self, - browser: &str, - version: &str, - actual_version: Option, - ) -> Result<(), String> { - if let Some(info) = self + pub fn mark_download_completed(&mut self, browser: &str, version: &str) -> Result<(), String> { + if self .browsers - .get_mut(browser) - .and_then(|versions| versions.get_mut(version)) + .get(browser) + .and_then(|versions| versions.get(version)) + .is_some() { - info.verified = true; - info.actual_version = actual_version; Ok(()) } else { Err(format!("Browser {browser}:{version} not found in registry")) } } - fn is_rolling_release(browser: &str, version: &str) -> bool { - // Check if this is a rolling release like twilight - browser == "zen" && version.to_lowercase() == "twilight" - } - pub fn cleanup_failed_download( &mut self, browser: &str, @@ -180,32 +147,35 @@ impl DownloadedBrowsersRegistry { pub fn cleanup_unused_binaries( &mut self, active_profiles: &[(String, String)], // (browser, version) pairs + running_profiles: &[(String, String)], // (browser, version) pairs for running profiles ) -> Result, Box> { let active_set: std::collections::HashSet<(String, String)> = active_profiles.iter().cloned().collect(); + let running_set: std::collections::HashSet<(String, String)> = + running_profiles.iter().cloned().collect(); let mut cleaned_up = Vec::new(); // Collect all downloaded browsers that are not in active profiles let mut to_remove = Vec::new(); for (browser, versions) in &self.browsers { - for (version, info) in versions { - // Only remove verified downloads that are not used by any active profile - if info.verified && !active_set.contains(&(browser.clone(), version.clone())) { - // Double-check that this browser+version is truly not in use - // by looking for exact matches in the active profiles - let is_in_use = active_profiles - .iter() - .any(|(active_browser, active_version)| { - active_browser == browser && active_version == version - }); + for version in versions.keys() { + let browser_version = (browser.clone(), version.clone()); - if !is_in_use { - to_remove.push((browser.clone(), version.clone())); - println!("Marking for removal: {browser} {version} (not used by any profile)"); - } else { - println!("Keeping: {browser} {version} (in use by profile)"); - } + // Don't remove if it's used by any active profile + if active_set.contains(&browser_version) { + println!("Keeping: {browser} {version} (in use by profile)"); + continue; } + + // Don't remove if it's currently running (even if not in active profiles) + if running_set.contains(&browser_version) { + println!("Keeping: {browser} {version} (currently running)"); + continue; + } + + // Mark for removal + to_remove.push(browser_version); + println!("Marking for removal: {browser} {version} (not used by any profile)"); } } @@ -277,6 +247,18 @@ impl DownloadedBrowsersRegistry { Ok(cleaned_up) } + + /// Get all browsers and versions that are currently running + pub fn get_running_browser_versions( + &self, + profiles: &[crate::browser_runner::BrowserProfile], + ) -> Vec<(String, String)> { + profiles + .iter() + .filter(|profile| profile.process_id.is_some()) + .map(|profile| (profile.browser.clone(), profile.version.clone())) + .collect() + } } #[cfg(test)] @@ -295,12 +277,7 @@ mod tests { let info = DownloadedBrowserInfo { browser: "firefox".to_string(), version: "139.0".to_string(), - download_date: 1234567890, file_path: PathBuf::from("/test/path"), - verified: true, - actual_version: None, - file_size: None, - is_rolling_release: false, }; registry.add_browser(info.clone()); @@ -317,34 +294,19 @@ mod tests { let info1 = DownloadedBrowserInfo { browser: "firefox".to_string(), version: "139.0".to_string(), - download_date: 1234567890, file_path: PathBuf::from("/test/path1"), - verified: true, - actual_version: None, - file_size: None, - is_rolling_release: false, }; let info2 = DownloadedBrowserInfo { browser: "firefox".to_string(), version: "140.0".to_string(), - download_date: 1234567891, file_path: PathBuf::from("/test/path2"), - verified: false, // Not verified, should not be included - actual_version: None, - file_size: None, - is_rolling_release: false, }; let info3 = DownloadedBrowserInfo { browser: "firefox".to_string(), version: "141.0".to_string(), - download_date: 1234567892, file_path: PathBuf::from("/test/path3"), - verified: true, - actual_version: None, - file_size: None, - is_rolling_release: false, }; registry.add_browser(info1); @@ -352,10 +314,10 @@ mod tests { registry.add_browser(info3); let versions = registry.get_downloaded_versions("firefox"); - assert_eq!(versions.len(), 2); + assert_eq!(versions.len(), 3); assert!(versions.contains(&"139.0".to_string())); + assert!(versions.contains(&"140.0".to_string())); assert!(versions.contains(&"141.0".to_string())); - assert!(!versions.contains(&"140.0".to_string())); } #[test] @@ -365,15 +327,15 @@ mod tests { // Mark download started registry.mark_download_started("firefox", "139.0", PathBuf::from("/test/path")); - // Should not be considered downloaded yet - assert!(!registry.is_browser_downloaded("firefox", "139.0")); + // Should be considered downloaded immediately + assert!(registry.is_browser_downloaded("firefox", "139.0")); // Mark as completed registry - .mark_download_completed_with_actual_version("firefox", "139.0", Some("139.0".to_string())) + .mark_download_completed("firefox", "139.0") .unwrap(); - // Now should be considered downloaded + // Should still be considered downloaded assert!(registry.is_browser_downloaded("firefox", "139.0")); } @@ -383,12 +345,7 @@ mod tests { let info = DownloadedBrowserInfo { browser: "firefox".to_string(), version: "139.0".to_string(), - download_date: 1234567890, file_path: PathBuf::from("/test/path"), - verified: true, - actual_version: None, - file_size: None, - is_rolling_release: false, }; registry.add_browser(info); @@ -400,15 +357,13 @@ mod tests { } #[test] - fn test_twilight_rolling_release() { + fn test_twilight_download() { let mut registry = DownloadedBrowsersRegistry::new(); // Mark twilight download started registry.mark_download_started("zen", "twilight", PathBuf::from("/test/zen-twilight")); - // Check that it's marked as rolling release - let zen_versions = ®istry.browsers["zen"]; - let twilight_info = &zen_versions["twilight"]; - assert!(twilight_info.is_rolling_release); + // Check that it's registered + assert!(registry.is_browser_downloaded("zen", "twilight")); } } diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index d008ce9..c249247 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -72,6 +72,8 @@ use group_manager::{ get_groups_with_profile_counts, get_profile_groups, update_profile_group, }; +use browser_runner::cleanup_unused_binaries; + // Trait to extend WebviewWindow with transparent titlebar functionality pub trait WindowExt { #[cfg(target_os = "macos")] @@ -337,6 +339,22 @@ pub fn run() { auto_updater::check_for_updates_with_progress(app_handle_auto_updater).await; }); + // Start periodic cleanup task for unused binaries + tauri::async_runtime::spawn(async move { + let mut interval = tokio::time::interval(tokio::time::Duration::from_secs(300)); // Every 5 minutes + + loop { + interval.tick().await; + + let browser_runner = crate::browser_runner::BrowserRunner::new(); + if let Err(e) = browser_runner.cleanup_unused_binaries_internal() { + eprintln!("Periodic cleanup failed: {e}"); + } else { + println!("Periodic cleanup completed successfully"); + } + } + }); + let app_handle_update = app.handle().clone(); tauri::async_runtime::spawn(async move { println!("Starting app update check at startup..."); @@ -447,6 +465,7 @@ pub fn run() { delete_profile_group, assign_profiles_to_group, delete_selected_profiles, + cleanup_unused_binaries, ]) .run(tauri::generate_context!()) .expect("error while running tauri application");