From a681a7e283dd76bea23c5138a0de81ebda3c3cf0 Mon Sep 17 00:00:00 2001 From: Leonard de Ruijter Date: Wed, 7 Oct 2026 21:10:55 +0200 Subject: [PATCH] Show the download window through wx-utils' progress module The update download window moved to wx-utils 0.4.2 as its progress module, together with its lifecycle guard and 99 percent gauge cap. start_download now hands the download to run_with_progress instead of running its own worker, update thread and dialog bookkeeping. On macOS the work also prepares the installation, and the window pulses with "Preparing update..." meanwhile, as before. The window now waits for any other wx-utils progress window in the application, instead of showing on top of it. Assisted-by: Claude:claude-opus-5-5 --- Cargo.lock | 15 ++++- Cargo.toml | 1 + README.md | 11 ++-- src/ui.rs | 155 ++++++++++++++------------------------------- src/ui/progress.rs | 127 ------------------------------------- 5 files changed, 68 insertions(+), 241 deletions(-) delete mode 100644 src/ui/progress.rs diff --git a/Cargo.lock b/Cargo.lock index 60afde3..de3cab4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -865,9 +865,9 @@ checksum = "57c0d7b74b563b49d38dae00a0c37d4d6de9b432382b2892f0574ddcae73fd0a" [[package]] name = "patois" -version = "0.2.2" +version = "0.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2d8a7a8ed1b780906a3ee4cfb797cf467c0a758176701c10fd783ca0f2d39938" +checksum = "75b8d37756e8066067907b01d2cb1ffb7f2886707af4909280a523b2f98f59cc" dependencies = [ "inventory", "patois-macros", @@ -1223,6 +1223,7 @@ dependencies = [ "serde", "tempfile", "ureq", + "wx-utils", "wxdragon", ] @@ -1770,6 +1771,16 @@ version = "0.6.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3ad82d2a33cdc9674dc7465672f271e096168fcdbe0f799d9e6db8c5892679dc" +[[package]] +name = "wx-utils" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b9ec56dbb82e24c9eff07f9e637f5a44410a4166ac9c8f25fe0fce34d8c58d17" +dependencies = [ + "patois", + "wxdragon", +] + [[package]] name = "wxdragon" version = "0.9.22" diff --git a/Cargo.toml b/Cargo.toml index b9270e4..8c9df98 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,6 +20,7 @@ patois = "0.2.2" pulldown-cmark = { version = "0.13.4", default-features = false } serde = { version = "1.0.229", default-features = false, features = ["derive"] } ureq = { version = "3.4.2", default-features = false, features = ["json", "rustls"] } +wx-utils = "0.4.2" # default-features = false so this does not decide for the application # which parts of wxWidgets get built; nothing here needs any of them. wxdragon = { version = "0.9.22", default-features = false } diff --git a/README.md b/README.md index b9e14eb..6ba2c6b 100644 --- a/README.md +++ b/README.md @@ -98,10 +98,13 @@ that identity, fixtures are ad-hoc signed and demonstrate the manual fallback in trusts only its generated local fixture and keeps the production signer checks. It is a repository test harness, excluded from the published crate package. -The download gauge stays below 100 until verification and preparation finish. Native progress -updates can yield to the event loop, so completion waits for the active Update/Pulse call to -return before destroying the dialog or calling the shutdown handler. Regression tests cover this -ordering without requiring a graphical session. +The download window comes from the `progress` module of +[wx-utils](https://github.com/trypsynth/wx-utils). Its gauge stays below 100 until verification +and preparation finish. Native progress updates can yield to the event loop, so completion waits +for the active Update/Pulse call to return before destroying the dialog or calling the shutdown +handler. wx-utils' regression tests cover this ordering without requiring a graphical session. +wx-utils shows one progress window at a time, so a download that starts while the application +shows another one waits for it. Run `cargo test` on macOS for coverage of cancellation, helper timeouts, replacement and rollback, failed restore and launch, competing destination changes, protected locations, bundle selection, diff --git a/src/ui.rs b/src/ui.rs index 7ce57cb..b3c74fb 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -1,14 +1,15 @@ use std::{ + path::PathBuf, process, ptr, sync::{ Arc, - atomic::{AtomicBool, AtomicU64, Ordering}, + atomic::{AtomicBool, Ordering}, }, thread, - time::Duration, }; use patois::t; +use wx_utils::progress::{Ended, Progress, run_with_progress}; use wxdragon::{ffi, prelude::*, window::WxWidget}; pub use self::markdown::markdown_to_text; @@ -19,15 +20,6 @@ use crate::{ }; mod markdown; -mod progress; - -use progress::{ProgressLifecycle, download_percent}; - -const PROGRESS_INTERVAL: Duration = Duration::from_millis(200); - -thread_local! { - static ACTIVE_PROGRESS: ProgressLifecycle = const { ProgressLifecycle::new() }; -} /// Guards against a second update-check flow (silent startup check, manual "Check for /// Updates", or an impatient double-click while a download is stuck) from starting while @@ -78,16 +70,6 @@ pub enum CheckTrigger { Manual, } -#[derive(Default)] -struct DownloadProgress { - downloaded: AtomicU64, - total: AtomicU64, - finished: AtomicBool, - #[cfg(target_os = "macos")] - preparing: AtomicBool, - cancelled: AtomicBool, -} - /// Show the "update available" dialog and return `true` if the user accepted. /// /// `app_display_name` appears in the body label (e.g. `"A new version of My App is available."`). @@ -203,99 +185,56 @@ fn start_download( on_exit: Box, ) { let title = t("%s Update").replace("%s", &config.app_display_name); - let progress_dialog = ProgressDialog::builder(&parent, &title, &t("Downloading update..."), 100) - .with_style( - ProgressDialogStyle::AutoHide - | ProgressDialogStyle::AppModal - | ProgressDialogStyle::RemainingTime - | ProgressDialogStyle::CanAbort, - ) - .build(); - ACTIVE_PROGRESS.with(|p| p.set(progress_dialog)); - let progress = Arc::new(DownloadProgress::default()); - spawn_progress_heartbeat(Arc::clone(&progress)); - thread::spawn(move || { - let download = - download_update_file(&config, &result.download_url, &result.signature_url, &progress.cancelled, |d, t| { - progress.downloaded.store(d, Ordering::Relaxed); - progress.total.store(t, Ordering::Relaxed); - }); - #[cfg(target_os = "macos")] - let download = if progress.cancelled.load(Ordering::Relaxed) { - if let Ok(ref path) = download { - platform::remove_private_download(path); - } - Err(t("Update cancelled.")) - } else { - download.map_err(|e| format!("{}: {e}", t("Update failed"))).and_then(|path| { - progress.preparing.store(true, Ordering::Relaxed); - let result = platform::install_with_cancel(&config, &path, &progress.cancelled); - // A manual fallback still needs its DMG. Automatic preparation has copied the - // bundle, so its private verified download can be removed. - if !matches!(result, Ok(InstallOutcome::ManualStep(_))) { - platform::remove_private_download(&path); - } - result - }) - }; - progress.finished.store(true, Ordering::Relaxed); - wxdragon::call_after(Box::new(move || { - ACTIVE_PROGRESS.with(|p| { - p.finish(Box::new(move || { - ACTIVE_PROGRESS.with(ProgressLifecycle::clear); - if !progress.cancelled.load(Ordering::Relaxed) { - #[cfg(not(target_os = "macos"))] - let download = download - .map_err(|e| format!("{}: {e}", t("Update failed"))) - .and_then(|path| platform::install(&config, &path)); - install_update(parent, download, on_exit); - } - drop(check); - })); - }); - })); - wxdragon::wake_up_idle(); + #[cfg(not(target_os = "macos"))] + let install_config = Arc::clone(&config); + #[cfg(not(target_os = "macos"))] + let work = move |progress: &Progress| fetch(&config, &result, progress); + #[cfg(target_os = "macos")] + let work = move |progress: &Progress| prepare(&config, fetch(&config, &result, progress), progress); + run_with_progress(&parent, &title, &t("Downloading update..."), work, move |download, ended| { + if ended == Ended::Completed { + #[cfg(not(target_os = "macos"))] + let download = download + .map_err(|e| format!("{}: {e}", t("Update failed"))) + .and_then(|path| platform::install(&install_config, &path)); + install_update(parent, download, on_exit); + } + drop(check); }); } -/// Updates the progress dialog from the main thread every [`PROGRESS_INTERVAL`] until the -/// download finishes or is cancelled. -fn spawn_progress_heartbeat(progress: Arc) { - thread::spawn(move || { - while !progress.finished.load(Ordering::Relaxed) && !progress.cancelled.load(Ordering::Relaxed) { - let downloaded = progress.downloaded.load(Ordering::Relaxed); - let total = progress.total.load(Ordering::Relaxed); - let progress = Arc::clone(&progress); - wxdragon::call_after(Box::new(move || update_progress_dialog(&progress, downloaded, total))); - wxdragon::wake_up_idle(); - thread::sleep(PROGRESS_INTERVAL); - } - }); +/// Downloads and verifies the update, moving the download window's gauge as it goes. +fn fetch(config: &UpdaterConfig, result: &UpdateAvailableResult, progress: &Progress) -> Result { + download_update_file(config, &result.download_url, &result.signature_url, progress.cancel_flag(), |done, total| { + progress.set(done, Some(total)); + }) } -fn update_progress_dialog(progress: &DownloadProgress, downloaded: u64, total: u64) { - ACTIVE_PROGRESS.with(|p| { - let Some(update) = p.begin_update() else { - return; - }; - let dialog = update.dialog(); - #[cfg(target_os = "macos")] - let preparing = progress.preparing.load(Ordering::Relaxed); - #[cfg(not(target_os = "macos"))] - let preparing = false; - let keep_going = if preparing { - dialog.pulse(Some(&t("Preparing update..."))) - } else { - download_percent(downloaded, total) - .map_or_else(|| dialog.pulse(None), |percent| dialog.update(percent, None)) - }; - if !keep_going { - progress.cancelled.store(true, Ordering::Relaxed); - p.clear(); +/// Prepares the downloaded update for installation, while the download window pulses. +#[cfg(target_os = "macos")] +fn prepare( + config: &UpdaterConfig, + download: Result, + progress: &Progress, +) -> Result { + if progress.is_cancelled() { + if let Ok(ref path) = download { + platform::remove_private_download(path); } - // Dropping `update` invokes any completion queued during the native event-loop yield, - // after Update/Pulse returns and its reference to the dialog is released. - }); + Err(t("Update cancelled.")) + } else { + download.map_err(|e| format!("{}: {e}", t("Update failed"))).and_then(|path| { + progress.set_message(&t("Preparing update...")); + progress.set(0, None); + let result = platform::install_with_cancel(config, &path, progress.cancel_flag()); + // A manual fallback still needs its DMG. Automatic preparation has copied the + // bundle, so its private verified download can be removed. + if !matches!(result, Ok(InstallOutcome::ManualStep(_))) { + platform::remove_private_download(&path); + } + result + }) + } } fn install_update(parent: ParentWindow, outcome: Result, on_exit: Box) { diff --git a/src/ui/progress.rs b/src/ui/progress.rs deleted file mode 100644 index 2fded34..0000000 --- a/src/ui/progress.rs +++ /dev/null @@ -1,127 +0,0 @@ -//! wx progress updates yield to the event loop. Keep dialog ownership and completion separate -//! so a nested callback cannot destroy a dialog, or borrow its storage, during Update/Pulse. -use std::{ - cell::{Cell, RefCell}, - rc::Rc, -}; - -pub(super) struct ProgressLifecycle { - dialog: RefCell>>, - updating: Cell, - completion: RefCell>>, -} - -impl ProgressLifecycle { - pub(super) const fn new() -> Self { - Self { dialog: RefCell::new(None), updating: Cell::new(false), completion: RefCell::new(None) } - } - - pub(super) fn set(&self, dialog: D) { - *self.dialog.borrow_mut() = Some(Rc::new(dialog)); - } - - pub(super) fn clear(&self) { - // Native dialog destruction can process events too. Release the borrow before dropping. - let dialog = self.dialog.borrow_mut().take(); - drop(dialog); - } - - pub(super) fn begin_update(&self) -> Option> { - if self.updating.get() { - return None; - } - let dialog = self.dialog.borrow().as_ref().map(Rc::clone)?; - self.updating.set(true); - Some(ProgressUpdate { state: self, dialog: Some(dialog) }) - } - - pub(super) fn finish(&self, completion: Box) { - if self.updating.get() { - *self.completion.borrow_mut() = Some(completion); - } else { - completion(); - } - } -} - -pub(super) struct ProgressUpdate<'a, D> { - state: &'a ProgressLifecycle, - dialog: Option>, -} - -impl ProgressUpdate<'_, D> { - pub(super) fn dialog(&self) -> &D { - self.dialog.as_ref().expect("progress update owns its dialog") - } -} - -impl Drop for ProgressUpdate<'_, D> { - fn drop(&mut self) { - // Native Update/Pulse has returned. Release its reference before handing off shutdown. - drop(self.dialog.take()); - self.state.updating.set(false); - let completion = self.state.completion.borrow_mut().take(); - if let Some(completion) = completion { - completion(); - } - } -} - -/// Keep the gauge below its native completion value until preparation and verification finish. -/// `Update(100)` without `AutoHide` opens a nested modal loop and waits for the user to close it. -pub(super) fn download_percent(downloaded: u64, total: u64) -> Option { - downloaded.saturating_mul(100).checked_div(total).map(|value| i32::try_from(value.min(99)).unwrap_or(99)) -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn completed_download_never_enters_native_completion_loop() { - assert_eq!(download_percent(50, 100), Some(50)); - assert_eq!(download_percent(100, 100), Some(99)); - assert_eq!(download_percent(200, 100), Some(99)); - assert_eq!(download_percent(u64::MAX, 1), Some(99)); - assert_eq!(download_percent(0, 0), None); - } - - #[test] - fn nested_completion_waits_for_native_update_to_return_and_destroy_dialog() { - struct Dialog(Rc>>); - impl Drop for Dialog { - fn drop(&mut self) { - self.0.borrow_mut().push("destroyed"); - } - } - let events = Rc::new(RefCell::new(Vec::new())); - let state = Rc::new(ProgressLifecycle::new()); - state.set(Dialog(Rc::clone(&events))); - let guard = state.begin_update().unwrap(); - assert!(state.begin_update().is_none()); - let complete_state = Rc::clone(&state); - let complete_events = Rc::clone(&events); - state.finish(Box::new(move || { - complete_state.clear(); - complete_events.borrow_mut().push("shutdown"); - })); - assert!(events.borrow().is_empty()); - // A reentrant callback can borrow the storage while Update/Pulse is active. - assert!(state.dialog.try_borrow_mut().is_ok()); - let _ = guard.dialog(); - drop(guard); - assert_eq!(*events.borrow(), ["destroyed", "shutdown"]); - assert!(state.begin_update().is_none()); - } - - #[test] - fn cancellation_keeps_native_dialog_alive_until_update_returns() { - let state = ProgressLifecycle::new(); - state.set(42); - let guard = state.begin_update().unwrap(); - state.clear(); - assert_eq!(*guard.dialog(), 42); - drop(guard); - assert!(state.begin_update().is_none()); - } -}