polish(desktop): final cross-cutting fixes
Two real bugs from final review: - Clear recovery state on window 'closed' so macOS dock-activate doesn't drop the recovery surface (window recreated with stale recoveryStore.mode) - Hoist setOnQuitRequested before createWindow so synchronous boot failures reach a wired Quit handler Three polish items: - Tray's Change Instance now routes through handleRecoveryAction so both paths share one implementation; recovery action's change-instance also show()+focus() for hidden-window tray clicks - install-update action guards against state.updateState !== 'downloaded' (defense in depth against malicious or buggy renderers) - Object.freeze rationale documented in RecoveryStateStore.update
This commit is contained in:
@@ -372,6 +372,11 @@ function createWindow(): void {
|
|||||||
mainWindow.on('closed', () => {
|
mainWindow.on('closed', () => {
|
||||||
setMainWindow(null);
|
setMainWindow(null);
|
||||||
mainWindow = null;
|
mainWindow = null;
|
||||||
|
// Clear recovery state so macOS dock-activate (which calls createWindow again)
|
||||||
|
// gets a clean slate. Without this, the new window inherits inRecoveryMode=true
|
||||||
|
// and the recovery surface is silently lost.
|
||||||
|
recoveryStore.markRecoveryExited();
|
||||||
|
recoveryStore.update({ mode: 'normal', reason: null });
|
||||||
});
|
});
|
||||||
|
|
||||||
// Window focus IPC for notification suppression
|
// Window focus IPC for notification suppression
|
||||||
@@ -895,23 +900,22 @@ if (!gotTheLock) {
|
|||||||
});
|
});
|
||||||
|
|
||||||
registerIpcHandlers();
|
registerIpcHandlers();
|
||||||
|
// Wire the recovery module's quit callback to the local requestQuit BEFORE
|
||||||
|
// createWindow so a synchronous did-fail-load on first load finds a wired
|
||||||
|
// Quit handler. requestQuit is a function declaration and is hoisted.
|
||||||
|
setOnQuitRequested(() => requestQuit());
|
||||||
createWindow();
|
createWindow();
|
||||||
createTray();
|
createTray();
|
||||||
|
|
||||||
// Wire the recovery module's quit callback to the local requestQuit.
|
|
||||||
setOnQuitRequested(() => requestQuit());
|
|
||||||
|
|
||||||
// Tray + macOS app-menu actions. Defined once so the subscriber and the
|
// Tray + macOS app-menu actions. Defined once so the subscriber and the
|
||||||
// initial-fire share one implementation (no drift on future menu changes).
|
// initial-fire share one implementation (no drift on future menu changes).
|
||||||
const trayActions = {
|
const trayActions = {
|
||||||
onShow: () => { mainWindow?.show(); mainWindow?.focus(); },
|
onShow: () => { mainWindow?.show(); mainWindow?.focus(); },
|
||||||
onHide: () => mainWindow?.hide(),
|
onHide: () => mainWindow?.hide(),
|
||||||
onChangeInstance: () => {
|
// Delegate to handleRecoveryAction so both the tray and the recovery
|
||||||
clearInstanceUrl();
|
// surface share one implementation path (avoids drift and ensures
|
||||||
mainWindow?.loadFile(getPickerPath());
|
// recovery state is always cleared on a Change Instance action).
|
||||||
mainWindow?.show();
|
onChangeInstance: () => handleRecoveryAction('change-instance'),
|
||||||
mainWindow?.focus();
|
|
||||||
},
|
|
||||||
onCheckForUpdates: () => handleRecoveryAction('check-update'),
|
onCheckForUpdates: () => handleRecoveryAction('check-update'),
|
||||||
onRestartToInstall: () => handleRecoveryAction('install-update'),
|
onRestartToInstall: () => handleRecoveryAction('install-update'),
|
||||||
onQuit: () => requestQuit(),
|
onQuit: () => requestQuit(),
|
||||||
|
|||||||
@@ -36,6 +36,7 @@ const INITIAL_STATE: RecoveryState = {
|
|||||||
};
|
};
|
||||||
|
|
||||||
export class RecoveryStateStore {
|
export class RecoveryStateStore {
|
||||||
|
// Freeze so the initial get() return cannot be mutated by callers.
|
||||||
private state: RecoveryState = Object.freeze({ ...INITIAL_STATE }) as RecoveryState;
|
private state: RecoveryState = Object.freeze({ ...INITIAL_STATE }) as RecoveryState;
|
||||||
private listeners = new Set<(s: RecoveryState) => void>();
|
private listeners = new Set<(s: RecoveryState) => void>();
|
||||||
private inRecoveryMode = false;
|
private inRecoveryMode = false;
|
||||||
@@ -45,6 +46,9 @@ export class RecoveryStateStore {
|
|||||||
}
|
}
|
||||||
|
|
||||||
update(partial: Partial<RecoveryState>): void {
|
update(partial: Partial<RecoveryState>): void {
|
||||||
|
// Freeze so consumers (incl. subscribers, IPC-cloned renderer reads via
|
||||||
|
// get-recovery-state) cannot mutate the shared state through the live
|
||||||
|
// reference returned by get(). Compile-time Readonly<> is a hint only.
|
||||||
this.state = Object.freeze({ ...this.state, ...partial }) as RecoveryState;
|
this.state = Object.freeze({ ...this.state, ...partial }) as RecoveryState;
|
||||||
// Snapshot before iterating: a listener can subscribe/unsubscribe others
|
// Snapshot before iterating: a listener can subscribe/unsubscribe others
|
||||||
// (or itself) during notification without affecting the current notify pass.
|
// (or itself) during notification without affecting the current notify pass.
|
||||||
@@ -370,6 +374,10 @@ export function handleRecoveryAction(action: RecoveryAction): void {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
case 'install-update': {
|
case 'install-update': {
|
||||||
|
// Defense in depth: only act when an update is actually downloaded. The
|
||||||
|
// recovery.html UI hides the Restart button unless updateState='downloaded',
|
||||||
|
// but a buggy/malicious renderer could send this action at any time.
|
||||||
|
if (recoveryStore.get().updateState !== 'downloaded') return;
|
||||||
// Direct quitAndInstall — does NOT rely on autoInstallOnAppQuit.
|
// Direct quitAndInstall — does NOT rely on autoInstallOnAppQuit.
|
||||||
// Force-kill-fix: if user reaches recovery and clicks here, install
|
// Force-kill-fix: if user reaches recovery and clicks here, install
|
||||||
// happens cleanly even if the on-quit hook would otherwise be bypassed.
|
// happens cleanly even if the on-quit hook would otherwise be bypassed.
|
||||||
@@ -381,6 +389,12 @@ export function handleRecoveryAction(action: RecoveryAction): void {
|
|||||||
recoveryStore.markRecoveryExited();
|
recoveryStore.markRecoveryExited();
|
||||||
recoveryStore.update({ mode: 'normal', reason: null });
|
recoveryStore.update({ mode: 'normal', reason: null });
|
||||||
mainWindowRef?.loadFile(getPickerPath());
|
mainWindowRef?.loadFile(getPickerPath());
|
||||||
|
// Ensure visible — tray clicks may happen with window hidden, and the
|
||||||
|
// recovery surface should also remain visible during the navigation.
|
||||||
|
// When invoked from recovery.html (window already showing), these are
|
||||||
|
// idempotent no-ops.
|
||||||
|
mainWindowRef?.show();
|
||||||
|
mainWindowRef?.focus();
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
case 'open-releases': {
|
case 'open-releases': {
|
||||||
|
|||||||
Reference in New Issue
Block a user