fix(mobile): retire browser pointer queues with their pane (#34825)

* fix(mobile): retire browser pointer queues with their pane

* test(mobile): use T[] array types in pointer retirement test

---------

Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>

mobile/src/browser/use-mobile-browser-commands.ts

@@ -17,13 +17,10 @@ import {
   browserPointerClick,
   browserPointerDown,
   browserPointerMove,
-  browserPointerUp,
-  browserPointerWheel
+  browserPointerUp
 } from './mobile-browser-command-operations'
 import type { BrowserPageCommandSend, BrowserPageParams } from './use-mobile-browser-request'
 import {
-  browserWheelDeltaFromScreen,
-  computeBrowserFrameGeometry,
   computeBrowserTouchClickRadiusCss,
   mapScreenToBrowserPoint,
   type BrowserPoint,
@@ -32,15 +29,9 @@ import {
 } from './browser-touch-geometry'
 import type { BrowserPointerModifier } from './MobileBrowserPointerModifiers'
 import type { BrowserDialogState } from './mobile-browser-stream-events'
+import { useMobileBrowserWheelQueue } from './use-mobile-browser-wheel-queue'
 
 const TOUCH_CLICK_RADIUS_DIP = 14
-type PendingWheelCommand = {
-  base: BrowserPageParams
-  point: BrowserPoint
-  gestureId: number
-  dx: number
-  dy: number
-}
 type SendBrowserRequest = (
   send: BrowserPageCommandSend,
   options?: { showBusy?: boolean; suppressError?: boolean; timeoutMs?: number }
@@ -79,8 +70,15 @@ export function useMobileBrowserCommands(args: MobileBrowserCommandArgs) {
     zoomRef
   } = args
 
-  const pendingWheelCommandRef = useRef<PendingWheelCommand | null>(null)
   const dialogAnswerTokenRef = useRef(0)
+  const { commandGenerationRef, commandsMountedRef, sendWheel } = useMobileBrowserWheelQueue({
+    client,
+    frameMetadataRef,
+    layoutRef,
+    pageParams,
+    setError,
+    zoomRef
+  })
   // A send claims the draft here, so a repeat tap before re-render finds it empty. The pane remounts
   // per page, so the draft outlives a reconnect; only unmount ends a send's claim on it.
   const keyboardDraftRef = useRef({ mounted: true, value: keyboardValue })
@@ -95,48 +93,13 @@ export function useMobileBrowserCommands(args: MobileBrowserCommandArgs) {
     }
   }, [])
 
-  const wheelCommandInFlightRef = useRef(false)
-
-  const flushPendingWheelCommand = useCallback(() => {
-    if (wheelCommandInFlightRef.current) {
-      return
-    }
-    const pending = pendingWheelCommandRef.current
-    if (!pending || !client) {
-      return
-    }
-    pendingWheelCommandRef.current = null
-    wheelCommandInFlightRef.current = true
-    void (async () => {
-      try {
-        const moveReply = await browserPointerMove.request(client, {
-          ...pending.base,
-          x: pending.point.x,
-          y: pending.point.y
-        })
-        browserPointerMove.interpret(moveReply)
-        const wheelReply = await browserPointerWheel.request(client, {
-          ...pending.base,
-          dx: pending.dx,
-          dy: pending.dy
-        })
-        browserPointerWheel.interpret(wheelReply)
-        setError(null)
-      } catch {
-        // Scroll bursts race navigation; keep usable stream instead of replacing it with transient selector errors.
-      } finally {
-        wheelCommandInFlightRef.current = false
-        flushPendingWheelCommand()
-      }
-    })()
-  }, [client])
-
   const sendPointerClick = useCallback(
     async (point: BrowserPoint, button: 'left' | 'right') => {
       const base = pageParams()
-      if (!client || !base) {
+      if (!commandsMountedRef.current || !client || !base) {
         return
       }
+      const generation = commandGenerationRef.current
       try {
         browserPointerClick.interpret(
           await browserPointerClick.request(
@@ -161,11 +124,17 @@ export function useMobileBrowserCommands(args: MobileBrowserCommandArgs) {
             { timeoutMs: 5_000 }
           )
         )
-        setError(null)
+        if (generation === commandGenerationRef.current) {
+          setError(null)
+        }
         return
       } catch (error) {
         // Why: a timed-out click may still run on the host, and the move/down/up replay drops modifiers.
-        if (isRpcDeliveryUnknown(error) || pointerModifiers.length > 0) {
+        if (
+          generation !== commandGenerationRef.current ||
+          isRpcDeliveryUnknown(error) ||
+          pointerModifiers.length > 0
+        ) {
           return
         }
       }
@@ -176,11 +145,17 @@ export function useMobileBrowserCommands(args: MobileBrowserCommandArgs) {
           y: point.y
         })
         browserPointerMove.interpret(moveReply)
+        if (generation !== commandGenerationRef.current) {
+          return
+        }
         const downReply = await browserPointerDown.request(client, { ...base, button })
         browserPointerDown.interpret(downReply)
+        // Release an admitted press even when its pane retired while mouseDown was pending.
         const upReply = await browserPointerUp.request(client, { ...base, button })
         browserPointerUp.interpret(upReply)
-        setError(null)
+        if (generation === commandGenerationRef.current) {
+          setError(null)
+        }
       } catch {
         // Pointer sends race navigation; keep stream and surface actionable failures through navigation/stream paths.
       }
@@ -196,33 +171,6 @@ export function useMobileBrowserCommands(args: MobileBrowserCommandArgs) {
     )
   }, [])
 
-  const sendWheel = useCallback(
-    (point: BrowserPoint, screenDx: number, screenDy: number, gestureId: number) => {
-      const base = pageParams()
-      if (!client || !base) {
-        return
-      }
-      const geometry = computeBrowserFrameGeometry(layoutRef.current, frameMetadataRef.current)
-      const delta = browserWheelDeltaFromScreen(screenDx, screenDy, geometry, zoomRef.current.scale)
-      if (Math.abs(delta.dx) < 1 && Math.abs(delta.dy) < 1) {
-        return
-      }
-      const pending = pendingWheelCommandRef.current
-      pendingWheelCommandRef.current =
-        pending && pending.base.page === base.page && pending.gestureId === gestureId
-          ? {
-              base,
-              point,
-              gestureId,
-              dx: pending.dx + delta.dx,
-              dy: pending.dy + delta.dy
-            }
-          : { base, point, gestureId, ...delta }
-      flushPendingWheelCommand()
-    },
-    [client, flushPendingWheelCommand, pageParams]
-  )
-
   const mapTouchPoint = useCallback((locationX: number, locationY: number): BrowserPoint | null => {
     return mapScreenToBrowserPoint(
       locationX,

mobile/src/browser/use-mobile-browser-pointer-retirement.test.tsx

@@ -0,0 +1,268 @@
+import { createElement } from 'react'
+import { act, create, type ReactTestRenderer } from 'react-test-renderer'
+import { afterEach, describe, expect, it } from 'vitest'
+import type { RpcClient } from '../transport/rpc-client'
+import { stubRpcClient } from '../transport/rpc-client-test-stub'
+import type { RpcResponse } from '../transport/types'
+import { useMobileBrowserCommands } from './use-mobile-browser-commands'
+import { useMobileBrowserRequest } from './use-mobile-browser-request'
+
+function deferred<Value>() {
+  let resolve!: (value: Value) => void
+  const promise = new Promise<Value>((settle) => {
+    resolve = settle
+  })
+  return { promise, resolve }
+}
+
+const accepted: RpcResponse = { id: 'reply', ok: true, result: {} }
+const refused: RpcResponse = {
+  id: 'reply',
+  ok: false,
+  error: { code: 'unknown-method', message: 'Unknown method' }
+}
+type Commands = ReturnType<typeof useMobileBrowserCommands>
+const renderers: ReactTestRenderer[] = []
+
+afterEach(() => {
+  act(() => {
+    for (const renderer of renderers.splice(0)) {
+      renderer.unmount()
+    }
+  })
+})
+
+function mount(client: RpcClient) {
+  let commands: Commands | null = null
+  const errors: (string | null)[] = []
+  function Screen({ currentClient }: { currentClient: RpcClient }) {
+    const { pageParams, sendBrowserRequest } = useMobileBrowserRequest({
+      busyRef: { current: false },
+      client: currentClient,
+      pageId: 'page',
+      setBusy: () => {},
+      setError: () => {},
+      worktreeId: 'workspace'
+    })
+    commands = useMobileBrowserCommands({
+      client: currentClient,
+      pageParams,
+      sendBrowserRequest,
+      frameMetadataRef: { current: null },
+      keyboardValue: '',
+      layoutRef: { current: null },
+      onToast: () => {},
+      pointerModifiers: [],
+      setDialog: () => {},
+      setError: (value) => {
+        if (typeof value !== 'function') {
+          errors.push(value)
+        }
+      },
+      setKeyboardValue: () => {},
+      setPointerModifiers: () => {},
+      zoomRef: { current: { scale: 1, offsetX: 0, offsetY: 0 } }
+    })
+    return null
+  }
+  let renderer!: ReactTestRenderer
+  act(() => {
+    renderer = create(createElement(Screen, { currentClient: client }))
+  })
+  renderers.push(renderer)
+  return {
+    get commands(): Commands {
+      if (!commands) {
+        throw new Error('not mounted')
+      }
+      return commands
+    },
+    errors,
+    unmount: () => act(() => renderer.unmount()),
+    replace: (currentClient: RpcClient) =>
+      act(() => renderer.update(createElement(Screen, { currentClient })))
+  }
+}
+
+describe('browser pointer commands after owner retirement', () => {
+  it.each([8, 80])('does not drain scroll work for %i departed panes', async (owners) => {
+    const move = deferred<RpcResponse>()
+    const calls: string[] = []
+    const client = stubRpcClient({
+      sendRequest: async (method) => {
+        calls.push(method)
+        return method === 'browser.mouseMove' ? move.promise : accepted
+      }
+    })
+    for (let index = 0; index < owners; index++) {
+      const screen = mount(client)
+      screen.commands.sendWheel({ x: 10, y: 20 }, 0, 10, index)
+      screen.commands.sendWheel({ x: 10, y: 25 }, 0, 20, index)
+      screen.unmount()
+    }
+    expect(calls).toEqual(Array.from({ length: owners }, () => 'browser.mouseMove'))
+    await act(async () => {
+      move.resolve(accepted)
+      await move.promise
+    })
+    expect(calls).toHaveLength(owners)
+
+    const liveCalls: { method: string; params: unknown }[] = []
+    const live = mount(
+      stubRpcClient({
+        sendRequest: async (method, params) => {
+          liveCalls.push({ method, params })
+          return accepted
+        }
+      })
+    )
+    await act(async () => {
+      live.commands.sendWheel({ x: 5, y: 6 }, 0, 30, 1)
+    })
+    expect(liveCalls).toEqual([
+      {
+        method: 'browser.mouseMove',
+        params: { worktree: 'id:workspace', page: 'page', x: 5, y: 6 }
+      },
+      {
+        method: 'browser.mouseWheel',
+        params: { worktree: 'id:workspace', page: 'page', dx: 0, dy: -30 }
+      }
+    ])
+    expect(live.errors).toEqual([null])
+  })
+
+  it('does not replay a late refused click after leaving the pane', async () => {
+    const click = deferred<RpcResponse>()
+    const calls: string[] = []
+    const screen = mount(
+      stubRpcClient({
+        sendRequest: async (method) => {
+          calls.push(method)
+          return method === 'browser.mouseClick' ? click.promise : accepted
+        }
+      })
+    )
+    const pending = screen.commands.sendPointerClick({ x: 10, y: 20 }, 'left')
+    screen.unmount()
+    await act(async () => {
+      click.resolve(refused)
+      await pending
+    })
+    expect(calls).toEqual(['browser.mouseClick'])
+    expect(screen.errors).toEqual([])
+  })
+
+  it('releases an already admitted mouse down after leaving the pane', async () => {
+    const down = deferred<RpcResponse>()
+    const calls: string[] = []
+    const screen = mount(
+      stubRpcClient({
+        sendRequest: async (method) => {
+          calls.push(method)
+          if (method === 'browser.mouseClick') {
+            return refused
+          }
+          return method === 'browser.mouseDown' ? down.promise : accepted
+        }
+      })
+    )
+    let pending!: Promise<void>
+    await act(async () => {
+      pending = screen.commands.sendPointerClick({ x: 10, y: 20 }, 'left')
+    })
+    expect(calls).toEqual(['browser.mouseClick', 'browser.mouseMove', 'browser.mouseDown'])
+    screen.unmount()
+    await act(async () => {
+      down.resolve(accepted)
+      await pending
+    })
+    expect(calls.at(-1)).toBe('browser.mouseUp')
+    expect(screen.errors).toEqual([])
+  })
+
+  it('does not start a fallback press after its move outlives the pane', async () => {
+    const move = deferred<RpcResponse>()
+    const calls: string[] = []
+    const screen = mount(
+      stubRpcClient({
+        sendRequest: async (method) => {
+          calls.push(method)
+          if (method === 'browser.mouseClick') {
+            return refused
+          }
+          return method === 'browser.mouseMove' ? move.promise : accepted
+        }
+      })
+    )
+    let pending!: Promise<void>
+    await act(async () => {
+      pending = screen.commands.sendPointerClick({ x: 10, y: 20 }, 'left')
+    })
+    expect(calls).toEqual(['browser.mouseClick', 'browser.mouseMove'])
+    screen.unmount()
+    await act(async () => {
+      move.resolve(accepted)
+      await pending
+    })
+    expect(calls).toEqual(['browser.mouseClick', 'browser.mouseMove'])
+  })
+
+  it('lets a replacement client scroll without an old callback draining its queue', async () => {
+    const oldMove = deferred<RpcResponse>()
+    const oldCalls: string[] = []
+    const screen = mount(
+      stubRpcClient({
+        sendRequest: async (method) => {
+          oldCalls.push(method)
+          return method === 'browser.mouseMove' ? oldMove.promise : accepted
+        }
+      })
+    )
+    screen.commands.sendWheel({ x: 1, y: 2 }, 0, 3, 1)
+    const nextMove = deferred<RpcResponse>()
+    const newCalls: { method: string; params: unknown }[] = []
+    screen.replace(
+      stubRpcClient({
+        sendRequest: async (method, params) => {
+          newCalls.push({ method, params })
+          return method === 'browser.mouseMove' && newCalls.length === 1
+            ? nextMove.promise
+            : accepted
+        }
+      })
+    )
+    screen.commands.sendWheel({ x: 4, y: 5 }, 0, 6, 2)
+    screen.commands.sendWheel({ x: 7, y: 8 }, 0, 9, 2)
+    screen.commands.sendWheel({ x: 7, y: 8 }, 0, 12, 2)
+    expect(newCalls.map(({ method }) => method)).toEqual(['browser.mouseMove'])
+    await act(async () => {
+      oldMove.resolve(accepted)
+      await oldMove.promise
+    })
+    expect(oldCalls).toEqual(['browser.mouseMove'])
+    expect(newCalls).toHaveLength(1)
+    await act(async () => {
+      nextMove.resolve(accepted)
+      await nextMove.promise
+    })
+    expect(newCalls).toEqual([
+      {
+        method: 'browser.mouseMove',
+        params: { worktree: 'id:workspace', page: 'page', x: 4, y: 5 }
+      },
+      {
+        method: 'browser.mouseWheel',
+        params: { worktree: 'id:workspace', page: 'page', dx: 0, dy: -6 }
+      },
+      {
+        method: 'browser.mouseMove',
+        params: { worktree: 'id:workspace', page: 'page', x: 7, y: 8 }
+      },
+      {
+        method: 'browser.mouseWheel',
+        params: { worktree: 'id:workspace', page: 'page', dx: 0, dy: -21 }
+      }
+    ])
+  })
+})

mobile/src/browser/use-mobile-browser-wheel-queue.ts

@@ -0,0 +1,119 @@
+import { useCallback, useLayoutEffect, useRef, type Dispatch, type SetStateAction } from 'react'
+import type { RpcClient } from '../transport/rpc-client'
+import type { BrowserScreencastFrameMetadata } from '../transport/browser-screencast-protocol'
+import { browserPointerMove, browserPointerWheel } from './mobile-browser-command-operations'
+import type { BrowserPageParams } from './use-mobile-browser-request'
+import {
+  browserWheelDeltaFromScreen,
+  computeBrowserFrameGeometry,
+  type BrowserPoint,
+  type BrowserTouchLayout,
+  type BrowserZoomState
+} from './browser-touch-geometry'
+
+type PendingWheelCommand = {
+  base: BrowserPageParams
+  point: BrowserPoint
+  gestureId: number
+  dx: number
+  dy: number
+}
+
+type BrowserWheelQueueArgs = {
+  client: RpcClient | null
+  frameMetadataRef: { current: BrowserScreencastFrameMetadata | null }
+  layoutRef: { current: BrowserTouchLayout | null }
+  pageParams: () => BrowserPageParams | null
+  setError: Dispatch<SetStateAction<string | null>>
+  zoomRef: { current: BrowserZoomState }
+}
+
+// Retires queued wheel work with its pane/client so departed owners cannot drain into a live one.
+export function useMobileBrowserWheelQueue(args: BrowserWheelQueueArgs) {
+  const { client, frameMetadataRef, layoutRef, pageParams, setError, zoomRef } = args
+  const pendingWheelCommandRef = useRef<PendingWheelCommand | null>(null)
+  const wheelCommandInFlightRef = useRef(false)
+  const commandGenerationRef = useRef(0)
+  const commandsMountedRef = useRef(false)
+
+  useLayoutEffect(() => {
+    commandsMountedRef.current = true
+    return () => {
+      commandsMountedRef.current = false
+      commandGenerationRef.current += 1
+      pendingWheelCommandRef.current = null
+      wheelCommandInFlightRef.current = false
+    }
+  }, [client, pageParams])
+
+  const flushPendingWheelCommand = useCallback(() => {
+    if (!commandsMountedRef.current || wheelCommandInFlightRef.current) {
+      return
+    }
+    const pending = pendingWheelCommandRef.current
+    if (!pending || !client) {
+      return
+    }
+    pendingWheelCommandRef.current = null
+    wheelCommandInFlightRef.current = true
+    const generation = commandGenerationRef.current
+    void (async () => {
+      try {
+        const moveReply = await browserPointerMove.request(client, {
+          ...pending.base,
+          x: pending.point.x,
+          y: pending.point.y
+        })
+        browserPointerMove.interpret(moveReply)
+        if (generation !== commandGenerationRef.current) {
+          return
+        }
+        const wheelReply = await browserPointerWheel.request(client, {
+          ...pending.base,
+          dx: pending.dx,
+          dy: pending.dy
+        })
+        browserPointerWheel.interpret(wheelReply)
+        if (generation === commandGenerationRef.current) {
+          setError(null)
+        }
+      } catch {
+        // Scroll bursts race navigation; keep usable stream instead of replacing it with transient selector errors.
+      } finally {
+        if (generation === commandGenerationRef.current) {
+          wheelCommandInFlightRef.current = false
+          flushPendingWheelCommand()
+        }
+      }
+    })()
+  }, [client])
+
+  const sendWheel = useCallback(
+    (point: BrowserPoint, screenDx: number, screenDy: number, gestureId: number) => {
+      const base = pageParams()
+      if (!commandsMountedRef.current || !client || !base) {
+        return
+      }
+      const geometry = computeBrowserFrameGeometry(layoutRef.current, frameMetadataRef.current)
+      const delta = browserWheelDeltaFromScreen(screenDx, screenDy, geometry, zoomRef.current.scale)
+      if (Math.abs(delta.dx) < 1 && Math.abs(delta.dy) < 1) {
+        return
+      }
+      const pending = pendingWheelCommandRef.current
+      pendingWheelCommandRef.current =
+        pending && pending.base.page === base.page && pending.gestureId === gestureId
+          ? {
+              base,
+              point,
+              gestureId,
+              dx: pending.dx + delta.dx,
+              dy: pending.dy + delta.dy
+            }
+          : { base, point, gestureId, ...delta }
+      flushPendingWheelCommand()
+    },
+    [client, flushPendingWheelCommand, pageParams]
+  )
+
+  return { commandGenerationRef, commandsMountedRef, sendWheel }
+}