From c4e58fac56cb14138c80d52dbca4ba296dce084e Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Thu, 24 Sep 2026 14:47:12 -0400 Subject: [PATCH] fix(mobile): restart the streamed browser pane on every return to the app (#22694) * fix(mobile): restart the streamed browser pane on every return to the app The browser pane stops its screencast when the app leaves the foreground and starts a new one when it comes back, keyed on an `appActive` boolean. When the leave and the return are handled in one React render (the JS thread was held across the whole trip, as a suspended or frozen app is), React applies false-then-true as no change, so the stream effect never re-runs: the old subscription is kept and no new one starts. If the desktop ended that subscription while the phone was away (it evicts a viewer whose socket refuses 90 frames in a row), the pane shows the last frame it had indefinitely, with no error and nothing that would restart it. The pane now keeps `foregroundVisit`: null while the app is away and a new id on each return. A batched leave-and-return still moves it to a new value, so every return starts a fresh stream, and the host's start snapshot repaints the pane. * refactor(mobile): drop a busy reset both stream-effect branches overwrite --- mobile/src/browser/MobileBrowserPane.tsx | 16 +- .../mobile-browser-app-resume.test.tsx | 151 ++++++++++++++++++ .../src/browser/use-mobile-browser-stream.ts | 10 +- 3 files changed, 167 insertions(+), 10 deletions(-) create mode 100644 mobile/src/browser/mobile-browser-app-resume.test.tsx diff --git a/mobile/src/browser/MobileBrowserPane.tsx b/mobile/src/browser/MobileBrowserPane.tsx index f9a32c2d66f..3ab40f1e444 100644 --- a/mobile/src/browser/MobileBrowserPane.tsx +++ b/mobile/src/browser/MobileBrowserPane.tsx @@ -92,7 +92,11 @@ export function MobileBrowserPane({ const [pointerModifiers, setPointerModifiers] = useState([]) const [zoom, setZoom] = useState(DEFAULT_ZOOM) const [layout, setLayout] = useState(null) - const [appActive, setAppActive] = useState(AppState.currentState === 'active') + // Why: a new id per return, so a leave and return that React batches into one render still restart the stream. + const [foregroundVisit, setForegroundVisit] = useState( + AppState.currentState === 'active' ? 0 : null + ) + const foregroundVisitCountRef = useRef(0) const streamGenerationRef = useRef(0) const layoutRef = useRef(null) const frameMetadataRef = useRef( @@ -139,11 +143,13 @@ export function MobileBrowserPane({ useEffect(() => { const subscription = AppState.addEventListener('change', (nextState) => { - const active = nextState === 'active' - if (!active) { + if (nextState !== 'active') { clearCachedBrowserFramesForWorktree(worktreeId) + setForegroundVisit(null) + return } - setAppActive(active) + foregroundVisitCountRef.current += 1 + setForegroundVisit(foregroundVisitCountRef.current) }) return () => { subscription.remove() @@ -185,7 +191,6 @@ export function MobileBrowserPane({ const { frameGeometry, frameLayers, pageParams, renderedFrameSource, sendBrowserRequest } = useMobileBrowserStream({ - appActive, binaryScreencastGranted, browserViewMode, busyRef, @@ -193,6 +198,7 @@ export function MobileBrowserPane({ client, frameMetadata, frameMetadataRef, + foregroundVisit, initialFrameUri: cachedInitialFrame?.uri ?? null, lastStreamCacheKeyRef, lastZoomResetUrlRef, diff --git a/mobile/src/browser/mobile-browser-app-resume.test.tsx b/mobile/src/browser/mobile-browser-app-resume.test.tsx new file mode 100644 index 00000000000..968e32d3847 --- /dev/null +++ b/mobile/src/browser/mobile-browser-app-resume.test.tsx @@ -0,0 +1,151 @@ +import { createElement } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { RpcClient } from '../transport/rpc-client' +import { MobileBrowserPane, type MobileBrowserTab } from './MobileBrowserPane' + +const appState = vi.hoisted(() => ({ + listeners: new Set<(state: string) => void>(), + emit(state: string) { + for (const listener of appState.listeners) { + listener(state) + } + } +})) + +vi.mock('./use-browser-binary-screencast-grant', () => ({ + useBrowserBinaryScreencastGrant: () => true +})) + +vi.mock('react-native', () => ({ + ActivityIndicator: 'ActivityIndicator', + AppState: { + currentState: 'active', + addEventListener: (_type: string, listener: (state: string) => void) => { + appState.listeners.add(listener) + return { remove: () => appState.listeners.delete(listener) } + } + }, + Image: 'Image', + PanResponder: { create: () => ({ panHandlers: {} }) }, + PixelRatio: { get: () => 2 }, + Platform: { OS: 'android' }, + Pressable: 'Pressable', + StyleSheet: { + absoluteFillObject: { position: 'absolute', top: 0, left: 0, right: 0, bottom: 0 }, + create: (styles: unknown) => styles + }, + Text: 'Text', + TextInput: 'TextInput', + View: 'View' +})) + +vi.mock('lucide-react-native', () => ({ + ArrowUp: 'ArrowUp', + ChevronLeft: 'ChevronLeft', + ChevronRight: 'ChevronRight', + Monitor: 'Monitor', + RefreshCw: 'RefreshCw', + Smartphone: 'Smartphone' +})) + +type Subscription = { closed: boolean } + +let pageCounter = 0 +let renderer: ReactTestRenderer | null = null + +afterEach(() => { + act(() => renderer?.unmount()) + renderer = null + appState.listeners.clear() +}) + +async function renderStreamingPane(): Promise { + pageCounter += 1 + const subscriptions: Subscription[] = [] + const client: RpcClient = { + sendRequest: vi.fn(), + subscribe: () => { + const subscription = { closed: false } + subscriptions.push(subscription) + return () => { + subscription.closed = true + } + }, + updateTerminalSubscriptionViewport: vi.fn(), + getState: () => 'connected', + getReconnectAttempt: () => 0, + getLastConnectedAt: () => null, + onStateChange: () => () => {}, + notifyForeground: vi.fn(), + close: vi.fn() + } + const tab: MobileBrowserTab = { + type: 'browser', + id: `tab-${pageCounter}`, + title: 'Dashboard', + browserWorkspaceId: 'bw-1', + browserPageId: `page-${pageCounter}`, + url: 'https://dashboard.example', + loading: false, + canGoBack: false, + canGoForward: false, + isActive: true + } + await act(async () => { + renderer = create( + createElement(MobileBrowserPane, { + client, + worktreeId: `wt-${pageCounter}`, + tab, + screencastSupported: true, + keyboardLift: 0, + bottomInset: 0, + onToast: () => {} + }), + { createNodeMock: () => ({ setNativeProps: () => {} }) } + ) + await Promise.resolve() + }) + // Host components are strings under the react-native double. + const hostView: string = 'View' + const viewport = renderer?.root.find( + (node) => node.type === hostView && typeof node.props.onLayout === 'function' + ) + act(() => { + viewport?.props.onLayout({ nativeEvent: { layout: { width: 360, height: 640 } } }) + }) + expect(subscriptions).toHaveLength(1) + return subscriptions +} + +function openStreams(subscriptions: Subscription[]): number { + return subscriptions.filter((subscription) => !subscription.closed).length +} + +describe('MobileBrowserPane across leaving the app and coming back', () => { + it('stops the stream in the background and starts a new one on return', async () => { + const subscriptions = await renderStreamingPane() + + act(() => appState.emit('background')) + expect(openStreams(subscriptions)).toBe(0) + + act(() => appState.emit('active')) + expect(subscriptions).toHaveLength(2) + expect(openStreams(subscriptions)).toBe(1) + }) + + // Why: a quick leave and return can land in one React batch, which nets the two changes out. + it('starts a new stream when the leave and the return land in one render', async () => { + const subscriptions = await renderStreamingPane() + + act(() => { + appState.emit('background') + appState.emit('active') + }) + + expect(subscriptions).toHaveLength(2) + expect(subscriptions[0].closed).toBe(true) + expect(openStreams(subscriptions)).toBe(1) + }) +}) diff --git a/mobile/src/browser/use-mobile-browser-stream.ts b/mobile/src/browser/use-mobile-browser-stream.ts index 1e04d8cd524..4f3597e0b0e 100644 --- a/mobile/src/browser/use-mobile-browser-stream.ts +++ b/mobile/src/browser/use-mobile-browser-stream.ts @@ -29,7 +29,6 @@ import { createBrowserFramePacer } from './browser-frame-pacer' import { useMobileBrowserRequest } from './use-mobile-browser-request' type MobileBrowserStreamArgs = { - appActive: boolean binaryScreencastGranted: boolean browserViewMode: MobileBrowserViewMode busyRef: { current: boolean } @@ -37,6 +36,8 @@ type MobileBrowserStreamArgs = { client: RpcClient | null frameMetadata: BrowserScreencastFrameMetadata | null frameMetadataRef: { current: BrowserScreencastFrameMetadata | null } + /** Null while the app is away; each return to the foreground is a new value. */ + foregroundVisit: number | null initialFrameUri: string | null lastStreamCacheKeyRef: { current: string | null } lastZoomResetUrlRef: { current: string } @@ -57,7 +58,6 @@ type MobileBrowserStreamArgs = { export function useMobileBrowserStream(args: MobileBrowserStreamArgs) { const { - appActive, binaryScreencastGranted, browserViewMode, busyRef, @@ -65,6 +65,7 @@ export function useMobileBrowserStream(args: MobileBrowserStreamArgs) { client, frameMetadata, frameMetadataRef, + foregroundVisit, initialFrameUri, lastStreamCacheKeyRef, lastZoomResetUrlRef, @@ -155,7 +156,6 @@ export function useMobileBrowserStream(args: MobileBrowserStreamArgs) { frameMetadataRef.current = cachedFrame?.metadata ?? null setFrameMetadata(cachedFrame?.metadata ?? null) } - busyRef.current = false setDialog(null) setError(null) if ( @@ -163,7 +163,7 @@ export function useMobileBrowserStream(args: MobileBrowserStreamArgs) { !binaryScreencastGranted || screencastSupported !== true || !tab.browserPageId || - !appActive || + foregroundVisit === null || !streamRequest ) { busyRef.current = false @@ -238,9 +238,9 @@ export function useMobileBrowserStream(args: MobileBrowserStreamArgs) { unsubscribe() } }, [ - appActive, binaryScreencastGranted, client, + foregroundVisit, framePacer, resetBrowserZoomState, screencastSupported,