Improve browser performance (#41830)

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(

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<DiscardedBrowserTab, 'tabId'>[] {
-  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<string> {
@@ -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<string>()
   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. */