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

@@ -92,7 +92,11 @@ export function MobileBrowserPane({
   const [pointerModifiers, setPointerModifiers] = useState<BrowserPointerModifier[]>([])
   const [zoom, setZoom] = useState<BrowserZoomState>(DEFAULT_ZOOM)
   const [layout, setLayout] = useState<BrowserTouchLayout | null>(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<number | null>(
+    AppState.currentState === 'active' ? 0 : null
+  )
+  const foregroundVisitCountRef = useRef(0)
   const streamGenerationRef = useRef(0)
   const layoutRef = useRef<BrowserTouchLayout | null>(null)
   const frameMetadataRef = useRef<BrowserScreencastFrameMetadata | null>(
@@ -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,

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<Subscription[]> {
+  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)
+  })
+})

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,