diff --git a/Cargo.lock b/Cargo.lock index db5a6641..7eb0f54f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -711,6 +711,7 @@ dependencies = [ "tokio", "tracing", "uuid", + "webview2-com", "windows", ] diff --git a/apps/desktop-tauri/src-tauri/Cargo.toml b/apps/desktop-tauri/src-tauri/Cargo.toml index 7ee2994d..2c7c84b0 100644 --- a/apps/desktop-tauri/src-tauri/Cargo.toml +++ b/apps/desktop-tauri/src-tauri/Cargo.toml @@ -21,6 +21,10 @@ tracing = "0.1" uuid = { version = "1", features = ["v4"] } [target.'cfg(windows)'.dependencies] +# WebView2's `ProcessFailed` event (#410). Already in the lock file through +# wry; a direct dependency only so `shell/webview_lifecycle.rs` can name the +# interfaces `tauri::webview::PlatformWebview::controller()` hands out. +webview2-com = "0.38" windows = { version = "0.61", features = [ "Win32_Foundation", "Win32_System_Com", diff --git a/apps/desktop-tauri/src-tauri/src/floatbar/window.rs b/apps/desktop-tauri/src-tauri/src/floatbar/window.rs index a2458d55..d491a3dc 100644 --- a/apps/desktop-tauri/src-tauri/src/floatbar/window.rs +++ b/apps/desktop-tauri/src-tauri/src/floatbar/window.rs @@ -107,6 +107,7 @@ pub fn show( .visible(false) .build() .map_err(|e| e.to_string())?; + crate::shell::webview_lifecycle::watch(app, &win); // Restore prior geometry if we have one. Otherwise, taskbar style opens // near the bottom while the original floating style keeps its top-center diff --git a/apps/desktop-tauri/src-tauri/src/main.rs b/apps/desktop-tauri/src-tauri/src/main.rs index e6de735b..263d715f 100644 --- a/apps/desktop-tauri/src-tauri/src/main.rs +++ b/apps/desktop-tauri/src-tauri/src/main.rs @@ -254,6 +254,7 @@ fn main() { } } if let Some(window) = app.get_webview_window("main") { + shell::window_recovery::register_main(app.handle(), &window); shell::dwm::force_dark_caption(&window); window.hide()?; } diff --git a/apps/desktop-tauri/src-tauri/src/shell/flyout_window.rs b/apps/desktop-tauri/src-tauri/src/shell/flyout_window.rs index 83c62082..5835c62a 100644 --- a/apps/desktop-tauri/src-tauri/src/shell/flyout_window.rs +++ b/apps/desktop-tauri/src-tauri/src/shell/flyout_window.rs @@ -57,6 +57,11 @@ pub fn save_stored_size(width: u32, height: u32) { /// precedent) — callers must invoke this from an async context (an `async` /// command, or `tauri::async_runtime::spawn`), never a sync command handler. pub fn open_or_focus(app: &AppHandle, position: Option<(i32, i32)>) -> Result<(), String> { + // A ProcessFailed teardown of this very window may still be waiting for + // its label; opening into that would show the dying frame or build a + // second window under the label (#410). + super::window_recovery::wait_for_flyout_rebuild(); + // A WebView2 process exit leaves the hidden frame with no content; drop it // so the build path below runs instead of showing a blank window (#410). crate::webview_recovery::reclaim_dead_window(app, FLYOUT_LABEL)?; @@ -98,6 +103,7 @@ pub fn open_or_focus(app: &AppHandle, position: Option<(i32, i32)>) -> Result<() .disable_drag_drop_handler() .visible(false); let win = builder.build().map_err(|e| e.to_string())?; + super::webview_lifecycle::watch(app, &win); super::dwm::force_dark_caption(&win); @@ -176,6 +182,17 @@ pub fn handle_window_event(window: &tauri::Window, event: &tauri::WindowEvent) - if crate::proof_harness::is_proof_mode(app) { return true; } + // A window that has never been shown cannot lose focus in any + // sense the user meant. Windows still reports one when a hidden + // window is activated without foreground rights (e.g. right after + // build, before the frontend's reveal); treating that as a + // dismiss would clear the pending reveal and leave the flyout + // invisible. A failed query (the window is mid-teardown) is + // treated the same way: swallowing one blur is cheaper than + // dismissing a flyout that is about to be rebuilt. + if !window.is_visible().unwrap_or(false) { + return true; + } let Some(st) = app.try_state::>() else { return true; }; diff --git a/apps/desktop-tauri/src-tauri/src/shell/mod.rs b/apps/desktop-tauri/src-tauri/src/shell/mod.rs index d80bd7e7..2719746f 100644 --- a/apps/desktop-tauri/src-tauri/src/shell/mod.rs +++ b/apps/desktop-tauri/src-tauri/src/shell/mod.rs @@ -12,7 +12,9 @@ mod geometry; mod position; pub mod settings_window; mod transition; +pub(crate) mod webview_lifecycle; mod window; +pub(crate) mod window_recovery; #[cfg(test)] mod tests; diff --git a/apps/desktop-tauri/src-tauri/src/shell/settings_window.rs b/apps/desktop-tauri/src-tauri/src/shell/settings_window.rs index 7469ab7c..f6a3a48b 100644 --- a/apps/desktop-tauri/src-tauri/src/shell/settings_window.rs +++ b/apps/desktop-tauri/src-tauri/src/shell/settings_window.rs @@ -5,7 +5,7 @@ use tauri::{Emitter, Manager, PhysicalPosition, WebviewUrl}; use crate::surface::SurfaceMode; -const SETTINGS_LABEL: &str = "settings"; +pub(crate) const SETTINGS_LABEL: &str = "settings"; const SETTINGS_WIDTH: f64 = 720.0; const SETTINGS_HEIGHT: f64 = 580.0; @@ -45,6 +45,7 @@ pub fn open_or_focus(app: &tauri::AppHandle, tab: &str) -> Result<(), String> { .resizable(true) .build() .map_err(|e| e.to_string())?; + super::webview_lifecycle::watch(app, &win); // Force DWM caption to dark; keep WS_THICKFRAME since window is resizable super::dwm::force_dark_caption_resizable(&win); diff --git a/apps/desktop-tauri/src-tauri/src/shell/transition.rs b/apps/desktop-tauri/src-tauri/src/shell/transition.rs index e0f5103c..93728fd7 100644 --- a/apps/desktop-tauri/src-tauri/src/shell/transition.rs +++ b/apps/desktop-tauri/src-tauri/src/shell/transition.rs @@ -249,9 +249,19 @@ fn apply_transition_request( force_same_mode_apply: bool, ) -> Result { let _transition_guard = SHELL_TRANSITION_SERIAL.lock().unwrap(); - let window = app - .get_webview_window("main") - .ok_or_else(|| "main window unavailable".to_string())?; + // #410: a dead `main` would show as an empty transparent frame. The + // guard rebuilds it off-thread and replays this request afterwards, so + // returning here is the recovery starting, not the request being lost. + let Some(window) = super::window_recovery::resolve_live_main( + app, + Some(super::window_recovery::MainRequest { + mode: request.mode, + target: request.target.clone(), + position: request.position, + }), + ) else { + return Ok(SurfaceMode::Hidden); + }; let st = app .try_state::>() .ok_or_else(|| "app state unavailable".to_string())?; @@ -509,7 +519,10 @@ pub(super) fn restore_surface_snapshot(state: &mut AppState, snapshot: &SurfaceS } } -fn commit_surface_snapshot(app: &AppHandle, snapshot: &SurfaceSnapshot) -> Result<(), String> { +pub(super) fn commit_surface_snapshot( + app: &AppHandle, + snapshot: &SurfaceSnapshot, +) -> Result<(), String> { let st = app .try_state::>() .ok_or_else(|| "app state unavailable".to_string())?; diff --git a/apps/desktop-tauri/src-tauri/src/shell/webview_lifecycle.rs b/apps/desktop-tauri/src-tauri/src/shell/webview_lifecycle.rs new file mode 100644 index 00000000..184cbf8f --- /dev/null +++ b/apps/desktop-tauri/src-tauri/src/shell/webview_lifecycle.rs @@ -0,0 +1,451 @@ +//! Recover from WebView2 process failures as they happen (#410). +//! +//! [`crate::webview_recovery`] and [`super::window_recovery`] catch a dead +//! webview *lazily*: the next time something tries to show the window, the +//! probe notices the missing WebView2 child and the window is rebuilt. That +//! covers hidden windows well, but a window that is on screen when its +//! browser process dies just sits there as a transparent frame until the +//! user happens to toggle it. +//! +//! This module closes that gap by subscribing to WebView2's own +//! `ProcessFailed` event on every window the app builds. When the browser +//! process is gone the control is unusable, so the affected window is torn +//! down immediately and — if it was visible — rebuilt and shown again +//! through the same first-build path it came from, so the recovery never +//! paints a frame of its own (the flyout keeps its `visible(false)` → +//! frontend-reveal handshake, `main` inherits `"visible": false` from the +//! config and is shown by the replayed surface transition). A crashed +//! render process is cheaper: WebView2 creates a new one by itself and only +//! the page has to be reloaded. +//! +//! Tearing the dead window down right away is also what lets the lazy +//! guards stay simple: a destroyed window vanishes from Tauri's label map, +//! so the next open takes the plain first-build path with no probe involved. +//! No extra "this window is dead" state has to be kept in sync. +//! +//! Nothing else in the stack surfaces `ProcessFailed` — not wry, not +//! tauri-runtime-wry, not tauri — so this is the one place the app talks to +//! WebView2's COM interfaces directly, through the `webview2-com` crate that +//! wry already depends on. +//! +//! **Threading.** WebView2 delivers `ProcessFailed` as a COM callback on the +//! UI thread, i.e. inside the very event loop that has to process a window's +//! `Destroyed` event. The handler therefore only reads what it needs from the +//! window and hands the destroy + rebuild to a background thread via +//! `window_recovery`; it never blocks, and never destroys a window inline. + +use tauri::{AppHandle, WebviewWindow}; + +/// What the handler does about a window whose webview reported a failure. +/// +/// Pure so the label → action policy is testable away from COM. `main` is +/// always rebuilt (hidden, as at startup) because it is the app's primary +/// surface and a hidden rebuild makes the next tray click instant; the two +/// windows that are built on demand are only rebuilt when the user was +/// looking at them, otherwise their next open builds them fresh anyway. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum RecoveryPlan { + /// Rebuild `main` hidden; replay the current surface if it was showing. + RebuildMain { replay: bool }, + /// Destroy `settings`; reopen it on its default tab if it was showing. + RebuildSettings { reopen: bool }, + /// Destroy the flyout; reopen it (via the reveal handshake) if it was showing. + RebuildFlyout { reopen: bool }, + /// Destroy the float bar and re-apply it from settings: it is on screen + /// for as long as it is enabled, so there is no "next open" to wait for. + RebuildFloatBar, + /// A window this module has no rebuild recipe for — log and leave it. + Unsupported, +} + +pub(crate) fn recovery_plan(label: &str, was_visible: bool) -> RecoveryPlan { + match label { + super::window_recovery::MAIN_LABEL => RecoveryPlan::RebuildMain { + replay: was_visible, + }, + super::settings_window::SETTINGS_LABEL => RecoveryPlan::RebuildSettings { + reopen: was_visible, + }, + super::flyout_window::FLYOUT_LABEL => RecoveryPlan::RebuildFlyout { + reopen: was_visible, + }, + crate::floatbar::FLOATBAR_LABEL => RecoveryPlan::RebuildFloatBar, + _ => RecoveryPlan::Unsupported, + } +} + +/// Subscribe `window` to WebView2's `ProcessFailed` event. +/// +/// Call once per built window, after `build()`; a rebuilt window gets a new +/// WebView2 instance and needs its own subscription. Registration is +/// best-effort: failing to subscribe is logged and the lazy guards still +/// apply. The subscription lives as long as the WebView2 instance does, so +/// the registration token is deliberately not kept — there is nothing to +/// unsubscribe from once the webview is gone. +#[cfg(windows)] +pub(crate) fn watch(app: &AppHandle, window: &WebviewWindow) { + let label = window.label().to_string(); + let app = app.clone(); + let result = window.with_webview(move |webview| { + let handler_label = label.clone(); + let handler_app = app.clone(); + match win32::subscribe(&webview, move |kind| { + on_process_failed(&handler_app, &handler_label, kind); + }) { + Ok(()) => tracing::debug!( + label, + "webview_lifecycle: subscribed to WebView2 ProcessFailed" + ), + Err(error) => tracing::warn!( + %error, + label, + "webview_lifecycle: could not subscribe to WebView2 ProcessFailed; \ + relying on the liveness guards" + ), + } + }); + if let Err(error) = result { + tracing::warn!( + %error, + label = window.label(), + "webview_lifecycle: could not reach the platform webview to subscribe" + ); + } +} + +#[cfg(not(windows))] +pub(crate) fn watch(_app: &AppHandle, _window: &WebviewWindow) {} + +/// Runs inside the COM callback on the UI thread: read, log, dispatch. +#[cfg(windows)] +fn on_process_failed(app: &AppHandle, label: &str, kind: win32::FailedKind) { + use tauri::Manager; + + let kind_name = win32::describe(kind); + // The warn is part of the contract: it is how an organic WebView2 death + // — previously invisible until a window came back blank — shows up in + // the log. + match win32::response_for(kind) { + win32::FailureResponse::LogOnly => { + tracing::warn!( + label, + kind = kind_name, + "webview_lifecycle: WebView2 reported a failed process; no recovery needed" + ); + return; + } + win32::FailureResponse::Reload => { + tracing::warn!( + label, + kind = kind_name, + "webview_lifecycle: WebView2 render process failed; reloading the page (#410)" + ); + reload_on_main_thread(app, label); + return; + } + win32::FailureResponse::Rebuild => {} + } + tracing::warn!( + label, + kind = kind_name, + "webview_lifecycle: WebView2 process failed; rebuilding the window (#410)" + ); + + // The native window is still there (only its WebView2 child died), so + // these cheap reads are safe and, on the main thread, synchronous. + let window = app.get_webview_window(label); + let was_visible = window + .as_ref() + .and_then(|window| window.is_visible().ok()) + .unwrap_or(false); + let position = window + .as_ref() + .and_then(|window| window.outer_position().ok()) + .map(|position| (position.x, position.y)); + let plan = recovery_plan(label, was_visible); + tracing::debug!( + label, + exists = window.is_some(), + was_visible, + ?position, + ?plan, + "webview_lifecycle: dispatching recovery" + ); + + use super::window_recovery; + match plan { + RecoveryPlan::RebuildMain { replay } => { + window_recovery::recover_main_after_loss(app, replay, position); + } + RecoveryPlan::RebuildSettings { reopen } => { + window_recovery::recover_settings_after_loss(app, reopen); + } + RecoveryPlan::RebuildFlyout { reopen } => { + window_recovery::recover_flyout_after_loss(app, reopen); + } + RecoveryPlan::RebuildFloatBar => { + window_recovery::recover_floatbar_after_loss(app); + } + RecoveryPlan::Unsupported => { + tracing::warn!( + label, + "webview_lifecycle: no rebuild recipe for this window; leaving it as is" + ); + } + } +} + +/// Reload the page in `label`'s window on the next turn of the event loop. +/// +/// WebView2 has already replaced the render process; the control itself is +/// fine, only its page is gone. Deferred through `run_on_main_thread` rather +/// than called inline so the reload never re-enters WebView2 from inside its +/// own `ProcessFailed` callback. `?tab=` and the other URL state survive, so +/// Settings comes back on the tab it was showing. +#[cfg(windows)] +fn reload_on_main_thread(app: &AppHandle, label: &str) { + use tauri::Manager; + + let Some(window) = app.get_webview_window(label) else { + return; + }; + let label = label.to_string(); + let dispatched = app.run_on_main_thread(move || { + if let Err(error) = window.reload() { + tracing::warn!( + %error, + label, + "webview_lifecycle: reloading after a render process failure failed" + ); + } + }); + if let Err(error) = dispatched { + tracing::warn!( + %error, + "webview_lifecycle: could not dispatch the reload to the main thread" + ); + } +} + +/// The COM-facing half: the only code that names WebView2 interfaces. +#[cfg(windows)] +mod win32 { + use tauri::webview::PlatformWebview; + use webview2_com::Microsoft::Web::WebView2::Win32::{ + COREWEBVIEW2_PROCESS_FAILED_KIND, COREWEBVIEW2_PROCESS_FAILED_KIND_BROWSER_PROCESS_EXITED, + COREWEBVIEW2_PROCESS_FAILED_KIND_FRAME_RENDER_PROCESS_EXITED, + COREWEBVIEW2_PROCESS_FAILED_KIND_GPU_PROCESS_EXITED, + COREWEBVIEW2_PROCESS_FAILED_KIND_PPAPI_BROKER_PROCESS_EXITED, + COREWEBVIEW2_PROCESS_FAILED_KIND_PPAPI_PLUGIN_PROCESS_EXITED, + COREWEBVIEW2_PROCESS_FAILED_KIND_RENDER_PROCESS_EXITED, + COREWEBVIEW2_PROCESS_FAILED_KIND_RENDER_PROCESS_UNRESPONSIVE, + COREWEBVIEW2_PROCESS_FAILED_KIND_SANDBOX_HELPER_PROCESS_EXITED, + COREWEBVIEW2_PROCESS_FAILED_KIND_UNKNOWN_PROCESS_EXITED, + COREWEBVIEW2_PROCESS_FAILED_KIND_UTILITY_PROCESS_EXITED, + }; + use webview2_com::ProcessFailedEventHandler; + + pub(super) type FailedKind = COREWEBVIEW2_PROCESS_FAILED_KIND; + + /// What a failure of this kind needs from the app. + #[derive(Debug, Clone, Copy, PartialEq, Eq)] + pub(super) enum FailureResponse { + /// The control is unusable: destroy the window and build a new one. + Rebuild, + /// The control is fine, its page is gone: reload it. + Reload, + /// WebView2 recovers on its own: log and leave it. + LogOnly, + } + + /// Per the `ICoreWebView2ProcessFailedEventArgs` documentation: + /// `BrowserProcessExited` leaves the control "closed and unusable", so + /// it is the one kind that needs a new window. `RenderProcessExited` + /// gets a fresh render process from WebView2 automatically and "the + /// application should reload the page to recover". Everything else — + /// an unresponsive renderer WebView2 may still recover, one iframe's + /// renderer, and the helper processes (GPU, utility, sandbox, plugin) it + /// restarts by itself — would trade a transient glitch for a lost + /// window if acted on. + pub(super) fn response_for(kind: FailedKind) -> FailureResponse { + match kind { + COREWEBVIEW2_PROCESS_FAILED_KIND_BROWSER_PROCESS_EXITED => FailureResponse::Rebuild, + COREWEBVIEW2_PROCESS_FAILED_KIND_RENDER_PROCESS_EXITED => FailureResponse::Reload, + _ => FailureResponse::LogOnly, + } + } + + /// Human-readable name for the log line; unknown values are printed raw + /// so a newer WebView2 runtime can never make the warn less informative. + pub(super) fn describe(kind: FailedKind) -> String { + let name = match kind { + COREWEBVIEW2_PROCESS_FAILED_KIND_BROWSER_PROCESS_EXITED => "BrowserProcessExited", + COREWEBVIEW2_PROCESS_FAILED_KIND_RENDER_PROCESS_EXITED => "RenderProcessExited", + COREWEBVIEW2_PROCESS_FAILED_KIND_RENDER_PROCESS_UNRESPONSIVE => { + "RenderProcessUnresponsive" + } + COREWEBVIEW2_PROCESS_FAILED_KIND_FRAME_RENDER_PROCESS_EXITED => { + "FrameRenderProcessExited" + } + COREWEBVIEW2_PROCESS_FAILED_KIND_UTILITY_PROCESS_EXITED => "UtilityProcessExited", + COREWEBVIEW2_PROCESS_FAILED_KIND_SANDBOX_HELPER_PROCESS_EXITED => { + "SandboxHelperProcessExited" + } + COREWEBVIEW2_PROCESS_FAILED_KIND_GPU_PROCESS_EXITED => "GpuProcessExited", + COREWEBVIEW2_PROCESS_FAILED_KIND_PPAPI_PLUGIN_PROCESS_EXITED => { + "PpapiPluginProcessExited" + } + COREWEBVIEW2_PROCESS_FAILED_KIND_PPAPI_BROKER_PROCESS_EXITED => { + "PpapiBrokerProcessExited" + } + COREWEBVIEW2_PROCESS_FAILED_KIND_UNKNOWN_PROCESS_EXITED => "UnknownProcessExited", + other => return format!("Unknown({})", other.0), + }; + name.to_string() + } + + /// Register `on_failed` for `ProcessFailed` on the webview's core. + /// + /// Must run on the UI thread (`WebviewWindow::with_webview` guarantees + /// that). The handler is invoked by WebView2 on that same thread; it + /// receives only the failure kind so nothing COM-flavoured leaks out. + pub(super) fn subscribe( + webview: &PlatformWebview, + mut on_failed: impl FnMut(FailedKind) + 'static, + ) -> Result<(), String> { + // SAFETY: plain COM calls on interfaces Tauri handed us for this + // webview, made on the thread that owns them. `add_ProcessFailed` + // AddRefs the handler, so dropping our reference afterwards is fine. + unsafe { + let core = webview + .controller() + .CoreWebView2() + .map_err(|error| error.to_string())?; + let handler = ProcessFailedEventHandler::create(Box::new(move |_sender, args| { + let Some(args) = args else { + return Ok(()); + }; + let mut kind = FailedKind::default(); + args.ProcessFailedKind(&mut kind)?; + on_failed(kind); + Ok(()) + })); + let mut token = 0i64; + core.add_ProcessFailed(&handler, &mut token) + .map_err(|error| error.to_string()) + } + } + + #[cfg(test)] + mod tests { + use super::*; + + #[test] + fn only_a_dead_browser_process_means_a_new_window() { + assert_eq!( + response_for(COREWEBVIEW2_PROCESS_FAILED_KIND_BROWSER_PROCESS_EXITED), + FailureResponse::Rebuild + ); + } + + #[test] + fn a_dead_render_process_only_needs_the_page_reloaded() { + // WebView2 spawns the replacement renderer itself; tearing the + // window down would cost the Settings tab and a full rebuild + // for what a reload fixes in place. + assert_eq!( + response_for(COREWEBVIEW2_PROCESS_FAILED_KIND_RENDER_PROCESS_EXITED), + FailureResponse::Reload + ); + } + + #[test] + fn recoverable_and_helper_failures_are_log_only() { + for kind in [ + COREWEBVIEW2_PROCESS_FAILED_KIND_RENDER_PROCESS_UNRESPONSIVE, + COREWEBVIEW2_PROCESS_FAILED_KIND_FRAME_RENDER_PROCESS_EXITED, + COREWEBVIEW2_PROCESS_FAILED_KIND_GPU_PROCESS_EXITED, + COREWEBVIEW2_PROCESS_FAILED_KIND_UTILITY_PROCESS_EXITED, + COREWEBVIEW2_PROCESS_FAILED_KIND_SANDBOX_HELPER_PROCESS_EXITED, + COREWEBVIEW2_PROCESS_FAILED_KIND_PPAPI_PLUGIN_PROCESS_EXITED, + COREWEBVIEW2_PROCESS_FAILED_KIND_PPAPI_BROKER_PROCESS_EXITED, + COREWEBVIEW2_PROCESS_FAILED_KIND_UNKNOWN_PROCESS_EXITED, + ] { + assert_eq!( + response_for(kind), + FailureResponse::LogOnly, + "{}", + describe(kind) + ); + } + } + + #[test] + fn a_kind_this_build_does_not_know_is_never_acted_on() { + // A newer runtime may add kinds; an unknown value must neither + // rebuild nor lose its numeric value in the log. + let future = COREWEBVIEW2_PROCESS_FAILED_KIND(99); + assert_eq!(response_for(future), FailureResponse::LogOnly); + assert_eq!(describe(future), "Unknown(99)"); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn main_is_always_rebuilt_and_replayed_only_when_it_was_showing() { + assert_eq!( + recovery_plan("main", true), + RecoveryPlan::RebuildMain { replay: true } + ); + assert_eq!( + recovery_plan("main", false), + RecoveryPlan::RebuildMain { replay: false } + ); + } + + #[test] + fn on_demand_windows_are_reopened_only_when_they_were_showing() { + assert_eq!( + recovery_plan("settings", true), + RecoveryPlan::RebuildSettings { reopen: true } + ); + assert_eq!( + recovery_plan("settings", false), + RecoveryPlan::RebuildSettings { reopen: false } + ); + assert_eq!( + recovery_plan("flyout", true), + RecoveryPlan::RebuildFlyout { reopen: true } + ); + assert_eq!( + recovery_plan("flyout", false), + RecoveryPlan::RebuildFlyout { reopen: false } + ); + } + + #[test] + fn the_float_bar_is_rebuilt_whether_or_not_it_was_showing() { + // Its lazy guard in `floatbar::window::show` only runs on a show, + // and an enabled bar is never re-shown, so hidden-or-not it is torn + // down and re-applied from settings (a disabled bar stays down). + assert_eq!( + recovery_plan("floatbar", true), + RecoveryPlan::RebuildFloatBar + ); + assert_eq!( + recovery_plan("floatbar", false), + RecoveryPlan::RebuildFloatBar + ); + } + + #[test] + fn windows_without_a_rebuild_recipe_are_left_alone() { + // A typo'd label must never be routed into somebody else's rebuild. + assert_eq!(recovery_plan("Main", true), RecoveryPlan::Unsupported); + assert_eq!(recovery_plan("about", true), RecoveryPlan::Unsupported); + } +} diff --git a/apps/desktop-tauri/src-tauri/src/shell/window.rs b/apps/desktop-tauri/src-tauri/src/shell/window.rs index b82a03dc..5d340222 100644 --- a/apps/desktop-tauri/src-tauri/src/shell/window.rs +++ b/apps/desktop-tauri/src-tauri/src/shell/window.rs @@ -269,9 +269,6 @@ where P: FnOnce(SurfaceMode) -> bool, { let _transition_guard = SHELL_TRANSITION_SERIAL.lock().unwrap(); - let window = app - .get_webview_window("main") - .ok_or_else(|| "main window unavailable".to_string())?; let st = app .try_state::>() .ok_or_else(|| "app state unavailable".to_string())?; @@ -284,6 +281,15 @@ where return Ok(None); }; + // The state now reads Hidden. A dead `main` is rebuilt in the background + // and comes back hidden, which is exactly what this path wanted, so + // there is nothing to replay; an open queued by another caller during + // the rebuild is kept (#410). Probed after the eligibility check so an + // event this path ignores never starts a rebuild on its own. + let Some(window) = super::window_recovery::resolve_live_main(app, None) else { + return Ok(Some(SurfaceMode::Hidden)); + }; + if let Some(transition) = plan.transition { apply_transition(app, &window, &transition, &plan.previous, plan.target, None).map(Some) } else { diff --git a/apps/desktop-tauri/src-tauri/src/shell/window_recovery.rs b/apps/desktop-tauri/src-tauri/src/shell/window_recovery.rs new file mode 100644 index 00000000..772b1e59 --- /dev/null +++ b/apps/desktop-tauri/src-tauri/src/shell/window_recovery.rs @@ -0,0 +1,746 @@ +//! Off-thread window rebuilds (#410). +//! +//! [`crate::webview_recovery`] can tell whether a window still hosts a live +//! WebView2 instance and, for the windows that are opened from an async +//! context, drops a dead one inline so the caller's first-build path runs. +//! `main` cannot be handled that way: `tray_bridge::handle_menu_event` calls +//! `shell::transition_to_target` synchronously on the menu-event thread and +//! `commands::set_surface_mode` is a *sync* Tauri command, so its guard is +//! reached on the main thread, where waiting for the destroyed window's label +//! to be released would block the very event loop that has to process +//! `Destroyed` — a guaranteed stall followed by a failed rebuild. +//! +//! So the `main` guard hands the destroy + rebuild to a background thread and +//! tells its caller to abandon this attempt. The request is kept as the +//! rebuild's pending replay and re-issued on the main thread once the window +//! is healthy again, so the click that hit the dead window still ends up +//! doing what the user asked — just a beat later. Every path that can notice +//! a dead `main` (the transition guards, the hide-to-tray guard and the +//! `ProcessFailed` handler) merges into the same pending replay, so it does +//! not matter which of them wins the race to start the rebuild. +//! +//! The guard itself never asks the event loop for anything: it probes the +//! native handle captured when the window was built. Transitions hold +//! `SHELL_TRANSITION_SERIAL` while they run, some of them from spawned tasks, +//! and a marshalled window getter under that lock can wait on a main thread +//! that is itself waiting for the lock. +//! +//! The same background rebuilds serve [`super::webview_lifecycle`], which +//! learns about a dead webview from WebView2 itself (inside a COM callback +//! on the UI thread, so it is under the same must-not-block rule) rather +//! than from a probe at show time. The `*_after_loss` entry points are its +//! side of the contract, and they exist for every window because the shared +//! browser process takes all of them down at once. + +use std::sync::Mutex; +use std::sync::atomic::{AtomicBool, AtomicIsize, Ordering}; +use std::time::Duration; + +use tauri::{AppHandle, Manager, WebviewWindow}; + +use crate::surface::SurfaceMode; +use crate::surface_target::SurfaceTarget; +use crate::webview_recovery; + +pub(crate) const MAIN_LABEL: &str = "main"; + +/// How long an open may wait for a lifecycle-driven rebuild of the same +/// window to release its label (see `wait_for_flyout_rebuild`). +const REBUILD_WAIT_POLLS: usize = 200; +const REBUILD_WAIT_POLL: Duration = Duration::from_millis(10); + +/// `main`'s native handle, captured right after each build so the liveness +/// probe never has to ask the event loop for it. Zero until the first build +/// is registered, which the probe treats as "alive" (never rebuild on a +/// guess). +static MAIN_HWND: AtomicIsize = AtomicIsize::new(0); + +/// The one `main` rebuild that may be in flight, and what to replay once it +/// lands. A later request replaces an earlier one; a hide (`None`) never +/// clears a pending replay. See `merge_replay`. +struct MainRebuild { + in_flight: bool, + replay: Option, +} + +impl MainRebuild { + /// Queue `request` as the replay and claim the in-flight slot if it is + /// free. Returns whether the caller now owns the slot and must start + /// the rebuild thread. + fn enqueue(&mut self, request: Option) -> bool { + self.replay = merge_replay(self.replay.take(), request); + if self.in_flight { + return false; + } + self.in_flight = true; + true + } + + /// Release the in-flight slot and claim the queued replay in one step. + /// + /// Both must happen under the same lock: a request that lands between + /// them would be merged into a queue nobody reads (`enqueue` sees the + /// slot taken and does not spawn; this thread has already taken its + /// replay) and stay there until the next crash. Done together, it is + /// either claimed here or finds the slot free and starts its own + /// rebuild. + fn finish(&mut self) -> Option { + self.in_flight = false; + self.replay.take() + } +} + +static MAIN_REBUILD: Mutex = Mutex::new(MainRebuild { + in_flight: false, + replay: None, +}); + +/// Set while a rebuild thread is between `destroy()` and a released label, +/// so a burst of `ProcessFailed` events queues one recovery per window. +static SETTINGS_REBUILD_IN_FLIGHT: AtomicBool = AtomicBool::new(false); +static FLYOUT_REBUILD_IN_FLIGHT: AtomicBool = AtomicBool::new(false); +static FLOATBAR_REBUILD_IN_FLIGHT: AtomicBool = AtomicBool::new(false); + +/// An in-flight flag that clears itself when dropped, so a rebuild thread +/// that panics cannot leave its window unrecoverable for the session. +struct InFlight(&'static AtomicBool); + +impl InFlight { + fn claim(flag: &'static AtomicBool) -> Option { + (!flag.swap(true, Ordering::SeqCst)).then_some(Self(flag)) + } +} + +impl Drop for InFlight { + fn drop(&mut self) { + self.0.store(false, Ordering::SeqCst); + } +} + +fn main_rebuild() -> std::sync::MutexGuard<'static, MainRebuild> { + lock_rebuild(&MAIN_REBUILD) +} + +fn lock_rebuild(slot: &Mutex) -> std::sync::MutexGuard<'_, MainRebuild> { + slot.lock().unwrap_or_else(|poisoned| poisoned.into_inner()) +} + +/// The rebuild thread's hold on the `main` in-flight slot. +/// +/// Clears the slot if the thread panics before `finish`, so a panic cannot +/// leave `main` unrecoverable for the session; a pending replay survives +/// the panic and is picked up by the next dispatch. Once `finish` has run +/// the drop is a no-op: `finish` already released the slot under the lock, +/// and a caller queued on that lock may have claimed it in the meantime. +/// Clearing again would wipe that claim and let the next caller start a +/// second rebuild of `main` while the first is still running. +struct Ticket<'a> { + slot: &'a Mutex, + armed: bool, +} + +impl<'a> Ticket<'a> { + fn new(slot: &'a Mutex) -> Self { + Self { slot, armed: true } + } + + /// Release the slot and take the replay together (`MainRebuild::finish`), + /// then disarm so the drop leaves the slot alone. + fn finish(&mut self) -> Option { + self.armed = false; + lock_rebuild(self.slot).finish() + } +} + +impl Drop for Ticket<'_> { + fn drop(&mut self) { + if self.armed { + lock_rebuild(self.slot).in_flight = false; + } + } +} + +/// What a caller should do with a window it is about to show. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum GuardAction { + /// The window is there and its webview is live — carry on. + Proceed, + /// The window needs rebuilding and this caller should start it. + Rebuild, + /// A rebuild is already running; abandon this attempt without starting + /// a second one. + Waiting, +} + +/// Decide what to do about a window, from facts a caller can cheaply gather. +/// +/// Split out from the Win32 probe and the thread spawning so the policy is +/// testable on its own: a missing window is as recoverable as a dead one +/// (Tauri drops a window from its label map once `Destroyed` is processed, +/// so an interrupted earlier recovery leaves exactly that shape), and an +/// in-flight rebuild always wins so concurrent callers cannot stack up +/// destroy/build cycles on the same label. +pub(crate) fn guard_action(exists: bool, alive: bool, rebuild_in_flight: bool) -> GuardAction { + if rebuild_in_flight { + return GuardAction::Waiting; + } + if exists && alive { + return GuardAction::Proceed; + } + GuardAction::Rebuild +} + +/// The replay to keep when `incoming` arrives while `pending` is queued. +/// +/// The newest request is what the user asked for most recently, so it wins; +/// a hide (`None`) has nothing to replay and must not erase a queued open, +/// or a `Focused(false)` that reaches the dying window a few milliseconds +/// before the `ProcessFailed` callback would silently cancel the reopen. +fn merge_replay( + pending: Option, + incoming: Option, +) -> Option { + incoming.or(pending) +} + +/// The transition to replay once `main` has been rebuilt. +/// +/// The rebuilt window is hidden and the surface state is reset to `Hidden` +/// with it (see `rebuild_main`), so a plain `transition_to_target` always +/// resolves as a mode change and shows the window. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct MainRequest { + pub mode: SurfaceMode, + pub target: SurfaceTarget, + pub position: Option<(i32, i32)>, +} + +/// Remember a freshly built `main` window: capture its native handle for +/// the liveness probe and subscribe it to `ProcessFailed`. Call right after +/// `build()`, at startup and on every rebuild. +pub(crate) fn register_main(app: &AppHandle, window: &WebviewWindow) { + MAIN_HWND.store( + webview_recovery::native_hwnd(window).unwrap_or(0), + Ordering::SeqCst, + ); + super::webview_lifecycle::watch(app, window); +} + +/// Whether the registered `main` window still hosts a live webview. Pure +/// Win32 on the captured handle: nothing here waits on the event loop. +fn main_webview_is_alive() -> bool { + match MAIN_HWND.load(Ordering::SeqCst) { + 0 => true, + hwnd => webview_recovery::hwnd_has_webview_child(hwnd), + } +} + +/// `main`'s webview was reported dead by WebView2 itself: rebuild it now, +/// hidden, exactly as at startup. When it was on screen, replay the current +/// surface at `position` so it comes back where the user had it. +pub(crate) fn recover_main_after_loss(app: &AppHandle, replay: bool, position: Option<(i32, i32)>) { + let request = replay + .then(|| { + let state = app.try_state::>()?; + let snapshot = { + let guard = state.lock().unwrap_or_else(|error| error.into_inner()); + super::transition::current_surface_snapshot(&guard) + }; + tracing::debug!( + mode = ?snapshot.mode, + target = ?snapshot.target, + "window_recovery: surface to replay after the main rebuild" + ); + (snapshot.mode != SurfaceMode::Hidden).then_some(MainRequest { + mode: snapshot.mode, + target: snapshot.target, + position, + }) + }) + .flatten(); + dispatch_main_rebuild(app, request); +} + +/// Resolve the `main` window, or start recovering it. +/// +/// `Some(window)` means the window is live and the caller may proceed with +/// its transition. `None` means the caller must abandon this attempt: a +/// rebuild is either already running or has just been dispatched, and +/// `request` (when given) is queued as its replay. +pub(crate) fn resolve_live_main( + app: &AppHandle, + request: Option, +) -> Option { + let window = app.get_webview_window(MAIN_LABEL); + let alive = window.is_some() && main_webview_is_alive(); + let in_flight = main_rebuild().in_flight; + + match guard_action(window.is_some(), alive, in_flight) { + GuardAction::Proceed => return window, + GuardAction::Waiting => { + tracing::debug!( + queued = request.is_some(), + "window_recovery: main rebuild already in flight; deferring this open" + ); + } + GuardAction::Rebuild => { + tracing::warn!( + label = MAIN_LABEL, + "window_recovery: main window is not usable; rebuilding it (#410)" + ); + } + } + dispatch_main_rebuild(app, request); + None +} + +/// Queue `request` as the pending replay and, unless one is already +/// running, destroy and rebuild `main` on a background thread. +/// +/// Spawns rather than blocking because the caller may be the main thread; +/// `std::thread::spawn` (not the async runtime) matches the existing +/// precedent in `transition.rs`'s startup reveal fallback and keeps the +/// blocking label wait off a tokio worker. +fn dispatch_main_rebuild(app: &AppHandle, request: Option) { + if !main_rebuild().enqueue(request) { + return; + } + + let app = app.clone(); + let _ = std::thread::spawn(move || { + let mut ticket = Ticket::new(&MAIN_REBUILD); + let result = rebuild_main(&app); + let replay = ticket.finish(); + + match result { + Ok(()) => { + tracing::info!(label = MAIN_LABEL, "window_recovery: main window rebuilt"); + if let Some(request) = replay { + replay_on_main_thread(&app, request); + } + } + Err(error) => { + tracing::error!( + %error, + label = MAIN_LABEL, + replay_dropped = replay.is_some(), + "window_recovery: could not rebuild the main window" + ); + } + } + }); +} + +/// Replay `request` against the rebuilt `main` — on the main thread, never +/// on the rebuild thread. +/// +/// A transition holds `SHELL_TRANSITION_SERIAL` while it calls window +/// getters, and the main thread's window-event handler takes that same lock +/// (`hide_to_tray_if_current` on `Focused(false)`). Run from a background +/// thread, the replay can therefore deadlock: it holds the lock and waits +/// for the main thread to answer a getter, while the main thread sits in an +/// event handler waiting for the lock. That is not hypothetical — a freshly +/// built window receives a `Focused(true)`/`Focused(false)` pair right after +/// `build()`, so the race was hit on every rebuild of a visible `main` +/// (observed 2026-09-11). On the main thread every window call takes the +/// runtime's direct path and the event handler cannot run concurrently, which +/// is exactly why the tray and menu paths run their transitions there. +/// +/// The window is healthy by now, so this pass takes the normal path — +/// `resolve_live_main` returns it and no further rebuild is dispatched. +fn replay_on_main_thread(app: &AppHandle, request: MainRequest) { + let handle = app.clone(); + let dispatched = app.run_on_main_thread(move || { + if let Err(error) = + super::transition_to_target(&handle, request.mode, request.target, request.position) + { + tracing::warn!( + %error, + "window_recovery: replaying the transition after rebuild failed" + ); + } + }); + if let Err(error) = dispatched { + tracing::warn!( + %error, + "window_recovery: could not dispatch the post-rebuild replay to the main thread" + ); + } +} + +/// Rebuild `main` from the same `tauri.conf.json` entry the first build uses. +/// +/// `main` is declared in the config rather than built in code, so the only +/// faithful recipe is `WebviewWindowBuilder::from_config` over that entry — +/// which also means the rebuilt window inherits `"visible": false` and the +/// rest of its declared properties, exactly as at startup. What `setup` does +/// to it afterwards (register it, `force_dark_caption`, `hide()`) is repeated +/// here so a recovered window is indistinguishable from a freshly-launched +/// one, and in particular can never flash a frame of its own before the +/// surface machinery decides to show it. +/// +/// The surface state is reset to `Hidden` to match: it still reads the mode +/// the dead window was showing, and a transition back to that mode would +/// otherwise resolve as a no-op and leave the rebuilt window hidden forever. +fn rebuild_main(app: &AppHandle) -> Result<(), String> { + if let Some(window) = app.get_webview_window(MAIN_LABEL) { + webview_recovery::destroy_and_release(app, &window)?; + } + + let config = app + .config() + .app + .windows + .first() + .ok_or_else(|| "no window is declared in tauri.conf.json".to_string())?; + + let window = tauri::WebviewWindowBuilder::from_config(app, config) + .map_err(|error| error.to_string())? + .build() + .map_err(|error| error.to_string())?; + register_main(app, &window); + + super::dwm::force_dark_caption(&window); + window.hide().map_err(|error| error.to_string())?; + super::transition::commit_surface_snapshot(app, &super::transition::hidden_surface_snapshot()) +} + +/// `settings` lost its webview: tear it down now. When it was on screen, +/// reopen it on the default tab — the tab it was showing died with the +/// webview, and the default is where the user lands from the tray too. +/// +/// `settings_window::open_or_focus` may run on the main thread (tray menu), +/// so unlike the flyout it cannot wait for this teardown; an open that lands +/// in the few hundred milliseconds between `destroy()` and the released +/// label is handled by `webview_recovery::reclaim_dead_window` as best it +/// can. +pub(crate) fn recover_settings_after_loss(app: &AppHandle, reopen: bool) { + let Some(ticket) = InFlight::claim(&SETTINGS_REBUILD_IN_FLIGHT) else { + return; + }; + let tab = reopen.then( + || match SurfaceTarget::default_for_mode(SurfaceMode::Settings) { + SurfaceTarget::Settings { tab } => tab, + _ => "general".to_string(), + }, + ); + let app = app.clone(); + let _ = std::thread::spawn(move || { + let destroyed = match app.get_webview_window(super::settings_window::SETTINGS_LABEL) { + Some(window) => webview_recovery::destroy_and_release(&app, &window), + None => Ok(()), + }; + drop(ticket); + + let tab = match (destroyed, tab) { + (Err(error), _) => { + tracing::error!( + %error, + "window_recovery: could not release the settings window for rebuild" + ); + return; + } + (Ok(()), None) => { + tracing::info!( + label = super::settings_window::SETTINGS_LABEL, + "window_recovery: settings window torn down; it will be rebuilt on next open" + ); + return; + } + (Ok(()), Some(tab)) => tab, + }; + // Re-entering `open_or_focus` rather than duplicating its builder + // keeps the window's size/geometry/theme recipe in one place; with + // the label released it takes the first-build branch, which is + // precisely "the same path the first build uses". + match super::settings_window::open_or_focus(&app, &tab) { + Ok(()) => tracing::info!( + label = super::settings_window::SETTINGS_LABEL, + "window_recovery: settings window rebuilt" + ), + Err(error) => tracing::warn!( + %error, + "window_recovery: reopening Settings after rebuild failed" + ), + } + }); +} + +/// Block, bounded, while a lifecycle-driven flyout teardown is between +/// `destroy()` and the released label, so an open that races it neither +/// shows the dying window nor builds a second one under the same label. +/// +/// Like `flyout_window::open_or_focus` itself, this must only run from an +/// async context: on the main thread it would wait on the very event loop +/// the teardown needs. +pub(crate) fn wait_for_flyout_rebuild() { + for _ in 0..REBUILD_WAIT_POLLS { + if !FLYOUT_REBUILD_IN_FLIGHT.load(Ordering::SeqCst) { + return; + } + std::thread::sleep(REBUILD_WAIT_POLL); + } + tracing::warn!( + "window_recovery: flyout rebuild still in flight after the wait; opening anyway" + ); +} + +/// The flyout lost its webview: tear it down now and, when it was on +/// screen, reopen it. +/// +/// `flyout_window::open_or_focus` is the first-build path, so the reopened +/// window is built `visible(false)` and revealed by the frontend after its +/// first layout pass — the same handshake as any other open, which is what +/// keeps the recovery from flashing a blank frame. It re-anchors above the +/// tray on its own, so no position is carried over. `open_or_focus` may +/// block on `build()`, hence the background thread. +pub(crate) fn recover_flyout_after_loss(app: &AppHandle, reopen: bool) { + let Some(ticket) = InFlight::claim(&FLYOUT_REBUILD_IN_FLIGHT) else { + return; + }; + let app = app.clone(); + let _ = std::thread::spawn(move || { + let destroyed = match app.get_webview_window(super::flyout_window::FLYOUT_LABEL) { + Some(window) => webview_recovery::destroy_and_release(&app, &window), + None => Ok(()), + }; + // Released before `open_or_focus`, which waits on this very flag. + drop(ticket); + + match destroyed { + Err(error) => tracing::error!( + %error, + "window_recovery: could not release the flyout window for rebuild" + ), + Ok(()) if !reopen => tracing::info!( + label = super::flyout_window::FLYOUT_LABEL, + "window_recovery: flyout torn down; it will be rebuilt on next open" + ), + Ok(()) => match super::flyout_window::open_or_focus(&app, None) { + Ok(()) => tracing::info!( + label = super::flyout_window::FLYOUT_LABEL, + "window_recovery: flyout rebuilt" + ), + Err(error) => tracing::warn!( + %error, + "window_recovery: reopening the flyout after rebuild failed" + ), + }, + } + }); +} + +/// The float bar lost its webview: tear it down and let +/// `floatbar::apply_state` build it again from the persisted settings. +/// +/// The bar has no "next open" to fall back on — it is on screen for as long +/// as it is enabled, and neither its z-order guard (which only rebuilds a +/// *missing* window) nor a settings save would touch a frame that is still +/// there — so the dead frame would stay blank until the user toggled the +/// feature. `apply_state` is the same recipe startup uses, and a disabled +/// bar simply stays torn down. +pub(crate) fn recover_floatbar_after_loss(app: &AppHandle) { + let Some(ticket) = InFlight::claim(&FLOATBAR_REBUILD_IN_FLIGHT) else { + return; + }; + let app = app.clone(); + let _ = std::thread::spawn(move || { + let _ticket = ticket; + let destroyed = match app.get_webview_window(crate::floatbar::FLOATBAR_LABEL) { + Some(window) => webview_recovery::destroy_and_release(&app, &window), + None => Ok(()), + }; + if let Err(error) = destroyed { + tracing::error!( + %error, + "window_recovery: could not release the float bar window for rebuild" + ); + return; + } + let settings = codexbar::settings::Settings::load(); + match crate::floatbar::apply_state(&app, &settings) { + Ok(()) => tracing::info!( + label = crate::floatbar::FLOATBAR_LABEL, + enabled = settings.float_bar_enabled, + "window_recovery: float bar torn down and re-applied from settings" + ), + Err(error) => tracing::warn!( + %error, + "window_recovery: re-applying the float bar after rebuild failed" + ), + } + }); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_live_window_is_shown_as_is() { + assert_eq!(guard_action(true, true, false), GuardAction::Proceed); + } + + #[test] + fn a_dead_window_is_rebuilt() { + assert_eq!(guard_action(true, false, false), GuardAction::Rebuild); + } + + #[test] + fn a_missing_window_is_rebuilt_too() { + // An earlier recovery that was interrupted after `destroy()` leaves + // no window under the label at all; that is recoverable, not fatal. + assert_eq!(guard_action(false, false, false), GuardAction::Rebuild); + } + + #[test] + fn an_in_flight_rebuild_wins_over_every_other_shape() { + // Concurrent tray clicks must not stack destroy/build cycles on one + // label — including the case where the half-rebuilt window already + // probes as alive. + assert_eq!(guard_action(true, false, true), GuardAction::Waiting); + assert_eq!(guard_action(false, false, true), GuardAction::Waiting); + assert_eq!(guard_action(true, true, true), GuardAction::Waiting); + } + + fn open(mode: SurfaceMode) -> MainRequest { + MainRequest { + mode, + target: SurfaceTarget::default_for_mode(mode), + position: None, + } + } + + #[test] + fn the_newest_open_request_is_the_one_replayed() { + let pending = open(SurfaceMode::PopOut); + let incoming = open(SurfaceMode::TrayPanel); + assert_eq!( + merge_replay(Some(pending), Some(incoming.clone())), + Some(incoming) + ); + } + + #[test] + fn a_hide_never_cancels_a_queued_open() { + // A `Focused(false)` reaching the dying window just before the + // ProcessFailed callback (or a tray hide during the rebuild) has + // nothing to replay; it must not erase the reopen already queued. + let pending = open(SurfaceMode::PopOut); + assert_eq!(merge_replay(Some(pending.clone()), None), Some(pending)); + assert_eq!(merge_replay(None, None), None); + } + + #[test] + fn only_the_first_enqueue_owns_the_rebuild() { + let mut rebuild = MainRebuild { + in_flight: false, + replay: None, + }; + assert!(rebuild.enqueue(Some(open(SurfaceMode::PopOut)))); + assert!(!rebuild.enqueue(Some(open(SurfaceMode::TrayPanel)))); + assert!(rebuild.in_flight); + assert_eq!(rebuild.replay, Some(open(SurfaceMode::TrayPanel))); + } + + #[test] + fn finishing_releases_the_slot_and_claims_the_replay_together() { + let mut rebuild = MainRebuild { + in_flight: true, + replay: Some(open(SurfaceMode::PopOut)), + }; + assert_eq!(rebuild.finish(), Some(open(SurfaceMode::PopOut))); + assert!(!rebuild.in_flight); + assert_eq!(rebuild.replay, None); + } + + #[test] + fn a_finished_ticket_leaves_a_later_claim_alone() { + // `finish` releases the slot; a caller queued on the lock claims it + // right after; the ticket then goes out of scope. That drop used to + // clear the slot unconditionally, wiping the new claim, so the + // caller after that would start a second rebuild of `main`. + let slot = Mutex::new(MainRebuild { + in_flight: false, + replay: None, + }); + assert!(lock_rebuild(&slot).enqueue(Some(open(SurfaceMode::PopOut)))); + let mut ticket = Ticket::new(&slot); + + assert_eq!(ticket.finish(), Some(open(SurfaceMode::PopOut))); + assert!(lock_rebuild(&slot).enqueue(Some(open(SurfaceMode::TrayPanel)))); + drop(ticket); + + let state = lock_rebuild(&slot); + assert!(state.in_flight, "the later claim must survive the ticket"); + assert_eq!(state.replay, Some(open(SurfaceMode::TrayPanel))); + } + + #[test] + fn a_ticket_dropped_before_finish_releases_the_slot() { + // The panic path: the thread unwinds before `finish`, the slot is + // freed, and the replay stays queued for the next dispatch. + let slot = Mutex::new(MainRebuild { + in_flight: false, + replay: None, + }); + assert!(lock_rebuild(&slot).enqueue(Some(open(SurfaceMode::PopOut)))); + drop(Ticket::new(&slot)); + + let state = lock_rebuild(&slot); + assert!(!state.in_flight); + assert_eq!(state.replay, Some(open(SurfaceMode::PopOut))); + } + + #[test] + fn a_request_racing_the_completion_is_never_stranded() { + // The lost-request shape is `in_flight == false` with a replay + // still queued: the worker took its replay, the request was merged + // behind it, and the worker then released the slot without looking + // again. Race `enqueue` against `finish` on a shared state and + // require that every interleaving hands the request to exactly one + // side — either the finishing worker replays it or the enqueuer is + // told to start a rebuild of its own. + use std::sync::{Arc, Barrier}; + + for _ in 0..500 { + let rebuild = Arc::new(Mutex::new(MainRebuild { + in_flight: true, + replay: None, + })); + let gate = Arc::new(Barrier::new(2)); + + let worker = { + let rebuild = Arc::clone(&rebuild); + let gate = Arc::clone(&gate); + std::thread::spawn(move || { + gate.wait(); + rebuild.lock().unwrap().finish() + }) + }; + let caller = { + let rebuild = Arc::clone(&rebuild); + let gate = Arc::clone(&gate); + std::thread::spawn(move || { + gate.wait(); + rebuild + .lock() + .unwrap() + .enqueue(Some(open(SurfaceMode::TrayPanel))) + }) + }; + + let replayed = worker.join().unwrap().is_some(); + let spawned = caller.join().unwrap(); + assert!( + replayed ^ spawned, + "the request must be claimed by exactly one side (replayed={replayed}, spawned={spawned})" + ); + let state = rebuild.lock().unwrap(); + assert!( + state.in_flight || state.replay.is_none(), + "a replay must never be queued with no rebuild in flight" + ); + } + } +} diff --git a/apps/desktop-tauri/src-tauri/src/webview_recovery.rs b/apps/desktop-tauri/src-tauri/src/webview_recovery.rs index f1f656e7..91c53c81 100644 --- a/apps/desktop-tauri/src-tauri/src/webview_recovery.rs +++ b/apps/desktop-tauri/src-tauri/src/webview_recovery.rs @@ -12,40 +12,95 @@ //! Detection walks the window's child HWNDs: a live webview always hosts a //! `Chrome_*` render host, and a dead one has none. //! -//! The `main` window is deliberately not handled here yet: its open path runs +//! The `main` window cannot use [`reclaim_dead_window`]: its open path runs //! from synchronous Tauri commands, where building a window deadlocks on -//! Windows, so rebuilding it needs an async path first. +//! Windows. `shell::window_recovery` rebuilds it on a background thread +//! instead, using [`is_webview_alive`] and [`destroy_and_release`] from here; +//! `shell::webview_lifecycle` drives the same rebuilds from WebView2's own +//! `ProcessFailed` event so a window that is on screen when its browser +//! process dies does not have to wait for the next open. + +use std::time::Duration; use tauri::{Manager, WebviewWindow}; +/// How many times `destroy_and_release` re-checks the label before giving up +/// on the rebuild, and the pause between checks. Counted in polls rather than +/// wall-clock time on purpose: each check may block on the event loop, and +/// while another window is being built on the main thread (a `ProcessFailed` +/// burst rebuilds several) that can be longer than any sane timeout. Time +/// spent blocked is not time the label was overdue. +const LABEL_RELEASE_POLLS: usize = 200; +const LABEL_RELEASE_POLL: Duration = Duration::from_millis(10); + /// Class-name prefixes WebView2 gives the windows it creates under a Tauri /// window. The render host paints the actual content. fn class_is_webview(class_name: &str) -> bool { class_name.starts_with("Chrome_") } -/// Whether `window` still has live WebView2 content. +/// The window's Win32 handle, or `None` when it no longer exposes one. A +/// destroyed window keeps its label until the event loop processes +/// `Destroyed`, but its native handle is already gone by then. /// -/// Returns `true` when the check cannot be performed, so an unreadable window -/// handle never causes a spurious rebuild. +/// Off the main thread this is a marshalled getter that waits on the event +/// loop; callers holding a lock the main thread may want must not use it. #[cfg(windows)] -pub fn is_webview_alive(window: &WebviewWindow) -> bool { +pub(crate) fn native_hwnd(window: &WebviewWindow) -> Option { use raw_window_handle::HasWindowHandle; - let Ok(handle) = window.window_handle() else { - return true; - }; - let raw_window_handle::RawWindowHandle::Win32(handle) = handle.as_raw() else { - return true; - }; - win32::has_webview_child(handle.hwnd.get()) + let handle = window.window_handle().ok()?; + match handle.as_raw() { + raw_window_handle::RawWindowHandle::Win32(handle) => Some(handle.hwnd.get()), + _ => None, + } +} + +#[cfg(not(windows))] +pub(crate) fn native_hwnd(_window: &WebviewWindow) -> Option { + None +} + +/// Whether the window behind `hwnd` still has live WebView2 content. Pure +/// Win32; safe from any thread and under any lock. A handle that is no +/// longer a window has no children and reads as dead. +#[cfg(windows)] +pub(crate) fn hwnd_has_webview_child(hwnd: isize) -> bool { + win32::has_webview_child(hwnd) } #[cfg(not(windows))] -pub fn is_webview_alive(_window: &WebviewWindow) -> bool { +pub(crate) fn hwnd_has_webview_child(_hwnd: isize) -> bool { true } +/// Whether `window` still has live WebView2 content. +/// +/// Returns `true` when the check cannot be performed, so an unreadable window +/// handle never causes a spurious rebuild. +pub fn is_webview_alive(window: &WebviewWindow) -> bool { + match native_hwnd(window) { + Some(hwnd) => hwnd_has_webview_child(hwnd), + None => true, + } +} + +/// Whether the label a destroyed window held (native handle `destroyed`) has +/// been given up, judged from what currently sits under it: `None` when no +/// window does, `Some(handle)` for the window that does. +/// +/// Nothing under the label means it is free. A window with a *different* +/// handle means someone else already rebuilt it, so the label was free in +/// between and there is nothing left to wait for. The same handle, or a +/// window with no handle at all, is still the destroyed one on its way out. +fn label_released(destroyed: Option, current: Option>) -> bool { + match current { + None => true, + Some(Some(hwnd)) => Some(hwnd) != destroyed, + Some(None) => false, + } +} + /// Drop `label`'s window when its webview is dead, so the caller's normal /// "window is missing -> build it" path runs. /// @@ -70,6 +125,47 @@ pub fn reclaim_dead_window(app: &tauri::AppHandle, label: &str) -> Result Result<(), String> { + let label = window.label().to_string(); + let destroyed = native_hwnd(window); + if let Err(error) = window.destroy() { + tracing::warn!( + %error, + label, + "destroy() failed; waiting for the label to be released anyway" + ); + } + + for _ in 0..LABEL_RELEASE_POLLS { + let current = app + .get_webview_window(&label) + .map(|window| native_hwnd(&window)); + if label_released(destroyed, current) { + return Ok(()); + } + std::thread::sleep(LABEL_RELEASE_POLL); + } + Err(format!( + "window `{label}` did not release its label after destroy" + )) +} + #[cfg(windows)] mod win32 { #[link(name = "user32")] @@ -114,7 +210,32 @@ mod win32 { #[cfg(test)] mod tests { - use super::class_is_webview; + use super::{class_is_webview, label_released}; + + #[test] + fn a_label_with_nothing_under_it_is_released() { + assert!(label_released(Some(0x10), None)); + assert!(label_released(None, None)); + } + + #[test] + fn the_destroyed_window_still_under_its_label_is_not_released() { + // Same handle: Tauri has not processed `Destroyed` yet. + assert!(!label_released(Some(0x10), Some(Some(0x10)))); + // No handle at all: half-destroyed, label still registered. + assert!(!label_released(Some(0x10), Some(None))); + assert!(!label_released(None, Some(None))); + } + + #[test] + fn a_different_window_under_the_label_means_it_was_released_meanwhile() { + // Observed live: a widget click rebuilt the flyout while the + // ProcessFailed teardown was still waiting, and the wait timed out + // against the *new* window. That rebuild is the desired outcome, + // not a failure. + assert!(label_released(Some(0x10), Some(Some(0x20)))); + assert!(label_released(None, Some(Some(0x20)))); + } #[test] fn webview_classes_are_recognized_as_live_content() {