Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/main/browser/browser-client-host-attach-request.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,28 @@ describe('browser client host attach request', () => {
expect(attach.params).not.toHaveProperty('leaseReconnectProtocolVersion')
})

it('asks for returning-host reclaim only beside page reconciliation', () => {
const options = {
pairing,
authorityRuntimeId: 'runtime-a',
browserHostClientId: 'host-a',
hostCapabilities: ['webview'],
pageCommandProtocolVersion: 1 as const,
onPageCommand: vi.fn(() => ({ status: 'completed' as const })),
pageInventoryProtocolVersion: 1 as const,
getPageInventory: () => [],
returningHostReclaimProtocolVersion: 1 as const
}

expect(
createBrowserClientHostAttachRequest({ ...options, pageReconciliationProtocolVersion: 1 })
.params
).toMatchObject({ returningHostReclaimProtocolVersion: 1 })
expect(createBrowserClientHostAttachRequest(options).params).not.toHaveProperty(
'returningHostReclaimProtocolVersion'
)
})

it('includes reconnect negotiation only beside an encoded inventory snapshot', () => {
const attach = createBrowserClientHostAttachRequest({
pairing,
Expand Down
7 changes: 6 additions & 1 deletion src/main/browser/browser-client-host-attach-request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ export function createBrowserClientHostAttachRequest(
const fileChannelProtocolVersion = pageCommandProtocolVersion
? options.fileChannelProtocolVersion
: undefined
const returningHostReclaimProtocolVersion = pageReconciliationProtocolVersion
? options.returningHostReclaimProtocolVersion
: undefined
const params = BrowserClientHostAttachParams.parse({
authorityRuntimeId: options.authorityRuntimeId,
browserHostClientId: options.browserHostClientId,
Expand All @@ -63,14 +66,16 @@ export function createBrowserClientHostAttachRequest(
: {}),
...(leaseReconnectProtocolVersion ? { leaseReconnectProtocolVersion } : {}),
...(pageReconciliationProtocolVersion ? { pageReconciliationProtocolVersion } : {}),
...(fileChannelProtocolVersion ? { fileChannelProtocolVersion } : {})
...(fileChannelProtocolVersion ? { fileChannelProtocolVersion } : {}),
...(returningHostReclaimProtocolVersion ? { returningHostReclaimProtocolVersion } : {})
})
return {
pageCommandProtocolVersion,
pageInventoryProtocolVersion,
leaseReconnectProtocolVersion,
pageReconciliationProtocolVersion,
fileChannelProtocolVersion,
returningHostReclaimProtocolVersion,
params
}
}
36 changes: 0 additions & 36 deletions src/main/browser/browser-client-host-authority-replacement-wait.ts

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { describe, expect, it } from 'vitest'
import { BROWSER_CLIENT_HOST_AUTHORITY_MISMATCH_CODE } from '../../shared/browser-client-host-protocol'
import { isBrowserClientHostAuthorityReplaced } from './browser-client-host-authority-replacement'
import { BrowserClientHostAuthorityReplacementWait } from './browser-client-host-authority-replacement-wait'

function errorWithCode(message: string, code: unknown): Error {
return Object.assign(new Error(message), { code })
Expand Down Expand Up @@ -72,77 +71,3 @@ describe('browser client host authority replacement', () => {
)
})
})

describe('BrowserClientHostAuthorityReplacementWait', () => {
beforeEach(() => {
vi.useFakeTimers()
})

afterEach(() => {
vi.useRealTimers()
})

it('expires exactly once, at the grace deadline', () => {
const wait = new BrowserClientHostAuthorityReplacementWait(1_000)
const expire = vi.fn()

wait.arm(expire)

expect(wait.armed).toBe(true)
vi.advanceTimersByTime(999)
expect(expire).not.toHaveBeenCalled()
vi.advanceTimersByTime(1)
expect(expire).toHaveBeenCalledOnce()
expect(wait.armed).toBe(false)
vi.advanceTimersByTime(10_000)
expect(expire).toHaveBeenCalledOnce()
})

// A replaced runtime typically produces a burst of mismatch errors, one per in-flight attach.
// Re-arming per error would push the deadline out indefinitely, which is the unbounded hold the
// class exists to prevent.
it('keeps the first deadline when armed again while already armed', () => {
const wait = new BrowserClientHostAuthorityReplacementWait(1_000)
const first = vi.fn()
const second = vi.fn()

wait.arm(first)
vi.advanceTimersByTime(900)
wait.arm(second)
vi.advanceTimersByTime(100)

expect(first).toHaveBeenCalledOnce()
expect(second).not.toHaveBeenCalled()
})

it('never expires after a cancel', () => {
const wait = new BrowserClientHostAuthorityReplacementWait(1_000)
const expire = vi.fn()
wait.arm(expire)

wait.cancel()

expect(wait.armed).toBe(false)
vi.advanceTimersByTime(10_000)
expect(expire).not.toHaveBeenCalled()
})

it('is re-armable after a cancel and after an expiry', () => {
const wait = new BrowserClientHostAuthorityReplacementWait(1_000)
const cancelled = vi.fn()
const rearmed = vi.fn()
wait.arm(cancelled)
wait.cancel()

wait.arm(rearmed)
vi.advanceTimersByTime(1_000)

expect(cancelled).not.toHaveBeenCalled()
expect(rearmed).toHaveBeenCalledOnce()

const afterExpiry = vi.fn()
wait.arm(afterExpiry)
vi.advanceTimersByTime(1_000)
expect(afterExpiry).toHaveBeenCalledOnce()
})
})
5 changes: 2 additions & 3 deletions src/main/browser/browser-client-host-authority-replacement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@ import { BROWSER_CLIENT_HOST_AUTHORITY_MISMATCH_CODE } from '../../shared/browse
/**
* True when a lease failed because the runtime it named has been replaced by a newer one.
*
* This is the restart signal, and it is the one host error that must not retire the environment:
* the webview guests are still alive and still ours, and the replacement authority arrives moments
* later to reclaim them. Every other host error means the host itself is unusable.
* This is the restart signal: the webview guests are still alive and still ours, and the
* replacement authority arrives later to reclaim them, so the composition parks for it.
*/
export function isBrowserClientHostAuthorityReplaced(error: unknown): boolean {
if (!(error instanceof Error)) {
Expand Down
39 changes: 39 additions & 0 deletions src/main/browser/browser-client-host-parked-publication.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { BrowserWindow } from 'electron'
import {
BROWSER_CLIENT_HOST_PARKED_CHANNEL,
type BrowserClientHostParkedEvent
} from '../../shared/browser-client-host-parked'

const parkedEnvironmentIds = new Set<string>()

export function isBrowserClientHostEnvironmentParked(environmentId: string): boolean {
return parkedEnvironmentIds.has(environmentId)
}

/** Tells every renderer, so the offline strip follows the browser host and not the control link. */
export function publishBrowserClientHostParked(
environmentId: string,
parked: boolean,
error?: Error
): void {
if (parked) {
parkedEnvironmentIds.add(environmentId)
console.warn('[browser-client-host] runtime unreachable; keeping pages until it returns:', {
environmentId,
error: error?.message
})
} else {
parkedEnvironmentIds.delete(environmentId)
}
const event: BrowserClientHostParkedEvent = { environmentId, parked }
for (const window of BrowserWindow.getAllWindows()) {
if (window.isDestroyed()) {
continue
}
try {
window.webContents.send(BROWSER_CLIENT_HOST_PARKED_CHANNEL, event)
} catch {
/* A renderer can close during publication. */
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export type BrowserClientPageCommandExecutorDependencies = BrowserClientPageAuth
signal: AbortSignal
): Promise<BrowserClientPageNetworkRoute>
selectRenderer(): BrowserClientPageRenderer
routeSessions: Pick<BrowserRouteSessionRegistry, 'preparePage'>
routeSessions: Pick<BrowserRouteSessionRegistry, 'preparePage' | 'retargetPartitionProxy'>
routeWebContents: BrowserClientPageLifecycleRegistry
guestBinding: BrowserClientPageGuestBinding
executeAutomation(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ export function createHarness(options: { maxPages?: number } = {}) {
preparePage: vi.fn(async () => {
order.push('prepare-page')
return routeSession
}),
retargetPartitionProxy: vi.fn(async () => {
order.push('retarget-partition-proxy')
})
},
executeAutomation: vi.fn(async () => ({ clicked: true })),
Expand Down
11 changes: 3 additions & 8 deletions src/main/browser/browser-client-page-command-executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,19 +107,14 @@ export class BrowserClientPageCommandExecutor {
case 'restorePage':
return executeBrowserClientPageReconciliationCommand(
{
routeWebContents: this.dependencies.routeWebContents,
routeSessions: this.dependencies.routeSessions,
retainNetworkRoute: this.dependencies.retainNetworkRoute,
pages: this.pages,
failedPages: this.failedPages,
routeWebContents: this.dependencies.routeWebContents,
assertAvailable: () =>
this.navigationFence.assertAvailable(this.closed || this.authorityTransitioning),
createPage: (command, commandSignal) => this.createPage(command, commandSignal),
navigate: (command, commandSignal) =>
navigateBrowserClientPageCommand(
this.pages,
this.dependencies.routeWebContents,
command,
commandSignal
),
retirePage: (browserPageId, generation) => this.retirePage(browserPageId, generation),
cleanupPage: (page, previousRendererPage) =>
this.cleanupPage(page, previousRendererPage)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ describe('BrowserClientPageCommandExecutor integration', () => {
await expect(executor.retirePage('page-a', 8)).resolves.toBe(true)
expect(guest.webContents.close).not.toHaveBeenCalled()
expect(rendererRetire).toHaveBeenCalledOnce()
expect(routeRelease).toHaveBeenCalledOnce()
// Once for the route the reclaim moved the page off, once for the route it retired with.
expect(routeRelease).toHaveBeenCalledTimes(2)
expect(clearPolicies).toHaveBeenCalledOnce()
expect(sessionRegistry.isAllowedPartition(partition)).toBe(false)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,10 @@ function createHarness() {
storageScope: 'a'.repeat(64),
retainNetworkRoute: vi.fn(async () => route),
selectRenderer: vi.fn(() => renderer),
routeSessions: { preparePage: vi.fn(async () => routeSession) },
routeSessions: {
preparePage: vi.fn(async () => routeSession),
retargetPartitionProxy: vi.fn(async () => {})
},
executeAutomation: vi.fn(async () => undefined),
retireAutomation: vi.fn(async () => {}),
guestBinding: { bind: vi.fn(), release: vi.fn() },
Expand Down Expand Up @@ -165,6 +168,53 @@ describe('browser client page reconciliation adapters', () => {
])
})

it('moves a reclaimed guest onto the current lease route and lets the old one go', async () => {
const { dependencies, executor } = createHarness()
await executor.handle(command('createPage'), new AbortController().signal)
const previousRoute = await dependencies.retainNetworkRoute.mock.results[0]!.value
const currentRoute = {
...previousRoute,
proxyEndpoint: { host: '127.0.0.1' as const, port: 43124 },
release: vi.fn()
}
dependencies.retainNetworkRoute.mockResolvedValueOnce(currentRoute)

await expect(
executor.handle(command('reclaimPage'), new AbortController().signal)
).resolves.toEqual({ status: 'completed' })

// The old route died with the replaced lease; without the retarget the kept page never loads.
expect(dependencies.routeSessions.retargetPartitionProxy).toHaveBeenCalledWith(
partition,
currentRoute.proxyEndpoint
)
expect(previousRoute.release).toHaveBeenCalledOnce()
expect(currentRoute.release).not.toHaveBeenCalled()
expect(executor.hasPage('page-a', 8)).toBe(true)
})

it('closes a reclaimed guest that cannot be moved onto the current route', async () => {
const { dependencies, executor } = createHarness()
await executor.handle(command('createPage'), new AbortController().signal)
const currentRoute = {
...(await dependencies.retainNetworkRoute.mock.results[0]!.value),
proxyEndpoint: { host: '127.0.0.1' as const, port: 43124 },
release: vi.fn()
}
dependencies.retainNetworkRoute.mockResolvedValueOnce(currentRoute)
dependencies.routeSessions.retargetPartitionProxy.mockRejectedValueOnce(
new Error('browser_route_partition_proxy_verification_failed')
)

await expect(
executor.handle(command('reclaimPage'), new AbortController().signal)
).resolves.toEqual(expect.objectContaining({ status: 'failed' }))

expect(currentRoute.release).toHaveBeenCalledOnce()
expect(executor.hasPage('page-a', 7)).toBe(false)
expect(executor.hasPage('page-a', 8)).toBe(false)
})

it('rejects DOM-preserving reclaim across runtime authorities', async () => {
const { executor, renderer, routeWebContents } = createHarness()
await executor.handle(command('createPage'), new AbortController().signal)
Expand Down
Loading
Loading