diff --git a/src/renderer/src/components/browser-pane/BrowserPane.tsx b/src/renderer/src/components/browser-pane/BrowserPane.tsx index 0b493bf6f..bc23d1eb2 100644 --- a/src/renderer/src/components/browser-pane/BrowserPane.tsx +++ b/src/renderer/src/components/browser-pane/BrowserPane.tsx @@ -101,6 +101,7 @@ import { useGrabMode } from './useGrabMode' import { formatGrabPayloadAsText } from './GrabConfirmationSheet' import { formatBrowserAnnotationsAsMarkdown } from './browser-annotation-output' import { isEditableKeyboardTarget } from './browser-keyboard' +import { getBrowserPagesForWorkspace } from './browser-pane-page-selection' import BrowserAddressBar from './BrowserAddressBar' import { BrowserToolbarMenu } from './BrowserToolbarMenu' import BrowserFind from './BrowserFind' @@ -147,6 +148,7 @@ import { onBrowserDriverChange, type BrowserDriverState } from '@/lib/pane-manager/browser-mobile-driver-state' +import { shouldPollChromiumErrorPage } from './chromium-error-page-polling' type BrowserTabPageState = Partial< Pick< @@ -242,7 +244,6 @@ type PendingRemoteBrowserWheel = { dy: number } -const EMPTY_BROWSER_PAGES: BrowserPageState[] = [] const EMPTY_BROWSER_ANNOTATIONS: BrowserPageAnnotation[] = [] const PENDING_ANNOTATION_CARD_HEIGHT = 330 const WHEEL_DELTA_LINE = 1 @@ -776,8 +777,9 @@ export default function BrowserPane({ const activeRuntimeEnvironmentId = useAppStore( (s) => s.settings?.activeRuntimeEnvironmentId ?? null ) - const browserPagesByWorkspace = useAppStore((s) => s.browserPagesByWorkspace) - const browserPages = browserPagesByWorkspace[browserTab.id] ?? EMPTY_BROWSER_PAGES + const browserPages = useAppStore((s) => + getBrowserPagesForWorkspace(s.browserPagesByWorkspace, browserTab.id) + ) const activeBrowserPage = browserPages.find((page) => page.id === browserTab.activePageId) ?? browserPages[0] ?? null const updateBrowserPageState = useAppStore((s) => s.updateBrowserPageState) @@ -3663,7 +3665,7 @@ function BrowserPagePane({ }, [browserTab.url, focusWebviewNow]) useEffect(() => { - if (!browserTab.loading) { + if (!shouldPollChromiumErrorPage({ isActive, loading: browserTab.loading })) { return } @@ -3696,12 +3698,13 @@ function BrowserPagePane({ // Why: some Electron builds paint Chromium's internal chrome-error page // without delivering a timely did-fail-load event to the renderer webview. - // Polling only while the tab is "loading" gives Orca a last-resort path to - // swap the black guest surface for the explicit unreachable-page overlay. + // Polling only while the active tab is "loading" gives Orca a last-resort + // path to swap the black guest surface without waking every retained + // inactive browser pane on a 250ms loop. detectChromiumErrorPage() const intervalId = window.setInterval(detectChromiumErrorPage, 250) return () => window.clearInterval(intervalId) - }, [browserTab.id, browserTab.loading]) + }, [browserTab.id, browserTab.loading, isActive]) const startGrabIntent = useCallback( (nextIntent: GrabIntent): void => { diff --git a/src/renderer/src/components/browser-pane/browser-pane-page-selection.test.ts b/src/renderer/src/components/browser-pane/browser-pane-page-selection.test.ts new file mode 100644 index 000000000..54455d706 --- /dev/null +++ b/src/renderer/src/components/browser-pane/browser-pane-page-selection.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, it } from 'vitest' +import { getBrowserPagesForWorkspace } from './browser-pane-page-selection' +import type { BrowserPage } from '../../../../shared/types' + +function makeBrowserPage(id: string): BrowserPage { + return { + id, + workspaceId: 'workspace-a', + worktreeId: 'worktree-a', + url: `https://example.com/${id}`, + title: id, + loading: false, + faviconUrl: null, + canGoBack: false, + canGoForward: false, + loadError: null, + createdAt: 1 + } +} + +describe('getBrowserPagesForWorkspace', () => { + it('returns only the owning workspace page array so unrelated page updates keep the selector stable', () => { + const pages = [makeBrowserPage('page-1')] + const browserPagesByWorkspace = { + workspaceA: pages, + workspaceB: [makeBrowserPage('page-2')] + } + + expect(getBrowserPagesForWorkspace(browserPagesByWorkspace, 'workspaceA')).toBe(pages) + expect( + getBrowserPagesForWorkspace( + { ...browserPagesByWorkspace, workspaceB: [makeBrowserPage('page-3')] }, + 'workspaceA' + ) + ).toBe(pages) + expect(getBrowserPagesForWorkspace(browserPagesByWorkspace, 'missing')).toBe( + getBrowserPagesForWorkspace({}, 'missing') + ) + }) +}) diff --git a/src/renderer/src/components/browser-pane/browser-pane-page-selection.ts b/src/renderer/src/components/browser-pane/browser-pane-page-selection.ts new file mode 100644 index 000000000..240b0dec6 --- /dev/null +++ b/src/renderer/src/components/browser-pane/browser-pane-page-selection.ts @@ -0,0 +1,10 @@ +import type { BrowserPage } from '../../../../shared/types' + +const EMPTY_BROWSER_PAGES: BrowserPage[] = [] + +export function getBrowserPagesForWorkspace( + browserPagesByWorkspace: Record, + workspaceId: string +): BrowserPage[] { + return browserPagesByWorkspace[workspaceId] ?? EMPTY_BROWSER_PAGES +} diff --git a/src/renderer/src/components/browser-pane/chromium-error-page-polling.test.ts b/src/renderer/src/components/browser-pane/chromium-error-page-polling.test.ts new file mode 100644 index 000000000..f7ad76c4f --- /dev/null +++ b/src/renderer/src/components/browser-pane/chromium-error-page-polling.test.ts @@ -0,0 +1,11 @@ +import { describe, expect, it } from 'vitest' +import { shouldPollChromiumErrorPage } from './chromium-error-page-polling' + +describe('shouldPollChromiumErrorPage', () => { + it('runs the fallback chrome-error poll only for the active loading browser pane', () => { + expect(shouldPollChromiumErrorPage({ isActive: true, loading: true })).toBe(true) + expect(shouldPollChromiumErrorPage({ isActive: false, loading: true })).toBe(false) + expect(shouldPollChromiumErrorPage({ isActive: true, loading: false })).toBe(false) + expect(shouldPollChromiumErrorPage({ isActive: false, loading: false })).toBe(false) + }) +}) diff --git a/src/renderer/src/components/browser-pane/chromium-error-page-polling.ts b/src/renderer/src/components/browser-pane/chromium-error-page-polling.ts new file mode 100644 index 000000000..c9e2f883e --- /dev/null +++ b/src/renderer/src/components/browser-pane/chromium-error-page-polling.ts @@ -0,0 +1,6 @@ +export function shouldPollChromiumErrorPage(args: { + isActive: boolean + loading: boolean +}): boolean { + return args.isActive && args.loading +} diff --git a/src/renderer/src/store/slices/browser.test.ts b/src/renderer/src/store/slices/browser.test.ts index 42e3b60f1..decaefb0a 100644 --- a/src/renderer/src/store/slices/browser.test.ts +++ b/src/renderer/src/store/slices/browser.test.ts @@ -58,6 +58,31 @@ function settingsWithRuntime(id: string): AppState['settings'] { return { activeRuntimeEnvironmentId: id } as AppState['settings'] } +function seedUnifiedBrowserTab( + store: ReturnType, + entityId: string, + label: string +): void { + store.setState({ + unifiedTabsByWorktree: { + 'wt-1': [ + { + id: 'unified-browser-tab', + entityId, + groupId: 'group-1', + worktreeId: 'wt-1', + contentType: 'browser', + label, + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + } + }) +} + function makeAnnotation(pageId: string, id = 'annotation-1'): BrowserPageAnnotation { return { id, @@ -134,6 +159,155 @@ describe('createBrowserSlice annotations', () => { expect(store.getState().browserAnnotationsByPageId[pageId]).toBeUndefined() }) + it('preserves browser map references when a page-state update is unchanged', () => { + const store = createTestStore() + const tab = store.getState().createBrowserTab('wt-1', 'https://example.com', { + title: 'Example' + }) + const pageId = tab.activePageId + if (!pageId) { + throw new Error('Expected a new browser page') + } + const page = store.getState().browserPagesByWorkspace[tab.id]?.[0] + if (!page) { + throw new Error('Expected page state') + } + const browserPagesByWorkspace = store.getState().browserPagesByWorkspace + const browserTabsByWorktree = store.getState().browserTabsByWorktree + + store.getState().updateBrowserPageState(pageId, { + title: page.title, + loading: page.loading, + faviconUrl: page.faviconUrl, + canGoBack: page.canGoBack, + canGoForward: page.canGoForward, + loadError: page.loadError + }) + + expect(store.getState().browserPagesByWorkspace).toBe(browserPagesByWorkspace) + expect(store.getState().browserTabsByWorktree).toBe(browserTabsByWorktree) + }) + + it('repairs a stale active browser unified-tab label on an otherwise unchanged title update', () => { + const store = createTestStore() + const tab = store.getState().createBrowserTab('wt-1', 'https://example.com', { + title: 'Example' + }) + const pageId = tab.activePageId + if (!pageId) { + throw new Error('Expected a new browser page') + } + seedUnifiedBrowserTab(store, tab.id, 'Stale label') + const browserPagesByWorkspace = store.getState().browserPagesByWorkspace + const browserTabsByWorktree = store.getState().browserTabsByWorktree + + store.getState().updateBrowserPageState(pageId, { title: 'Example' }) + + expect(store.getState().unifiedTabsByWorktree['wt-1']?.[0]?.label).toBe('Example') + expect(store.getState().browserPagesByWorkspace).toBe(browserPagesByWorkspace) + expect(store.getState().browserTabsByWorktree).toBe(browserTabsByWorktree) + }) + + it('repairs stale active browser workspace metadata on an otherwise unchanged page update', () => { + const store = createTestStore() + const tab = store.getState().createBrowserTab('wt-1', 'https://example.com', { + title: 'Example' + }) + const pageId = tab.activePageId + if (!pageId) { + throw new Error('Expected a new browser page') + } + store.setState((state) => ({ + browserTabsByWorktree: { + ...state.browserTabsByWorktree, + 'wt-1': (state.browserTabsByWorktree['wt-1'] ?? []).map((workspace) => + workspace.id === tab.id + ? { + ...workspace, + title: 'Stale workspace', + url: 'https://stale.example.com', + loading: false, + canGoBack: true, + canGoForward: true + } + : workspace + ) + } + })) + const browserPagesByWorkspace = store.getState().browserPagesByWorkspace + + store.getState().updateBrowserPageState(pageId, { title: 'Example' }) + + const repaired = store + .getState() + .browserTabsByWorktree['wt-1']?.find((entry) => entry.id === tab.id) + expect(repaired).toMatchObject({ + title: 'Example', + url: 'https://example.com', + loading: true, + canGoBack: false, + canGoForward: false + }) + expect(store.getState().browserPagesByWorkspace).toBe(browserPagesByWorkspace) + }) + + it('updates the active browser unified-tab label without a second tab-label write', () => { + const store = createTestStore() + const tab = store.getState().createBrowserTab('wt-1', 'https://example.com', { + title: 'Example' + }) + const pageId = tab.activePageId + if (!pageId) { + throw new Error('Expected a new browser page') + } + seedUnifiedBrowserTab(store, tab.id, 'Example') + + store.getState().updateBrowserPageState(pageId, { title: 'Next', loading: false }) + + expect(store.getState().unifiedTabsByWorktree['wt-1']?.[0]?.label).toBe('Next') + expect(store.getState().setTabLabel).not.toHaveBeenCalled() + }) + + it('updates inactive browser pages without relabeling or rebuilding the workspace map', () => { + const store = createTestStore() + const tab = store.getState().createBrowserTab('wt-1', 'https://example.com', { + title: 'Example' + }) + const activePageId = tab.activePageId + if (!activePageId) { + throw new Error('Expected a new browser page') + } + const inactivePage = store + .getState() + .createBrowserPage(tab.id, 'https://example.com/inactive', { + title: 'Inactive', + activate: false + }) + if (!inactivePage) { + throw new Error('Expected inactive browser page') + } + seedUnifiedBrowserTab(store, tab.id, 'Example') + const browserPagesByWorkspace = store.getState().browserPagesByWorkspace + const browserTabsByWorktree = store.getState().browserTabsByWorktree + + store.getState().updateBrowserPageState(inactivePage.id, { + title: 'Inactive next', + loading: false + }) + + expect(store.getState().browserPagesByWorkspace).not.toBe(browserPagesByWorkspace) + expect(store.getState().browserTabsByWorktree).toBe(browserTabsByWorktree) + expect( + store.getState().browserPagesByWorkspace[tab.id]?.find((page) => page.id === inactivePage.id) + ).toMatchObject({ title: 'Inactive next', loading: false }) + expect(store.getState().browserTabsByWorktree['wt-1']?.[0]).toMatchObject({ + activePageId, + title: 'Example' + }) + expect(store.getState().unifiedTabsByWorktree['wt-1']?.[0]?.label).toBe('Example') + expect(store.getState().setTabLabel).not.toHaveBeenCalled() + }) + it('caps stored browser annotations per page', () => { const store = createTestStore() const tab = store.getState().createBrowserTab('wt-1', 'https://example.com') diff --git a/src/renderer/src/store/slices/browser.ts b/src/renderer/src/store/slices/browser.ts index 978dd4c95..8176fbeaf 100644 --- a/src/renderer/src/store/slices/browser.ts +++ b/src/renderer/src/store/slices/browser.ts @@ -317,6 +317,26 @@ function mirrorWorkspaceFromActivePage( } } +function browserWorkspaceMirrorFieldsEqual( + workspace: BrowserWorkspace, + mirrored: BrowserWorkspace +): boolean { + const workspacePageIds = workspace.pageIds ?? [] + const mirroredPageIds = mirrored.pageIds ?? [] + return ( + workspace.activePageId === mirrored.activePageId && + workspacePageIds.length === mirroredPageIds.length && + workspacePageIds.every((pageId, index) => pageId === mirroredPageIds[index]) && + workspace.url === mirrored.url && + workspace.title === mirrored.title && + workspace.loading === mirrored.loading && + workspace.faviconUrl === mirrored.faviconUrl && + workspace.canGoBack === mirrored.canGoBack && + workspace.canGoForward === mirrored.canGoForward && + workspace.loadError === mirrored.loadError + ) +} + function getFallbackTabTypeForWorktree( worktreeId: string, openFiles: AppState['openFiles'], @@ -335,26 +355,46 @@ function getFallbackTabTypeForWorktree( return 'terminal' } +const browserWorkspaceByIdCache = new WeakMap< + Record, + Map +>() +const browserPageByIdCache = new WeakMap, Map>() + function findWorkspace( browserTabsByWorktree: Record, workspaceId: string ): BrowserWorkspace | null { - return ( - Object.values(browserTabsByWorktree) - .flat() - .find((workspace) => workspace.id === workspaceId) ?? null - ) + const cached = browserWorkspaceByIdCache.get(browserTabsByWorktree) + if (cached) { + return cached.get(workspaceId) ?? null + } + const workspaceById = new Map() + for (const workspaces of Object.values(browserTabsByWorktree)) { + for (const workspace of workspaces) { + workspaceById.set(workspace.id, workspace) + } + } + browserWorkspaceByIdCache.set(browserTabsByWorktree, workspaceById) + return workspaceById.get(workspaceId) ?? null } function findPage( browserPagesByWorkspace: Record, pageId: string ): BrowserPage | null { - return ( - Object.values(browserPagesByWorkspace) - .flat() - .find((page) => page.id === pageId) ?? null - ) + const cached = browserPageByIdCache.get(browserPagesByWorkspace) + if (cached) { + return cached.get(pageId) ?? null + } + const pageById = new Map() + for (const pages of Object.values(browserPagesByWorkspace)) { + for (const page of pages) { + pageById.set(page.id, page) + } + } + browserPageByIdCache.set(browserPagesByWorkspace, pageById) + return pageById.get(pageId) ?? null } export const createBrowserSlice: StateCreator = (set, get) => ({ @@ -1094,48 +1134,90 @@ export const createBrowserSlice: StateCreator = if (!workspace) { return s } - const nextPages = (s.browserPagesByWorkspace[workspace.id] ?? []).map((entry) => - entry.id === pageId - ? { - ...entry, - title: - updates.title === undefined - ? entry.title - : normalizeBrowserTitle(updates.title, entry.url), - loading: updates.loading ?? entry.loading, - faviconUrl: updates.faviconUrl === undefined ? entry.faviconUrl : updates.faviconUrl, - canGoBack: updates.canGoBack ?? entry.canGoBack, - canGoForward: updates.canGoForward ?? entry.canGoForward, - loadError: updates.loadError === undefined ? entry.loadError : updates.loadError - } - : entry - ) + const nextPage = { + ...page, + title: + updates.title === undefined ? page.title : normalizeBrowserTitle(updates.title, page.url), + loading: updates.loading ?? page.loading, + faviconUrl: updates.faviconUrl === undefined ? page.faviconUrl : updates.faviconUrl, + canGoBack: updates.canGoBack ?? page.canGoBack, + canGoForward: updates.canGoForward ?? page.canGoForward, + loadError: updates.loadError === undefined ? page.loadError : updates.loadError + } + const unifiedTabs = s.unifiedTabsByWorktree[workspace.worktreeId] ?? [] + const unifiedIndex = + workspace.activePageId === pageId && updates.title !== undefined + ? unifiedTabs.findIndex( + (entry) => entry.contentType === 'browser' && entry.entityId === workspace.id + ) + : -1 + const unifiedLabelNeedsRepair = + unifiedIndex !== -1 && unifiedTabs[unifiedIndex]?.label !== nextPage.title + const pageStateUnchanged = + nextPage.title === page.title && + nextPage.loading === page.loading && + nextPage.faviconUrl === page.faviconUrl && + nextPage.canGoBack === page.canGoBack && + nextPage.canGoForward === page.canGoForward && + nextPage.loadError === page.loadError + const currentPages = s.browserPagesByWorkspace[workspace.id] ?? [] + const mirroredWorkspace = pageStateUnchanged + ? mirrorWorkspaceFromActivePage(workspace, currentPages) + : null + const workspaceNeedsRepair = + mirroredWorkspace !== null && + !browserWorkspaceMirrorFieldsEqual(workspace, mirroredWorkspace) + if (pageStateUnchanged && !unifiedLabelNeedsRepair && !workspaceNeedsRepair) { + return s + } + if (pageStateUnchanged) { + const nextState: Partial = {} + if (workspaceNeedsRepair && mirroredWorkspace) { + nextState.browserTabsByWorktree = { + ...s.browserTabsByWorktree, + [workspace.worktreeId]: (s.browserTabsByWorktree[workspace.worktreeId] ?? []).map( + (tab) => (tab.id === workspace.id ? mirroredWorkspace : tab) + ) + } + } + if (unifiedLabelNeedsRepair) { + nextState.unifiedTabsByWorktree = { + ...s.unifiedTabsByWorktree, + [workspace.worktreeId]: unifiedTabs.map((entry, index) => + index === unifiedIndex ? { ...entry, label: nextPage.title } : entry + ) + } + } + return nextState + } + const nextPages = currentPages.map((entry) => (entry.id === pageId ? nextPage : entry)) const nextWorkspace = mirrorWorkspaceFromActivePage(workspace, nextPages) - return { + const nextState: Partial = { browserPagesByWorkspace: { ...s.browserPagesByWorkspace, [workspace.id]: nextPages - }, - browserTabsByWorktree: { + } + } + if (!browserWorkspaceMirrorFieldsEqual(workspace, nextWorkspace)) { + nextState.browserTabsByWorktree = { ...s.browserTabsByWorktree, [workspace.worktreeId]: (s.browserTabsByWorktree[workspace.worktreeId] ?? []).map((tab) => tab.id === workspace.id ? nextWorkspace : tab ) } } + if (workspace.activePageId === pageId && updates.title !== undefined && unifiedIndex !== -1) { + if (unifiedLabelNeedsRepair || unifiedTabs[unifiedIndex]?.label !== nextWorkspace.title) { + nextState.unifiedTabsByWorktree = { + ...s.unifiedTabsByWorktree, + [workspace.worktreeId]: unifiedTabs.map((entry, index) => + index === unifiedIndex ? { ...entry, label: nextWorkspace.title } : entry + ) + } + } + } + return nextState }) - - const page = findPage(get().browserPagesByWorkspace, pageId) - if (!page) { - return - } - const workspace = findWorkspace(get().browserTabsByWorktree, page.workspaceId) - const item = Object.values(get().unifiedTabsByWorktree) - .flat() - .find((entry) => entry.contentType === 'browser' && entry.entityId === page.workspaceId) - if (item && workspace && workspace.activePageId === pageId && updates.title) { - get().setTabLabel(item.id, workspace.title) - } }, setBrowserTabUrl: (pageId, url) => get().setBrowserPageUrl(pageId, url),