diff --git a/src/main/browser/browser-screencast-cdp-events.ts b/src/main/browser/browser-screencast-cdp-events.ts index dc34ef42d08..c787aca7044 100644 --- a/src/main/browser/browser-screencast-cdp-events.ts +++ b/src/main/browser/browser-screencast-cdp-events.ts @@ -1,5 +1,6 @@ import { Buffer } from 'node:buffer' import type { WebContents } from 'electron' +import type { BrowserScreencastFrameMetadata } from '../../shared/browser-screencast-protocol' import { sendDebuggerCommand } from './browser-screencast-debugger-command' import { enrichFrameMetadata, readFrameMetadata } from './browser-screencast-frame-metadata' import { readBrowserScreencastImageSize } from './browser-screencast-image-size' @@ -19,6 +20,8 @@ type BrowserScreencastMessageHandlerDeps = { scheduleNavigationFrameCapture: () => void clearNavigationCaptureTimer: () => void bumpSnapshotGeneration: () => void + /** True when the frame showed the guest at a size the stream must re-emulate for, so it is stale. */ + observeGuestViewSize: (metadata: BrowserScreencastFrameMetadata) => boolean setDialogOpen: (open: boolean) => void } @@ -28,7 +31,7 @@ export function createBrowserScreencastMessageHandler( const { dbg, options, isClosed, isStopping, queueFrame, ackScreencastFrame } = deps const { scheduleNavigationFrameCapture, clearNavigationCaptureTimer, bumpSnapshotGeneration } = deps - const { setDialogOpen } = deps + const { setDialogOpen, observeGuestViewSize } = deps return (_event: unknown, method: string, params: unknown): void => { if (isClosed()) { @@ -88,6 +91,11 @@ export function createBrowserScreencastMessageHandler( } try { + const metadata = readFrameMetadata(payload.metadata) + if (observeGuestViewSize(metadata)) { + ackScreencastFrame(sessionId) + return + } const decoded = Buffer.from(data, 'base64') const image = decoded.buffer.byteLength === decoded.byteLength ? decoded : new Uint8Array(decoded) @@ -105,7 +113,7 @@ export function createBrowserScreencastMessageHandler( bumpSnapshotGeneration() clearNavigationCaptureTimer() queueFrame({ - metadata: enrichFrameMetadata(readFrameMetadata(payload.metadata), imageSize, options), + metadata: enrichFrameMetadata(metadata, imageSize, options), image, sessionId }) diff --git a/src/main/browser/browser-screencast-guest-view-limit.test.ts b/src/main/browser/browser-screencast-guest-view-limit.test.ts new file mode 100644 index 00000000000..4c5bd8ccd39 --- /dev/null +++ b/src/main/browser/browser-screencast-guest-view-limit.test.ts @@ -0,0 +1,233 @@ +import { Buffer } from 'node:buffer' +import { describe, expect, it, vi } from 'vitest' + +import { decodeBrowserScreencastFrame } from '../../shared/browser-screencast-protocol' +import { clampScreencastViewportToGuestView } from './browser-screencast-guest-view-limit' +import { startBrowserScreencast } from './browser-screencast-stream' +import { createMockScreencastWebContents } from './browser-screencast-web-contents-test-double' + +function jpegWithSize(width: number, height: number): Buffer { + return Buffer.from([ + 0xff, + 0xd8, + 0xff, + 0xc0, + 0x00, + 0x11, + 0x08, + (height >> 8) & 0xff, + height & 0xff, + (width >> 8) & 0xff, + width & 0xff, + 0x03, + 0x01, + 0x11, + 0x00, + 0x02, + 0x11, + 0x00, + 0x03, + 0x11, + 0x00, + 0xff, + 0xd9 + ]) +} + +function createGuest(type: 'webview' | 'window') { + return Object.assign(createMockScreencastWebContents(), { getType: vi.fn(() => type) }) +} + +// What a guest streams: its element's size, whatever viewport is emulated. +function emitGuestFrame( + guest: ReturnType, + sessionId: number, + view: { width: number; height: number } +): void { + guest.debugger.emit('message', {}, 'Page.screencastFrame', { + data: jpegWithSize(view.width, view.height).toString('base64'), + sessionId, + metadata: { deviceWidth: view.width, deviceHeight: view.height } + }) +} + +// Each distinct emulated size in order; the stream re-applies one before every snapshot. +function emulatedSizes(guest: ReturnType): string[] { + const sizes = guest.debugger.sendCommand.mock.calls + .filter(([method]) => method === 'Emulation.setDeviceMetricsOverride') + .map(([, params]) => `${params.width}x${params.height}`) + return sizes.filter((size, index) => size !== sizes[index - 1]) +} + +async function startOversizedStream(guest: ReturnType, onFrame = vi.fn()) { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the double implements every WebContents member the stream calls; the other screencast tests pass it the same way. + return startBrowserScreencast(guest as never, { + format: 'jpeg', + quality: 70, + maxWidth: 3840, + maxHeight: 2160, + viewportWidth: 1833, + viewportHeight: 1179, + deviceScaleFactor: 2, + everyNthFrame: 1, + minFrameIntervalMs: 0, + onFrame + }) +} + +describe('clampScreencastViewportToGuestView', () => { + it('shrinks each side the guest cannot paint and keeps the rest of the request', () => { + expect( + clampScreencastViewportToGuestView( + { viewportWidth: 1833, viewportHeight: 1179, deviceScaleFactor: 2, mobile: false }, + { width: 1463, height: 849 } + ) + ).toEqual({ viewportWidth: 1463, viewportHeight: 849, deviceScaleFactor: 2, mobile: false }) + expect( + clampScreencastViewportToGuestView( + { viewportWidth: 390, viewportHeight: 900 }, + { width: 1463, height: 849 } + ) + ).toEqual({ viewportWidth: 390, viewportHeight: 849 }) + }) + + it('never goes below the smallest viewport a client may request', () => { + expect( + clampScreencastViewportToGuestView( + { viewportWidth: 800, viewportHeight: 600 }, + { width: 1, height: 1 } + ) + ).toEqual({ viewportWidth: 320, viewportHeight: 240 }) + }) + + it('leaves a sizeless request alone', () => { + expect(clampScreencastViewportToGuestView({}, { width: 600, height: 400 })).toEqual({}) + }) +}) + +describe('screencast of a guest smaller than the requested viewport (#30341)', () => { + it('re-emulates at the size the guest can paint and labels frames with it', async () => { + const guest = createGuest('webview') + const onFrame = vi.fn() + const session = await startOversizedStream(guest, onFrame) + await vi.waitFor(() => + expect(guest.debugger.sendCommand).toHaveBeenCalledWith( + 'Page.captureScreenshot', + expect.anything() + ) + ) + expect(emulatedSizes(guest)).toEqual(['1833x1179']) + + // This frame is the top-left crop of a 1833x1179 layout: it must not reach the client. + emitGuestFrame(guest, 1, { width: 1463, height: 849 }) + await vi.waitFor(() => expect(emulatedSizes(guest)).toEqual(['1833x1179', '1463x849'])) + expect(guest.debugger.sendCommand).toHaveBeenCalledWith('Page.screencastFrameAck', { + sessionId: 1 + }) + expect(onFrame).not.toHaveBeenCalled() + + emitGuestFrame(guest, 2, { width: 1463, height: 849 }) + await vi.waitFor(() => expect(onFrame).toHaveBeenCalledTimes(1)) + expect(decodeBrowserScreencastFrame(onFrame.mock.calls[0][0])?.metadata).toMatchObject({ + deviceWidth: 1463, + deviceHeight: 849 + }) + + session.stop() + await session.done + }) + + it('gives the requested size back once the guest grows, and follows a new request', async () => { + const guest = createGuest('webview') + const session = await startOversizedStream(guest) + emitGuestFrame(guest, 1, { width: 1463, height: 849 }) + await vi.waitFor(() => expect(emulatedSizes(guest)).toHaveLength(2)) + + emitGuestFrame(guest, 2, { width: 2000, height: 1300 }) + await vi.waitFor(() => expect(emulatedSizes(guest).at(-1)).toBe('1833x1179')) + + await session.updateViewport({ viewportWidth: 1028, viewportHeight: 1179 }) + expect(emulatedSizes(guest).at(-1)).toBe('1028x1179') + + emitGuestFrame(guest, 3, { width: 1463, height: 849 }) + await vi.waitFor(() => expect(emulatedSizes(guest).at(-1)).toBe('1028x849')) + + session.stop() + await session.done + }) + + it('keeps dropping guest frames until the fitted size is actually emulated', async () => { + const guest = createGuest('webview') + let releaseSnapshot!: () => void + const snapshotHeld = new Promise((resolve) => { + releaseSnapshot = resolve + }) + guest.debugger.sendCommand.mockImplementation(async (method: string) => { + if (method === 'Page.captureScreenshot') { + await snapshotHeld + } + return {} + }) + const onFrame = vi.fn() + const session = await startOversizedStream(guest, onFrame) + + emitGuestFrame(guest, 1, { width: 1463, height: 849 }) + emitGuestFrame(guest, 2, { width: 1463, height: 849 }) + await vi.waitFor(() => + expect(guest.debugger.sendCommand).toHaveBeenCalledWith('Page.screencastFrameAck', { + sessionId: 2 + }) + ) + expect(onFrame).not.toHaveBeenCalled() + expect(emulatedSizes(guest)).toEqual(['1833x1179']) + + releaseSnapshot() + await vi.waitFor(() => expect(emulatedSizes(guest)).toEqual(['1833x1179', '1463x849'])) + emitGuestFrame(guest, 3, { width: 1463, height: 849 }) + await vi.waitFor(() => expect(onFrame).toHaveBeenCalledTimes(1)) + + session.stop() + await session.done + }) + + it('resumes live frames once the fitted size is emulated, before its snapshot returns', async () => { + const guest = createGuest('webview') + let screenshots = 0 + guest.debugger.sendCommand.mockImplementation(async (method: string) => { + if (method === 'Page.captureScreenshot') { + screenshots += 1 + if (screenshots > 1) { + await new Promise(() => {}) + } + } + return {} + }) + const onFrame = vi.fn() + const session = await startOversizedStream(guest, onFrame) + await vi.waitFor(() => expect(screenshots).toBe(1)) + + emitGuestFrame(guest, 1, { width: 1463, height: 849 }) + await vi.waitFor(() => expect(screenshots).toBe(2)) + expect(emulatedSizes(guest)).toEqual(['1833x1179', '1463x849']) + emitGuestFrame(guest, 2, { width: 1463, height: 849 }) + await vi.waitFor(() => expect(onFrame).toHaveBeenCalledTimes(1)) + + session.stop() + }) + + it('keeps the full request for a window guest, which can grow to it', async () => { + const guest = createGuest('window') + const session = await startOversizedStream(guest) + + emitGuestFrame(guest, 1, { width: 1463, height: 849 }) + await vi.waitFor(() => + expect(guest.debugger.sendCommand).toHaveBeenCalledWith('Page.screencastFrameAck', { + sessionId: 1 + }) + ) + expect(emulatedSizes(guest)).toEqual(['1833x1179']) + + session.stop() + await session.done + }) +}) diff --git a/src/main/browser/browser-screencast-guest-view-limit.ts b/src/main/browser/browser-screencast-guest-view-limit.ts new file mode 100644 index 00000000000..cb3d0c4898f --- /dev/null +++ b/src/main/browser/browser-screencast-guest-view-limit.ts @@ -0,0 +1,63 @@ +import type { WebContents } from 'electron' +import type { BrowserScreencastFrameMetadata } from '../../shared/browser-screencast-protocol' +import type { BrowserScreencastViewport } from './browser-screencast-stream-types' + +export type BrowserScreencastGuestViewSize = { + width: number + height: number +} + +// The runtime's own floor for a requested viewport (normalizeScreencastViewport). +const MIN_VIEWPORT_WIDTH = 320 +const MIN_VIEWPORT_HEIGHT = 240 + +/** + * Whether this guest can only paint inside its embedder's element. A `` guest's + * compositor surface is the element's size: an emulated viewport larger than that is laid out + * at full size but its frames and `capturePage` hold only the element's top-left part, and + * `Emulation.setVisibleSize` does not grow it (measured on Electron 43, 2026-10-18). An + * offscreen `BrowserWindow` does grow, so it keeps the full requested size. + */ +export function isEmbedderSizedScreencastGuest(webContents: Pick): boolean { + return typeof webContents.getType === 'function' && webContents.getType() === 'webview' +} + +/** For an embedder-sized guest every live frame reports the element's CSS size, whatever is emulated. */ +export function readScreencastGuestViewSize( + metadata: BrowserScreencastFrameMetadata +): BrowserScreencastGuestViewSize | null { + const width = metadata.deviceWidth + const height = metadata.deviceHeight + if ( + typeof width !== 'number' || + typeof height !== 'number' || + !Number.isFinite(width) || + !Number.isFinite(height) || + width < 1 || + height < 1 + ) { + return null + } + // Why floor: a fractional element must not be asked for the pixel row it cannot paint. + return { width: Math.floor(width), height: Math.floor(height) } +} + +/** + * The viewport to emulate: the client's request, shrunk to what the guest can paint. Frames then + * cover the whole emulated page and report its real size, so a client letterboxes and maps + * input against what it was sent instead of a stretched crop. + */ +export function clampScreencastViewportToGuestView( + requested: BrowserScreencastViewport, + view: BrowserScreencastGuestViewSize | null +): BrowserScreencastViewport { + const { viewportWidth, viewportHeight } = requested + if (!view || viewportWidth === undefined || viewportHeight === undefined) { + return { ...requested } + } + return { + ...requested, + viewportWidth: Math.min(viewportWidth, Math.max(MIN_VIEWPORT_WIDTH, view.width)), + viewportHeight: Math.min(viewportHeight, Math.max(MIN_VIEWPORT_HEIGHT, view.height)) + } +} diff --git a/src/main/browser/browser-screencast-lifecycle.test.ts b/src/main/browser/browser-screencast-lifecycle.test.ts index fdf586b0ef8..a4a80e59f9c 100644 --- a/src/main/browser/browser-screencast-lifecycle.test.ts +++ b/src/main/browser/browser-screencast-lifecycle.test.ts @@ -144,6 +144,7 @@ describe('browser screencast lifecycle', () => { scheduleNavigationFrameCapture: vi.fn(), clearNavigationCaptureTimer: vi.fn(), bumpSnapshotGeneration: vi.fn(), + observeGuestViewSize: () => false, setDialogOpen: vi.fn() }) diff --git a/src/main/browser/browser-screencast-live-frame-allocation.test.ts b/src/main/browser/browser-screencast-live-frame-allocation.test.ts index f4801931b26..daf938794fb 100644 --- a/src/main/browser/browser-screencast-live-frame-allocation.test.ts +++ b/src/main/browser/browser-screencast-live-frame-allocation.test.ts @@ -55,6 +55,7 @@ function createLiveFrameRoute( scheduleNavigationFrameCapture: () => {}, clearNavigationCaptureTimer: () => {}, bumpSnapshotGeneration: () => {}, + observeGuestViewSize: () => false, setDialogOpen: () => {} }) return { diff --git a/src/main/browser/browser-screencast-stream.ts b/src/main/browser/browser-screencast-stream.ts index 6456f4e3102..c9d57522be5 100644 --- a/src/main/browser/browser-screencast-stream.ts +++ b/src/main/browser/browser-screencast-stream.ts @@ -6,6 +6,12 @@ import { sendDebuggerCommand } from './browser-screencast-debugger-command' import { createBrowserScreencastDeviceMetrics } from './browser-screencast-device-metrics' import { createBrowserScreencastFramePacer } from './browser-screencast-frame-pacer' import { createBrowserScreencastSnapshotCapture } from './browser-screencast-snapshot-capture' +import { + clampScreencastViewportToGuestView, + isEmbedderSizedScreencastGuest, + readScreencastGuestViewSize, + type BrowserScreencastGuestViewSize +} from './browser-screencast-guest-view-limit' import type { BrowserScreencastFrameBudget, BrowserScreencastOptions, @@ -55,6 +61,30 @@ export async function startBrowserScreencast( const isClosed = (): boolean => closed const isStopping = (): boolean => stopping + // Why: `options` carries what is emulated; the client's own request is kept apart so the guest + // view limit can shrink it and later give the size back when the guest grows. + const embedderSizedGuest = isEmbedderSizedScreencastGuest(webContents) + let requestedViewport: BrowserScreencastViewport = { + viewportWidth: options.viewportWidth, + viewportHeight: options.viewportHeight, + deviceScaleFactor: options.deviceScaleFactor, + mobile: options.mobile + } + let guestViewSize: BrowserScreencastGuestViewSize | null = null + // Bumped when the emulated size changes; caught up once a device-metrics write at it lands. + let emulationGeneration = 0 + let appliedEmulationGeneration = 0 + const applyEffectiveViewport = (): boolean => { + const next = clampScreencastViewportToGuestView(requestedViewport, guestViewSize) + const changed = + next.viewportWidth !== options.viewportWidth || next.viewportHeight !== options.viewportHeight + Object.assign(options, next) + if (changed) { + emulationGeneration += 1 + } + return changed + } + const deviceMetrics = createBrowserScreencastDeviceMetrics(webContents, dbg, options) const framePacer = createBrowserScreencastFramePacer({ dbg, options, isClosed, isStopping }) const snapshotCapture = createBrowserScreencastSnapshotCapture({ @@ -65,7 +95,17 @@ export async function startBrowserScreencast( isStopping, getSeq: framePacer.getSeq, queueFrame: framePacer.queueFrame, - applyDeviceMetricsOverride: deviceMetrics.apply + applyDeviceMetricsOverride: () => { + const generation = emulationGeneration + const applied = deviceMetrics.apply() + // Why also on failure: a refused write must not hold every live frame back for good. + // Why not awaited here: the capture behind it keeps the exact timing it had. + const markApplied = (): void => { + appliedEmulationGeneration = Math.max(appliedEmulationGeneration, generation) + } + applied.then(markApplied, markApplied) + return applied + } }) const handleMessage = createBrowserScreencastMessageHandler({ dbg, @@ -77,6 +117,21 @@ export async function startBrowserScreencast( scheduleNavigationFrameCapture: snapshotCapture.scheduleNavigationFrameCapture, clearNavigationCaptureTimer: snapshotCapture.clearNavigationCaptureTimer, bumpSnapshotGeneration: snapshotCapture.bumpGeneration, + observeGuestViewSize: (metadata) => { + if (!embedderSizedGuest) { + return false + } + const size = readScreencastGuestViewSize(metadata) + if (size && (guestViewSize?.width !== size.width || guestViewSize.height !== size.height)) { + guestViewSize = size + if (applyEffectiveViewport()) { + void reemulate().catch(() => {}) + } + } + // Why until applied: a queued re-emulation can wait behind a slow snapshot, and every frame + // until the fitted metrics land is a crop of the old layout that passes the new size check. + return appliedEmulationGeneration < emulationGeneration + }, setDialogOpen: (open: boolean) => { dialogOpen = open // A new dialog is a new answer, and a closed one leaves nothing to answer. @@ -85,6 +140,24 @@ export async function startBrowserScreencast( } }) + // Re-applies the emulated viewport and sends a frame at it, queued behind earlier updates. + const reemulate = (): Promise => { + pendingUpdate = pendingUpdate + .catch(() => {}) + .then(async () => { + if (closed || stopping) { + return + } + snapshotCapture.bumpGeneration() + snapshotCapture.clearNavigationCaptureTimer() + framePacer.clearPending(true) + // Why: initialOnly skips every stream that ever emitted a frame, which would + // leave the old device metrics applied and the new subscriber frameless. + await snapshotCapture.emitSnapshotFrame(false) + }) + return pendingUpdate + } + const startScreencast = (): Promise => sendDebuggerCommand(dbg, 'Page.startScreencast', { format: options.format, @@ -168,21 +241,11 @@ export async function startBrowserScreencast( return settlement }, updateViewport: (viewport: BrowserScreencastViewport) => { - pendingUpdate = pendingUpdate - .catch(() => {}) - .then(async () => { - if (closed || stopping) { - return - } - Object.assign(options, viewport) - snapshotCapture.bumpGeneration() - snapshotCapture.clearNavigationCaptureTimer() - framePacer.clearPending(true) - // Why: initialOnly skips every stream that ever emitted a frame, which would - // leave the old device metrics applied and the new subscriber frameless. - await snapshotCapture.emitSnapshotFrame(false) - }) - return pendingUpdate + if (!closed && !stopping) { + requestedViewport = { ...viewport } + applyEffectiveViewport() + } + return reemulate() }, updateFrameBudget: (budget: BrowserScreencastFrameBudget) => { pendingUpdate = pendingUpdate diff --git a/src/renderer/src/components/browser-pane/stream-remote/remote-browser-frame-style.test.ts b/src/renderer/src/components/browser-pane/stream-remote/remote-browser-frame-style.test.ts index e348439cb3c..d7a6a23374c 100644 --- a/src/renderer/src/components/browser-pane/stream-remote/remote-browser-frame-style.test.ts +++ b/src/renderer/src/components/browser-pane/stream-remote/remote-browser-frame-style.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from 'vitest' import { getRemoteBrowserFrameStyle } from './remote-browser-frame-style' describe('getRemoteBrowserFrameStyle', () => { - it('fills the client viewport even when stale metadata reports an oversized bitmap', () => { + it('fits the frame by the CSS viewport it shows, not by the bitmap size', () => { expect( getRemoteBrowserFrameStyle({ imageWidth: 2266, @@ -11,6 +11,16 @@ describe('getRemoteBrowserFrameStyle', () => { deviceHeight: 609 }) ).toEqual({ + inset: 0, + margin: 'auto', + width: `min(100cqw, calc(100cqh * ${958 / 609}))`, + height: `min(100cqh, calc(100cqw / ${958 / 609}))`, + objectFit: 'fill' + }) + }) + + it('fills the pane when the frame does not say what it shows', () => { + expect(getRemoteBrowserFrameStyle({ imageWidth: 800, imageHeight: 600 })).toEqual({ width: '100%', height: '100%', objectFit: 'fill', diff --git a/src/renderer/src/components/browser-pane/stream-remote/remote-browser-frame-style.ts b/src/renderer/src/components/browser-pane/stream-remote/remote-browser-frame-style.ts index 19014dff557..640136a0b01 100644 --- a/src/renderer/src/components/browser-pane/stream-remote/remote-browser-frame-style.ts +++ b/src/renderer/src/components/browser-pane/stream-remote/remote-browser-frame-style.ts @@ -1,16 +1,26 @@ import type { CSSProperties } from 'react' import type { BrowserScreencastFrameMetadata } from '../../../../../shared/browser-screencast-protocol' +import { getPositiveFiniteNumber } from './remote-browser-page-input-model' +// Why the device size and not the bitmap's: the host labels every frame with the CSS viewport it +// shows, while the bitmap's own size moves with DPR and the frame budget's scaling. export function getRemoteBrowserFrameStyle( - _metadata: BrowserScreencastFrameMetadata | null + metadata: BrowserScreencastFrameMetadata | null ): CSSProperties { - // Why: the runtime now rejects server-sized frames when a client viewport was - // requested. The renderer should display that authoritative viewport exactly, - // not infer crop/contain behavior from transient bitmap dimensions. + const width = getPositiveFiniteNumber(metadata?.deviceWidth) + const height = getPositiveFiniteNumber(metadata?.deviceHeight) + if (!width || !height) { + return { width: '100%', height: '100%', objectFit: 'fill', objectPosition: 'top left' } + } + // Why contain: a host can stream a smaller viewport than this pane asked for (it cannot paint + // past its own view), and a frame from before a resize lands in a pane of another shape. + // Stretching either distorts the page. Pointer input reads this box back off the image. + const aspect = width / height return { - width: '100%', - height: '100%', - objectFit: 'fill', - objectPosition: 'top left' + inset: 0, + margin: 'auto', + width: `min(100cqw, calc(100cqh * ${aspect}))`, + height: `min(100cqh, calc(100cqw / ${aspect}))`, + objectFit: 'fill' } } diff --git a/src/renderer/src/components/browser-pane/stream-remote/remote-browser-page-input-model.test.ts b/src/renderer/src/components/browser-pane/stream-remote/remote-browser-page-input-model.test.ts index c54cd9e5bc9..97254e5a703 100644 --- a/src/renderer/src/components/browser-pane/stream-remote/remote-browser-page-input-model.test.ts +++ b/src/renderer/src/components/browser-pane/stream-remote/remote-browser-page-input-model.test.ts @@ -3,6 +3,7 @@ import { buildRemoteContextMenuExpression, getPositiveFiniteNumber, getRemoteBrowserMouseButton, + mapRemoteBrowserClientPoint, readRemoteContextMenuResult, readRemoteCssViewportSize, resolveRemoteBrowserCssViewport @@ -110,4 +111,60 @@ describe('remote browser page input model', () => { ).toEqual(naturalSize) }) }) + + // #30341: the host can only paint 1463x849 of the 1833x1179 this pane asked for, so the frame + // is letterboxed (contain) inside the pane and input must go through that same box. + describe('pointer mapping for a frame smaller than the pane', () => { + const pane = { left: 0, top: 0, width: 1833, height: 1179 } + const frame = { width: 1463, height: 849 } + const scale = Math.min(pane.width / frame.width, pane.height / frame.height) + const frameRect = { + width: frame.width * scale, + height: frame.height * scale, + left: (pane.width - frame.width * scale) / 2, + top: (pane.height - frame.height * scale) / 2 + } + const cssViewport = resolveRemoteBrowserCssViewport({ + cssViewportSize: { width: 1833, height: 1179 }, + requestedViewportSize: { width: 1833, height: 1179 }, + frameMetadata: { deviceWidth: frame.width, deviceHeight: frame.height }, + naturalSize: { width: 2926, height: 1698 } + }) + const screenOf = (x: number, y: number) => ({ + clientX: frameRect.left + x * scale, + clientY: frameRect.top + y * scale + }) + + it('lands on the page point drawn under the pointer, corners included', () => { + expect(cssViewport).toEqual(frame) + for (const [x, y] of [ + [0, 0], + [731, 424], + [1463, 849], + [1200, 100] + ]) { + expect(mapRemoteBrowserClientPoint({ ...screenOf(x, y), frameRect, cssViewport })).toEqual({ + x, + y + }) + } + }) + + it('clamps a pointer in the letterbox to the page edge', () => { + expect( + mapRemoteBrowserClientPoint({ clientX: 10, clientY: 2, frameRect, cssViewport }) + ).toEqual({ x: 8, y: 0 }) + }) + + it('refuses a frame that has no box yet', () => { + expect( + mapRemoteBrowserClientPoint({ + clientX: 1, + clientY: 1, + frameRect: { left: 0, top: 0, width: 0, height: 0 }, + cssViewport + }) + ).toBeNull() + }) + }) }) diff --git a/src/renderer/src/components/browser-pane/stream-remote/remote-browser-page-input-model.ts b/src/renderer/src/components/browser-pane/stream-remote/remote-browser-page-input-model.ts index e40dcaef0fd..f4d0ceefb6c 100644 --- a/src/renderer/src/components/browser-pane/stream-remote/remote-browser-page-input-model.ts +++ b/src/renderer/src/components/browser-pane/stream-remote/remote-browser-page-input-model.ts @@ -189,6 +189,31 @@ export function resolveRemoteBrowserCssViewport(input: { } } +// Why the image's own box and not the pane's: the frame is letterboxed inside the pane, so only +// the rect it is drawn in says which page pixel sits under the pointer. +export function mapRemoteBrowserClientPoint(input: { + clientX: number + clientY: number + frameRect: Pick + cssViewport: RemoteBrowserViewportSize +}): RemoteBrowserImagePoint | null { + const { frameRect, cssViewport } = input + if ( + frameRect.width <= 0 || + frameRect.height <= 0 || + cssViewport.width <= 0 || + cssViewport.height <= 0 + ) { + return null + } + const x = ((input.clientX - frameRect.left) / frameRect.width) * cssViewport.width + const y = ((input.clientY - frameRect.top) / frameRect.height) * cssViewport.height + return { + x: Math.round(Math.min(cssViewport.width, Math.max(0, x))), + y: Math.round(Math.min(cssViewport.height, Math.max(0, y))) + } +} + export function getPositiveFiniteNumber(value: unknown): number | null { return typeof value === 'number' && Number.isFinite(value) && value > 0 ? value : null } diff --git a/src/renderer/src/components/browser-pane/stream-remote/remote-browser-page-pane.tsx b/src/renderer/src/components/browser-pane/stream-remote/remote-browser-page-pane.tsx index f447b5432d1..851def90ade 100644 --- a/src/renderer/src/components/browser-pane/stream-remote/remote-browser-page-pane.tsx +++ b/src/renderer/src/components/browser-pane/stream-remote/remote-browser-page-pane.tsx @@ -323,11 +323,11 @@ export function RemoteBrowserPagePane({ const markup = useMarkupMode({ getCaptureContext: useCallback((): MarkupCaptureContext | null => { const element = imageRef.current - const container = remoteViewportRef.current - if (!element || !container) { + if (!element || !remoteViewportRef.current) { return null } - const rect = container.getBoundingClientRect() + // Why the image's box: the frame is letterboxed, and the pane's shape would distort it. + const rect = element.getBoundingClientRect() if (rect.width <= 0 || rect.height <= 0) { return null } diff --git a/src/renderer/src/components/browser-pane/stream-remote/remote-browser-page-viewport.tsx b/src/renderer/src/components/browser-pane/stream-remote/remote-browser-page-viewport.tsx index 31ae068e8b7..f4d6a3f966e 100644 --- a/src/renderer/src/components/browser-pane/stream-remote/remote-browser-page-viewport.tsx +++ b/src/renderer/src/components/browser-pane/stream-remote/remote-browser-page-viewport.tsx @@ -88,15 +88,19 @@ export function RemoteBrowserPageViewport({
{markup.isActive && markup.baseImage ? ( - void markup.complete(input)} - onCancel={markup.cancel} - /> + // Why the frame's own box: markup draws over the letterboxed page, not the whole pane. +
+ void markup.complete(input)} + onCancel={markup.cancel} + /> +
) : null} {frameUrl ? ( { expect(harness.currentStatusKind).toBe('live') }) + // #30341: the resize found no subscription to restart, so the stream kept the opening size. + it('restarts at the new size when the pane was resized while its stream was opening', async () => { + const harness = createHarness() + const subscribe = harness.holdNextSubscribe() + harness.lifecycle.open() + await settle() + + harness.setViewportSize({ width: 1400, height: 900 }) + expect(harness.lifecycle.restartForViewport('page-1')).toBe(false) + subscribe.release() + await settle() + + expect(harness.streams.map((stream) => stream.viewportWidth)).toEqual([800, 1400]) + expect(harness.streams[0].unsubscribeCount).toBe(1) + }) + it('ignores a viewport change that is within measurement jitter', async () => { const harness = createHarness() await openStreamAndConfirmReady(harness) diff --git a/src/renderer/src/components/browser-pane/stream-remote/remote-browser-stream-lifecycle.ts b/src/renderer/src/components/browser-pane/stream-remote/remote-browser-stream-lifecycle.ts index 91af28a938d..8efd0b4b734 100644 --- a/src/renderer/src/components/browser-pane/stream-remote/remote-browser-stream-lifecycle.ts +++ b/src/renderer/src/components/browser-pane/stream-remote/remote-browser-stream-lifecycle.ts @@ -132,7 +132,8 @@ export class RemoteBrowserStreamLifecycle { // Why: the runtime stream validates frames against its start viewport, so a resize needs a fresh // subscription or the new-size frames get rejected. - restartForViewport(pageId: string): void { + // Returns whether a restart was started. + restartForViewport(pageId: string): boolean { const current = this.subscription const nextViewportSize = this.deps.readViewportSize() if ( @@ -142,7 +143,7 @@ export class RemoteBrowserStreamLifecycle { areRemoteViewportSizesNear(this.streamViewportSize, nextViewportSize) || !this.tokens.isCurrentStreamToken(current.token) ) { - return + return false } // Why: cancel() cannot recall a retry already dispatched into an await. Bumping the operation // generation — as every other supersession site does — makes that attempt's own token guard @@ -183,6 +184,7 @@ export class RemoteBrowserStreamLifecycle { // A resize during a blip tears down a live subscription and never reaches the budget. this.setStatus(remoteBrowserStreamStopped(failure.message)) }) + return true } // Drops every handle this pane holds on a runtime page that is gone. @@ -222,6 +224,9 @@ export class RemoteBrowserStreamLifecycle { return } this.subscription = subscription + // Why: a resize while this stream was opening found no subscription to restart, and nothing + // else would re-send the size, so the pane kept the old one until the next resize. + this.restartForViewport(subscription.token.remotePageId) } private async startStream(pageId: string): Promise { diff --git a/src/renderer/src/components/browser-pane/stream-remote/use-remote-browser-page-input.ts b/src/renderer/src/components/browser-pane/stream-remote/use-remote-browser-page-input.ts index c58a5c56a1a..1b9cc44226b 100644 --- a/src/renderer/src/components/browser-pane/stream-remote/use-remote-browser-page-input.ts +++ b/src/renderer/src/components/browser-pane/stream-remote/use-remote-browser-page-input.ts @@ -14,6 +14,7 @@ import type { import type { BrowserScreencastFrameMetadata } from '../../../../../shared/browser-screencast-protocol' import { getRemoteBrowserMouseButton, + mapRemoteBrowserClientPoint, resolveRemoteBrowserCssViewport, type PendingRemoteBrowserWheel, type RemoteBrowserPaneNotice, @@ -107,24 +108,20 @@ export function useRemoteBrowserPageInput({ const getRemoteImagePoint = useCallback( (event: { clientX: number; clientY: number }): { x: number; y: number } | null => { const image = imageRef.current - const viewport = remoteViewportRef.current - if (!image || !viewport) { + if (!image || !remoteViewportRef.current) { return null } - const rect = viewport.getBoundingClientRect() - const { width: viewportWidth, height: viewportHeight } = resolveRemoteBrowserCssViewport({ - cssViewportSize: remoteCssViewportSizeRef.current, - requestedViewportSize: remoteViewportSizeRef.current, - frameMetadata, - naturalSize: { width: image.naturalWidth, height: image.naturalHeight } + return mapRemoteBrowserClientPoint({ + clientX: event.clientX, + clientY: event.clientY, + frameRect: image.getBoundingClientRect(), + cssViewport: resolveRemoteBrowserCssViewport({ + cssViewportSize: remoteCssViewportSizeRef.current, + requestedViewportSize: remoteViewportSizeRef.current, + frameMetadata, + naturalSize: { width: image.naturalWidth, height: image.naturalHeight } + }) }) - if (rect.width <= 0 || rect.height <= 0 || viewportWidth <= 0 || viewportHeight <= 0) { - return null - } - return { - x: Math.round(((event.clientX - rect.left) / rect.width) * viewportWidth), - y: Math.round(((event.clientY - rect.top) / rect.height) * viewportHeight) - } }, [frameMetadata, imageRef, remoteCssViewportSizeRef, remoteViewportRef, remoteViewportSizeRef] ) @@ -147,6 +144,15 @@ export function useRemoteBrowserPageInput({ } event.preventDefault() image.focus() + // Why: a release over the letterbox gutter or outside the pane must still send mouseUp, or + // the host page keeps the button held. + if (typeof image.setPointerCapture === 'function') { + try { + image.setPointerCapture(event.pointerId) + } catch { + // The pointer is already gone; its release will not arrive anywhere to capture. + } + } setPaneNotice(null) enqueueRemoteInput(async () => { if (!isCurrentRemoteOperationToken(operationToken)) { diff --git a/src/renderer/src/components/browser-pane/stream-remote/use-remote-browser-page-stream.ts b/src/renderer/src/components/browser-pane/stream-remote/use-remote-browser-page-stream.ts index 5ebd1c91bd3..fb3430d1f82 100644 --- a/src/renderer/src/components/browser-pane/stream-remote/use-remote-browser-page-stream.ts +++ b/src/renderer/src/components/browser-pane/stream-remote/use-remote-browser-page-stream.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useLayoutEffect, useState } from 'react' +import { useCallback, useEffect, useLayoutEffect, useRef, useState } from 'react' import { callRuntimeRpc } from '@/runtime/runtime-rpc-client' import { decodeBrowserScreencastFrame } from '../../../../../shared/browser-screencast-protocol' @@ -12,7 +12,6 @@ import type { import { useRemoteBrowserStreamActivation } from './use-remote-browser-stream-activation' import { decodeRemoteBrowserFrameUrl, - getRemoteBrowserDeviceScaleFactor, readRemoteCssViewportSize, type RemoteBrowserPaneNotice, type RemoteBrowserRuntimeTarget, @@ -72,6 +71,7 @@ export function useRemoteBrowserPageStream({ } { // Bumped by Reconnect to re-run the open effect from scratch. See reconnectRemoteStream. const [reopenNonce, setReopenNonce] = useState(0) + const lastFrameDeviceSizeRef = useRef(null) const rememberRemoteViewportSize = useCallback( (next: RemoteBrowserViewportSize): RemoteBrowserViewportSize => { @@ -130,19 +130,9 @@ export function useRemoteBrowserPageStream({ if (!target || !size) { return } - await callRuntimeRpc( - target, - 'browser.viewport', - { - worktree: runtimeWorktree, - page: pageId, - width: size.width, - height: size.height, - deviceScaleFactor: getRemoteBrowserDeviceScaleFactor(), - mobile: false - }, - { timeoutMs: 15_000, suppressFeatureInteraction: true } - ) + // Why no browser.viewport here: the screencast subscription already carries this pane's + // size and the host's stream owns the emulation. A second, unowned override re-applied + // the full pane size over the host's fitted one, so its frames came back cropped (#30341). try { // Why: the streamed bitmap can include the host compositor surface, but CDP input wants the guest page's CSS viewport coords. const viewport = await callRuntimeRpc( @@ -155,9 +145,10 @@ export function useRemoteBrowserPageStream({ }, { timeoutMs: 15_000, suppressFeatureInteraction: true } ) - remoteCssViewportSizeRef.current = readRemoteCssViewportSize(viewport) ?? size + remoteCssViewportSizeRef.current = readRemoteCssViewportSize(viewport) } catch { - remoteCssViewportSizeRef.current = size + // Unknown: pointer mapping falls back to the requested and per-frame sizes. + remoteCssViewportSizeRef.current = null } }, [readRemoteViewportSize, remoteCssViewportSizeRef, runtimeTarget, runtimeWorktree] @@ -182,9 +173,13 @@ export function useRemoteBrowserPageStream({ if (!pageId || !isActiveRef.current) { return } - void syncRemoteViewport(pageId) - .then(() => lifecycle.restartForViewport(pageId)) - .catch(() => {}) + // Why restart first: the new stream applies the size, and its ready handler re-reads the + // page's CSS viewport after that. Reading it now would only see the old emulation. + if (lifecycle.restartForViewport(pageId)) { + remoteCssViewportSizeRef.current = null + } else { + void syncRemoteViewport(pageId).catch(() => {}) + } }, 150) } scheduleSync() @@ -202,6 +197,7 @@ export function useRemoteBrowserPageStream({ isActiveRef, lifecycle, readRemoteViewportSize, + remoteCssViewportSizeRef, remoteViewportRef, remoteViewportTimerRef, syncRemoteViewport @@ -236,6 +232,18 @@ export function useRemoteBrowserPageStream({ } const prevUrl = streamFrameUrlRef.current streamFrameUrlRef.current = nextUrl + // Why: the host can change the viewport it shows on its own (it fits it to the view it + // can paint), and the cached CSS size then describes a layout that is gone. + const deviceSize = `${frame.metadata.deviceWidth}x${frame.metadata.deviceHeight}` + const previousDeviceSize = lastFrameDeviceSizeRef.current + lastFrameDeviceSizeRef.current = deviceSize + if (previousDeviceSize !== null && previousDeviceSize !== deviceSize) { + remoteCssViewportSizeRef.current = null + const pageId = lifecycle.tokens.remotePage + if (pageId) { + void syncRemoteViewport(pageId).catch(() => {}) + } + } setFrameMetadata(frame.metadata) setFrameUrl(nextUrl) setPaneBusy(false) @@ -250,7 +258,9 @@ export function useRemoteBrowserPageStream({ [ lifecycle, pendingFrameDecodeRef, + remoteCssViewportSizeRef, setFrameMetadata, + syncRemoteViewport, setFrameUrl, setPaneBusy, streamFrameUrlRef