From fc4b97abc2d64f220209814b8b44726ecc002f4d Mon Sep 17 00:00:00 2001 From: oxoxDev <164490987+oxoxDev@users.noreply.github.com> Date: Sat, 25 Apr 2026 04:16:00 +0530 Subject: [PATCH] feat(accounts): loading overlay for first-time webview opens (#867) (#887) Co-authored-by: Claude Opus 4.7 (1M context) Co-authored-by: Steven Enamakel --- .../permissions/allow-core-process.toml | 2 + app/src-tauri/src/cdp/mod.rs | 4 +- app/src-tauri/src/cdp/session.rs | 96 ++++- app/src-tauri/src/lib.rs | 1 + app/src-tauri/src/webview_accounts/mod.rs | 392 +++++++++++++++++- app/src/components/accounts/WebviewHost.tsx | 33 +- ...webviewAccountService.loadListener.test.ts | 173 ++++++++ app/src/services/webviewAccountService.ts | 111 ++++- app/src/types/accounts.ts | 9 +- 9 files changed, 785 insertions(+), 36 deletions(-) create mode 100644 app/src/services/__tests__/webviewAccountService.loadListener.test.ts diff --git a/app/src-tauri/permissions/allow-core-process.toml b/app/src-tauri/permissions/allow-core-process.toml index 534d53343..bfcecfb4d 100644 --- a/app/src-tauri/permissions/allow-core-process.toml +++ b/app/src-tauri/permissions/allow-core-process.toml @@ -15,7 +15,9 @@ allow = [ "unregister_dictation_hotkey", "webview_account_open", "webview_account_close", + "webview_account_purge", "webview_account_bounds", + "webview_account_reveal", "webview_account_hide", "webview_account_show", "webview_recipe_event", diff --git a/app/src-tauri/src/cdp/mod.rs b/app/src-tauri/src/cdp/mod.rs index 0272337cb..31c77b5fb 100644 --- a/app/src-tauri/src/cdp/mod.rs +++ b/app/src-tauri/src/cdp/mod.rs @@ -16,7 +16,9 @@ pub mod target; pub use conn::CdpConn; pub use emulation::{set_user_agent_override, UaSpec}; -pub use session::{placeholder_marker, placeholder_url, spawn_session, target_url_fragment}; +pub use session::{ + placeholder_marker, placeholder_url, spawn_session, target_url_fragment, SpawnedSession, +}; pub use snapshot::Snapshot; pub use target::{ browser_ws_url, connect_and_attach_matching, detach_session, find_page_target_where, diff --git a/app/src-tauri/src/cdp/session.rs b/app/src-tauri/src/cdp/session.rs index e47b16831..3109e73e3 100644 --- a/app/src-tauri/src/cdp/session.rs +++ b/app/src-tauri/src/cdp/session.rs @@ -17,16 +17,23 @@ use std::time::Duration; use serde_json::json; +use tauri::{AppHandle, Runtime}; use tokio::task::JoinHandle; use tokio::time::sleep; use super::{browser_ws_url, find_page_target_where, set_user_agent_override, CdpConn, UaSpec}; +use crate::webview_accounts::emit_load_finished; /// Backoff between failed attach attempts / reconnects. Intentionally /// short — once the webview is open, the target usually shows up within /// 500ms. const ATTACH_BACKOFF: Duration = Duration::from_secs(2); +/// Watchdog budget before we synthesise a `webview-account:load` event with +/// `state: "timeout"` so the frontend never holds its loading spinner open on +/// a flaky network. Matches the timeout documented in issue #867. +const LOAD_TIMEOUT: Duration = Duration::from_secs(15); + /// Returns the unique marker substring that the account's initial /// placeholder URL contains so `Target.getTargets` can identify it. pub fn placeholder_marker(account_id: &str) -> String { @@ -56,21 +63,53 @@ fn target_matches_account_url(target_url: &str, account_id: &str) -> bool { target_url.ends_with(&marker_fragment) || target_url.ends_with(&fragment) } -/// Spawn the per-account CDP session. Returns immediately; the background -/// task keeps the session alive and retries on disconnect. Idempotent at -/// the call site — the caller is expected to only call this once per -/// `webview_account_open`. -/// -/// **Shutdown**: returns the `JoinHandle` for the spawned loop so the -/// caller (`webview_account_close` / `webview_account_purge`) can -/// `abort()` it when the account goes away. Without abort the loop -/// would keep retrying `attach_to_target` against a vanished target -/// forever and accumulate across reopen cycles. -pub fn spawn_session(account_id: String, real_url: String) -> JoinHandle<()> { - tokio::spawn(async move { run_session_forever(account_id, real_url).await }) +/// Per-account spawn result. Both handles are owned by `WebviewAccountsState` +/// (see `cdp_sessions` and `load_watchdogs`) so close/purge can abort each one +/// without leaking tasks across reopen cycles. +pub struct SpawnedSession { + pub session: JoinHandle<()>, + pub watchdog: JoinHandle<()>, } -async fn run_session_forever(account_id: String, real_url: String) { +/// Spawn the per-account CDP session. Returns immediately; the background +/// task keeps the session alive and retries on disconnect. Also spawns a +/// 15 s watchdog task that fires a `webview-account:load{state:"timeout"}` +/// event if neither the native `on_page_load` nor CDP `Page.loadEventFired` +/// signals arrive in time. +/// +/// Both `JoinHandle`s inside the returned [`SpawnedSession`] must be stored +/// by the caller and aborted on account close/purge to prevent task leaks +/// across reopen cycles. +pub fn spawn_session( + app: AppHandle, + account_id: String, + real_url: String, +) -> SpawnedSession { + // Load-overlay watchdog — independent of the session loop. Emits a + // `timeout` signal after LOAD_TIMEOUT so the frontend's loading spinner + // is always released even if neither the native `on_page_load` nor the + // CDP `Page.loadEventFired` signal arrives (flaky network, provider + // blocking, CDP socket hiccup). + // + // `emit_load_finished` dedups via `WebviewAccountsState.loaded_accounts` + // so a late watchdog is a no-op once either signal has fired. The + // returned `JoinHandle` is stored in `WebviewAccountsState.load_watchdogs` + // and aborted on close/purge so a watchdog spawned for a vanished + // account can't fire a stale timeout against a freshly-reused id. + let watchdog = { + let app = app.clone(); + let account_id = account_id.clone(); + let real_url = real_url.clone(); + tokio::spawn(async move { + sleep(LOAD_TIMEOUT).await; + emit_load_finished(&app, &account_id, "timeout", &real_url); + }) + }; + let session = tokio::spawn(async move { run_session_forever(app, account_id, real_url).await }); + SpawnedSession { session, watchdog } +} + +async fn run_session_forever(app: AppHandle, account_id: String, real_url: String) { log::info!( "[cdp-session][{}] up real_url={} marker={}", account_id, @@ -81,7 +120,7 @@ async fn run_session_forever(account_id: String, real_url: String) { // `/json/version`. The placeholder URL is trivial so this is quick. sleep(Duration::from_millis(500)).await; loop { - match run_session_cycle(&account_id, &real_url).await { + match run_session_cycle(&app, &account_id, &real_url).await { Ok(()) => { log::info!( "[cdp-session][{}] session ended cleanly, reconnecting", @@ -96,7 +135,11 @@ async fn run_session_forever(account_id: String, real_url: String) { } } -async fn run_session_cycle(account_id: &str, real_url: &str) -> Result<(), String> { +async fn run_session_cycle( + app: &AppHandle, + account_id: &str, + real_url: &str, +) -> Result<(), String> { let browser_ws = browser_ws_url().await?; let mut cdp = CdpConn::open(&browser_ws).await?; @@ -200,6 +243,12 @@ async fn run_session_cycle(account_id: &str, real_url: &str) -> Result<(), Strin account_id ); + // Enable the Page domain so `Page.loadEventFired` reaches our + // `pump_events` callback below. Must happen BEFORE `Page.navigate` so + // the first top-level load event for the real provider URL isn't missed. + cdp.call("Page.enable", json!({}), Some(&session_id)) + .await?; + // Drive the webview from the placeholder to the real provider URL. // Fragment survives same-origin navigations so scanners can match on // it indefinitely. Skip navigation if the target is already on the @@ -226,10 +275,19 @@ async fn run_session_cycle(account_id: &str, real_url: &str) -> Result<(), Strin // override reverts when we detach, so we intentionally block here. // pump_events returns when the CDP ws closes (browser process exits // or `Target.detachFromTarget` is called from elsewhere). - cdp.pump_events(&session_id, |_method, _params| { - // We don't subscribe to any domain here — the session exists - // purely to keep the UA override resident. Per-provider scanners - // attach their own sessions for Network / IndexedDB / DOMSnapshot. + // + // The callback emits `webview-account:load{state:"finished"}` on the + // first `Page.loadEventFired` as a belt-and-braces fallback to the + // native `WebviewBuilder::on_page_load` handler wired in + // `webview_account_open`. `emit_load_finished` dedups across both paths + // so the frontend only sees one signal per cold open. + let cb_app = app.clone(); + let cb_account_id = account_id.to_string(); + let cb_real_url = real_url.to_string(); + cdp.pump_events(&session_id, move |method, _params| { + if method == "Page.loadEventFired" { + emit_load_finished(&cb_app, &cb_account_id, "finished", &cb_real_url); + } }) .await } diff --git a/app/src-tauri/src/lib.rs b/app/src-tauri/src/lib.rs index f67bb6a31..11c417b8f 100644 --- a/app/src-tauri/src/lib.rs +++ b/app/src-tauri/src/lib.rs @@ -1049,6 +1049,7 @@ pub fn run() { webview_accounts::webview_account_close, webview_accounts::webview_account_purge, webview_accounts::webview_account_bounds, + webview_accounts::webview_account_reveal, webview_accounts::webview_account_hide, webview_accounts::webview_account_show, webview_accounts::webview_recipe_event, diff --git a/app/src-tauri/src/webview_accounts/mod.rs b/app/src-tauri/src/webview_accounts/mod.rs index 516bcf856..cf4c7da71 100644 --- a/app/src-tauri/src/webview_accounts/mod.rs +++ b/app/src-tauri/src/webview_accounts/mod.rs @@ -340,6 +340,21 @@ pub struct WebviewAccountsState { /// close/purge so reopen cycles don't stack multiple live loops. #[cfg(feature = "cef")] cdp_sessions: Mutex>>, + /// account_id -> 15s `webview-account:load{state:"timeout"}` watchdog. + /// Aborted in close/purge so a watchdog spawned for a now-closed + /// account can't fire a stale timeout against a freshly-reused id. + #[cfg(feature = "cef")] + load_watchdogs: Mutex>>, + /// account_id of webviews that have already emitted their first + /// `webview-account:load{state:"finished"}` event. Used to dedup + /// triple-signal fires (native on_page_load, CDP `Page.loadEventFired`, + /// 15 s watchdog) so the frontend only reveals once per cold open. + loaded_accounts: Mutex>, + /// Last bounds requested by the frontend for a given account, captured at + /// `webview_account_open` time so the off-screen-spawned webview can be + /// revealed at the right rect without the frontend having to round-trip + /// them again. + requested_bounds: Mutex>, /// Runtime notification-bypass controls used by the settings UI. notification_bypass: Mutex, } @@ -773,6 +788,150 @@ pub struct WebviewEvent { pub ts: Option, } +/// Strip query string and fragment from a URL before emitting to the log. +/// Provider URLs occasionally embed auth material (Telegram WebApp data, +/// OAuth callback codes, sometimes session tokens) and we don't want those +/// to land in the long-lived shell log file. Returns the original input on +/// parse failure so we still surface *something* useful for debugging. +fn redact_url_for_log(raw: &str) -> String { + match Url::parse(raw) { + Ok(mut u) => { + u.set_query(None); + u.set_fragment(None); + u.to_string() + } + Err(_) => { + // Fallback: drop everything from the first '?' or '#'. + raw.split(['?', '#']).next().unwrap_or(raw).to_string() + } + } +} + +/// Grow the first-cold-open webview back to its full requested bounds and +/// notify the frontend — exactly once per account open. Called from the +/// three independent signals (native `WebviewBuilder::on_page_load`, CDP +/// `Page.loadEventFired`, 15 s watchdog) — the first one wins and the rest +/// short-circuit via `WebviewAccountsState.loaded_accounts`. Resetting +/// happens in `webview_account_close` / `webview_account_purge` so a reopen +/// fires again. +/// +/// Doing the `set_size` server-side (instead of waiting for the frontend to +/// invoke `webview_account_reveal`) avoids an extra IPC round-trip and the +/// brief blank frame that would otherwise sit between the load event and +/// the frontend's reveal call. +pub(crate) fn emit_load_finished( + app: &AppHandle, + account_id: &str, + state: &str, + url: &str, +) { + let Some(app_state) = app.try_state::() else { + // No state => emit anyway so the frontend doesn't hang; best-effort. + log::warn!( + "[webview-accounts][{}] WebviewAccountsState missing — emitting without reveal", + account_id + ); + let _ = app.emit( + "webview-account:load", + serde_json::json!({"account_id": account_id, "state": state, "url": url}), + ); + return; + }; + + let is_first = app_state + .loaded_accounts + .lock() + .unwrap() + .insert(account_id.to_string()); + if !is_first { + log::debug!( + "[webview-accounts][{}] load event deduped state={} url={}", + account_id, + state, + url + ); + return; + } + + // Restore the webview to its full requested size. The spawn path created + // it at 1×1 so the React loading spinner wasn't covered; now that the page + // is painted we can grow it into the placeholder rect. + let label = app_state.inner.lock().unwrap().get(account_id).cloned(); + let bounds = app_state + .requested_bounds + .lock() + .unwrap() + .get(account_id) + .copied(); + match (label, bounds) { + (Some(label), Some(b)) => { + if let Some(wv) = app.get_webview(&label) { + if let Err(e) = wv.set_size(LogicalSize::new(b.width, b.height)) { + log::warn!( + "[webview-accounts][{}] reveal set_size failed: {}", + account_id, + e + ); + } + if let Err(e) = wv.set_position(LogicalPosition::new(b.x, b.y)) { + log::warn!( + "[webview-accounts][{}] reveal set_position failed: {}", + account_id, + e + ); + } + let _ = wv.show(); + log::info!( + "[webview-accounts][{}] revealed label={} bounds={:?} state={}", + account_id, + label, + b, + state + ); + } else { + log::warn!( + "[webview-accounts][{}] reveal: webview {} missing", + account_id, + label + ); + } + } + _ => { + log::info!( + "[webview-accounts][{}] reveal skipped (account closed before load) state={}", + account_id, + state + ); + } + } + + // Redact the URL in the log: providers like Telegram (`#tgWebAppData=…`) + // and OAuth callbacks embed auth material in the query/fragment. The full + // URL still flows to the frontend listener over the Tauri event so any + // consumer that needs it has access; we just don't persist it to the + // shell's log file. + log::info!( + "[webview-accounts][{}] load event state={} url={}", + account_id, + state, + redact_url_for_log(url) + ); + if let Err(err) = app.emit( + "webview-account:load", + serde_json::json!({ + "account_id": account_id, + "state": state, + "url": url, + }), + ) { + log::warn!( + "[webview-accounts][{}] emit webview-account:load failed: {}", + account_id, + err + ); + } +} + /// Reject any `account_id` that isn't strictly `[A-Za-z0-9_-]+`. The ID comes /// from IPC (React shell, but also from injected recipe code running inside /// third-party origins via `webview_recipe_event`), so treat it as untrusted. @@ -949,6 +1108,11 @@ pub async fn webview_account_open( if let Some(b) = args.bounds { let _ = existing.set_position(LogicalPosition::new(b.x, b.y)); let _ = existing.set_size(LogicalSize::new(b.width, b.height)); + state + .requested_bounds + .lock() + .unwrap() + .insert(args.account_id.clone(), b); } let _ = existing.show(); log::info!( @@ -956,6 +1120,26 @@ pub async fn webview_account_open( existing_label, args.account_id ); + // Warm re-open: the page is already painted, so skip the + // loading overlay cycle and tell the frontend to go straight + // to `open`. We bypass `emit_load_finished` because the + // `loaded_accounts` dedup set would swallow the emit after + // the first cold open of this account. + let reuse_url = existing.url().map(|u| u.to_string()).unwrap_or_default(); + if let Err(err) = app.emit( + "webview-account:load", + serde_json::json!({ + "account_id": args.account_id, + "state": "reused", + "url": reuse_url, + }), + ) { + log::warn!( + "[webview-accounts][{}] emit reused event failed: {}", + args.account_id, + err + ); + } return Ok(existing_label); } // Stale entry — fall through and rebuild @@ -1124,6 +1308,33 @@ pub async fn webview_account_open( builder = builder.user_agent(ua); } + // Wire the native page-load signal so the frontend can hide its spinner as + // soon as CEF's LoadHandler reports the main frame finished. Dedup against + // the CDP `Page.loadEventFired` subscription and the 15 s watchdog through + // `emit_load_finished` so we only fire the first winning signal per open. + // + // Skip `data:` URLs: the initial CDP placeholder also fires `Finished` + // before the real provider URL is navigated to — emitting on the + // placeholder would release the overlay too early and the user would + // briefly see the blank placeholder before the real page paints. + let page_load_app = app.clone(); + let page_load_account_id = args.account_id.clone(); + builder = builder.on_page_load(move |_webview, payload| { + if !matches!(payload.event(), tauri::webview::PageLoadEvent::Finished) { + return; + } + let url = payload.url(); + if url.scheme() == "data" { + return; + } + emit_load_finished( + &page_load_app, + &page_load_account_id, + "finished", + url.as_str(), + ); + }); + let bounds = args.bounds.unwrap_or(Bounds { x: 0.0, y: 0.0, @@ -1131,18 +1342,67 @@ pub async fn webview_account_open( height: 600.0, }); + // Park the webview off-screen during its first page load so the React + // placeholder's loading spinner is not covered by the native CEF subview. + // `webview_account_reveal` (invoked from the frontend after the load event + // arrives, or by the 15 s watchdog) moves it back to `bounds` + shows it. + // We use positive coords well below the parent window rather than large + // negative values so multi-monitor layouts stay well-defined. + // + // Warm-open reuse (when a webview already exists for this account) earlier + // in this function returns before we get here, so existing webviews keep + // their current position — we only off-screen the first cold spawn. + // Spawn strategy: keep the webview at the caller's requested position + // but shrink the initial size to 1×1 under CEF so the native subview + // doesn't paint over the React loading spinner. `webview_account_reveal` + // grows it back to `bounds.width × bounds.height` once the page-loaded + // signal arrives. + // + // Why not move off-screen: moving the NSView after a cold CEF spawn on + // macOS sometimes leaves the page painted but not repainted at the new + // origin, leaving the user looking at a blank viewport until they + // reload. Keeping the position stable and only toggling size sidesteps + // that repaint edge case while still keeping the webview visually + // hidden (1 px under the overlay) during load. + // + // Warm-open reuse returned earlier in this function, so this only + // affects the first cold spawn. + #[cfg(feature = "cef")] + let initial_size = if skip_cdp_for_debug { + LogicalSize::new(bounds.width, bounds.height) + } else { + LogicalSize::new(1.0, 1.0) + }; + #[cfg(not(feature = "cef"))] + let initial_size = LogicalSize::new(bounds.width, bounds.height); + let initial_position = LogicalPosition::new(bounds.x, bounds.y); + + // Remember the bounds the frontend wanted so `webview_account_reveal` has a + // rect to restore to even if the frontend's bounds cache is empty (e.g. + // after a page reload races the load event). + state + .requested_bounds + .lock() + .unwrap() + .insert(args.account_id.clone(), bounds); + // Defensive reset: if a prior close/purge was raced by a stale emit we + // could still have the account marked as "already loaded". Clear here so + // the fresh spawn is allowed to fire the first event again. + state + .loaded_accounts + .lock() + .unwrap() + .remove(&args.account_id); + let webview = parent_window - .add_child( - builder, - LogicalPosition::new(bounds.x, bounds.y), - LogicalSize::new(bounds.width, bounds.height), - ) + .add_child(builder, initial_position, initial_size) .map_err(|e| format!("add_child failed: {e}"))?; log::info!( - "[webview-accounts] spawned label={} bounds={:?}", + "[webview-accounts] spawned label={} requested_bounds={:?} initial_size={:?}", webview.label(), - bounds + bounds, + initial_size ); state @@ -1168,9 +1428,22 @@ pub async fn webview_account_open( args.account_id ); } else { - let handle = cdp::spawn_session(args.account_id.clone(), real_url_str.clone()); - let mut sessions = state.cdp_sessions.lock().unwrap(); - if let Some(old) = sessions.insert(args.account_id.clone(), handle) { + let cdp::SpawnedSession { session, watchdog } = + cdp::spawn_session(app.clone(), args.account_id.clone(), real_url_str.clone()); + if let Some(old) = state + .cdp_sessions + .lock() + .unwrap() + .insert(args.account_id.clone(), session) + { + old.abort(); + } + if let Some(old) = state + .load_watchdogs + .lock() + .unwrap() + .insert(args.account_id.clone(), watchdog) + { old.abort(); } } @@ -1374,7 +1647,31 @@ pub async fn webview_account_close( args.account_id ); } + if let Some(task) = state + .load_watchdogs + .lock() + .unwrap() + .remove(&args.account_id) + { + task.abort(); + log::debug!( + "[webview-accounts] aborted load watchdog for account={}", + args.account_id + ); + } } + // Reset load-overlay bookkeeping so the next open of this account starts + // with a fresh "not yet loaded" state. + state + .loaded_accounts + .lock() + .unwrap() + .remove(&args.account_id); + state + .requested_bounds + .lock() + .unwrap() + .remove(&args.account_id); log::info!("[webview-accounts] closed label={}", label); Ok(()) } @@ -1445,7 +1742,29 @@ pub async fn webview_account_purge( args.account_id ); } + if let Some(task) = state + .load_watchdogs + .lock() + .unwrap() + .remove(&args.account_id) + { + task.abort(); + log::debug!( + "[webview-accounts] purge aborted load watchdog for account={}", + args.account_id + ); + } } + state + .loaded_accounts + .lock() + .unwrap() + .remove(&args.account_id); + state + .requested_bounds + .lock() + .unwrap() + .remove(&args.account_id); let data_dir = data_directory_for(&app, &args.account_id)?; if data_dir.exists() { @@ -1492,6 +1811,59 @@ pub async fn webview_account_bounds( label, args.bounds ); + // Keep the in-state bounds synced so `webview_account_reveal` has the + // latest rect even if the frontend's own cache is cleared between the + // `webview_account_open` call and the `webview-account:load` signal. + state + .requested_bounds + .lock() + .unwrap() + .insert(args.account_id.clone(), args.bounds); + Ok(()) +} + +/// Move an off-screen-spawned webview back to the frontend's desired rect and +/// show it. Invoked by the frontend when it receives the `webview-account:load` +/// event so the loading spinner is uncovered only after the page has painted. +/// +/// Called as the final step of the first-open flow: +/// 1. `webview_account_open` — CEF subview spawned off-screen +/// 2. native `on_page_load` OR CDP `Page.loadEventFired` OR 15 s watchdog +/// 3. frontend listener → `webview_account_reveal` +#[tauri::command] +pub async fn webview_account_reveal( + app: AppHandle, + state: tauri::State<'_, WebviewAccountsState>, + args: BoundsArgs, +) -> Result<(), String> { + let label_opt = state.inner.lock().unwrap().get(&args.account_id).cloned(); + let Some(label) = label_opt else { + // Reveal race: the webview was closed before the load event arrived. + // Return Ok so the frontend doesn't surface an error. + log::debug!( + "[webview-accounts] reveal: no webview for account {}", + args.account_id + ); + return Ok(()); + }; + let wv = app + .get_webview(&label) + .ok_or_else(|| format!("webview {label} missing"))?; + wv.set_position(LogicalPosition::new(args.bounds.x, args.bounds.y)) + .map_err(|e| format!("set_position: {e}"))?; + wv.set_size(LogicalSize::new(args.bounds.width, args.bounds.height)) + .map_err(|e| format!("set_size: {e}"))?; + wv.show().map_err(|e| format!("show: {e}"))?; + state + .requested_bounds + .lock() + .unwrap() + .insert(args.account_id.clone(), args.bounds); + log::info!( + "[webview-accounts] revealed label={} -> {:?}", + label, + args.bounds + ); Ok(()) } diff --git a/app/src/components/accounts/WebviewHost.tsx b/app/src/components/accounts/WebviewHost.tsx index 769e4691b..909b423d2 100644 --- a/app/src/components/accounts/WebviewHost.tsx +++ b/app/src/components/accounts/WebviewHost.tsx @@ -6,7 +6,8 @@ import { openWebviewAccount, setWebviewAccountBounds, } from '../../services/webviewAccountService'; -import type { AccountProvider } from '../../types/accounts'; +import { useAppSelector } from '../../store/hooks'; +import type { AccountProvider, AccountStatus } from '../../types/accounts'; const log = debug('webview-accounts:host'); @@ -15,12 +16,19 @@ interface WebviewHostProps { provider: AccountProvider; } +const LOADING_STATUSES: ReadonlySet = new Set(['pending', 'loading']); + /** * Reserves a rectangular slot in the React layout that the native child * webview is glued to. We measure the placeholder's bounding rect and * tell Rust to position the webview at the same spot. On unmount or * route change the webview is hidden (not destroyed) so its session * stays warm in the background. + * + * During the first-open cycle the CEF subview is parked off-screen by Rust so + * the React loading overlay below isn't covered by an empty native view. The + * overlay is dismissed when the `webview-account:load` event flips the account + * status out of `pending`/`loading`. */ const WebviewHost = ({ accountId, provider }: WebviewHostProps) => { const ref = useRef(null); @@ -28,6 +36,14 @@ const WebviewHost = ({ accountId, provider }: WebviewHostProps) => { null ); const openedRef = useRef(false); + const status = useAppSelector(s => s.accounts.accounts[accountId]?.status); + // Only render the spinner when the account is *actively* loading. We used + // to also treat `status === undefined` as loading, but that meant a host + // mounted for an account that's not in the store (e.g. a render race with + // `addAccount`) would spin forever. The brief microtask between mount and + // the `setAccountStatus('pending')` dispatch in `openWebviewAccount` is + // visually indistinguishable from no overlay, so this is safe. + const isLoading = status !== undefined && LOADING_STATUSES.has(status); // Spawn / show + keep bounds synced on every layout change. // IMPORTANT: both refs are reset on cleanup so switching accountIds @@ -105,8 +121,19 @@ const WebviewHost = ({ accountId, provider }: WebviewHostProps) => {
+ aria-label={`webview host for account ${accountId}`}> + {isLoading ? ( +
+
+ Loading… +
+ ) : null} +
); }; diff --git a/app/src/services/__tests__/webviewAccountService.loadListener.test.ts b/app/src/services/__tests__/webviewAccountService.loadListener.test.ts new file mode 100644 index 000000000..a70eeb2cb --- /dev/null +++ b/app/src/services/__tests__/webviewAccountService.loadListener.test.ts @@ -0,0 +1,173 @@ +import { invoke } from '@tauri-apps/api/core'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { store } from '../../store'; +import { addAccount, resetAccountsState } from '../../store/accountsSlice'; +import { + closeWebviewAccount, + openWebviewAccount, + setWebviewAccountBounds, + startWebviewAccountService, + stopWebviewAccountService, +} from '../webviewAccountService'; + +// Capture the handlers attached via `listen(...)` so tests can fire synthetic +// events and verify downstream behaviour without actually wiring Tauri IPC. +type EventHandler = (evt: { payload: unknown }) => void; +const listeners = new Map(); + +vi.mock('@tauri-apps/api/core', () => ({ + invoke: vi.fn().mockResolvedValue(undefined), + isTauri: vi.fn().mockReturnValue(true), +})); + +vi.mock('@tauri-apps/api/event', () => ({ + listen: vi.fn(async (event: string, handler: EventHandler) => { + listeners.set(event, handler); + return () => { + listeners.delete(event); + }; + }), +})); + +// The service pulls in heavy deps for unrelated flows (Meet transcript + core +// RPC). Stub them so the listener test doesn't drag the whole dependency graph +// through its setup. +vi.mock('../api/threadApi', () => ({ threadApi: { createNewThread: vi.fn() } })); +vi.mock('../chatService', () => ({ chatSend: vi.fn() })); +vi.mock('../coreRpcClient', () => ({ callCoreRpc: vi.fn() })); +vi.mock('../notificationService', () => ({ ingestNotification: vi.fn() })); + +const ACCOUNT_ID = 'acct-123'; + +function seedAccount(): void { + store.dispatch(resetAccountsState()); + store.dispatch( + addAccount({ + id: ACCOUNT_ID, + provider: 'telegram', + label: 'Test', + createdAt: new Date().toISOString(), + status: 'closed', + }) + ); +} + +async function fireLoadEvent(payload: { state: string; url?: string }): Promise { + const handler = listeners.get('webview-account:load'); + if (!handler) throw new Error('webview-account:load listener not attached'); + handler({ payload: { account_id: ACCOUNT_ID, url: '', ...payload } }); + // Drain to a macrotask so chained `.catch()` / `.then()` on the + // `invoke()` promise inside the handler also settle before we assert. + await new Promise(r => setTimeout(r, 0)); +} + +describe('webviewAccountService load listener', () => { + beforeEach(async () => { + listeners.clear(); + stopWebviewAccountService(); + // Tear down any per-account state left from the previous test (bounds + // cache + loading flag) before re-arming the listener for this one. + // `stopWebviewAccountService` already clears the module-level Maps; + // `closeWebviewAccount` is the no-Tauri-side close path (the invoke is + // mocked) and is here only as belt-and-braces. + await closeWebviewAccount(ACCOUNT_ID); + // Single mock reset so individual tests can rely on the `invoke` + // resolved-value config they set up after this hook returns. + vi.clearAllMocks(); + seedAccount(); + startWebviewAccountService(); + }); + + it('transitions pending → loading on openWebviewAccount resolve', async () => { + const bounds = { x: 10, y: 20, width: 800, height: 600 }; + await openWebviewAccount({ accountId: ACCOUNT_ID, provider: 'telegram', bounds }); + + expect(store.getState().accounts.accounts[ACCOUNT_ID]?.status).toBe('loading'); + expect(vi.mocked(invoke)).toHaveBeenCalledWith( + 'webview_account_open', + expect.objectContaining({ + args: expect.objectContaining({ account_id: ACCOUNT_ID, provider: 'telegram' }), + }) + ); + }); + + it('reveals with cached bounds + flips to open on finished event', async () => { + const bounds = { x: 5, y: 15, width: 1024, height: 768 }; + await openWebviewAccount({ accountId: ACCOUNT_ID, provider: 'telegram', bounds }); + vi.mocked(invoke).mockClear(); + + await fireLoadEvent({ state: 'finished', url: 'https://web.telegram.org/' }); + + expect(vi.mocked(invoke)).toHaveBeenCalledWith('webview_account_reveal', { + args: { account_id: ACCOUNT_ID, bounds }, + }); + expect(store.getState().accounts.accounts[ACCOUNT_ID]?.status).toBe('open'); + }); + + it('reveals with latest bounds when resize landed during loading', async () => { + const initial = { x: 0, y: 0, width: 800, height: 600 }; + await openWebviewAccount({ accountId: ACCOUNT_ID, provider: 'telegram', bounds: initial }); + + // Resize during loading — invoke should be skipped, cache should still update. + vi.mocked(invoke).mockClear(); + const resized = { x: 0, y: 0, width: 1200, height: 900 }; + await setWebviewAccountBounds(ACCOUNT_ID, resized); + expect(vi.mocked(invoke)).not.toHaveBeenCalledWith('webview_account_bounds', expect.anything()); + + await fireLoadEvent({ state: 'finished', url: 'x' }); + + expect(vi.mocked(invoke)).toHaveBeenCalledWith('webview_account_reveal', { + args: { account_id: ACCOUNT_ID, bounds: resized }, + }); + }); + + it('forwards bounds once loading is done', async () => { + const initial = { x: 0, y: 0, width: 800, height: 600 }; + await openWebviewAccount({ accountId: ACCOUNT_ID, provider: 'telegram', bounds: initial }); + await fireLoadEvent({ state: 'finished', url: 'x' }); + vi.mocked(invoke).mockClear(); + + const next = { x: 10, y: 10, width: 900, height: 700 }; + await setWebviewAccountBounds(ACCOUNT_ID, next); + + expect(vi.mocked(invoke)).toHaveBeenCalledWith('webview_account_bounds', { + args: { account_id: ACCOUNT_ID, bounds: next }, + }); + }); + + it('still reveals on timeout fallback', async () => { + const bounds = { x: 0, y: 0, width: 800, height: 600 }; + await openWebviewAccount({ accountId: ACCOUNT_ID, provider: 'telegram', bounds }); + vi.mocked(invoke).mockClear(); + + await fireLoadEvent({ state: 'timeout', url: '' }); + + expect(vi.mocked(invoke)).toHaveBeenCalledWith('webview_account_reveal', { + args: { account_id: ACCOUNT_ID, bounds }, + }); + expect(store.getState().accounts.accounts[ACCOUNT_ID]?.status).toBe('open'); + }); + + it('treats `reused` event as finished (warm re-open path)', async () => { + const bounds = { x: 0, y: 0, width: 800, height: 600 }; + await openWebviewAccount({ accountId: ACCOUNT_ID, provider: 'telegram', bounds }); + vi.mocked(invoke).mockClear(); + + await fireLoadEvent({ state: 'reused', url: 'https://web.telegram.org/' }); + + expect(vi.mocked(invoke)).toHaveBeenCalledWith('webview_account_reveal', { + args: { account_id: ACCOUNT_ID, bounds }, + }); + expect(store.getState().accounts.accounts[ACCOUNT_ID]?.status).toBe('open'); + }); + + it('skips reveal when the account has already unmounted', async () => { + // Fire load event without ever having opened the account (no cached bounds). + vi.mocked(invoke).mockClear(); + + await fireLoadEvent({ state: 'finished', url: 'x' }); + + expect(vi.mocked(invoke)).not.toHaveBeenCalled(); + }); +}); diff --git a/app/src/services/webviewAccountService.ts b/app/src/services/webviewAccountService.ts index 5cb7c3283..620f8c244 100644 --- a/app/src/services/webviewAccountService.ts +++ b/app/src/services/webviewAccountService.ts @@ -59,6 +59,22 @@ interface NotificationClickPayload { provider: string; } +interface WebviewAccountLoadPayload { + account_id: string; + // `'finished'` — native `on_page_load` or CDP `Page.loadEventFired` fired + // `'timeout'` — 15 s watchdog elapsed; reveal anyway so spinner isn't stuck + // `'reused'` — warm re-open of already-loaded account; reveal synchronously + state: 'finished' | 'timeout' | 'reused' | string; + url: string; +} + +interface WebviewAccountBounds { + x: number; + y: number; + width: number; + height: number; +} + interface RecipeNotifyPayload { title?: string; body?: string; @@ -70,9 +86,23 @@ interface RecipeNotifyPayload { let unlisten: UnlistenFn | null = null; let unlistenNotifyClick: UnlistenFn | null = null; +let unlistenLoad: UnlistenFn | null = null; let started = false; let permissionChecked = false; +// Last bounds the frontend handed to Rust per account. Updated on every +// `setWebviewAccountBounds` call (even when the invoke itself is skipped +// because the account is still loading). The `webview-account:load` listener +// reads back from here so it can issue `webview_account_reveal` with the +// correct rect without a second round-trip. +const lastBoundsByAccount = new Map(); + +// Track which accounts are still in their initial load cycle (spawned +// off-screen, waiting for the first page-loaded signal). Bounds updates for +// these are cached but NOT forwarded to Rust — moving the off-screen webview +// to the on-screen rect prematurely would defeat the loading overlay. +const loadingAccounts = new Set(); + export function startWebviewAccountService(): void { if (started) return; if (!isTauri()) { @@ -100,6 +130,17 @@ export function startWebviewAccountService(): void { } catch (err) { errLog('failed to attach notification:click listener', err); } + try { + // Rust emits `webview-account:load` from three independent signals + // (native `on_page_load`, CDP `Page.loadEventFired`, 15 s watchdog). + // It dedups server-side so we see exactly one event per cold open. + unlistenLoad = await listen('webview-account:load', evt => { + handleWebviewAccountLoad(evt.payload); + }); + log('webview-account:load listener attached'); + } catch (err) { + errLog('failed to attach webview-account:load listener', err); + } })(); } @@ -112,9 +153,55 @@ export function stopWebviewAccountService(): void { unlistenNotifyClick(); unlistenNotifyClick = null; } + if (unlistenLoad) { + unlistenLoad(); + unlistenLoad = null; + } + // Drop module-level state so a subsequent start (HMR / shutdown→restart) + // doesn't see stale per-account entries that survived the listener + // teardown. Otherwise an account whose webview was destroyed mid-load + // would resurface as "still loading" on restart and silently drop bounds + // updates because `loadingAccounts.has(...)` is true. + lastBoundsByAccount.clear(); + loadingAccounts.clear(); started = false; } +function handleWebviewAccountLoad(payload: WebviewAccountLoadPayload) { + const accountId = payload?.account_id; + if (!accountId) { + errLog('webview-account:load missing account_id — ignoring: %o', payload); + return; + } + log('load event account=%s state=%s url=%s', accountId, payload.state, payload.url); + loadingAccounts.delete(accountId); + + // Rust already resized the webview to `requested_bounds` as part of + // `emit_load_finished`, so the native side is already correct. We still + // issue `webview_account_reveal` here as a belt-and-braces idempotent + // no-op: if the frontend bounds diverged from the Rust-stored ones (e.g. + // a resize landed during the load window) this reapplies the latest + // measured rect. When the cache is empty (host already unmounted) we + // simply skip. + // + // Dispatch `'open'` after the reveal settles (success or failure) so the + // spinner is only dismissed once the webview is actually positioned. On + // error we still flip to `'open'` so the spinner never hangs indefinitely — + // the webview will have been positioned server-side by `emit_load_finished`. + const bounds = lastBoundsByAccount.get(accountId); + if (bounds) { + invoke('webview_account_reveal', { args: { account_id: accountId, bounds } }) + .catch(err => { + errLog('webview_account_reveal failed account=%s: %o', accountId, err); + }) + .finally(() => { + store.dispatch(setAccountStatus({ accountId, status: 'open' })); + }); + } else { + store.dispatch(setAccountStatus({ accountId, status: 'open' })); + } +} + function handleNotificationClick(payload: NotificationClickPayload) { const accountId = payload?.account_id; const provider = payload?.provider; @@ -720,16 +807,23 @@ export async function openWebviewAccount(args: OpenAccountArgs): Promise { if (!isTauri()) throw new Error('webview accounts require the desktop app'); log('open account=%s provider=%s', args.accountId, args.provider); store.dispatch(setAccountStatus({ accountId: args.accountId, status: 'pending' })); + lastBoundsByAccount.set(args.accountId, args.bounds); + loadingAccounts.add(args.accountId); void ensureNotificationPermission(); try { await invoke('webview_account_open', { args: { account_id: args.accountId, provider: args.provider, bounds: args.bounds }, }); - store.dispatch(setAccountStatus({ accountId: args.accountId, status: 'open' })); + // Rust confirmed `add_child`. The webview is spawned off-screen; keep us + // in the loading state until `webview-account:load` arrives (at which point + // the listener dispatches `'open'`). Warm re-opens are resolved by the + // `'reused'` event which the listener also handles. + store.dispatch(setAccountStatus({ accountId: args.accountId, status: 'loading' })); void setFocusedAccount(args.accountId); } catch (err) { const msg = err instanceof Error ? err.message : String(err); errLog('open failed: %s', msg); + loadingAccounts.delete(args.accountId); store.dispatch( setAccountStatus({ accountId: args.accountId, status: 'error', lastError: msg }) ); @@ -739,9 +833,18 @@ export async function openWebviewAccount(args: OpenAccountArgs): Promise { export async function setWebviewAccountBounds( accountId: string, - bounds: { x: number; y: number; width: number; height: number } + bounds: WebviewAccountBounds ): Promise { if (!isTauri()) return; + // Always keep the cache fresh — the load-event listener needs it whether or + // not we forward this particular call to Rust. + lastBoundsByAccount.set(accountId, bounds); + if (loadingAccounts.has(accountId)) { + // Webview is parked off-screen waiting for its first page-loaded signal. + // Skip the invoke so we don't drag the CEF subview back on-screen over + // the React loading overlay. + return; + } try { await invoke('webview_account_bounds', { args: { account_id: accountId, bounds } }); } catch (err) { @@ -771,6 +874,8 @@ export async function closeWebviewAccount(accountId: string): Promise { if (!isTauri()) return; log('close account=%s', accountId); await flushMeetingIfAny(accountId, 'webview-closed'); + lastBoundsByAccount.delete(accountId); + loadingAccounts.delete(accountId); try { await invoke('webview_account_close', { args: { account_id: accountId } }); store.dispatch(setAccountStatus({ accountId, status: 'closed' })); @@ -787,6 +892,8 @@ export async function purgeWebviewAccount(accountId: string): Promise { if (!isTauri()) return; log('purge account=%s', accountId); await flushMeetingIfAny(accountId, 'webview-purged'); + lastBoundsByAccount.delete(accountId); + loadingAccounts.delete(accountId); try { await invoke('webview_account_purge', { args: { account_id: accountId } }); store.dispatch(setAccountStatus({ accountId, status: 'closed' })); diff --git a/app/src/types/accounts.ts b/app/src/types/accounts.ts index 27daae677..8c9de025e 100644 --- a/app/src/types/accounts.ts +++ b/app/src/types/accounts.ts @@ -11,7 +11,14 @@ export type AccountProvider = | 'zoom' | 'browserscan'; -export type AccountStatus = 'pending' | 'open' | 'error' | 'closed'; +// Status lifecycle for an embedded webview account: +// 'pending' — openWebviewAccount invoked, Rust-side add_child not yet confirmed +// 'loading' — CEF child webview spawned off-screen, waiting for first page-loaded +// signal; WebviewHost shows its spinner +// 'open' — page loaded, webview_account_reveal completed, webview on-screen +// 'closed' — webview destroyed +// 'error' — open/reveal failed (lastError populated) +export type AccountStatus = 'pending' | 'loading' | 'open' | 'error' | 'closed'; export interface Account { id: string;