mirror of
https://github.com/nimbold/Firelink.git
synced 2026-08-08 18:33:39 +00:00
fix(properties): fence stale renderer actions
This commit is contained in:
@@ -50,6 +50,7 @@ export const PropertiesWindowApp = () => {
|
||||
const { t } = useTranslation();
|
||||
const currentWindow = useMemo(() => getCurrentWindow(), []);
|
||||
const windowLabel = currentWindow.label;
|
||||
const sessionId = useMemo(() => crypto.randomUUID(), []);
|
||||
const [downloadId, setDownloadId] = useState<string | null>(null);
|
||||
const [snapshot, setSnapshot] = useState<PropertiesSnapshot | null>(null);
|
||||
const [activeTab, setActiveTab] = useState<PropertiesTab>('overview');
|
||||
@@ -190,7 +191,9 @@ export const PropertiesWindowApp = () => {
|
||||
if (cancelled) return;
|
||||
setDownloadId(id);
|
||||
unlistenSnapshot = await listen<PropertiesSnapshotEvent>(PROPERTIES_WINDOW_SNAPSHOT, async event => {
|
||||
if (event.payload.windowLabel !== windowLabel || event.payload.downloadId !== id) return;
|
||||
if (event.payload.windowLabel !== windowLabel
|
||||
|| event.payload.downloadId !== id
|
||||
|| event.payload.sessionId !== sessionId) return;
|
||||
if (event.payload.revision <= latestSnapshotRevisionRef.current) return;
|
||||
latestSnapshotRevisionRef.current = event.payload.revision;
|
||||
await changeAppLocale(event.payload.snapshot.appearance.locale);
|
||||
@@ -218,7 +221,9 @@ export const PropertiesWindowApp = () => {
|
||||
}
|
||||
});
|
||||
unlistenResult = await listen<PropertiesActionResult>(PROPERTIES_WINDOW_ACTION_RESULT, event => {
|
||||
if (event.payload.windowLabel !== windowLabel || event.payload.downloadId !== id) return;
|
||||
if (event.payload.windowLabel !== windowLabel
|
||||
|| event.payload.downloadId !== id
|
||||
|| event.payload.sessionId !== sessionId) return;
|
||||
if (event.payload.requestId !== requestIdRef.current) return;
|
||||
setIsSaving(false);
|
||||
const completedAction = pendingActionRef.current;
|
||||
@@ -253,7 +258,7 @@ export const PropertiesWindowApp = () => {
|
||||
setNotice(t($ => $.downloadTable.noDownloads));
|
||||
}
|
||||
});
|
||||
await sendPropertiesReady();
|
||||
await sendPropertiesReady(sessionId);
|
||||
if (cancelled) return;
|
||||
// Tauri event listeners are registered asynchronously. If the main
|
||||
// bridge was still installing its listener, the first ready event can
|
||||
@@ -261,7 +266,7 @@ export const PropertiesWindowApp = () => {
|
||||
// the handshake rather than leaving a permanently hidden window.
|
||||
readyRetryTimer = window.setInterval(() => {
|
||||
if (cancelled || hasRevealedWindowRef.current) return;
|
||||
void sendPropertiesReady().catch(() => undefined);
|
||||
void sendPropertiesReady(sessionId).catch(() => undefined);
|
||||
}, 500);
|
||||
} catch (error) {
|
||||
if (!cancelled) setErrorMessage(errorText(error));
|
||||
@@ -277,7 +282,7 @@ export const PropertiesWindowApp = () => {
|
||||
appearanceCleanupRef.current?.();
|
||||
appearanceCleanupRef.current = null;
|
||||
};
|
||||
}, [currentWindow, hydrateDraft, t, windowLabel]);
|
||||
}, [currentWindow, hydrateDraft, sessionId, t, windowLabel]);
|
||||
|
||||
useEffect(() => {
|
||||
if (!snapshot || draftTab !== null) return;
|
||||
@@ -331,6 +336,7 @@ export const PropertiesWindowApp = () => {
|
||||
await sendPropertiesActionRequest({
|
||||
windowLabel,
|
||||
downloadId,
|
||||
sessionId,
|
||||
requestId,
|
||||
action,
|
||||
payload,
|
||||
@@ -342,7 +348,7 @@ export const PropertiesWindowApp = () => {
|
||||
switchAfterSaveRef.current = null;
|
||||
setErrorMessage(errorText(error));
|
||||
}
|
||||
}, [downloadId, windowLabel]);
|
||||
}, [downloadId, sessionId, windowLabel]);
|
||||
|
||||
const applyActiveTab = useCallback(async () => {
|
||||
if (!snapshot || !isEditableStatus(snapshot.status)) {
|
||||
|
||||
@@ -16,13 +16,16 @@ import {
|
||||
applySecretPatch,
|
||||
beginExclusivePropertiesAction,
|
||||
createFrameCoalescer,
|
||||
enqueuePropertiesAction,
|
||||
getPropertiesLifecycleAction,
|
||||
sanitizePropertiesSnapshot,
|
||||
sendPropertiesActionResult,
|
||||
sendPropertiesRemoved,
|
||||
sendPropertiesSnapshot,
|
||||
shouldAcceptPropertiesActionRequest,
|
||||
type PropertiesActionRequest,
|
||||
type PropertiesPatch,
|
||||
type PropertiesWindowRegistration,
|
||||
type PropertiesWindowReady,
|
||||
} from '../propertiesBridge';
|
||||
import { invokeCommand as invoke } from '../ipc';
|
||||
@@ -108,25 +111,28 @@ const copyEditablePatch = (rawPatch: PropertiesPatch): Partial<DownloadItem> =>
|
||||
|
||||
export const PropertiesWindowBridgeHost = () => {
|
||||
useEffect(() => {
|
||||
const windows = new Map<string, string>();
|
||||
const windows = new Map<string, PropertiesWindowRegistration>();
|
||||
const snapshotRevisions = new Map<string, number>();
|
||||
const actionsInFlight = new Set<string>();
|
||||
const actionChains = new Map<string, Promise<void>>();
|
||||
let disposed = false;
|
||||
let unlistenReady: UnlistenFn | undefined;
|
||||
let unlistenAction: UnlistenFn | undefined;
|
||||
let unlistenClosed: UnlistenFn | undefined;
|
||||
const snapshotCoalescer = createFrameCoalescer(
|
||||
windowLabel => {
|
||||
const downloadId = windows.get(windowLabel);
|
||||
if (downloadId) void sendFor(windowLabel, downloadId).catch(() => undefined);
|
||||
const registration = windows.get(windowLabel);
|
||||
if (registration) void sendFor(windowLabel, registration.downloadId).catch(() => undefined);
|
||||
},
|
||||
callback => window.requestAnimationFrame(callback),
|
||||
handle => window.cancelAnimationFrame(handle),
|
||||
);
|
||||
|
||||
const sendFor = async (windowLabel: string, downloadId: string) => {
|
||||
const registration = windows.get(windowLabel);
|
||||
if (!registration || registration.downloadId !== downloadId || disposed) return false;
|
||||
const item = useDownloadStore.getState().downloads.find(download => download.id === downloadId);
|
||||
if (!item || disposed) return false;
|
||||
if (!item) return false;
|
||||
const settings = useSettingsStore.getState();
|
||||
const progress = useDownloadProgressStore.getState();
|
||||
const revision = (snapshotRevisions.get(windowLabel) ?? 0) + 1;
|
||||
@@ -134,6 +140,7 @@ export const PropertiesWindowBridgeHost = () => {
|
||||
await sendPropertiesSnapshot(windowLabel, {
|
||||
windowLabel,
|
||||
downloadId,
|
||||
sessionId: registration.sessionId,
|
||||
revision,
|
||||
snapshot: sanitizePropertiesSnapshot(item, {
|
||||
theme: settings.theme,
|
||||
@@ -149,6 +156,25 @@ export const PropertiesWindowBridgeHost = () => {
|
||||
return true;
|
||||
};
|
||||
|
||||
const synchronizeRegistration = (
|
||||
windowLabel: string,
|
||||
downloadId: string,
|
||||
sessionId: string,
|
||||
) => {
|
||||
const previous = windows.get(windowLabel);
|
||||
const sessionChanged = previous?.downloadId !== downloadId || previous.sessionId !== sessionId;
|
||||
windows.set(windowLabel, {
|
||||
downloadId,
|
||||
sessionId,
|
||||
latestRequestId: sessionChanged ? 0 : (previous?.latestRequestId ?? 0),
|
||||
});
|
||||
if (sessionChanged) {
|
||||
snapshotRevisions.set(windowLabel, 0);
|
||||
} else if (!snapshotRevisions.has(windowLabel)) {
|
||||
snapshotRevisions.set(windowLabel, 0);
|
||||
}
|
||||
};
|
||||
|
||||
const handleReady = async (payload: PropertiesWindowReady) => {
|
||||
try {
|
||||
await invoke('validate_properties_window_request', payload);
|
||||
@@ -157,8 +183,7 @@ export const PropertiesWindowBridgeHost = () => {
|
||||
await sendPropertiesRemoved(payload.windowLabel, payload.downloadId);
|
||||
return;
|
||||
}
|
||||
windows.set(payload.windowLabel, payload.downloadId);
|
||||
if (!snapshotRevisions.has(payload.windowLabel)) snapshotRevisions.set(payload.windowLabel, 0);
|
||||
synchronizeRegistration(payload.windowLabel, payload.downloadId, payload.sessionId);
|
||||
await sendFor(payload.windowLabel, payload.downloadId);
|
||||
} catch {
|
||||
// The child will show its own unavailable state. Do not log bridge
|
||||
@@ -166,14 +191,17 @@ export const PropertiesWindowBridgeHost = () => {
|
||||
}
|
||||
};
|
||||
|
||||
const handleAction = async (request: PropertiesActionRequest) => {
|
||||
const processAction = async (request: PropertiesActionRequest) => {
|
||||
let ok = false;
|
||||
let error: string | undefined;
|
||||
const actionKey = `${request.windowLabel}:${request.downloadId}`;
|
||||
let releaseAction: (() => void) | undefined;
|
||||
try {
|
||||
await invoke('validate_properties_window_request', request);
|
||||
if (windows.get(request.windowLabel) !== request.downloadId) {
|
||||
const registration = windows.get(request.windowLabel);
|
||||
if (!registration
|
||||
|| registration.downloadId !== request.downloadId
|
||||
|| registration.sessionId !== request.sessionId) {
|
||||
throw new Error('Properties window is no longer registered');
|
||||
}
|
||||
releaseAction = beginExclusivePropertiesAction(actionsInFlight, actionKey);
|
||||
@@ -285,6 +313,7 @@ export const PropertiesWindowBridgeHost = () => {
|
||||
await sendPropertiesActionResult(request.windowLabel, {
|
||||
windowLabel: request.windowLabel,
|
||||
downloadId: request.downloadId,
|
||||
sessionId: request.sessionId,
|
||||
requestId: request.requestId,
|
||||
ok,
|
||||
...(error ? { error } : {}),
|
||||
@@ -295,18 +324,44 @@ export const PropertiesWindowBridgeHost = () => {
|
||||
}
|
||||
};
|
||||
|
||||
const handleAction = async (request: PropertiesActionRequest) => {
|
||||
const actionKey = `${request.windowLabel}:${request.downloadId}`;
|
||||
try {
|
||||
// The native command validates the caller, download binding, and
|
||||
// renderer session. If a ready event is delayed or lost, this valid
|
||||
// action can also establish the main-window registration.
|
||||
await invoke('validate_properties_window_request', request);
|
||||
synchronizeRegistration(request.windowLabel, request.downloadId, request.sessionId);
|
||||
} catch {
|
||||
// Stale renderer actions are deliberately ignored. The current child
|
||||
// session cannot safely consume a result for a superseded renderer.
|
||||
return;
|
||||
}
|
||||
|
||||
const registration = windows.get(request.windowLabel);
|
||||
if (!registration) return;
|
||||
if (!shouldAcceptPropertiesActionRequest(registration, request)) return;
|
||||
registration.latestRequestId = request.requestId;
|
||||
|
||||
// Preserve user order for accepted requests. This keeps a pause from an
|
||||
// earlier request from running after a newer resume, while still
|
||||
// allowing the newer request to run after an already-started operation.
|
||||
await enqueuePropertiesAction(actionChains, actionKey, () => processAction(request));
|
||||
};
|
||||
|
||||
void listen<PropertiesWindowReady>(PROPERTIES_WINDOW_READY, event => void handleReady(event.payload)).then(value => { unlistenReady = value; });
|
||||
void listen<PropertiesActionRequest>(PROPERTIES_WINDOW_ACTION_REQUEST, event => void handleAction(event.payload)).then(value => { unlistenAction = value; });
|
||||
void listen<string>(PROPERTIES_WINDOW_CLOSED, event => {
|
||||
const downloadId = windows.get(event.payload);
|
||||
const registration = windows.get(event.payload);
|
||||
windows.delete(event.payload);
|
||||
snapshotRevisions.delete(event.payload);
|
||||
snapshotCoalescer.cancel(event.payload);
|
||||
if (downloadId) actionsInFlight.delete(`${event.payload}:${downloadId}`);
|
||||
if (registration) actionsInFlight.delete(`${event.payload}:${registration.downloadId}`);
|
||||
}).then(value => { unlistenClosed = value; });
|
||||
|
||||
const unsubscribeStore = useDownloadStore.subscribe((state, previous) => {
|
||||
for (const [windowLabel, downloadId] of windows) {
|
||||
for (const [windowLabel, registration] of windows) {
|
||||
const { downloadId } = registration;
|
||||
const next = state.downloads.find(download => download.id === downloadId);
|
||||
const before = previous.downloads.find(download => download.id === downloadId);
|
||||
if (!next) {
|
||||
@@ -321,7 +376,8 @@ export const PropertiesWindowBridgeHost = () => {
|
||||
}
|
||||
});
|
||||
const unsubscribeProgress = useDownloadProgressStore.subscribe((state, previous) => {
|
||||
for (const [windowLabel, downloadId] of windows) {
|
||||
for (const [windowLabel, registration] of windows) {
|
||||
const { downloadId } = registration;
|
||||
if (state.progressMap[downloadId] !== previous.progressMap[downloadId]
|
||||
|| state.moveProgressMap[downloadId] !== previous.moveProgressMap[downloadId]) {
|
||||
snapshotCoalescer.schedule(windowLabel);
|
||||
|
||||
+3
-3
@@ -157,10 +157,10 @@ type CommandMap = {
|
||||
remove_from_queue: { args: { id: string }; result: boolean };
|
||||
open_download_properties_window: { args: { id: string }; result: string };
|
||||
get_properties_window_download_id: { args: undefined; result: string };
|
||||
properties_window_send_ready: { args: undefined; result: void };
|
||||
properties_window_send_ready: { args: { sessionId: string }; result: void };
|
||||
properties_window_reveal: { args: undefined; result: void };
|
||||
properties_window_send_action: { args: { requestId: number; action: string; payload?: unknown }; result: void };
|
||||
validate_properties_window_request: { args: { windowLabel: string; downloadId: string }; result: void };
|
||||
properties_window_send_action: { args: { sessionId: string; requestId: number; action: string; payload?: unknown }; result: void };
|
||||
validate_properties_window_request: { args: { windowLabel: string; downloadId: string; sessionId: string; requestId?: number }; result: void };
|
||||
close_download_properties_window: { args: { id: string }; result: void };
|
||||
properties_window_registry_remove_for_download: { args: { id: string }; result: void };
|
||||
};
|
||||
|
||||
@@ -14,9 +14,11 @@ import {
|
||||
applySecretPatch,
|
||||
beginExclusivePropertiesAction,
|
||||
createFrameCoalescer,
|
||||
enqueuePropertiesAction,
|
||||
getPropertiesLifecycleAction,
|
||||
isExpectedPropertiesDiagnosticUnavailable,
|
||||
sanitizePropertiesSnapshot,
|
||||
shouldAcceptPropertiesActionRequest,
|
||||
} from './propertiesBridge';
|
||||
|
||||
describe('Properties window bridge', () => {
|
||||
@@ -157,6 +159,72 @@ describe('Properties window bridge', () => {
|
||||
expect(inFlight.has('window:download')).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects actions from a superseded renderer session and older request IDs', () => {
|
||||
const registration = {
|
||||
downloadId: 'download-1',
|
||||
sessionId: 'session-new',
|
||||
latestRequestId: 4,
|
||||
};
|
||||
|
||||
expect(shouldAcceptPropertiesActionRequest(registration, {
|
||||
downloadId: 'download-1',
|
||||
sessionId: 'session-old',
|
||||
requestId: 99,
|
||||
})).toBe(false);
|
||||
expect(shouldAcceptPropertiesActionRequest(registration, {
|
||||
downloadId: 'download-1',
|
||||
sessionId: 'session-new',
|
||||
requestId: 4,
|
||||
})).toBe(false);
|
||||
expect(shouldAcceptPropertiesActionRequest(registration, {
|
||||
downloadId: 'download-1',
|
||||
sessionId: 'session-new',
|
||||
requestId: 5,
|
||||
})).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects a request whose download binding does not match the window', () => {
|
||||
expect(shouldAcceptPropertiesActionRequest({
|
||||
downloadId: 'download-1',
|
||||
sessionId: 'session-1',
|
||||
latestRequestId: 0,
|
||||
}, {
|
||||
downloadId: 'download-2',
|
||||
sessionId: 'session-1',
|
||||
requestId: 1,
|
||||
})).toBe(false);
|
||||
});
|
||||
|
||||
it('serializes actions per window and continues after an earlier action fails', async () => {
|
||||
const chains = new Map<string, Promise<void>>();
|
||||
const events: string[] = [];
|
||||
let releaseFirst!: () => void;
|
||||
let markFirstStarted!: () => void;
|
||||
const firstGate = new Promise<void>(resolve => { releaseFirst = resolve; });
|
||||
const firstStarted = new Promise<void>(resolve => { markFirstStarted = resolve; });
|
||||
|
||||
const first = enqueuePropertiesAction(chains, 'window:download', async () => {
|
||||
events.push('first-start');
|
||||
markFirstStarted();
|
||||
await firstGate;
|
||||
events.push('first-end');
|
||||
throw new Error('first action failed');
|
||||
});
|
||||
const second = enqueuePropertiesAction(chains, 'window:download', async () => {
|
||||
events.push('second');
|
||||
});
|
||||
|
||||
await firstStarted;
|
||||
expect(events).toEqual(['first-start']);
|
||||
releaseFirst();
|
||||
const results = await Promise.allSettled([first, second]);
|
||||
|
||||
expect(results[0].status).toBe('rejected');
|
||||
expect(results[1].status).toBe('fulfilled');
|
||||
expect(events).toEqual(['first-start', 'first-end', 'second']);
|
||||
expect(chains.size).toBe(0);
|
||||
});
|
||||
|
||||
it('coalesces repeated snapshot requests to one callback per animation frame', () => {
|
||||
const frames = new Map<number, FrameRequestCallback>();
|
||||
const delivered: string[] = [];
|
||||
|
||||
+37
-2
@@ -154,11 +154,13 @@ export const beginExclusivePropertiesAction = (
|
||||
export type PropertiesWindowReady = {
|
||||
windowLabel: string;
|
||||
downloadId: string;
|
||||
sessionId: string;
|
||||
};
|
||||
|
||||
export type PropertiesActionRequest = {
|
||||
windowLabel: string;
|
||||
downloadId: string;
|
||||
sessionId: string;
|
||||
requestId: number;
|
||||
action: PropertiesAction;
|
||||
payload?: PropertiesPatch | { limit: string | null } | { maxPeers: string | null; peerSpeedLimit: string | null };
|
||||
@@ -167,6 +169,7 @@ export type PropertiesActionRequest = {
|
||||
export type PropertiesActionResult = {
|
||||
windowLabel: string;
|
||||
downloadId: string;
|
||||
sessionId: string;
|
||||
requestId: number;
|
||||
ok: boolean;
|
||||
error?: string;
|
||||
@@ -175,10 +178,41 @@ export type PropertiesActionResult = {
|
||||
export type PropertiesSnapshotEvent = {
|
||||
windowLabel: string;
|
||||
downloadId: string;
|
||||
sessionId: string;
|
||||
revision: number;
|
||||
snapshot: PropertiesSnapshot;
|
||||
};
|
||||
|
||||
export type PropertiesWindowRegistration = {
|
||||
downloadId: string;
|
||||
sessionId: string;
|
||||
latestRequestId: number;
|
||||
};
|
||||
|
||||
export const shouldAcceptPropertiesActionRequest = (
|
||||
registration: PropertiesWindowRegistration | undefined,
|
||||
request: Pick<PropertiesActionRequest, 'downloadId' | 'sessionId' | 'requestId'>,
|
||||
): boolean => registration !== undefined
|
||||
&& registration.downloadId === request.downloadId
|
||||
&& registration.sessionId === request.sessionId
|
||||
&& Number.isSafeInteger(request.requestId)
|
||||
&& request.requestId > registration.latestRequestId;
|
||||
|
||||
export const enqueuePropertiesAction = (
|
||||
chains: Map<string, Promise<void>>,
|
||||
key: string,
|
||||
action: () => Promise<void>,
|
||||
): Promise<void> => {
|
||||
const previous = chains.get(key) ?? Promise.resolve();
|
||||
const operation = previous.catch(() => undefined).then(action);
|
||||
let tracked: Promise<void>;
|
||||
tracked = operation.finally(() => {
|
||||
if (chains.get(key) === tracked) chains.delete(key);
|
||||
});
|
||||
chains.set(key, tracked);
|
||||
return tracked;
|
||||
};
|
||||
|
||||
const copyWithoutSecrets = (
|
||||
item: DownloadItem,
|
||||
appearance: DocumentAppearance,
|
||||
@@ -272,11 +306,12 @@ export const createFrameCoalescer = (
|
||||
export const openPropertiesWindow = (downloadId: string): Promise<string> =>
|
||||
invoke('open_download_properties_window', { id: downloadId });
|
||||
|
||||
export const sendPropertiesReady = (): Promise<void> =>
|
||||
invoke('properties_window_send_ready');
|
||||
export const sendPropertiesReady = (sessionId: string): Promise<void> =>
|
||||
invoke('properties_window_send_ready', { sessionId });
|
||||
|
||||
export const sendPropertiesActionRequest = (payload: PropertiesActionRequest): Promise<void> =>
|
||||
invoke('properties_window_send_action', {
|
||||
sessionId: payload.sessionId,
|
||||
requestId: payload.requestId,
|
||||
action: payload.action,
|
||||
payload: payload.payload,
|
||||
|
||||
Reference in New Issue
Block a user