starred/orca
1
0
Fork 0
mirror of https://github.com/stablyai/orca.git synced 2026-10-21 08:03:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(browser): serialize CDP user-agent writes and share one navigation URL

Browse Source 
setViewportOverride and the navigation path both wrote Emulation.setUserAgentOverride
for the same tab with no shared ordering, and resolved the tab's host from different
sources. Verified against real Electron 43.1.0: did-start-navigation fires while an
awaited sendCommand is pending, and getURL() keeps reporting the outgoing page until
commit — so a preset resuming mid-navigation resolved the wrong host. Route both
writers through viewportOpsByTabId and resolve the host from the in-flight navigation
target, held until commit or failure.
This commit is contained in:
Brennan Benson
2026-08-07 00:32:15 -07:00
parent 86bc670412
commit 0b90e8d4e5
2 changed files with 283 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/main/browser/browser-manager.test.ts
+206 -6
View File
@@ -2902,6 +2902,14 @@ describe('browserManager', () => {
const GUEST_CLEAN_UA =
'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/134.0.0.0 Safari/537.36'
// Why: viewport UA writes are queued on the per-tab chain, so draining it takes more than one
// microtask hop; loop until the chain is empty rather than guessing a tick count.
async function flushViewportOps(): Promise<void> {
for (let i = 0; i < 20; i++) {
await Promise.resolve()
}
}
function makeGuest(
id: number,
url = 'https://example.com/'
@@ -2911,16 +2919,19 @@ describe('browserManager', () => {
debuggerIsAttached: ReturnType<typeof vi.fn>
debuggerAttach: ReturnType<typeof vi.fn>
setGuestUserAgent: (ua: string) => void
commitNavigationTo: (nextUrl: string) => void
} {
const debuggerSendCommand = vi.fn().mockResolvedValue(undefined)
const debuggerIsAttached = vi.fn(() => true)
const debuggerAttach = vi.fn()
let currentUa = GUEST_ELECTRON_UA
// Why: getURL() reports the last COMMITTED url — it does not move at did-start-navigation.
let committedUrl = url
const guest = {
id,
isDestroyed: vi.fn(() => false),
getType: vi.fn(() => 'webview'),
getURL: vi.fn(() => url),
getURL: vi.fn(() => committedUrl),
getUserAgent: vi.fn(() => currentUa),
setUserAgent: vi.fn((ua: string) => {
currentUa = ua
@@ -2945,6 +2956,9 @@ describe('browserManager', () => {
debuggerAttach,
setGuestUserAgent: (ua: string) => {
currentUa = ua
},
commitNavigationTo: (nextUrl: string) => {
committedUrl = nextUrl
}
}
}
@@ -3091,7 +3105,7 @@ describe('browserManager', () => {
// Navigating to the auth host must move the standing override to the Firefox identity.
debuggerSendCommand.mockClear()
didStartNavigation(null, 'https://accounts.google.com/v3/signin/identifier', false, true)
await Promise.resolve()
await flushViewportOps()
expect(debuggerSendCommand).toHaveBeenCalledWith('Emulation.setUserAgentOverride', {
userAgent: googleAuthUserAgent()
})
@@ -3099,12 +3113,198 @@ describe('browserManager', () => {
// Leaving the auth host restores the clean Chrome-shaped preset UA.
debuggerSendCommand.mockClear()
didStartNavigation(null, 'https://example.com/', false, true)
await Promise.resolve()
await flushViewportOps()
expect(debuggerSendCommand).toHaveBeenCalledWith('Emulation.setUserAgentOverride', {
userAgent: GUEST_CLEAN_UA
})
})
// Why: setViewportOverride and the navigation path are two independent writers of the same CDP
// command. Verified against real Electron 43.1.0: did-start-navigation fires while an awaited
// sendCommand is still pending, and getURL() keeps reporting the OUTGOING page until commit —
// so a preset resuming mid-navigation resolves the wrong host unless both writers share a chain
// and a single URL source.
function lastUserAgentOverride(
debuggerSendCommand: ReturnType<typeof vi.fn>
): Record<string, unknown> | undefined {
const calls = debuggerSendCommand.mock.calls.filter(
(call) => call[0] === 'Emulation.setUserAgentOverride'
)
return calls.at(-1)?.[1] as Record<string, unknown> | undefined
}
// Why mobile: on the desktop branch the break is masked by coincidence — applyGoogleAuthUserAgent
// has already switched the WebContents UA to Firefox, and cleanElectronUserAgent passes a Firefox
// UA through untouched, so the stale-URL desktop path happens to emit Firefox anyway. The mobile
// branch derives a Chrome-shaped iPhone UA from that same base and exposes the real defect.
it('does not leave the Chrome preset UA standing when a mobile preset lands mid-navigation onto an auth host', async () => {
const { guest, debuggerSendCommand } = makeGuest(4251, 'https://example.com/')
// Hold the preset's first CDP command open so the navigation lands inside its await window.
let releaseMetrics = (): void => {}
const metricsGate = new Promise<void>((resolve) => {
releaseMetrics = () => resolve()
})
debuggerSendCommand.mockImplementation((method: string) =>
method === 'Emulation.setDeviceMetricsOverride' ? metricsGate : Promise.resolve(undefined)
)
webContentsFromIdMock.mockReturnValue(guest)
browserManager.attachGuestPolicies(guest as never)
browserManager.registerGuest({
browserPageId: 'tab-race-onto-auth',
webContentsId: guest.id as number,
rendererWebContentsId
})
const didStartNavigation = guestOnMock.mock.calls.find(
([event]) => event === 'did-start-navigation'
)?.[1] as (event: unknown, url: string, isInPlace: boolean, isMainFrame: boolean) => void
const presetDone = browserManager.setViewportOverride('tab-race-onto-auth', {
width: 375,
height: 667,
deviceScaleFactor: 2,
mobile: true
})
await flushViewportOps()
// The tab navigates to the auth host while the preset is still awaiting its first command.
didStartNavigation(null, 'https://accounts.google.com/v3/signin/identifier', false, true)
releaseMetrics()
await presetDone
await flushViewportOps()
// Without a shared chain + shared URL source, the preset resumes, reads getURL() as the
// pre-navigation host, and pins the tab to the Chrome-shaped UA while it is on the auth host.
expect(lastUserAgentOverride(debuggerSendCommand)).toEqual({
userAgent: googleAuthUserAgent()
})
})
it('does not leave the Firefox UA standing when a preset lands mid-navigation off an auth host', async () => {
const { guest, debuggerSendCommand } = makeGuest(4252, 'https://accounts.google.com/')
webContentsFromIdMock.mockReturnValue(guest)
browserManager.attachGuestPolicies(guest as never)
browserManager.registerGuest({
browserPageId: 'tab-race-off-auth',
webContentsId: guest.id as number,
rendererWebContentsId
})
const didStartNavigation = guestOnMock.mock.calls.find(
([event]) => event === 'did-start-navigation'
)?.[1] as (event: unknown, url: string, isInPlace: boolean, isMainFrame: boolean) => void
// Establish a standing preset while the tab really is on the auth host.
await browserManager.setViewportOverride('tab-race-off-auth', {
width: 1024,
height: 768,
deviceScaleFactor: 1,
mobile: false
})
await flushViewportOps()
expect(lastUserAgentOverride(debuggerSendCommand)).toEqual({
userAgent: googleAuthUserAgent()
})
// A second preset change is now in flight when the tab leaves the auth host.
let releaseMetrics = (): void => {}
const metricsGate = new Promise<void>((resolve) => {
releaseMetrics = () => resolve()
})
debuggerSendCommand.mockImplementation((method: string) =>
method === 'Emulation.setDeviceMetricsOverride' ? metricsGate : Promise.resolve(undefined)
)
const presetDone = browserManager.setViewportOverride('tab-race-off-auth', {
width: 1440,
height: 900,
deviceScaleFactor: 2,
mobile: false
})
await flushViewportOps()
didStartNavigation(null, 'https://example.com/', false, true)
releaseMetrics()
await presetDone
await flushViewportOps()
// Without the fix the resuming preset re-reads getURL() as the auth host and clobbers the
// navigation's correct write, stranding the Firefox UA on a non-auth page.
expect(lastUserAgentOverride(debuggerSendCommand)).toEqual({ userAgent: GUEST_CLEAN_UA })
})
it('falls back to the committed URL once a navigation commits or fails', async () => {
const { guest, debuggerSendCommand, commitNavigationTo } = makeGuest(
4253,
'https://example.com/'
)
webContentsFromIdMock.mockReturnValue(guest)
browserManager.attachGuestPolicies(guest as never)
browserManager.registerGuest({
browserPageId: 'tab-pending-cleared',
webContentsId: guest.id as number,
rendererWebContentsId
})
const didStartNavigation = guestOnMock.mock.calls.find(
([event]) => event === 'did-start-navigation'
)?.[1] as (event: unknown, url: string, isInPlace: boolean, isMainFrame: boolean) => void
const didFailLoad = guestOnMock.mock.calls.find(
([event]) => event === 'did-fail-load'
)?.[1] as (
event: unknown,
errorCode: number,
errorDescription: string,
validatedURL: string,
isMainFrame: boolean
) => void
const didNavigate = guestOnMock.mock.calls.find(
([event]) => event === 'did-navigate'
)?.[1] as (event: unknown, url: string) => void
await browserManager.setViewportOverride('tab-pending-cleared', {
width: 1024,
height: 768,
deviceScaleFactor: 1,
mobile: false
})
await flushViewportOps()
// An aborted navigation to the auth host must not leave that host standing as the tab's
// identity: the tab never went there, so a later preset must resolve the committed URL.
didStartNavigation(null, 'https://accounts.google.com/', false, true)
didFailLoad(null, -3, 'Aborted', 'https://accounts.google.com/', true)
await flushViewportOps()
// A mobile preset discriminates cleanly here: the auth branch would emit the Firefox UA, the
// non-auth branch an iPhone UA. Asserting the desktop branch instead would conflate this with
// applyGoogleAuthUserAgent leaving the WebContents UA on Firefox after an aborted auth nav,
// which is pre-existing behavior and out of scope for the ordering fix.
debuggerSendCommand.mockClear()
await browserManager.setViewportOverride('tab-pending-cleared', {
width: 375,
height: 667,
deviceScaleFactor: 2,
mobile: true
})
await flushViewportOps()
expect(lastUserAgentOverride(debuggerSendCommand)?.userAgent).toContain('iPhone')
// Same after a successful commit: the committed URL takes over from the pending target.
didStartNavigation(null, 'https://accounts.google.com/signin', false, true)
commitNavigationTo('https://accounts.google.com/signin')
didNavigate(null, 'https://accounts.google.com/signin')
await flushViewportOps()
debuggerSendCommand.mockClear()
await browserManager.setViewportOverride('tab-pending-cleared', {
width: 1024,
height: 768,
deviceScaleFactor: 1,
mobile: false
})
await flushViewportOps()
expect(lastUserAgentOverride(debuggerSendCommand)).toEqual({
userAgent: googleAuthUserAgent()
})
})
it('does not touch the UA override on navigation when no preset is standing', async () => {
const { guest, debuggerSendCommand } = makeGuest(4248)
webContentsFromIdMock.mockReturnValue(guest)
@@ -3119,7 +3319,7 @@ describe('browserManager', () => {
)?.[1] as (event: unknown, url: string, isInPlace: boolean, isMainFrame: boolean) => void
didStartNavigation(null, 'https://accounts.google.com/', false, true)
await Promise.resolve()
await flushViewportOps()
expect(debuggerSendCommand).not.toHaveBeenCalledWith(
'Emulation.setUserAgentOverride',
expect.anything()
@@ -3149,7 +3349,7 @@ describe('browserManager', () => {
debuggerSendCommand.mockClear()
didStartNavigation(null, 'https://accounts.google.com/', false, true)
await Promise.resolve()
await flushViewportOps()
expect(debuggerSendCommand).not.toHaveBeenCalledWith(
'Emulation.setUserAgentOverride',
expect.anything()
@@ -3179,7 +3379,7 @@ describe('browserManager', () => {
})
debuggerSendCommand.mockClear()
didStartNavigation(null, 'https://accounts.google.com/', false, true)
await Promise.resolve()
await flushViewportOps()
expect(debuggerSendCommand).not.toHaveBeenCalledWith(
'Emulation.setUserAgentOverride',
expect.anything()
src/main/browser/browser-manager.ts
+77 -38
View File
@@ -226,6 +226,9 @@ export class BrowserManager {
// Why: presence means a CDP UA override is standing for the tab (value = the preset's mobile flag),
// so navigation can re-issue it against the target URL's identity.
private readonly viewportUaOverrideMobileByTabId = new Map<string, boolean>()
// Why: the in-flight main-frame navigation target, held only until commit or failure — getURL()
// still reports the outgoing page until then. See resolveTabNavigationUrl.
private readonly pendingNavigationUrlByTabId = new Map<string, string>()
private readonly contextMenuCleanupByTabId = new Map<string, () => void>()
private readonly grabShortcutCleanupByTabId = new Map<string, () => void>()
private readonly shortcutForwardingCleanupByTabId = new Map<string, () => void>()
@@ -802,6 +805,8 @@ export class BrowserManager {
if (!isMainFrame) {
return
}
// Why: a nav that never committed must not leave its target standing as the tab's host.
this.clearPendingNavigationUrl(guest.id)
const browserPageId = this.tabIdByWebContentsId.get(guest.id)
const certificateFailure = browserPageId
? this.certificateTrustController?.getFailure(browserPageId)
@@ -845,6 +850,12 @@ export class BrowserManager {
if (!isMainFrame || isChromiumInternalErrorUrl(url)) {
return
}
const navigatingTabId = this.tabIdByWebContentsId.get(guest.id)
if (navigatingTabId) {
// 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.pendingNavigationUrlByTabId.set(navigatingTabId, url)
}
this.applyGoogleAuthUserAgent(guest, url)
this.certificateTrustController?.onMainFrameNavigationStarted(guest.id)
// Why: a pre-registration failure belongs only to its own nav; a replacement nav must not replay it.
@@ -861,6 +872,8 @@ export class BrowserManager {
}
const didNavigateHandler = (_event: Electron.Event, url: string): void => {
// Why: once committed, getURL() reports this url, so the pending target is redundant.
this.clearPendingNavigationUrl(guest.id)
// 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)
@@ -932,39 +945,78 @@ export class BrowserManager {
guest.setUserAgent(guest.session.getUserAgent())
}
if (browserPageId) {
this.reapplyViewportUserAgentOverride(guest, browserPageId, url)
this.reapplyViewportUserAgentOverride(guest, browserPageId)
}
}
private clearPendingNavigationUrl(guestId: number): void {
const browserTabId = this.tabIdByWebContentsId.get(guestId)
if (browserTabId) {
this.pendingNavigationUrlByTabId.delete(browserTabId)
}
}
// Why: webContents.getURL() reports the last COMMITTED url, so mid-navigation it names the host
// the tab is leaving, not the one it is entering. Every UA writer must resolve the host through
// here or two writers racing the same navigation will pick opposite identities.
private resolveTabNavigationUrl(guest: Electron.WebContents, browserTabId: string): string {
return this.pendingNavigationUrlByTabId.get(browserTabId) ?? guest.getURL()
}
// Why: setViewportOverride and the navigation path both write Emulation.setUserAgentOverride for
// the same tab. Sharing one chain makes the last-enqueued write the one that lands; without it the
// two interleave and the tab can keep the Firefox UA off the auth hosts, or the Chrome-shaped
// preset UA while on them — the exact mismatch this scope exists to remove.
private enqueueViewportOp<T>(browserTabId: string, op: () => Promise<T>): Promise<T> {
const prev = this.viewportOpsByTabId.get(browserTabId) ?? Promise.resolve()
const next = prev.catch(() => {}).then(op)
this.viewportOpsByTabId.set(browserTabId, next)
// Why: only clear if we're still the tail; a later call may have replaced the entry, and deleting would break serialization.
void next
.catch(() => {})
.then(() => {
if (this.viewportOpsByTabId.get(browserTabId) === next) {
this.viewportOpsByTabId.delete(browserTabId)
}
})
return next
}
// Why: Emulation.setUserAgentOverride is set once and stands across every later navigation,
// outranking setUserAgent for navigator.userAgent. A viewport preset applied before reaching an
// auth host would otherwise pin navigator.userAgent to the Chrome-shaped preset UA while the
// request header says Firefox — the two-layer disagreement this scope exists to remove.
private reapplyViewportUserAgentOverride(
guest: Electron.WebContents,
browserTabId: string,
url: string
browserTabId: string
): void {
// Why: enqueue unconditionally — a setViewportOverride already queued ahead may be about to
// install (or clear) the override, so the standing-preset check belongs inside the chain.
void this.enqueueViewportOp(browserTabId, () =>
this.sendViewportUserAgentOverride(guest, browserTabId)
).catch(() => {})
}
private async sendViewportUserAgentOverride(
guest: Electron.WebContents,
browserTabId: string
): Promise<void> {
const mobile = this.viewportUaOverrideMobileByTabId.get(browserTabId)
if (mobile === undefined) {
return
}
try {
if (!guest.debugger.isAttached()) {
if (guest.isDestroyed() || !guest.debugger.isAttached()) {
return
}
// Why: navigator.userAgent is read by page JS after commit, so landing this override
// asynchronously still beats every reader; the header layer already forces Firefox on the wire.
void guest.debugger
.sendCommand(
'Emulation.setUserAgentOverride',
buildViewportUserAgentOverride({
url,
mobile,
baseUserAgent: cleanElectronUserAgent(guest.getUserAgent())
})
)
.catch(() => {})
await guest.debugger.sendCommand(
'Emulation.setUserAgentOverride',
buildViewportUserAgentOverride({
url: this.resolveTabNavigationUrl(guest, browserTabId),
mobile,
baseUserAgent: cleanElectronUserAgent(guest.getUserAgent())
})
)
} catch {
// guest may be mid-teardown or the debugger taken over by DevTools
}
@@ -1152,6 +1204,7 @@ export class BrowserManager {
// Why: drop the viewport-op chain so the Map doesn't retain a promise keyed to a destroyed guest.
this.viewportOpsByTabId.delete(browserTabId)
this.viewportUaOverrideMobileByTabId.delete(browserTabId)
this.pendingNavigationUrlByTabId.delete(browserTabId)
this.annotationViewportBridgeOpsByTabId.delete(browserTabId)
}
@@ -1218,6 +1271,7 @@ export class BrowserManager {
this.sessionProfileIdByPageId.clear()
this.userAgentModeByPageId.clear()
this.viewportUaOverrideMobileByTabId.clear()
this.pendingNavigationUrlByTabId.clear()
this.pendingLoadFailuresByGuestId.clear()
this.loadErrorsByGuestId.clear()
this.clearedLoadErrorsByGuestId.clear()
@@ -1505,20 +1559,11 @@ export class BrowserManager {
browserTabId: string,
override: BrowserViewportOverride | null
): Promise<boolean> {
// Why: chain per-tab so rapid toggles don't interleave CDP commands and the last-requested override wins.
const prev = this.viewportOpsByTabId.get(browserTabId) ?? Promise.resolve()
const next = prev
.catch(() => {})
.then(() => this.doSetViewportOverrideImpl(browserTabId, override))
this.viewportOpsByTabId.set(browserTabId, next)
try {
return await next
} finally {
// Why: only clear if we're still the tail; a later call may have replaced the entry, and deleting would break serialization.
if (this.viewportOpsByTabId.get(browserTabId) === next) {
this.viewportOpsByTabId.delete(browserTabId)
}
}
// Why: chain per-tab so rapid toggles — and navigations racing them — don't interleave CDP
// commands; the last-enqueued write wins.
return this.enqueueViewportOp(browserTabId, () =>
this.doSetViewportOverrideImpl(browserTabId, override)
)
}
async setAnnotationViewportBridge(
@@ -1614,14 +1659,8 @@ export class BrowserManager {
// Why: remember the standing override so later navigations can re-issue it; it outranks
// setUserAgent and would otherwise pin the auth-host identity to whatever was set here.
this.viewportUaOverrideMobileByTabId.set(browserTabId, override.mobile)
await dbg.sendCommand(
'Emulation.setUserAgentOverride',
buildViewportUserAgentOverride({
url: guest.getURL(),
mobile: override.mobile,
baseUserAgent: cleanElectronUserAgent(guest.getUserAgent())
})
)
// Why: same sender as the navigation path, so both resolve the tab's host identically.
await this.sendViewportUserAgentOverride(guest, browserTabId)
}
} else {
this.viewportUaOverrideMobileByTabId.delete(browserTabId)