diff --git a/src/main/browser/browser-manager-guest-navigation-policy.ts b/src/main/browser/browser-manager-guest-navigation-policy.ts index 7c8f8c28e22..d34892206b1 100644 --- a/src/main/browser/browser-manager-guest-navigation-policy.ts +++ b/src/main/browser/browser-manager-guest-navigation-policy.ts @@ -91,12 +91,18 @@ export abstract class BrowserManagerGuestNavigationPolicy extends BrowserManager const didStartNavigationHandler = ( _event: Electron.Event, url: string, - _isInPlace: boolean, + isInPlace: boolean, isMainFrame: boolean ): void => { if (!isMainFrame || isChromiumInternalErrorUrl(url)) { return } + if (isInPlace) { + // Why: a same-document navigation never emits 'did-navigate', so a pending target recorded + // here would never be cleared and would leak into supersededUrls on the next real + // navigation. 'did-navigate-in-page' owns the same-document commit instead. + return + } // Why: getURL() still reports the previous committed URL until this navigation commits, so // every UA writer must read the in-flight target or they disagree about the tab's host. this.startPendingNavigation(guest.id, url) @@ -124,10 +130,26 @@ export abstract class BrowserManagerGuestNavigationPolicy extends BrowserManager this.notifyBrowserGuestStateChanged(guest.id) } + const didNavigateInPageHandler = ( + _event: Electron.Event, + _url: string, + isMainFrame: boolean + ): void => { + if (!isMainFrame) { + return + } + // Why: Electron does not emit 'did-navigate' for a same-document commit, so without this a + // pushed host snapshot keeps the pre-navigation url while a pull reads the new one — the same + // stale-push class this policy's committed-navigation notify fixes for full navigations. + this.pendingNavigationByGuestId.delete(guest.id) + this.notifyBrowserGuestStateChanged(guest.id) + } + guest.on('will-navigate', navigationGuard) guest.on('will-redirect', willRedirectHandler) guest.on('did-start-navigation', didStartNavigationHandler) guest.on('did-navigate', didNavigateHandler) + guest.on('did-navigate-in-page', didNavigateInPageHandler) guest.on('did-fail-load', didFailLoadHandler) const handleDestroyed = (): void => { // Why: guests can die before renderer registration, else attach-time closures leak until shutdown. @@ -146,6 +168,7 @@ export abstract class BrowserManagerGuestNavigationPolicy extends BrowserManager guest.off('will-redirect', willRedirectHandler) guest.off('did-start-navigation', didStartNavigationHandler) guest.off('did-navigate', didNavigateHandler) + guest.off('did-navigate-in-page', didNavigateInPageHandler) guest.off('did-fail-load', didFailLoadHandler) } } 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 9155e2846df..9cb281a83e2 100644 --- a/src/main/browser/browser-manager-load-failure-replay.test.ts +++ b/src/main/browser/browser-manager-load-failure-replay.test.ts @@ -92,6 +92,65 @@ describe('browserManager', () => { expect(stateChanged).toHaveBeenCalledWith('remote-worktree') }) + it('publishes a same-document offscreen navigation, and only for the main frame', () => { + const stateChanged = vi.fn() + const guest = { + id: 607, + 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/app#route') + } + webContentsFromIdMock.mockReturnValue(guest) + browserManager.setBrowserGuestStateChangedListener(stateChanged) + browserManager.registerOffscreenGuest({ + browserPageId: 'spa-page', + worktreeId: 'remote-worktree', + webContentsId: guest.id + }) + stateChanged.mockClear() + // Why this event and not 'did-navigate': Electron never emits a full commit for a + // same-document navigation, so pushState/replaceState/hash routes would otherwise leave the + // published snapshot on the pre-navigation url while a pull reads the new one. + const inPage = guest.on.mock.calls.find(([event]) => event === 'did-navigate-in-page')?.[1] + expect(inPage).toBeTypeOf('function') + inPage!(null, guest.getURL(), false) + expect(stateChanged).not.toHaveBeenCalled() + inPage!(null, guest.getURL(), true) + expect(stateChanged).toHaveBeenCalledWith('remote-worktree') + }) + + it('detaches the same-document navigation listener with the rest of the policy', () => { + const guest = { + id: 608, + 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/app') + } + webContentsFromIdMock.mockReturnValue(guest) + browserManager.registerOffscreenGuest({ + browserPageId: 'detach-page', + worktreeId: 'remote-worktree', + webContentsId: guest.id + }) + const destroyed = guest.on.mock.calls.find(([event]) => event === 'destroyed')?.[1] + expect(destroyed).toBeTypeOf('function') + destroyed!() + // Why assert the pairing: a listener added without a matching off() outlives its guest. + const attached = guest.on.mock.calls.map(([event]) => event).filter((e) => e !== 'destroyed') + const detached = new Set(guest.off.mock.calls.map(([event]) => event)) + for (const event of attached) { + expect(detached.has(event), `${event} was attached but never detached`).toBe(true) + } + }) + it('tracks offscreen load failures for the owning worktree snapshot', () => { const stateChanged = vi.fn() const offscreenGuest = {