From ff6f0be3d493f28e6ac2a0bb6bbfbba3001fe142 Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Fri, 11 Sep 2026 20:58:23 -0700 Subject: [PATCH] fix(browser): publish a committed navigation, and mount remote background pages Two defects from the same #19326 regression. Each is necessary; neither is sufficient. A 2x2 ablation is in the PR body. Missing publication edge: the offscreen guest policy notified the runtime on every navigation FAILURE path but not on a successful main-frame commit, so a page created blank and then navigated never invalidated the published snapshot. A pull (session.tabs.list) read the live WebContents and looked correct, while the pushed snapshot clients actually apply stayed at about:blank forever. Successful commit now uses the same worktree-scoped notifier the failure paths already use. Missing mount admission: #19326 deferred inactive browser pages behind a paintability gate; #19633 repaired that for local pages by admitting explicitly created ones, but scoped admission to local URL pages. A runtime-backed page opened in the background is never the active tab, has no local guest to retain, and was never admitted, so its pane never mounted. Document previews stay excluded. Guards are vitest, not e2e: e2e is not a required check in this repo. --- ...browser-manager-guest-navigation-policy.ts | 1 + ...rowser-manager-load-failure-replay.test.ts | 26 ++++++++ ...browser-background-mount-admission.test.ts | 60 +++++++++++++++++++ .../browser/browser-page-create-actions.ts | 4 +- .../slices/browser/browser-tab-actions.ts | 4 +- 5 files changed, 91 insertions(+), 4 deletions(-) create mode 100644 src/renderer/src/store/slices/browser-background-mount-admission.test.ts diff --git a/src/main/browser/browser-manager-guest-navigation-policy.ts b/src/main/browser/browser-manager-guest-navigation-policy.ts index abacd268640..7c8f8c28e22 100644 --- a/src/main/browser/browser-manager-guest-navigation-policy.ts +++ b/src/main/browser/browser-manager-guest-navigation-policy.ts @@ -121,6 +121,7 @@ export abstract class BrowserManagerGuestNavigationPolicy extends BrowserManager // Why: a committed nav makes the did-start-navigation stash obsolete; drop it so a later ERR_ABORTED can't restore an error over it. this.clearedLoadErrorsByGuestId.delete(guest.id) this.certificateTrustController?.onMainFrameNavigationCommitted(guest.id, url) + this.notifyBrowserGuestStateChanged(guest.id) } guest.on('will-navigate', navigationGuard) diff --git a/src/main/browser/browser-manager-load-failure-replay.test.ts b/src/main/browser/browser-manager-load-failure-replay.test.ts index 569771f2ffa..9155e2846df 100644 --- a/src/main/browser/browser-manager-load-failure-replay.test.ts +++ b/src/main/browser/browser-manager-load-failure-replay.test.ts @@ -66,6 +66,32 @@ describe('browserManager', () => { vi.useRealTimers() }) + it('publishes successful offscreen navigation to the owning worktree', () => { + const stateChanged = vi.fn() + const guest = { + id: 606, + isDestroyed: vi.fn(() => false), + getType: vi.fn(() => 'window'), + setBackgroundThrottling: vi.fn(), + setWindowOpenHandler: vi.fn(), + on: vi.fn(), + off: vi.fn(), + getURL: vi.fn(() => 'https://remote.test/link') + } + webContentsFromIdMock.mockReturnValue(guest) + browserManager.setBrowserGuestStateChangedListener(stateChanged) + browserManager.registerOffscreenGuest({ + browserPageId: 'background-page', + worktreeId: 'remote-worktree', + webContentsId: guest.id + }) + stateChanged.mockClear() + const committed = guest.on.mock.calls.find(([event]) => event === 'did-navigate')?.[1] + expect(committed).toBeTypeOf('function') + committed!(null, guest.getURL()) + expect(stateChanged).toHaveBeenCalledWith('remote-worktree') + }) + it('tracks offscreen load failures for the owning worktree snapshot', () => { const stateChanged = vi.fn() const offscreenGuest = { diff --git a/src/renderer/src/store/slices/browser-background-mount-admission.test.ts b/src/renderer/src/store/slices/browser-background-mount-admission.test.ts new file mode 100644 index 00000000000..2c147351ce0 --- /dev/null +++ b/src/renderer/src/store/slices/browser-background-mount-admission.test.ts @@ -0,0 +1,60 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { + isBrowserPageMountAdmitted, + releaseBrowserPageMount +} from '../../components/browser-pane/host-guest/browser-page-mount-admission' +import { createBrowserMockApi, createTestStore } from './browser-slice-test-harness' + +vi.mock('@/runtime/web-runtime-session', () => ({ + createWebRuntimeSessionBrowserTab: vi.fn() +})) + +const pagesToRelease: string[] = [] + +// @ts-expect-error test window mock +globalThis.window = { api: createBrowserMockApi(vi.fn()) } + +afterEach(() => { + pagesToRelease.splice(0).forEach(releaseBrowserPageMount) +}) + +describe('new background URL page admission', () => { + it.each([undefined, 'env-1'])( + 'admits a new workspace page on runtime %s without activation', + (environmentId) => { + const store = createTestStore() + const workspace = store.getState().createBrowserTab('wt-1', 'https://example.test/link', { + activate: false, + browserRuntimeEnvironmentId: environmentId + }) + const page = store.getState().browserPagesByWorkspace[workspace.id][0] + pagesToRelease.push(page.id) + + expect(isBrowserPageMountAdmitted(page.id)).toBe(true) + expect(store.getState().activeTabType).toBe('terminal') + expect(store.getState().createUnifiedTab).toHaveBeenCalledWith( + 'wt-1', + 'browser', + expect.objectContaining({ activate: false }) + ) + } + ) + + it.each([undefined, 'env-1'])( + 'admits a second background page on runtime %s without selecting it', + (environmentId) => { + const store = createTestStore() + const workspace = store.getState().createBrowserTab('wt-1', 'https://example.test/first') + const firstPageId = workspace.activePageId! + pagesToRelease.push(firstPageId) + const page = store.getState().createBrowserPage(workspace.id, 'https://example.test/link', { + activate: false, + browserRuntimeEnvironmentId: environmentId + })! + pagesToRelease.push(page.id) + + expect(isBrowserPageMountAdmitted(page.id)).toBe(true) + expect(store.getState().browserTabsByWorktree['wt-1'][0].activePageId).toBe(firstPageId) + } + ) +}) diff --git a/src/renderer/src/store/slices/browser/browser-page-create-actions.ts b/src/renderer/src/store/slices/browser/browser-page-create-actions.ts index 261299ce302..81b7d702453 100644 --- a/src/renderer/src/store/slices/browser/browser-page-create-actions.ts +++ b/src/renderer/src/store/slices/browser/browser-page-create-actions.ts @@ -37,8 +37,8 @@ export function createBrowserPageCreateActions( undefined, options?.docLocation ) - // Runtime-backed pages are streamed, not locally driven, but they still need the pane - // mounted to start that stream — #19633 admitted only local pages and left them deferred. + // Runtime-backed pages need their pane mounted to exist as a background tab at all; + // the stream itself still opens only on activation. #19633 admitted local pages only. if (!options?.docLocation) { admitBrowserPageMount(page.id) } diff --git a/src/renderer/src/store/slices/browser/browser-tab-actions.ts b/src/renderer/src/store/slices/browser/browser-tab-actions.ts index 9b51037b650..10be755f531 100644 --- a/src/renderer/src/store/slices/browser/browser-tab-actions.ts +++ b/src/renderer/src/store/slices/browser/browser-tab-actions.ts @@ -49,8 +49,8 @@ export function createBrowserTabActions( browserPageId, options?.docLocation ) - // Runtime-backed pages are streamed, not locally driven, but they still need the pane - // mounted to start that stream — #19633 admitted only local pages and left them deferred. + // Runtime-backed pages need their pane mounted to exist as a background tab at all; + // the stream itself still opens only on activation. #19633 admitted local pages only. if (!options?.docLocation) { admitBrowserPageMount(page.id) }