fix(mobile): keep the streamed browser pane flipping on slow phones, and stop double taps

[Jinwoo-H]

	Files	Added	Deleted	Net
Test	6	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​860	$\color{#cf222e}{\Huge{\mathbf{−}}}$​320	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​540
Prod	797	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​1191	$\color{#cf222e}{\Huge{\mathbf{−}}}$​1392	$\color{#cf222e}{\Huge{\mathbf{−}}}$​201

ELI5

The phone shows a browser tab by drawing a stream of screenshots. It loads each new picture in a hidden slot and swaps it in when it's ready. When pictures arrived faster than a slow phone could load them, each new one replaced the one still loading, so the swap never happened and the tab looked frozen while the page was busy. Now the phone finishes loading one picture before it starts the next. Separately, a tap that timed out could be sent a second time. The phone no longer re-sends a tap the computer may already have run.

What Changed

Frame pacing (freeze during page loads, menus, refresh).

• Before: the pane decoded each frame on a hidden <Image> layer and flipped layers on onLoad. While that decode ran, every newer frame re-pointed the same hidden layer, and the replaced load's onLoad was usually lost. If a decode took longer than the gap between frames (~100 ms), no onLoad arrived and the pane stayed on an old frame until the page went still. A pane re-render (pinch zoom, a toolbar toggle) also re-pointed both layers at the newest frame behind the frame code, which could freeze a blinking caret.
• After: one plain module, browser-frame-pacer.ts, owns the double buffer: pacing, decode, flip and reset, with one timer (the 100 ms interval).
  • Each layer records the source its <Image> holds and whether that source has answered (loading, ready, failed). A layer is written only when it is not loading and only with a different source, so every write gets exactly one native answer: an unchanged source reloads nothing on Android (ReactImageView.setSource) or iOS (ImageShadowNode), so the pacer never waits on one.
  • A decode is never cut short, and a reset (stream switch, app backgrounded) abandons nothing: the load under way still answers for its layer. Only the newest frame is held; it takes the hidden layer once that layer is free.
  • A frame the hidden layer already holds flips at once if it decoded (a blinking caret alternating two frames) and is skipped if it failed.
  • A native onLoad settles a layer only when its reported source.uri is the layer's current source. Android reports the view's current source at dispatch, so this drops loads dispatched before a newer write landed; it is not a general guard against every late load.
  • The <Image> source prop is only the frame the layers mount with; every later source write is the pacer's, so a re-render changes nothing on screen.
  • Frame metadata (and with it touch mapping) and the busy flag follow the frame on screen: the stream hook sets them when a frame is shown, not when it starts decoding.

Double taps.

• Before: when browser.mouseClick failed for any reason, including the 5 s timeout, sendPointerClick replayed it as mouseMove/mouseDown/mouseUp. A timed-out click is still queued on the host, so the replay could land a second tap on whatever the first tap opened, such as a dropdown option.
• After: the click is sent directly in a try/catch. Any answered click counts as delivered (the external-Chromium provider can answer a delivered click with null), and a delivery-unknown error (isRpcDeliveryUnknown) is not replayed. Only a refusal replays.

Why

• Pacing: decoding at the rate the phone can manage is simpler than the old "keep the hidden layer on the newest frame" rule, and it can't starve itself. An earlier revision added a 1.5 s watchdog that gave up on a slow decode; on the emulator under load that re-created the freeze (each give-up re-points the layer, and the next one cuts that decode too), so it was removed. With no re-pointing, native answers every load with onLoad or onError, and the web probe's decode() always settles, so nothing is left for a watchdog to catch.
• One owner: the pacing, decode tracking and flip had been split over three hooks and a helper, wired back through the stream hook and the pane. Keeping them in one closure removes 12 refs and one timer and makes the layer state (what each layer's Image holds, and whether it has answered) the single source of truth. An earlier revision cleared a layer's record when it gave up on a decode while the Image still held that source; the next identical frame was then a native no-op that never answered and the pane froze (for example, returning from the background to an unchanged page).
• Taps: the existing delivery-ambiguity marker is exactly the signal this needs, so no new plumbing was added.

This is phone-only, with no wire change. The host-side cause of slow taps (a ~2 s wait on every browser command) was fixed separately in #22528, #22534 and #22526, which are already on main.

Linked Issue

STA-8024 (Linear). Related: STA-4128 (#16122), STA-7002.

Visual Proof

N/A: measured on an emulator rather than recorded (see Testing).

Testing

• browser-frame-pacer.web.test.ts (the pacer against the web paint writers, asserting what each layer shows and which is visible): holds the newest frame while a layer decodes; shows a slow frame however long it takes, including a last frame with nothing newer; flips straight to a layer already holding the frame; skips a frame sent again after it failed and keeps streaming; settles a layer only for the source it holds; after a reset mid-decode, flips to the same frame when it is sent again, whether the decode settles before or after; keeps streaming after a layer remounts; paces to one frame per interval; reports each frame shown once.

• mobile-browser-pane-frame-layers.test.tsx mounts the real pane with a native model where a prop is whatever was written last, a same-source write loads nothing, and each change answers once with the layer's current source: a caret blinking across a re-render keeps blinking; a frame decoding when the app goes to the background is shown when sent again on return; a frame that failed and is sent again does not stop the stream (these two freeze on e859aa1); a remount of both Images mid-decode keeps the visible frame and the stream (fails with the remount re-arm or put-back deleted). Mutating the pacer's rules (decode gate, source check, interval, same-source flip, failed skip, remount re-arm and put-back) each fails at least one pacer or pane case.

• use-mobile-browser-commands-click-fallback.test.tsx: a delivery-unknown click is not replayed, an answered click with a null result is not replayed, and a refused click still is.

• Recording corpus re-recorded once (788 files): baseline in every golden, the seed checkpoint renamed clicked-by-fallback → tap-settled (it no longer clicks by fallback in most partitions), and one behaviour change: the result-null partition of matrix-browser.pointer-click-browser.mouseclick-1 sends only the click.

• Full mobile suite (876 files), mobile tsc, tests-typecheck ratchet, oxfmt --check, oxlint, check:code-quality:changed, React Doctor changed-lines, the page-closure suites (session pin drops two modules, 4219 → 4217 after merging main: three modules replaced by one) and the real-page pane render suite (flips the double buffer in Chromium).

• Android emulator A/B (headless API 36 arm64, Pixel 7 profile, 4 cores, CPU throttled with in-emulator busy loops, host streaming ~10 frames/s, 60 s per run; temporary instrumentation, not committed). This AVD is faster than the first run's, so the freeze needed 24 loops:

  Build	Page, loops	Flips/s	Longest gap	Frozen seconds	Stale loads
  Before this PR	animated, 12	7.3	460 ms	0	41
  Before this PR	animated, 24	0.8	13.5 s	27 of 43	296
  Previous head	animated, 24	3.4	507 ms	0	0
  This PR	animated, 0	9.4	145 ms	0	0
  This PR	animated, 12	8.2	312 ms	0	0
  This PR	animated, 24	2.5	684 ms	0	0

  Slow last frame: a page streams noise for 2.5 s, then stops on a final frame, which decodes in up to 2.4 s (12 loops) and 7.9 s (24 loops). This PR showed the final frame in 5/5 cycles at each load, every applied frame flipped (16/16) and no stale load. The watchdog revision it replaces, same setting: 41 applied, 11 flipped, 30 stale loads, final frame missed in 1 of 5 cycles.

  • Double tap (earlier run, throwaway 6 s host delay on the click): old build registered 2 clicks from one tap, this PR 1.

• Background/return on the emulator (24 busy loops; a 2.5 s noise burst ends on a green-centred frame, the app goes to the background while it decodes and returns 6 s later; pass = green 15 s after return): the previous head stayed on a burst frame for 45 s+ in 3 of 14 cycles; this PR in 1 of 14, and 1 of 26 with instrumentation. The instrumented miss logged no reset at all (the background and return never reached the stream effect) and the pacer flipping to the last frame it received, so it is not the path fixed here; its cause is not identified. The native-model test above is what certifies this fix.

• Not verified: caret blink across a pinch on a device (screen sampling was too slow to read a 500 ms blink; covered by the pane test above); iOS (the source-uri check reads what RN 0.83's Fabric ImageEventEmitter sends, the prop string, but was not run); whether busy-loop throttling matches a real slow phone.

AI Disclosure

Claude (Opus).

Notes

This overlaps with #16122 (STA-4128), which edits MobileBrowserPane.tsx for the reconnect case, and will need a rebase onto the pacer.

Checklist

• This PR is small and focused
• I explained what changed and why
• N/A with reason for visuals
• Self-reviewed for correctness, security, and performance
• Cross-platform (iOS/Android/web page), SSH/remote impact considered
• pnpm lint, pnpm typecheck, pnpm test, pnpm build (targeted locally; CI will cover the rest)

[pullfrog]

ℹ️ Minor suggestions only — one interaction between the new fast path and a pane re-render, plus a stale comment.

Reviewed changes

• Frame pacing (mobile/src/browser/use-mobile-browser-frame-apply.ts) — a hidden layer that is decoding is no longer re-pointed; the newest frame is held in the pacer slot and takes the layer once the decode settles, with a 1.5 s BROWSER_FRAME_DECODE_WATCHDOG_MS for a decode that never reports.
• Same-source fast path — a frame the target layer already holds settles without a decode, via the new layerUrisRef per-layer URI tracking.
• Drain wiring — drainQueuedFrame is returned from use-mobile-browser-stream.ts, passed as onFrameLayerSettled in MobileBrowserPane.tsx, and called from the pane's image load/error handlers and the arm callbacks.
• Double-tap fix (use-mobile-browser-commands.ts) — the mouseMove/mouseDown/mouseUp replay is skipped when browser.mouseClick failed with a delivery-unknown error, since a timed-out click may still run on the host.
• Tests and corpus — new use-mobile-browser-commands-click-fallback.test.tsx; reworked use-mobile-browser-frame-apply.web.test.tsx; a replay-free re-record of the two transport-rejection pointer-click partitions, and a session-closure pin bump for main's #22452.

The double-tap fix is correct as far as I can trace it: bindDeferredRpcOperation.request forwards client.sendRequest unchanged, so the timeout's markRpcDeliveryUnknown mark survives on the same Error identity down to isRpcDeliveryUnknown. The frame-pacing rewrite reads cleanly, and the held-frame and watchdog tests can actually fail against the old gating.

ℹ️ Nitpicks

• config/scripts/mobile-web-app-browser-pane-render.test.mjs:322 still says applyFrame writes the next frame's URI onto the hidden layer "as soon as the frame lands". Under the new pacing it only writes when drainQueuedFrame reaches applyFrame — after a settled/abandoned decode and the pacer interval — so the sentence no longer describes the path it is explaining.
• mobile/src/browser/use-mobile-browser-pane-layers.ts:66,86 settle/abandon on native onLoad/onError without the frameUriRef identity guard the arm callbacks use, so the claim in use-mobile-browser-frame-apply.ts:64 that "both arms … answer for the frame they were armed with" only holds on the web path. Pre-existing root and low probability (the author measured late native loads at 0.4–0.7 s, under the 1.5 s watchdog), but the watchdog's layer reuse widens the window slightly.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes

• Unified the double buffer under one pacer — browser-frame-pacer.ts replaces use-mobile-browser-frame-apply.ts, use-mobile-browser-pane-layers.ts and browser-frame-layer-flip.ts with a single closure owning the timer, decode watchdog, flip and reset, and the pane binds each layer's <View>/<Image> straight to it.
• Fixed the re-render clobber — MobileBrowserPane.tsx now derives renderedFrameSource from the frameUri state (first frame and replace only), so an unrelated render no longer re-sources both layers behind the pacer; this retires the prior review's finding.
• Made the decode deadline apply only when a newer frame waits — a slow last frame now lands however long it takes instead of being abandoned at 1.5 s and never shown.
• Guarded the native load flip by source — onLoad now checks event.nativeEvent.source?.uri, so a late load for a source the layer has moved off cannot flip early.
• Simplified the tap fallback — sendPointerClick calls browserPointerClick.request directly with the 5 s timeout rather than through sendBrowserRequest, preserving the delivery-unknown skip for timed-out clicks.
• Reworked the tests and pin — added browser-frame-pacer.web.test.ts and mobile-browser-pane-frame-layers.test.tsx, and dropped the session-closure pin back to 4218 for the two removed modules.

I traced the pacer's timer lifecycle and found no reachable state where a queued frame is left without a timer or where a newer frame is stranded behind a decode; both the watchdog-only-when-queued change and the reset() behavior are sound. The clobber fix checks out against RN 0.83's deep-diff of the source prop and React DOM's per-property style diffing on RN Web, so an unchanged URI commits nothing and the pacer's setNativeProps write survives. The prior review's open thread and both nitpicks are addressed.

  ^{｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes

Incremental review of the one commit since the prior pullfrog review (806134b7 → 2dbc509): the frame pacer no longer runs a decode watchdog.

• Removed the 1.5 s decode watchdog and its abandonment path — browser-frame-pacer.ts drops BROWSER_FRAME_DECODE_WATCHDOG_MS and the abandonDecode() call in drain(); drain() now returns while a decode is in flight and resumes when that decode's onLoad/onError (native) or decode() probe (web) settles, instead of cutting a slow decode short and re-pointing its layer.
• Rewrote the pacer tests for the new pacing — the watchdog/give-up cases are replaced by tests that a slow decode is waited out and then both the frame it held and the newest queued frame are shown.

Removing the watchdog is sound: a layer is never re-pointed mid-decode, so a native load always terminates in onLoad (whose source.uri matches the layer's tracked source) or onError, and the web probe's decode() promise always settles; reset()/replace() remain the only paths that drop a decode. I confirmed the iOS Fabric implementation emits onLoad with an ImageSource carrying uri (RCTImageComponentView.didReceiveImage), and no dangling BROWSER_FRAME_DECODE_WATCHDOG_MS reference remains.

  ^{｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

ℹ️ No critical issues — one documentation-accuracy nit inline.

Reviewed changes

Incremental review of the four commits since the prior pullfrog review (e859aa14f8 → 4e618a816a).

• Reworked the pacer's double buffer around a per-layer LayerState (uri + loading/ready/failed), a target frame and mountedUri, replacing layerUris/decoding/paint.
• Moved pacer construction into use-mobile-browser-stream (returned as frameLayers); the pane no longer creates it.
• Made settle(layer, uri) the sole flip signal, matching event.nativeEvent.source?.uri, and restored a layer's held source when its <Image> remounts.
• Changed the tap fallback so any answered click counts as delivered (including a null result), replaying only on a non-delivery-unknown refusal with no modifiers.
• Re-recorded the RPC corpus against pin 2c2b84ce9e and renamed the clicked-by-fallback partition to tap-settled, plus the pane/pacer/click tests.

I read the full source and test delta end to end. The pacer's target/settle/fail lifecycle has no reachable state that strands a queued frame — show only reaches write when drain's hidden-layer decode guard is clear, and every superseding path (reset/replace/attachImage) re-arms a decode. The answered-tap rule matches what require-result-or-throw-message actually throws on, so a decoded null or {ok:false} reply no longer replays. The three changed test files (19 tests) pass locally.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes

Incremental review of the one commit since the prior pullfrog review (4e618a81 → cb8fbb10): a small pane/pacer refactor, a docstring fix that retires the prior review's open thread, and a new remount test.

• Moved the pane's frameUri state into the stream hook — use-mobile-browser-stream now owns frameUri and returns renderedFrameSource; MobileBrowserPane destructures it instead of keeping its own copy and passing setFrameUri down.
• Trimmed the pacer's guards — drain's decode check dropped the mountedUri !== null && half, and attachImage's inner null check folded into its early return. Both are behavior-preserving: a layer only reaches loading through write/attachImage, which already require mountedUri non-null.
• Rewrote the pacer docstring to attribute the not-loading rule to the stream and document replace writing over a loading layer, addressing the prior review's inline note.
• Clarified the tap-fallback comment — the move/down/up replay drops pointer modifiers, which is why a modifier tap also returns instead of replaying.
• Added a remount test — both <Image> layers unmount (zero-size layout) and remount mid-decode, keeping the visible frame and the stream; the test's native model now loads the source a fresh <Image> mounts with.

I traced the state move and the two guard simplifications and found no behavioral difference, confirmed the dropped exports have no external references, and ran the three browser test files (20 tests) green locally. The prior review's open thread is resolved.

  ^{｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}