From 9e5fc43bbb719ea99e43af1a3bd819d01b877261 Mon Sep 17 00:00:00 2001 From: Nikolatesla-lj Date: Sat, 2 May 2026 04:39:13 +0800 Subject: [PATCH] fix(browser): clean up closed page webviews --- src/renderer/src/components/Terminal.tsx | 34 ++++++++---- .../browser-webview-cleanup.test.ts | 55 +++++++++++++++++++ .../browser-pane/browser-webview-cleanup.ts | 22 ++++++++ 3 files changed, 99 insertions(+), 12 deletions(-) create mode 100644 src/renderer/src/components/browser-pane/browser-webview-cleanup.test.ts create mode 100644 src/renderer/src/components/browser-pane/browser-webview-cleanup.ts diff --git a/src/renderer/src/components/Terminal.tsx b/src/renderer/src/components/Terminal.tsx index c45cabf8416..cdb2d02d1fe 100644 --- a/src/renderer/src/components/Terminal.tsx +++ b/src/renderer/src/components/Terminal.tsx @@ -31,6 +31,7 @@ import EditorAutosaveController from './editor/EditorAutosaveController' import type { TabGroupLayoutNode } from '../../../shared/types' import BrowserPane, { destroyPersistentWebview } from './browser-pane/BrowserPane' import BrowserPaneOverlayLayer from './browser-pane/BrowserPaneOverlayLayer' +import { collectBrowserWebviewIds } from './browser-pane/browser-webview-cleanup' import { handleSwitchTab, handleSwitchTerminalTab } from '../hooks/ipc-tab-switch' import TabGroupSplitLayout from './tab-group/TabGroupSplitLayout' import { shouldAutoCreateInitialTerminal } from './terminal/initial-terminal' @@ -904,29 +905,38 @@ function Terminal(): React.JSX.Element | null { }) }, []) - // Why: removeWorktree cleans up browser tab state in the store but cannot - // call destroyPersistentWebview (renderer-only DOM code). This subscriber - // detects when browser tabs disappear from a worktree (e.g. worktree deleted) - // and destroys orphaned webview elements to prevent memory leaks. - const prevBrowserTabIdsRef = useRef>(new Set()) + // Why: browser page state can disappear through store-only paths (CLI tab + // close, worktree deletion). The store cannot call destroyPersistentWebview + // because that function owns renderer DOM nodes, so this subscriber tears down + // webviews whose backing page records were removed. + const prevBrowserWebviewIdsRef = useRef>( + collectBrowserWebviewIds( + useAppStore.getState().browserTabsByWorktree, + useAppStore.getState().browserPagesByWorkspace + ) + ) useEffect(() => { let prevBrowserTabs = useAppStore.getState().browserTabsByWorktree + let prevBrowserPages = useAppStore.getState().browserPagesByWorkspace return useAppStore.subscribe((state) => { - if (state.browserTabsByWorktree === prevBrowserTabs) { + if ( + state.browserTabsByWorktree === prevBrowserTabs && + state.browserPagesByWorkspace === prevBrowserPages + ) { return } prevBrowserTabs = state.browserTabsByWorktree - const currentIds = new Set( - Object.values(state.browserTabsByWorktree) - .flat() - .map((tab) => tab.id) + prevBrowserPages = state.browserPagesByWorkspace + const currentIds = collectBrowserWebviewIds( + state.browserTabsByWorktree, + state.browserPagesByWorkspace ) - for (const prevId of prevBrowserTabIdsRef.current) { + for (const prevId of prevBrowserWebviewIdsRef.current) { if (!currentIds.has(prevId)) { destroyPersistentWebview(prevId) } } - prevBrowserTabIdsRef.current = currentIds + prevBrowserWebviewIdsRef.current = currentIds }) }, []) diff --git a/src/renderer/src/components/browser-pane/browser-webview-cleanup.test.ts b/src/renderer/src/components/browser-pane/browser-webview-cleanup.test.ts new file mode 100644 index 00000000000..de439ef80df --- /dev/null +++ b/src/renderer/src/components/browser-pane/browser-webview-cleanup.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, it } from 'vitest' +import { collectBrowserWebviewIds } from './browser-webview-cleanup' +import type { BrowserPage, BrowserWorkspace } from '../../../../shared/types' + +function workspace(id: string): BrowserWorkspace { + return { + id, + worktreeId: 'wt-1', + label: id, + sessionProfileId: null, + pageIds: [], + activePageId: null, + url: 'about:blank', + title: id, + loading: false, + faviconUrl: null, + canGoBack: false, + canGoForward: false, + loadError: null, + createdAt: 1 + } +} + +function page(id: string, workspaceId: string): BrowserPage { + return { + id, + workspaceId, + worktreeId: 'wt-1', + url: 'about:blank', + title: id, + loading: false, + faviconUrl: null, + canGoBack: false, + canGoForward: false, + loadError: null, + createdAt: 1 + } +} + +describe('collectBrowserWebviewIds', () => { + it('tracks browser page ids because webviews are keyed by page id', () => { + const ids = collectBrowserWebviewIds( + { 'wt-1': [workspace('workspace-1')] }, + { 'workspace-1': [page('page-1', 'workspace-1'), page('page-2', 'workspace-1')] } + ) + + expect([...ids].sort()).toEqual(['page-1', 'page-2']) + }) + + it('keeps legacy workspace ids only when no page records exist', () => { + const ids = collectBrowserWebviewIds({ 'wt-1': [workspace('legacy-workspace')] }, {}) + + expect([...ids]).toEqual(['legacy-workspace']) + }) +}) diff --git a/src/renderer/src/components/browser-pane/browser-webview-cleanup.ts b/src/renderer/src/components/browser-pane/browser-webview-cleanup.ts new file mode 100644 index 00000000000..5b9a4c45807 --- /dev/null +++ b/src/renderer/src/components/browser-pane/browser-webview-cleanup.ts @@ -0,0 +1,22 @@ +import type { BrowserPage, BrowserWorkspace } from '../../../../shared/types' + +export function collectBrowserWebviewIds( + browserTabsByWorktree: Record, + browserPagesByWorkspace: Record +): Set { + const ids = new Set() + for (const pages of Object.values(browserPagesByWorkspace)) { + for (const page of pages) { + ids.add(page.id) + } + } + + for (const tabs of Object.values(browserTabsByWorktree)) { + for (const tab of tabs) { + if ((browserPagesByWorkspace[tab.id] ?? []).length === 0) { + ids.add(tab.id) + } + } + } + return ids +}