From d83da0060426ef3ceeb1052a4feb4bb9119901f1 Mon Sep 17 00:00:00 2001 From: m4air Date: Wed, 16 Sep 2026 21:46:15 -0700 Subject: [PATCH] fix: fence viewport state after browser guest retirement --- .../README.md | 44 +++ .../baseline-results.json | 94 ++++++ .../baseline-source.txt | 219 ++++++++++++++ .../fix.patch | 18 ++ .../fixed-results.json | 260 ++++++++++++++++ .../source-versions.json | 98 ++++++ .../validation.json | 36 +++ .../vitest.config.mjs | 36 +++ ...browser-manager-viewport-ownership.test.ts | 283 ++++++++++++++++++ src/main/browser/browser-manager-viewport.ts | 11 +- 10 files changed, 1098 insertions(+), 1 deletion(-) create mode 100644 docs/audits/browser-viewport-owner-retention/README.md create mode 100644 docs/audits/browser-viewport-owner-retention/baseline-results.json create mode 100644 docs/audits/browser-viewport-owner-retention/baseline-source.txt create mode 100644 docs/audits/browser-viewport-owner-retention/fix.patch create mode 100644 docs/audits/browser-viewport-owner-retention/fixed-results.json create mode 100644 docs/audits/browser-viewport-owner-retention/source-versions.json create mode 100644 docs/audits/browser-viewport-owner-retention/validation.json create mode 100644 docs/audits/browser-viewport-owner-retention/vitest.config.mjs create mode 100644 src/main/browser/browser-manager-viewport-ownership.test.ts diff --git a/docs/audits/browser-viewport-owner-retention/README.md b/docs/audits/browser-viewport-owner-retention/README.md new file mode 100644 index 00000000000..bcd64e43a9a --- /dev/null +++ b/docs/audits/browser-viewport-owner-retention/README.md @@ -0,0 +1,44 @@ +# Retired browser viewport operation ownership + +The viewport operation captures a guest ID, then awaits CDP commands. Closing a tab deletes its viewport state, but the old continuation can subsequently recreate the UA-intent entry. A failed UA clear can also restore the old value over a replacement guest's completed desktop preset, or a late clear can delete the replacement's mobile intent. + +The correction reuses that captured guest ID at three mutation boundaries: before publishing an applied preset's UA intent, before reading/deleting a cleared preset's intent, and before failed-clear rollback. Same-owner rollback, native UA profiles, navigation behavior, and the per-tab promise chain are preserved. + +## Evidence + +The regression fixture calls the actual manager, registration, unregistration, and viewport implementation. Electron WebContents and pending debugger replies are controlled ports; no native browser or window is launched. + +- Baseline: **7 failing ownership regressions, 5 passing controls**. +- Fixed: **12/12 ownership cases**, plus **30 existing viewport, navigation, partial-failure, and UA cases**. +- Sixteen pending UA-clear rejections after `unregisterAll` leave **16 retired UA entries before, zero after**. Registration, preset, and promise maps remain empty. +- Other regressions cover closed-tab late success, failed-clear rollback, mobile/desktop replacement, and native-to-default profile replacement. +- Controls preserve ordinary serialized mobile/desktop/null operations, both native-profile presets, same-owner rollback, and the replacement promise tail while old queued operations settle. +- An independent reviewer ran all 12 candidate cases and reviewed the three mutation guards before promotion. + +The retained entries are tab ID strings and booleans. This does **not** demonstrate retained native WebContents, a process RSS slope, or gigabyte-scale memory growth. In-flight CDP work still owns its continuation until it settles. Positive and negative post-close command replies are injected schedules, not an affected-host trace. + +## Ordinary callers and compatibility + +The renderer requests overrides when the user selects a viewport preset and on guest `dom-ready`, including null presets. The trusted IPC handler validates dimensions before calling this manager. Navigation later reads the UA-intent map, so stale replacement values can alter the standing mobile/desktop identity. The fixture does not execute the renderer or IPC producer. + +Both local webview and host-side offscreen registrations use these maps. The correction changes no wire fields, protocol, execution-host ownership, native process lifecycle, folder/worktree handling, or UI layout. It only prevents an operation for a different guest from mutating the current registration's state. + +`source-versions.json` records 11 paths at audit checkpoint `4a09b1d1`, independent main `291b4ddd`, and reported v1.4.198 `e0826956`. The viewport implementation, registration, registry declarations, IPC handler, and toolbar producer match all three. Ten sources match independent main and eight match v1.4.198. The guest-session producer contains an earlier audit fix; historical navigation and fixture sources differ. This is a current-dependency replay with the exact historical viewport source, not a historical app-binary replay. + +The browsing activity in #19831 makes this path applicable in principle. The report does not establish the required overlap or tab count, and this small metadata mechanism does not account for its reported memory totals. + +## Reproduction + +From the worktree, run the fixed regression suite: + +```sh +ORCA_BACKGROUND_LAUNCH=1 node node_modules/vitest/vitest.mjs run --config docs/audits/browser-viewport-owner-retention/vitest.config.mjs +``` + +Run the same tests with the exact baseline viewport implementation; exit status 1 and seven failed cases are expected: + +```sh +ORCA_BACKGROUND_LAUNCH=1 ORCA_VIEWPORT_BASELINE=1 node node_modules/vitest/vitest.mjs run --config docs/audits/browser-viewport-owner-retention/vitest.config.mjs +``` + +The import overlay never rewrites product files. `baseline-source.txt` contains only the original viewport module; current support modules remain in use. `baseline-results.json`, `fixed-results.json`, and `validation.json` record the measured results and their scope. diff --git a/docs/audits/browser-viewport-owner-retention/baseline-results.json b/docs/audits/browser-viewport-owner-retention/baseline-results.json new file mode 100644 index 00000000000..a089d27272f --- /dev/null +++ b/docs/audits/browser-viewport-owner-retention/baseline-results.json @@ -0,0 +1,94 @@ +{ + "testFiles": 1, + "total": 12, + "passed": 5, + "failed": 7, + "cases": [ + { + "file": "src/main/browser/browser-manager-viewport-ownership.test.ts", + "title": "browser viewport operation ownership does not recreate closed-tab UA intent after a late touch completion", + "status": "failed", + "failures": [ + "AssertionError: expected true to be undefined\n at ./src/main/browser/browser-manager-viewport-ownership.test.ts:109:37\n at processTicksAndRejections (node:internal/process/task_queues:104:5)\n at file://./node_modules/.pnpm/@vitest+runner@4.1.11/node_modules/@vitest/runner/dist/chunk-artifact.js:1903:20" + ] + }, + { + "file": "src/main/browser/browser-manager-viewport-ownership.test.ts", + "title": "browser viewport operation ownership does not restore closed-tab UA intent after a failed clear", + "status": "failed", + "failures": [ + "AssertionError: expected true to be undefined\n at ./src/main/browser/browser-manager-viewport-ownership.test.ts:126:42\n at processTicksAndRejections (node:internal/process/task_queues:104:5)\n at file://./node_modules/.pnpm/@vitest+runner@4.1.11/node_modules/@vitest/runner/dist/chunk-artifact.js:1903:20" + ] + }, + { + "file": "src/main/browser/browser-manager-viewport-ownership.test.ts", + "title": "browser viewport operation ownership preserves replacement desktop intent after an old clear fails", + "status": "failed", + "failures": [ + "AssertionError: expected true to be false // Object.is equality\n at ./src/main/browser/browser-manager-viewport-ownership.test.ts:142:42\n at processTicksAndRejections (node:internal/process/task_queues:104:5)\n at file://./node_modules/.pnpm/@vitest+runner@4.1.11/node_modules/@vitest/runner/dist/chunk-artifact.js:1903:20" + ] + }, + { + "file": "src/main/browser/browser-manager-viewport-ownership.test.ts", + "title": "browser viewport operation ownership preserves replacement mobile intent after an old clear resumes", + "status": "failed", + "failures": [ + "AssertionError: expected false to be true // Object.is equality\n at ./src/main/browser/browser-manager-viewport-ownership.test.ts:160:42\n at processTicksAndRejections (node:internal/process/task_queues:104:5)\n at file://./node_modules/.pnpm/@vitest+runner@4.1.11/node_modules/@vitest/runner/dist/chunk-artifact.js:1903:20" + ] + }, + { + "file": "src/main/browser/browser-manager-viewport-ownership.test.ts", + "title": "browser viewport operation ownership same-owner clear failure still restores the legitimate earlier intent", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-ownership.test.ts", + "title": "browser viewport operation ownership old apply cannot overwrite a replacement guest desktop intent", + "status": "failed", + "failures": [ + "AssertionError: expected true to be false // Object.is equality\n at ./src/main/browser/browser-manager-viewport-ownership.test.ts:185:41\n at processTicksAndRejections (node:internal/process/task_queues:104:5)\n at file://./node_modules/.pnpm/@vitest+runner@4.1.11/node_modules/@vitest/runner/dist/chunk-artifact.js:1903:20" + ] + }, + { + "file": "src/main/browser/browser-manager-viewport-ownership.test.ts", + "title": "browser viewport operation ownership an old native profile cannot write UA intent after replacement with a default profile", + "status": "failed", + "failures": [ + "AssertionError: expected true to be undefined\n at ./src/main/browser/browser-manager-viewport-ownership.test.ts:197:49\n at processTicksAndRejections (node:internal/process/task_queues:104:5)\n at file://./node_modules/.pnpm/@vitest+runner@4.1.11/node_modules/@vitest/runner/dist/chunk-artifact.js:1903:20" + ] + }, + { + "file": "src/main/browser/browser-manager-viewport-ownership.test.ts", + "title": "browser viewport operation ownership old queued operations cannot remove or join a replacement promise tail", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-ownership.test.ts", + "title": "browser viewport operation ownership normal same-owner toggles preserve last-requested order and remove the promise tail", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-ownership.test.ts", + "title": "browser viewport operation ownership native UA mode remains unchanged with mobile=false", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-ownership.test.ts", + "title": "browser viewport operation ownership native UA mode remains unchanged with mobile=true", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-ownership.test.ts", + "title": "browser viewport operation ownership late rejected clears cannot repopulate all registries after unregisterAll", + "status": "failed", + "failures": [ + "AssertionError: expected 16 to be +0 // Object.is equality\n at ./src/main/browser/browser-manager-viewport-ownership.test.ts:278:28\n at processTicksAndRejections (node:internal/process/task_queues:104:5)\n at file://./node_modules/.pnpm/@vitest+runner@4.1.11/node_modules/@vitest/runner/dist/chunk-artifact.js:1903:20" + ] + } + ] +} diff --git a/docs/audits/browser-viewport-owner-retention/baseline-source.txt b/docs/audits/browser-viewport-owner-retention/baseline-source.txt new file mode 100644 index 00000000000..ce31dbe37e1 --- /dev/null +++ b/docs/audits/browser-viewport-owner-retention/baseline-source.txt @@ -0,0 +1,219 @@ +import { webContents } from 'electron' +import { + BROWSER_ANNOTATION_VIEWPORT_BRIDGE_WORLD_ID, + buildBrowserAnnotationViewportBridgeScript, + type BrowserAnnotationViewportBridgeOptions +} from '../../shared/browser-annotation-viewport-bridge' +import type { BrowserViewportOverride } from '../../shared/browser-workspace-types' +import { googleAuthUserAgent, isGoogleAuthUrl } from './browser-google-auth-ua' +import { BrowserManagerDownloadLifecycle } from './browser-manager-download-lifecycle' + +export abstract class BrowserManagerViewport extends BrowserManagerDownloadLifecycle { + // Why: guests are isolated from Orca's preload bridge, so main owns the devtools escape hatch after a tab→guest lookup. + async openDevTools(browserTabId: string): Promise { + const webContentsId = this.webContentsIdByTabId.get(browserTabId) + if (!webContentsId) { + return false + } + const guest = webContents.fromId(webContentsId) + if (!guest || guest.isDestroyed()) { + // Why: a stale guest must clear every per-tab registry entry, not just the WebContents maps. + this.unregisterGuest(browserTabId) + return false + } + // Offscreen guests have no visible window on this desktop; detaching DevTools would open it + // on the host display with no route back to the remote client. + if (this.offscreenGuestIds.has(webContentsId)) { + return false + } + guest.openDevTools({ mode: 'detach' }) + return true + } + + // Why: emulate viewport via CDP; never detach the debugger here or the agent bridge's per-guest state is cleared. + async setViewportOverride( + browserTabId: string, + override: BrowserViewportOverride | null + ): Promise { + // Why: chain per-tab so rapid toggles don't interleave CDP commands and the last-requested override wins. + const expectedWebContentsId = this.webContentsIdByTabId.get(browserTabId) + if (expectedWebContentsId !== undefined) { + // Keep host panning available while CDP applies the requested dimensions. The guest id fence + // prevents this intent from leaking to a replacement guest; clearing the preset removes it. + this.viewportPresetActiveByTabId.set(browserTabId, { + guestWebContentsId: expectedWebContentsId, + active: override !== null + }) + } + // The renderer resizes the host before CDP completes; discard the old geometry until it + // reports the new pane bounds so a pending preset cannot route wheel input using stale limits. + this.viewportScrollStateByTabId.delete(browserTabId) + const prev = this.viewportOpsByTabId.get(browserTabId) ?? Promise.resolve() + const next = prev + .catch(() => {}) + .then(() => this.doSetViewportOverrideImpl(browserTabId, override, expectedWebContentsId)) + 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) + } + } + } + + async setAnnotationViewportBridge( + browserTabId: string, + options: BrowserAnnotationViewportBridgeOptions, + resolveGuest: () => Electron.WebContents | null + ): Promise { + const prev = this.annotationViewportBridgeOpsByTabId.get(browserTabId) ?? Promise.resolve() + const next = prev + .catch(() => {}) + .then(() => this.doSetAnnotationViewportBridgeImpl(options, resolveGuest)) + this.annotationViewportBridgeOpsByTabId.set(browserTabId, next) + try { + return await next + } finally { + if (this.annotationViewportBridgeOpsByTabId.get(browserTabId) === next) { + this.annotationViewportBridgeOpsByTabId.delete(browserTabId) + } + } + } + + // Why the caller resolves the guest: the same bridge serves browsing pages and workspace + // documents, which live in different halves of the page registry. + // Why a resolver and not the guest itself: this op may have waited behind another one, and a + // cross-process navigation meanwhile swaps the tab's contents without destroying the old one — + // injecting into the guest the request named would bridge a page nobody is looking at. + // Why no tab id: with teardown gone this reaches only the guest the resolver hands back, and + // taking an id it cannot act on would invite the next reader to act on it. + protected async doSetAnnotationViewportBridgeImpl( + options: BrowserAnnotationViewportBridgeOptions, + resolveGuest: () => Electron.WebContents | null + ): Promise { + // Why no teardown here: the resolver already unregisters a page whose guest died, and the only + // case it uniquely leaves is an ownership mismatch on a healthy page — where tearing down would + // cancel that page's in-flight downloads and grabs over a request that was merely misaddressed. + const guest = resolveGuest() + if (!guest || guest.isDestroyed()) { + return false + } + + try { + // Why: run the scroll bridge in an isolated world so page scripts can't read the per-tab token or tamper with it. + await guest.executeJavaScriptInIsolatedWorld( + BROWSER_ANNOTATION_VIEWPORT_BRIDGE_WORLD_ID, + [{ code: buildBrowserAnnotationViewportBridgeScript(options) }], + false + ) + return true + } catch { + return false + } + } + + protected async doSetViewportOverrideImpl( + browserTabId: string, + override: BrowserViewportOverride | null, + expectedWebContentsId: number | undefined + ): Promise { + const webContentsId = this.webContentsIdByTabId.get(browserTabId) + if (!webContentsId || webContentsId !== expectedWebContentsId) { + return false + } + const guest = webContents.fromId(webContentsId) + if (!guest || guest.isDestroyed()) { + // Why: a stale guest must clear every per-tab registry entry, not just the WebContents maps. + this.unregisterGuest(browserTabId) + return false + } + + try { + if (!guest.debugger.isAttached()) { + guest.debugger.attach('1.3') + } + } catch (err) { + // Why: attach throws if DevTools is open on the guest; log context so this failure mode is diagnosable. + console.warn('[browser-manager] setViewportOverride: failed to attach debugger', { + browserTabId, + webContentsId, + error: err instanceof Error ? err.message : String(err) + }) + return false + } + + const dbg = guest.debugger + try { + if (override) { + await dbg.sendCommand('Emulation.setDeviceMetricsOverride', { + width: override.width, + height: override.height, + deviceScaleFactor: override.deviceScaleFactor, + mobile: override.mobile + }) + if (this.webContentsIdByTabId.get(browserTabId) === webContentsId) { + this.viewportPresetActiveByTabId.set(browserTabId, { + guestWebContentsId: webContentsId, + active: true + }) + } + await dbg.sendCommand('Emulation.setTouchEmulationEnabled', { + enabled: override.mobile, + maxTouchPoints: override.mobile ? 5 : 0 + }) + // Why: viewport sizing must not override a profile's explicit native-UA identity. + if (this.userAgentModeByPageId.get(browserTabId) !== 'native') { + // Navigation must see the preset intent while the final CDP command is in flight. + this.viewportUaOverrideMobileByTabId.set(browserTabId, override.mobile) + // Why: same sender as the navigation path, so both resolve the tab's host identically. + await this.sendViewportUserAgentOverride(guest, override.mobile) + } + } else { + await dbg.sendCommand('Emulation.clearDeviceMetricsOverride', {}) + if (this.webContentsIdByTabId.get(browserTabId) === webContentsId) { + this.viewportPresetActiveByTabId.set(browserTabId, { + guestWebContentsId: webContentsId, + active: false + }) + } + await dbg.sendCommand('Emulation.setTouchEmulationEnabled', { + enabled: false, + maxTouchPoints: 0 + }) + const trackedMobile = this.viewportUaOverrideMobileByTabId.get(browserTabId) + // A navigation after this point must not re-install the override behind the clear. + this.viewportUaOverrideMobileByTabId.delete(browserTabId) + try { + if (this.authUserAgentOverrideStateByGuestId.has(guest.id)) { + const url = this.resolveTabNavigationUrl(guest) + const restored = await this.applyAuthUserAgentOverrideOverCdp( + guest, + false, + url, + isGoogleAuthUrl(url) ? googleAuthUserAgent() : guest.session.getUserAgent() + ) + if (!restored) { + throw new Error('Failed to preserve auth user agent') + } + } else { + // Why: passing an empty string restores the session default UA. + await dbg.sendCommand('Emulation.setUserAgentOverride', { userAgent: '' }) + } + } catch (error) { + if (trackedMobile !== undefined) { + this.viewportUaOverrideMobileByTabId.set(browserTabId, trackedMobile) + } + throw error + } + } + if (this.webContentsIdByTabId.get(browserTabId) !== webContentsId) { + return false + } + return true + } catch { + return false + } + } +} diff --git a/docs/audits/browser-viewport-owner-retention/fix.patch b/docs/audits/browser-viewport-owner-retention/fix.patch new file mode 100644 index 00000000000..be47f36792b --- /dev/null +++ b/docs/audits/browser-viewport-owner-retention/fix.patch @@ -0,0 +1,18 @@ +diff --git a/src/main/browser/browser-manager-viewport.ts b/src/main/browser/browser-manager-viewport.ts +index ce31dbe37e..3f1fbb68fb 100644 +--- a/src/main/browser/browser-manager-viewport.ts ++++ b/src/main/browser/browser-manager-viewport.ts +@@ -165,0 +166,3 @@ export abstract class BrowserManagerViewport extends BrowserManagerDownloadLifec ++ if (this.webContentsIdByTabId.get(browserTabId) !== webContentsId) { ++ return false ++ } +@@ -184,0 +188,3 @@ export abstract class BrowserManagerViewport extends BrowserManagerDownloadLifec ++ if (this.webContentsIdByTabId.get(browserTabId) !== webContentsId) { ++ return false ++ } +@@ -205 +211,4 @@ export abstract class BrowserManagerViewport extends BrowserManagerDownloadLifec +- if (trackedMobile !== undefined) { ++ if ( ++ trackedMobile !== undefined && ++ this.webContentsIdByTabId.get(browserTabId) === webContentsId ++ ) { diff --git a/docs/audits/browser-viewport-owner-retention/fixed-results.json b/docs/audits/browser-viewport-owner-retention/fixed-results.json new file mode 100644 index 00000000000..3567f8d3eff --- /dev/null +++ b/docs/audits/browser-viewport-owner-retention/fixed-results.json @@ -0,0 +1,260 @@ +{ + "testFiles": 4, + "total": 42, + "passed": 42, + "failed": 0, + "cases": [ + { + "file": "src/main/browser/browser-manager-viewport-override.test.ts", + "title": "browserManager setViewportOverride returns false when the tab is not registered", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-override.test.ts", + "title": "browserManager setViewportOverride applies device metrics, touch emulation, and a mobile UA for mobile presets", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-override.test.ts", + "title": "browserManager setViewportOverride keeps the session UA for native-mode profiles when mobile=false", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-override.test.ts", + "title": "browserManager setViewportOverride keeps the session UA for native-mode profiles when mobile=true", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-override.test.ts", + "title": "browserManager setViewportOverride presents the Firefox UA for a preset applied on a Google auth host (mobile=false)", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-override.test.ts", + "title": "browserManager setViewportOverride presents the Firefox UA for a preset applied on a Google auth host (mobile=true)", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-override.test.ts", + "title": "browserManager setViewportOverride re-issues the standing UA override when navigating onto and back off an auth host", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-override.test.ts", + "title": "browserManager setViewportOverride does not leave the Chrome preset UA standing when a mobile preset lands mid-navigation onto an auth host", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-override.test.ts", + "title": "browserManager setViewportOverride does not leave the Firefox UA standing when a preset lands mid-navigation off an auth host", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-override.test.ts", + "title": "browserManager setViewportOverride falls back to the committed URL once a navigation commits or fails", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-override.test.ts", + "title": "browserManager setViewportOverride does not let a superseded navigation failure revert a newer target", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-override.test.ts", + "title": "browserManager setViewportOverride switches identity for a server redirect and restores it if the redirect fails", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-override.test.ts", + "title": "browserManager setViewportOverride preserves the auth identity when a viewport preset is cleared after a redirect", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-override.test.ts", + "title": "browserManager setViewportOverride does not inherit a mobile owner UA in a desktop popup", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-override.test.ts", + "title": "browserManager setViewportOverride reapplies a preset when navigation starts during its final UA write", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-override.test.ts", + "title": "browserManager setViewportOverride does not reinstall a preset while its final UA clear is in flight", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-override.test.ts", + "title": "browserManager setViewportOverride keeps tracking the standing override when the CDP clear fails", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-override.test.ts", + "title": "browserManager setViewportOverride does not touch the UA override on navigation when no preset is standing", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-override.test.ts", + "title": "browserManager setViewportOverride stops re-issuing the UA override once the preset is cleared", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-override.test.ts", + "title": "browserManager setViewportOverride leaves the UA override alone on navigation for native-UA profiles", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-override.test.ts", + "title": "browserManager setViewportOverride clears device metrics and disables touch for override=null", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-override.test.ts", + "title": "browserManager setViewportOverride attaches the debugger if not already attached and does not detach after", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-override.test.ts", + "title": "browserManager setViewportOverride returns false when debugger.attach throws (e.g. DevTools already open)", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-ownership.test.ts", + "title": "browser viewport operation ownership does not recreate closed-tab UA intent after a late touch completion", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-ownership.test.ts", + "title": "browser viewport operation ownership does not restore closed-tab UA intent after a failed clear", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-ownership.test.ts", + "title": "browser viewport operation ownership preserves replacement desktop intent after an old clear fails", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-ownership.test.ts", + "title": "browser viewport operation ownership preserves replacement mobile intent after an old clear resumes", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-ownership.test.ts", + "title": "browser viewport operation ownership same-owner clear failure still restores the legitimate earlier intent", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-ownership.test.ts", + "title": "browser viewport operation ownership old apply cannot overwrite a replacement guest desktop intent", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-ownership.test.ts", + "title": "browser viewport operation ownership an old native profile cannot write UA intent after replacement with a default profile", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-ownership.test.ts", + "title": "browser viewport operation ownership old queued operations cannot remove or join a replacement promise tail", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-ownership.test.ts", + "title": "browser viewport operation ownership normal same-owner toggles preserve last-requested order and remove the promise tail", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-ownership.test.ts", + "title": "browser viewport operation ownership native UA mode remains unchanged with mobile=false", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-ownership.test.ts", + "title": "browser viewport operation ownership native UA mode remains unchanged with mobile=true", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-ownership.test.ts", + "title": "browser viewport operation ownership late rejected clears cannot repopulate all registries after unregisterAll", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-partial-failure.test.ts", + "title": "browserManager viewport partial failure keeps wheel routing active when follow-up setup fails after metrics apply", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-manager-viewport-partial-failure.test.ts", + "title": "browserManager viewport partial failure keeps host panning available when metrics setup fails", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-viewport-user-agent.test.ts", + "title": "buildViewportUserAgentOverride presents the Firefox UA on Google auth hosts regardless of the preset", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-viewport-user-agent.test.ts", + "title": "buildViewportUserAgentOverride keeps the clean desktop UA off the auth hosts", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-viewport-user-agent.test.ts", + "title": "buildViewportUserAgentOverride splices the real Chrome major into the mobile UA and its client hints", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-viewport-user-agent.test.ts", + "title": "buildViewportUserAgentOverride falls back to a known Chrome major when the base UA carries none", + "status": "passed", + "failures": [] + }, + { + "file": "src/main/browser/browser-viewport-user-agent.test.ts", + "title": "buildViewportUserAgentOverride treats an unparseable URL as a non-auth host", + "status": "passed", + "failures": [] + } + ] +} diff --git a/docs/audits/browser-viewport-owner-retention/source-versions.json b/docs/audits/browser-viewport-owner-retention/source-versions.json new file mode 100644 index 00000000000..dabc3a96c01 --- /dev/null +++ b/docs/audits/browser-viewport-owner-retention/source-versions.json @@ -0,0 +1,98 @@ +{ + "refs": { + "audit": "4a09b1d108cfd8b57ffcc5d727b3a3bd71ae71fb", + "main": "291b4ddd6f1c1af480169885e0fda7f9c78ff053", + "v1.4.198": "e0826956fcfc532f5a1e55b5e081f2e57e553c43" + }, + "canonicalLineEndings": "LF", + "sources": [ + { + "path": "src/main/browser/browser-manager-viewport.ts", + "sha256": { + "audit": "701f9ea1a4310e509f409e08ee4ea0539f820bd209ff68f41001b8c91e5470e4", + "main": "701f9ea1a4310e509f409e08ee4ea0539f820bd209ff68f41001b8c91e5470e4", + "v1.4.198": "701f9ea1a4310e509f409e08ee4ea0539f820bd209ff68f41001b8c91e5470e4" + } + }, + { + "path": "src/main/browser/browser-manager-registration.ts", + "sha256": { + "audit": "12c149b1fb87b67b4192b6c2f23bd7f45b7df2d0c60368182f33b5a47b56f96f", + "main": "12c149b1fb87b67b4192b6c2f23bd7f45b7df2d0c60368182f33b5a47b56f96f", + "v1.4.198": "12c149b1fb87b67b4192b6c2f23bd7f45b7df2d0c60368182f33b5a47b56f96f" + } + }, + { + "path": "src/main/browser/browser-manager-navigation.ts", + "sha256": { + "audit": "a4c0e169ac35725b02d050c95843721e4274775a5bcb9ef2a5b7c835c4d8c234", + "main": "a4c0e169ac35725b02d050c95843721e4274775a5bcb9ef2a5b7c835c4d8c234", + "v1.4.198": "c93c060896351b4bc23db628a732ef4db4acd5b26760e4565e6bf029d5cf7531" + } + }, + { + "path": "src/main/browser/browser-manager-guest-policy.ts", + "sha256": { + "audit": "6d567b0c675091ec0a59d39ffe701fd36098ec2730b61ef0b80ab53962da6144", + "main": "6d567b0c675091ec0a59d39ffe701fd36098ec2730b61ef0b80ab53962da6144", + "v1.4.198": "6d567b0c675091ec0a59d39ffe701fd36098ec2730b61ef0b80ab53962da6144" + } + }, + { + "path": "src/main/browser/browser-manager-state.ts", + "sha256": { + "audit": "ea6e847afe227b9acbf432d68a6f21f0d15247748fc8b4f769ab73498d6a13c5", + "main": "ea6e847afe227b9acbf432d68a6f21f0d15247748fc8b4f769ab73498d6a13c5", + "v1.4.198": "ea6e847afe227b9acbf432d68a6f21f0d15247748fc8b4f769ab73498d6a13c5" + } + }, + { + "path": "src/main/browser/browser-manager-types.ts", + "sha256": { + "audit": "4d28f7c397aa82b067971ff769a44fff74adadceaac66774345a615f799bb64f", + "main": "4d28f7c397aa82b067971ff769a44fff74adadceaac66774345a615f799bb64f", + "v1.4.198": "4d28f7c397aa82b067971ff769a44fff74adadceaac66774345a615f799bb64f" + } + }, + { + "path": "src/main/browser/browser-manager-viewport-test-fixtures.ts", + "sha256": { + "audit": "0ef61054ae131db056b5268c6ed4f417b4486784d58d2c0d7d2285ad785666ee", + "main": "0ef61054ae131db056b5268c6ed4f417b4486784d58d2c0d7d2285ad785666ee", + "v1.4.198": "36d29f1d78bd559af3b235acc9be8dafe763e3dfb9ea4367272db04449eca707" + } + }, + { + "path": "src/main/browser/browser-manager-test-harness.ts", + "sha256": { + "audit": "66de235be9285ec2cd2a6d783c8e8a046d6106d4171aaf808b5d263c08a72038", + "main": "66de235be9285ec2cd2a6d783c8e8a046d6106d4171aaf808b5d263c08a72038", + "v1.4.198": "66de235be9285ec2cd2a6d783c8e8a046d6106d4171aaf808b5d263c08a72038" + } + }, + { + "path": "src/main/ipc/browser-guest-view-ipc.ts", + "sha256": { + "audit": "da4a441ce5f0851f1a1c6ec38c872c187ec191a9acad340f05ac557534731959", + "main": "da4a441ce5f0851f1a1c6ec38c872c187ec191a9acad340f05ac557534731959", + "v1.4.198": "da4a441ce5f0851f1a1c6ec38c872c187ec191a9acad340f05ac557534731959" + } + }, + { + "path": "src/renderer/src/components/browser-pane/host-guest/browser-page-webview-guest-session.ts", + "sha256": { + "audit": "a78cc98873e93834bf07b47bbf5d92da888dfbccef06551aa6ac3c8f6e9f29f8", + "main": "2fe17f0fe4f8ef6ca7cff7ca5731d9d725dbf4b5e7944264e30f12241317fc6d", + "v1.4.198": "6c089c0c8285b21b3f3c0b3e06bd297a25d941cfa8a4849d03fbd08a6c89c139" + } + }, + { + "path": "src/renderer/src/components/browser-pane/assemble-chrome/BrowserToolbarMenu.tsx", + "sha256": { + "audit": "e05e7d8d532d2f43f13a8dc1035540638ab513c9f3d6bdbd1531cf3e0e9462e1", + "main": "e05e7d8d532d2f43f13a8dc1035540638ab513c9f3d6bdbd1531cf3e0e9462e1", + "v1.4.198": "e05e7d8d532d2f43f13a8dc1035540638ab513c9f3d6bdbd1531cf3e0e9462e1" + } + } + ] +} diff --git a/docs/audits/browser-viewport-owner-retention/validation.json b/docs/audits/browser-viewport-owner-retention/validation.json new file mode 100644 index 00000000000..2a7de53af18 --- /dev/null +++ b/docs/audits/browser-viewport-owner-retention/validation.json @@ -0,0 +1,36 @@ +{ + "scope": "Actual manager and lifecycle; controlled Electron/CDP ports; no native guest, heap/RSS or incident attribution", + "baseline": { + "total": 12, + "failedOwnershipCases": 7, + "passedControls": 5 + }, + "fixed": { + "total": 42, + "passed": 42, + "testFiles": 4 + }, + "independentReview": { + "candidateTestsPassed": 12, + "findings": "No blocker; three map mutation guards preserve current guest and promise ownership" + }, + "typecheck": { + "node": "passed after correcting fixture-only protected-map reads and array typing", + "cli": "passed", + "web": "passed" + }, + "quality": { + "fullFileScans": 5, + "codeFiles": 3, + "newDiagnostics": 0 + }, + "sourceSha256": { + "src/main/browser/browser-manager-viewport.ts": "a839fd89cc5e687782323036ca3a8dd9de79bd838e77dd863a5e1ae101424b4c", + "src/main/browser/browser-manager-viewport-ownership.test.ts": "ec17f2acd6b766166d13cafc2ecd33f1d4f0de7a4b4e8896d0746b2d528341da" + }, + "limitations": [ + "Pending CDP response schedules are injected, not an affected-host capture", + "Retired map values are booleans; native objects and process RSS were not measured", + "Historical viewport source is exact; surrounding dependencies execute current audit versions" + ] +} diff --git a/docs/audits/browser-viewport-owner-retention/vitest.config.mjs b/docs/audits/browser-viewport-owner-retention/vitest.config.mjs new file mode 100644 index 00000000000..37cec186182 --- /dev/null +++ b/docs/audits/browser-viewport-owner-retention/vitest.config.mjs @@ -0,0 +1,36 @@ +import { readFileSync } from 'node:fs' +import { fileURLToPath } from 'node:url' +import base from '../../../config/vitest.config.ts' + +if (process.env.ORCA_BACKGROUND_LAUNCH !== '1') { + throw new Error('Set ORCA_BACKGROUND_LAUNCH=1 for the viewport ownership replay') +} + +const target = fileURLToPath( + new URL('../../../src/main/browser/browser-manager-viewport.ts', import.meta.url) +).replaceAll('\\', '/') + +export default { + ...base, + test: { + ...base.test, + include: ['src/main/browser/browser-manager-viewport-ownership.test.ts'] + }, + plugins: + process.env.ORCA_VIEWPORT_BASELINE === '1' + ? [ + { + name: 'viewport-owner-baseline', + enforce: 'pre', + transform(_source, id) { + return id.replaceAll('\\', '/').split('?')[0] === target + ? { + code: readFileSync(new URL('./baseline-source.txt', import.meta.url), 'utf8'), + map: null + } + : null + } + } + ] + : [] +} diff --git a/src/main/browser/browser-manager-viewport-ownership.test.ts b/src/main/browser/browser-manager-viewport-ownership.test.ts new file mode 100644 index 00000000000..7a98207a285 --- /dev/null +++ b/src/main/browser/browser-manager-viewport-ownership.test.ts @@ -0,0 +1,283 @@ +import { beforeEach, afterEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + appGetPathMock: vi.fn(() => '/downloads'), + shellOpenExternalMock: vi.fn(), + browserWindowFromWebContentsMock: vi.fn(), + menuBuildFromTemplateMock: vi.fn(), + guestOffMock: vi.fn(), + guestOnMock: vi.fn(), + guestSetBackgroundThrottlingMock: vi.fn(), + guestSetWindowOpenHandlerMock: vi.fn(), + guestOpenDevToolsMock: vi.fn(), + webContentsFromIdMock: vi.fn(), + screenGetCursorScreenPointMock: vi.fn(() => ({ x: 0, y: 0 })), + openPopupWithOriginBarMock: vi.fn() +})) + +vi.mock('electron', () => ({ + app: { getPath: mocks.appGetPathMock }, + BrowserWindow: { fromWebContents: mocks.browserWindowFromWebContentsMock }, + clipboard: { writeText: vi.fn() }, + shell: { openExternal: mocks.shellOpenExternalMock }, + Menu: { buildFromTemplate: mocks.menuBuildFromTemplateMock }, + screen: { getCursorScreenPoint: mocks.screenGetCursorScreenPointMock }, + webContents: { fromId: mocks.webContentsFromIdMock } +})) +vi.mock('./popup-origin-bar-window', () => ({ + openPopupWithOriginBar: mocks.openPopupWithOriginBarMock +})) + +import { browserManager } from './browser-manager' +import { resetBrowserManagerMocks, resetBrowserManagerState } from './browser-manager-test-harness' +import { createViewportGuestFactory } from './browser-manager-viewport-test-fixtures' + +const makeGuest = createViewportGuestFactory(mocks) +const mobile = { width: 375, height: 667, deviceScaleFactor: 2, mobile: true } +const desktop = { width: 1440, height: 900, deviceScaleFactor: 1, mobile: false } +const guests = new Map>() +const registeredGuests = readViewportStateMap('webContentsIdByTabId') +const uaIntents = readViewportStateMap('viewportUaOverrideMobileByTabId') +const presetIntents = readViewportStateMap('viewportPresetActiveByTabId') +const pendingOperations = readViewportStateMap('viewportOpsByTabId') + +function readViewportStateMap(name: string): Map { + const value: unknown = Reflect.get(browserManager, name) + if (!(value instanceof Map)) { + throw new Error(`Expected manager state map: ${name}`) + } + return value +} + +function register(tab: string, id: number, native = false) { + const handle = makeGuest(id) + guests.set(id, handle.guest) + expect( + browserManager.registerOffscreenGuest({ + browserPageId: tab, + webContentsId: id, + ...(native ? { userAgentMode: 'native' } : {}) + }) + ).toBe(true) + return handle +} + +function pause(handle: ReturnType, method: string) { + const entered = Promise.withResolvers() + const gate = Promise.withResolvers() + let blocked = false + handle.debuggerSendCommand.mockImplementation((next) => { + if (!blocked && next === method) { + blocked = true + entered.resolve() + return gate.promise + } + return Promise.resolve() + }) + return { entered: entered.promise, ...gate } +} + +describe('browser viewport operation ownership', () => { + beforeEach(() => { + expect(process.env.ORCA_BACKGROUND_LAUNCH).toBe('1') + resetBrowserManagerMocks(mocks) + resetBrowserManagerState() + guests.clear() + mocks.webContentsFromIdMock.mockImplementation((id) => guests.get(id)) + }) + afterEach(() => { + browserManager.unregisterAll() + vi.restoreAllMocks() + }) + + it('does not recreate closed-tab UA intent after a late touch completion', async () => { + const handle = register('closed', 100) + const gate = pause(handle, 'Emulation.setTouchEmulationEnabled') + const result = browserManager.setViewportOverride('closed', mobile) + await gate.entered + browserManager.unregisterGuest('closed') + expect(uaIntents.size).toBe(0) + const isDestroyed = handle.guest.isDestroyed + expect(vi.isMockFunction(isDestroyed)).toBe(true) + if (vi.isMockFunction(isDestroyed)) { + isDestroyed.mockReturnValue(true) + } + gate.resolve() + await expect(result).resolves.toBe(false) + expect(registeredGuests.size).toBe(0) + expect(presetIntents.size).toBe(0) + expect(uaIntents.get('closed')).toBeUndefined() + }) + + it('does not restore closed-tab UA intent after a failed clear', async () => { + const handle = register('clear-close', 101) + await expect(browserManager.setViewportOverride('clear-close', mobile)).resolves.toBe(true) + const gate = pause(handle, 'Emulation.setUserAgentOverride') + const result = browserManager.setViewportOverride('clear-close', null) + await gate.entered + browserManager.unregisterGuest('clear-close') + const isDestroyed = handle.guest.isDestroyed + expect(vi.isMockFunction(isDestroyed)).toBe(true) + if (vi.isMockFunction(isDestroyed)) { + isDestroyed.mockReturnValue(true) + } + gate.reject(new Error('Target closed')) + await expect(result).resolves.toBe(false) + expect(uaIntents.get('clear-close')).toBeUndefined() + }) + + it('preserves replacement desktop intent after an old clear fails', async () => { + const handle = register('replacement', 102) + await browserManager.setViewportOverride('replacement', mobile) + const gate = pause(handle, 'Emulation.setUserAgentOverride') + const result = browserManager.setViewportOverride('replacement', null) + await gate.entered + browserManager.unregisterGuest('replacement') + register('replacement', 103) + await expect(browserManager.setViewportOverride('replacement', desktop)).resolves.toBe(true) + expect(uaIntents.get('replacement')).toBe(false) + gate.reject(new Error('Old target closed')) + await expect(result).resolves.toBe(false) + expect(registeredGuests.get('replacement')).toBe(103) + expect(uaIntents.get('replacement')).toBe(false) + expect(presetIntents.get('replacement')).toEqual({ + guestWebContentsId: 103, + active: true + }) + }) + + it('preserves replacement mobile intent after an old clear resumes', async () => { + const handle = register('late-delete', 104) + await browserManager.setViewportOverride('late-delete', desktop) + const gate = pause(handle, 'Emulation.setTouchEmulationEnabled') + const result = browserManager.setViewportOverride('late-delete', null) + await gate.entered + browserManager.unregisterGuest('late-delete') + register('late-delete', 105) + await browserManager.setViewportOverride('late-delete', mobile) + gate.resolve() + await expect(result).resolves.toBe(false) + expect(uaIntents.has('late-delete')).toBe(true) + expect(registeredGuests.get('late-delete')).toBe(105) + }) + + it('same-owner clear failure still restores the legitimate earlier intent', async () => { + const handle = register('same-owner', 106) + await browserManager.setViewportOverride('same-owner', mobile) + const gate = pause(handle, 'Emulation.setUserAgentOverride') + const result = browserManager.setViewportOverride('same-owner', null) + await gate.entered + gate.reject(new Error('Protocol error')) + await expect(result).resolves.toBe(false) + expect(uaIntents.get('same-owner')).toBe(true) + }) + + it('old apply cannot overwrite a replacement guest desktop intent', async () => { + const old = register('late-apply', 110) + const gate = pause(old, 'Emulation.setTouchEmulationEnabled') + const pending = browserManager.setViewportOverride('late-apply', mobile) + await gate.entered + browserManager.unregisterGuest('late-apply') + register('late-apply', 111) + await browserManager.setViewportOverride('late-apply', desktop) + gate.resolve() + await expect(pending).resolves.toBe(false) + expect(uaIntents.get('late-apply')).toBe(false) + }) + + it('an old native profile cannot write UA intent after replacement with a default profile', async () => { + const old = register('native-replacement', 112, true) + const gate = pause(old, 'Emulation.setTouchEmulationEnabled') + const pending = browserManager.setViewportOverride('native-replacement', mobile) + await gate.entered + browserManager.unregisterGuest('native-replacement') + register('native-replacement', 113) + gate.resolve() + await expect(pending).resolves.toBe(false) + expect(uaIntents.get('native-replacement')).toBeUndefined() + }) + + it('old queued operations cannot remove or join a replacement promise tail', async () => { + const old = register('queued-replacement', 114) + const oldGate = pause(old, 'Emulation.setTouchEmulationEnabled') + const first = browserManager.setViewportOverride('queued-replacement', mobile) + const second = browserManager.setViewportOverride('queued-replacement', null) + await oldGate.entered + browserManager.unregisterGuest('queued-replacement') + const replacement = register('queued-replacement', 115) + const newGate = pause(replacement, 'Emulation.setTouchEmulationEnabled') + const replacementFirst = browserManager.setViewportOverride('queued-replacement', desktop) + const replacementSecond = browserManager.setViewportOverride('queued-replacement', mobile) + await newGate.entered + const tail = pendingOperations.get('queued-replacement') + oldGate.resolve() + await expect(first).resolves.toBe(false) + await expect(second).resolves.toBe(false) + expect(pendingOperations.get('queued-replacement')).toBe(tail) + newGate.resolve() + await expect(replacementFirst).resolves.toBe(true) + await expect(replacementSecond).resolves.toBe(true) + expect(pendingOperations.size).toBe(0) + expect(uaIntents.get('queued-replacement')).toBe(true) + }) + + it('normal same-owner toggles preserve last-requested order and remove the promise tail', async () => { + const handle = register('serialized', 116) + const gate = pause(handle, 'Emulation.setTouchEmulationEnabled') + const first = browserManager.setViewportOverride('serialized', mobile) + await gate.entered + const second = browserManager.setViewportOverride('serialized', desktop) + const third = browserManager.setViewportOverride('serialized', null) + gate.resolve() + expect(await Promise.all([first, second, third])).toEqual([true, true, true]) + expect(handle.debuggerSendCommand.mock.calls.map(([method]) => method)).toEqual([ + 'Emulation.setDeviceMetricsOverride', + 'Emulation.setTouchEmulationEnabled', + 'Emulation.setUserAgentOverride', + 'Emulation.setDeviceMetricsOverride', + 'Emulation.setTouchEmulationEnabled', + 'Emulation.setUserAgentOverride', + 'Emulation.clearDeviceMetricsOverride', + 'Emulation.setTouchEmulationEnabled', + 'Emulation.setUserAgentOverride' + ]) + expect(pendingOperations.size).toBe(0) + expect(uaIntents.size).toBe(0) + }) + + it.each([false, true])('native UA mode remains unchanged with mobile=%s', async (mobileMode) => { + const handle = register('native', 117, true) + await expect( + browserManager.setViewportOverride('native', mobileMode ? mobile : desktop) + ).resolves.toBe(true) + expect(handle.debuggerSendCommand).not.toHaveBeenCalledWith( + 'Emulation.setUserAgentOverride', + expect.anything() + ) + expect(uaIntents.size).toBe(0) + }) + + it('late rejected clears cannot repopulate all registries after unregisterAll', async () => { + const operations: { gate: ReturnType; pending: Promise }[] = [] + for (let index = 0; index < 16; index++) { + const tab = `all-closed-${index}` + const handle = register(tab, 200 + index) + await browserManager.setViewportOverride(tab, mobile) + const gate = pause(handle, 'Emulation.setUserAgentOverride') + const pending = browserManager.setViewportOverride(tab, null) + await gate.entered + operations.push({ gate, pending }) + } + browserManager.unregisterAll() + for (const { gate } of operations) { + gate.reject(new Error('Target closed')) + } + expect(await Promise.all(operations.map(({ pending }) => pending))).toEqual( + Array(16).fill(false) + ) + expect(uaIntents.size).toBe(0) + expect(registeredGuests.size).toBe(0) + expect(pendingOperations.size).toBe(0) + expect(presetIntents.size).toBe(0) + }) +}) diff --git a/src/main/browser/browser-manager-viewport.ts b/src/main/browser/browser-manager-viewport.ts index ce31dbe37e1..3f1fbb68fbe 100644 --- a/src/main/browser/browser-manager-viewport.ts +++ b/src/main/browser/browser-manager-viewport.ts @@ -163,6 +163,9 @@ export abstract class BrowserManagerViewport extends BrowserManagerDownloadLifec enabled: override.mobile, maxTouchPoints: override.mobile ? 5 : 0 }) + if (this.webContentsIdByTabId.get(browserTabId) !== webContentsId) { + return false + } // Why: viewport sizing must not override a profile's explicit native-UA identity. if (this.userAgentModeByPageId.get(browserTabId) !== 'native') { // Navigation must see the preset intent while the final CDP command is in flight. @@ -182,6 +185,9 @@ export abstract class BrowserManagerViewport extends BrowserManagerDownloadLifec enabled: false, maxTouchPoints: 0 }) + if (this.webContentsIdByTabId.get(browserTabId) !== webContentsId) { + return false + } const trackedMobile = this.viewportUaOverrideMobileByTabId.get(browserTabId) // A navigation after this point must not re-install the override behind the clear. this.viewportUaOverrideMobileByTabId.delete(browserTabId) @@ -202,7 +208,10 @@ export abstract class BrowserManagerViewport extends BrowserManagerDownloadLifec await dbg.sendCommand('Emulation.setUserAgentOverride', { userAgent: '' }) } } catch (error) { - if (trackedMobile !== undefined) { + if ( + trackedMobile !== undefined && + this.webContentsIdByTabId.get(browserTabId) === webContentsId + ) { this.viewportUaOverrideMobileByTabId.set(browserTabId, trackedMobile) } throw error