fix: address latest CodeRabbit review

This commit is contained in:
2026-04-10 19:22:01 -07:00
parent d81fe87982
commit c9ce337c1a
7 changed files with 54 additions and 34 deletions

View File

@@ -133,13 +133,12 @@ import {
} from './logger'; } from './logger';
import { createWindowTracker as createWindowTrackerCore } from './window-trackers'; import { createWindowTracker as createWindowTrackerCore } from './window-trackers';
import { import {
bindWindowsOverlayAboveMpvNative, bindWindowsOverlayAboveMpv,
clearWindowsOverlayOwnerNative, clearWindowsOverlayOwner,
ensureWindowsOverlayTransparencyNative, ensureWindowsOverlayTransparency,
getWindowsForegroundProcessNameNative, findWindowsMpvTargetWindowHandle,
queryWindowsTargetWindowHandle, getWindowsForegroundProcessName,
setWindowsOverlayOwnerNative, setWindowsOverlayOwner,
shouldUseWindowsTrackerPowershellFallback,
} from './window-trackers/windows-helper'; } from './window-trackers/windows-helper';
import { import {
commandNeedsOverlayStartupPrereqs, commandNeedsOverlayStartupPrereqs,
@@ -1940,18 +1939,9 @@ function resolveWindowsOverlayBindTargetHandle(targetMpvSocketPath?: string | nu
return null; return null;
} }
if (shouldUseWindowsTrackerPowershellFallback()) {
const helperTargetHwnd = queryWindowsTargetWindowHandle({ targetMpvSocketPath });
if (helperTargetHwnd !== null) {
return helperTargetHwnd;
}
}
try { try {
const win32 = require('./window-trackers/win32') as typeof import('./window-trackers/win32'); void targetMpvSocketPath;
const poll = win32.findMpvWindows(); return findWindowsMpvTargetWindowHandle();
const focused = poll.matches.find((m) => m.isForeground);
return focused?.hwnd ?? [...poll.matches].sort((a, b) => b.area - a.area)[0]?.hwnd ?? null;
} catch { } catch {
return null; return null;
} }
@@ -1990,7 +1980,7 @@ async function syncWindowsVisibleOverlayToMpvZOrder(): Promise<boolean> {
const overlayHwnd = getWindowsNativeWindowHandleNumber(mainWindow); const overlayHwnd = getWindowsNativeWindowHandleNumber(mainWindow);
const targetWindowHwnd = resolveWindowsOverlayBindTargetHandle(appState.mpvSocketPath); const targetWindowHwnd = resolveWindowsOverlayBindTargetHandle(appState.mpvSocketPath);
if (targetWindowHwnd !== null && bindWindowsOverlayAboveMpvNative(overlayHwnd, targetWindowHwnd)) { if (targetWindowHwnd !== null && bindWindowsOverlayAboveMpv(overlayHwnd, targetWindowHwnd)) {
(mainWindow as BrowserWindow & { setOpacity?: (opacity: number) => void }).setOpacity?.(1); (mainWindow as BrowserWindow & { setOpacity?: (opacity: number) => void }).setOpacity?.(1);
return true; return true;
} }
@@ -2082,7 +2072,7 @@ function maybePollWindowsVisibleOverlayForegroundProcess(): void {
return; return;
} }
const processName = getWindowsForegroundProcessNameNative(); const processName = getWindowsForegroundProcessName();
const normalizedProcessName = processName?.trim().toLowerCase() ?? null; const normalizedProcessName = processName?.trim().toLowerCase() ?? null;
const previousProcessName = lastWindowsVisibleOverlayForegroundProcessName; const previousProcessName = lastWindowsVisibleOverlayForegroundProcessName;
lastWindowsVisibleOverlayForegroundProcessName = normalizedProcessName; lastWindowsVisibleOverlayForegroundProcessName = normalizedProcessName;
@@ -2234,6 +2224,9 @@ function openRuntimeOptionsPalette(): void {
if (!opened) { if (!opened) {
showMpvOsd('Runtime options overlay unavailable.'); showMpvOsd('Runtime options overlay unavailable.');
} }
}).catch((error) => {
logger.error('Failed to open runtime options overlay.', error);
showMpvOsd('Runtime options overlay unavailable.');
}); });
} }
@@ -4106,7 +4099,7 @@ function createMainWindow(): BrowserWindow {
const window = createMainWindowHandler(); const window = createMainWindowHandler();
if (process.platform === 'win32') { if (process.platform === 'win32') {
const overlayHwnd = getWindowsNativeWindowHandleNumber(window); const overlayHwnd = getWindowsNativeWindowHandleNumber(window);
if (!ensureWindowsOverlayTransparencyNative(overlayHwnd)) { if (!ensureWindowsOverlayTransparency(overlayHwnd)) {
logger.warn('Failed to eagerly extend Windows overlay transparency via koffi'); logger.warn('Failed to eagerly extend Windows overlay transparency via koffi');
} }
} }
@@ -5149,7 +5142,7 @@ const { initializeOverlayRuntime: initializeOverlayRuntimeHandler } =
if (process.platform !== 'win32' || !mainWindow || mainWindow.isDestroyed()) return; if (process.platform !== 'win32' || !mainWindow || mainWindow.isDestroyed()) return;
const overlayHwnd = getWindowsNativeWindowHandleNumber(mainWindow); const overlayHwnd = getWindowsNativeWindowHandleNumber(mainWindow);
const targetWindowHwnd = resolveWindowsOverlayBindTargetHandle(appState.mpvSocketPath); const targetWindowHwnd = resolveWindowsOverlayBindTargetHandle(appState.mpvSocketPath);
if (targetWindowHwnd !== null && bindWindowsOverlayAboveMpvNative(overlayHwnd, targetWindowHwnd)) { if (targetWindowHwnd !== null && bindWindowsOverlayAboveMpv(overlayHwnd, targetWindowHwnd)) {
return; return;
} }
const tracker = appState.windowTracker; const tracker = appState.windowTracker;
@@ -5166,7 +5159,7 @@ const { initializeOverlayRuntime: initializeOverlayRuntimeHandler } =
})() })()
: null; : null;
if (!mpvResult) return; if (!mpvResult) return;
if (!setWindowsOverlayOwnerNative(overlayHwnd, mpvResult.hwnd)) { if (!setWindowsOverlayOwner(overlayHwnd, mpvResult.hwnd)) {
logger.warn('Failed to set overlay owner via koffi'); logger.warn('Failed to set overlay owner via koffi');
} }
}, },
@@ -5174,7 +5167,7 @@ const { initializeOverlayRuntime: initializeOverlayRuntimeHandler } =
const mainWindow = overlayManager.getMainWindow(); const mainWindow = overlayManager.getMainWindow();
if (process.platform !== 'win32' || !mainWindow || mainWindow.isDestroyed()) return; if (process.platform !== 'win32' || !mainWindow || mainWindow.isDestroyed()) return;
const overlayHwnd = getWindowsNativeWindowHandleNumber(mainWindow); const overlayHwnd = getWindowsNativeWindowHandleNumber(mainWindow);
if (!clearWindowsOverlayOwnerNative(overlayHwnd)) { if (!clearWindowsOverlayOwner(overlayHwnd)) {
logger.warn('Failed to clear overlay owner via koffi'); logger.warn('Failed to clear overlay owner via koffi');
} }
}, },

View File

@@ -89,6 +89,9 @@ export function createOverlayModalRuntimeService(
}; };
const isWindowReadyForIpc = (window: BrowserWindow): boolean => { const isWindowReadyForIpc = (window: BrowserWindow): boolean => {
if (window.isDestroyed()) {
return false;
}
if (window.webContents.isLoading()) { if (window.webContents.isLoading()) {
return false; return false;
} }

View File

@@ -16,6 +16,7 @@ test('on will quit cleanup handler runs all cleanup steps', () => {
unregisterAllGlobalShortcuts: () => calls.push('unregister-shortcuts'), unregisterAllGlobalShortcuts: () => calls.push('unregister-shortcuts'),
stopSubtitleWebsocket: () => calls.push('stop-ws'), stopSubtitleWebsocket: () => calls.push('stop-ws'),
stopTexthookerService: () => calls.push('stop-texthooker'), stopTexthookerService: () => calls.push('stop-texthooker'),
clearWindowsVisibleOverlayForegroundPollLoop: () => calls.push('clear-windows-visible-overlay-poll'),
destroyMainOverlayWindow: () => calls.push('destroy-main-overlay-window'), destroyMainOverlayWindow: () => calls.push('destroy-main-overlay-window'),
destroyModalOverlayWindow: () => calls.push('destroy-modal-overlay-window'), destroyModalOverlayWindow: () => calls.push('destroy-modal-overlay-window'),
destroyYomitanParserWindow: () => calls.push('destroy-yomitan-window'), destroyYomitanParserWindow: () => calls.push('destroy-yomitan-window'),
@@ -40,9 +41,10 @@ test('on will quit cleanup handler runs all cleanup steps', () => {
}); });
cleanup(); cleanup();
assert.equal(calls.length, 28); assert.equal(calls.length, 29);
assert.equal(calls[0], 'destroy-tray'); assert.equal(calls[0], 'destroy-tray');
assert.equal(calls[calls.length - 1], 'stop-discord-presence'); assert.equal(calls[calls.length - 1], 'stop-discord-presence');
assert.ok(calls.includes('clear-windows-visible-overlay-poll'));
assert.ok(calls.indexOf('flush-mpv-log') < calls.indexOf('destroy-socket')); assert.ok(calls.indexOf('flush-mpv-log') < calls.indexOf('destroy-socket'));
}); });

View File

@@ -6,6 +6,7 @@ export function createOnWillQuitCleanupHandler(deps: {
unregisterAllGlobalShortcuts: () => void; unregisterAllGlobalShortcuts: () => void;
stopSubtitleWebsocket: () => void; stopSubtitleWebsocket: () => void;
stopTexthookerService: () => void; stopTexthookerService: () => void;
clearWindowsVisibleOverlayForegroundPollLoop: () => void;
destroyMainOverlayWindow: () => void; destroyMainOverlayWindow: () => void;
destroyModalOverlayWindow: () => void; destroyModalOverlayWindow: () => void;
destroyYomitanParserWindow: () => void; destroyYomitanParserWindow: () => void;
@@ -36,6 +37,7 @@ export function createOnWillQuitCleanupHandler(deps: {
deps.unregisterAllGlobalShortcuts(); deps.unregisterAllGlobalShortcuts();
deps.stopSubtitleWebsocket(); deps.stopSubtitleWebsocket();
deps.stopTexthookerService(); deps.stopTexthookerService();
deps.clearWindowsVisibleOverlayForegroundPollLoop();
deps.destroyMainOverlayWindow(); deps.destroyMainOverlayWindow();
deps.destroyModalOverlayWindow(); deps.destroyModalOverlayWindow();
deps.destroyYomitanParserWindow(); deps.destroyYomitanParserWindow();

View File

@@ -87,6 +87,7 @@ test('cleanup deps builder returns handlers that guard optional runtime objects'
assert.ok(calls.includes('destroy-yomitan-settings-window')); assert.ok(calls.includes('destroy-yomitan-settings-window'));
assert.ok(calls.includes('stop-jellyfin-remote')); assert.ok(calls.includes('stop-jellyfin-remote'));
assert.ok(calls.includes('stop-discord-presence')); assert.ok(calls.includes('stop-discord-presence'));
assert.ok(calls.includes('clear-windows-visible-overlay-foreground-poll-loop'));
assert.equal(reconnectTimer, null); assert.equal(reconnectTimer, null);
assert.equal(immersionTracker, null); assert.equal(immersionTracker, null);
}); });

View File

@@ -21,7 +21,11 @@ test('runtime options open prefers dedicated modal window on first attempt', asy
}); });
return true; return true;
}, },
waitForModalOpen: async () => true, waitForModalOpen: async (modal, timeoutMs) => {
assert.equal(modal, 'runtime-options');
assert.equal(timeoutMs, 1500);
return true;
},
logWarn: () => { logWarn: () => {
throw new Error('should not warn on first-attempt success'); throw new Error('should not warn on first-attempt success');
}, },
@@ -52,7 +56,9 @@ test('runtime options open retries after an open timeout', async () => {
}); });
return true; return true;
}, },
waitForModalOpen: async () => { waitForModalOpen: async (modal, timeoutMs) => {
assert.equal(modal, 'runtime-options');
assert.equal(timeoutMs, 1500);
waitCalls += 1; waitCalls += 1;
return waitCalls === 2; return waitCalls === 2;
}, },
@@ -76,13 +82,18 @@ test('runtime options open retries after an open timeout', async () => {
}); });
test('runtime options open fails when no overlay window can be targeted', async () => { test('runtime options open fails when no overlay window can be targeted', async () => {
let waitCalls = 0;
const opened = await openRuntimeOptionsModal({ const opened = await openRuntimeOptionsModal({
ensureOverlayStartupPrereqs: () => {}, ensureOverlayStartupPrereqs: () => {},
ensureOverlayWindowsReadyForVisibilityActions: () => {}, ensureOverlayWindowsReadyForVisibilityActions: () => {},
sendToActiveOverlayWindow: () => false, sendToActiveOverlayWindow: () => false,
waitForModalOpen: async () => true, waitForModalOpen: async () => {
waitCalls += 1;
return true;
},
logWarn: () => {}, logWarn: () => {},
}); });
assert.equal(opened, false); assert.equal(opened, false);
assert.equal(waitCalls, 0);
}); });

View File

@@ -1,4 +1,5 @@
import type { OverlayHostedModal } from '../../shared/ipc/contracts'; import type { OverlayHostedModal } from '../../shared/ipc/contracts';
import { openOverlayHostedModal } from './overlay-hosted-modal-open';
const RUNTIME_OPTIONS_MODAL: OverlayHostedModal = 'runtime-options'; const RUNTIME_OPTIONS_MODAL: OverlayHostedModal = 'runtime-options';
const RUNTIME_OPTIONS_OPEN_TIMEOUT_MS = 1500; const RUNTIME_OPTIONS_OPEN_TIMEOUT_MS = 1500;
@@ -18,12 +19,19 @@ export async function openRuntimeOptionsModal(deps: {
logWarn: (message: string) => void; logWarn: (message: string) => void;
}): Promise<boolean> { }): Promise<boolean> {
const sendOpen = (): boolean => { const sendOpen = (): boolean => {
deps.ensureOverlayStartupPrereqs(); return openOverlayHostedModal(
deps.ensureOverlayWindowsReadyForVisibilityActions(); {
return deps.sendToActiveOverlayWindow('runtime-options:open', undefined, { ensureOverlayStartupPrereqs: deps.ensureOverlayStartupPrereqs,
restoreOnModalClose: RUNTIME_OPTIONS_MODAL, ensureOverlayWindowsReadyForVisibilityActions:
deps.ensureOverlayWindowsReadyForVisibilityActions,
sendToActiveOverlayWindow: deps.sendToActiveOverlayWindow,
},
{
channel: 'runtime-options:open',
modal: RUNTIME_OPTIONS_MODAL,
preferModalWindow: true, preferModalWindow: true,
}); },
);
}; };
if (!sendOpen()) { if (!sendOpen()) {