diff --git a/src/main/browser/extensions/tabs-and-windows/browser-extension-discarded-tabs.test.ts b/src/main/browser/extensions/tabs-and-windows/browser-extension-discarded-tabs.test.ts index 482e93828e4..d8dde11508b 100644 --- a/src/main/browser/extensions/tabs-and-windows/browser-extension-discarded-tabs.test.ts +++ b/src/main/browser/extensions/tabs-and-windows/browser-extension-discarded-tabs.test.ts @@ -265,6 +265,10 @@ beforeEach(() => { } }) discardedTabs.forgetBrowserPageGuestsGone() + // Retire ids an earlier test handed out, so each test starts with none listed. + for (const pageId of ['live', 'cold', 'doc', 'notes', 'job', 'remote', 'streamed']) { + discardedTabs.takeDiscardedBrowserTabId(pageId) + } registerTabsNamespace() guest(11, mocks.sessions.default, 'https://live.test/') mocks.pageByGuest.set(11, 'live') @@ -347,6 +351,24 @@ describe('restored browser pages without a guest are discarded tabs', () => { expect(await urls()).toContain('https://live.test/') }) + it('showing one restored page gives no other page a tab id extensions never saw', async () => { + recordActiveBrowserPage('cold') + guest(12, mocks.sessions.default, 'https://cold.test/') + mocks.pageByGuest.set(12, 'cold') + announceRegisteredBrowserExtensionTab('cold', 12) + saved.local.browserPagesByWorkspace = Object.fromEntries( + Object.entries(saved.local.browserPagesByWorkspace ?? {}).map(([id, pages]) => [ + id, + pages.filter((entry) => entry.id !== 'notes') + ]) + ) + writeListener?.('local') + expect(events('onRemoved')).toEqual([]) + expect((await query({ discarded: true })).map((entry) => entry.url)).toEqual([ + 'https://remote.test/' + ]) + }) + it('reports a discarded tab closed once, when the session save drops its page', async () => { const [cold] = await query({ discarded: true }) saved.local.browserPagesByWorkspace = Object.fromEntries( diff --git a/src/main/browser/extensions/tabs-and-windows/browser-extension-discarded-tabs.ts b/src/main/browser/extensions/tabs-and-windows/browser-extension-discarded-tabs.ts index 7f18c6b46cb..ee019d54609 100644 --- a/src/main/browser/extensions/tabs-and-windows/browser-extension-discarded-tabs.ts +++ b/src/main/browser/extensions/tabs-and-windows/browser-extension-discarded-tabs.ts @@ -1,7 +1,6 @@ import { webContents } from 'electron' import type { Store } from '../../../persistence/loading-store/store' -import type { BrowserWorkspace } from '../../../../shared/browser-workspace-types' -import type { WorkspaceSessionState } from '../../../../shared/workspace-session/workspace-session-state-types' +import type { BrowserPage, BrowserWorkspace } from '../../../../shared/browser-workspace-types' // oxlint-disable-next-line import/no-cycle -- browser subsystem singleton hub (browserManager); breaking it needs DI and test-mock updates, left for a human import { browserManager } from '../../guest-lifecycle/manager/browser-manager' import { emitBrowserExtensionEvent } from '../api-runtime/browser-extension-api-host' @@ -54,21 +53,52 @@ function extensionSessionFor(tab: BrowserWorkspace): Electron.Session | null { return profile ? getBrowserExtensionSessionForProfile(profile.id) : null } -function savedPagesIn(state: WorkspaceSessionState): Omit[] { - return Object.values(state.browserTabsByWorktree ?? {}).flatMap((tabs) => - tabs.flatMap((tab) => { - const session = extensionSessionFor(tab) - if (!session) { - return [] +/** + * Each saved web page with the session its guest would load in, in host order (a page id saved + * under two hosts comes first from the first). Stops once `visit` returns true. + */ +function visitSavedPages( + from: SavedBrowserTabsSource, + visit: (page: BrowserPage, session: Electron.Session) => boolean | void +): void { + for (const hostId of from.getWorkspaceSessionHostIds()) { + const state = from.getWorkspaceSession(hostId) + for (const tabs of Object.values(state.browserTabsByWorktree ?? {})) { + for (const tab of tabs) { + const session = extensionSessionFor(tab) + if (!session) { + continue + } + for (const page of state.browserPagesByWorkspace?.[tab.id] ?? []) { + // A document page is no web page; a runtime-hosted page streams, never a local guest. + if ( + page.docLocation || + (page.remoteBrowserPageId && !page.remoteBrowserPageClientHosted) + ) { + continue + } + if (visit(page, session)) { + return + } + } } - return (state.browserPagesByWorkspace?.[tab.id] ?? []).flatMap((page) => - // A document page is no web page; a runtime-hosted page streams, never a local guest. - page.docLocation || (page.remoteBrowserPageId && !page.remoteBrowserPageClientHosted) - ? [] - : [{ browserPageId: page.id, url: page.url, title: page.title, session }] - ) - }) - ) + } + } +} + +function withTabId(page: BrowserPage, session: Electron.Session): DiscardedBrowserTab { + let assigned = assignedByPageId.get(page.id) + if (!assigned) { + assigned = { tabId: nextTabId++, session } + assignedByPageId.set(page.id, assigned) + } + return { + browserPageId: page.id, + url: page.url, + title: page.title, + session, + tabId: assigned.tabId + } } function savedPageIdsIn(hostIds: readonly string[]): Set { @@ -109,11 +139,17 @@ export function setSavedBrowserTabsSource(next: SavedBrowserTabsSource | null): return } const allHostIds = next.getWorkspaceSessionHostIds() - const written = savedPageIdsIn(hostId ? [hostId] : allHostIds) const anywhere = savedPageIdsIn(allHostIds) - for (const pageId of unloadedPageIds) { - if (written.has(pageId) || !anywhere.has(pageId)) { - unloadedPageIds.delete(pageId) + if (unloadedPageIds.size > 0) { + // Why reuse: a write to every host, or to the only one, saves exactly `anywhere`. + const written = + !hostId || (allHostIds.length === 1 && allHostIds[0] === hostId) + ? anywhere + : savedPageIdsIn([hostId]) + for (const pageId of unloadedPageIds) { + if (written.has(pageId) || !anywhere.has(pageId)) { + unloadedPageIds.delete(pageId) + } } } // A listed page no save keeps was closed before its guest ever loaded. @@ -133,29 +169,32 @@ export function listDiscardedBrowserTabs(): DiscardedBrowserTab[] { } const seen = new Set() const tabs: DiscardedBrowserTab[] = [] - for (const hostId of source.getWorkspaceSessionHostIds()) { - for (const page of savedPagesIn(source.getWorkspaceSession(hostId))) { - const { browserPageId } = page - if (seen.has(browserPageId) || unloadedPageIds.has(browserPageId)) { - continue - } - seen.add(browserPageId) - if (hasLiveGuest(browserPageId)) { - continue - } - let assigned = assignedByPageId.get(browserPageId) - if (!assigned) { - assigned = { tabId: nextTabId++, session: page.session } - assignedByPageId.set(browserPageId, assigned) - } - tabs.push({ ...page, tabId: assigned.tabId }) + visitSavedPages(source, (page, session) => { + if (seen.has(page.id) || unloadedPageIds.has(page.id)) { + return } - } + seen.add(page.id) + if (!hasLiveGuest(page.id)) { + tabs.push(withTabId(page, session)) + } + }) return tabs } +/** Looks up one page without listing (and numbering) every other discarded tab. */ export function findDiscardedBrowserTabForPage(browserPageId: string): DiscardedBrowserTab | null { - return listDiscardedBrowserTabs().find((tab) => tab.browserPageId === browserPageId) ?? null + if (!source || unloadedPageIds.has(browserPageId) || hasLiveGuest(browserPageId)) { + return null + } + let found: DiscardedBrowserTab | null = null + visitSavedPages(source, (page, session) => { + if (page.id !== browserPageId) { + return false + } + found = withTabId(page, session) + return true + }) + return found } /** A page's guest registered: the synthetic id it was listed under (if any) retires with it. */