From 27aab69ec15066ae0094b513bafe4de533657ef4 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 28 Aug 2026 08:39:26 -0700 Subject: [PATCH] test(mobile): assert migrated screencast frames reach the original listener The shape-only assertion passed on a no-op callback, which is the dropped-frame bug this migration path exists to fix. Also pin what a superseded frame's late onLoad promotes. --- .../mobile-browser-frameless-stream.test.tsx | 51 +++++++++++++++++++ .../stable-logical-rpc-client.test.ts | 12 ++++- 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/mobile/src/browser/mobile-browser-frameless-stream.test.tsx b/mobile/src/browser/mobile-browser-frameless-stream.test.tsx index c4fed33e8ff..bc0050024c1 100644 --- a/mobile/src/browser/mobile-browser-frameless-stream.test.tsx +++ b/mobile/src/browser/mobile-browser-frameless-stream.test.tsx @@ -294,6 +294,57 @@ describe('MobileBrowserPane with a stream that reports ready but sends no frames expect(activeSubscriptionCount()).toBe(1) }) + // Why: the load handler keys on (layer, decoderEpoch) and a B->C replacement reuses both, + // so a late B onLoad does promote. Pin what that promotion actually lands on. + it('promotes the newest frame when a superseded frame reports its load late', async () => { + vi.useFakeTimers() + vi.setSystemTime(1_000) + const { renderer, stream, nativeMocks } = await renderPane() + + act(() => { + stream.onBinaryFrame?.(makeFrame('visible', 1)) + }) + const layers = getFrameLayers(renderer, nativeMocks) + + act(() => { + stream.onBinaryFrame?.(makeFrame('superseded', 2)) + vi.advanceTimersByTime(100) + }) + expect(latestNativeImageUri(layers[1].imageNative)).toBe(frameUri('superseded')) + + // The final frame replaces the hidden layer's source without a new transition. + act(() => { + stream.onBinaryFrame?.(makeFrame('final', 3)) + vi.advanceTimersByTime(100) + }) + expect(latestNativeImageUri(layers[1].imageNative)).toBe(frameUri('final')) + + for (const layer of layers) { + layer.wrapperNative.setNativeProps.mockClear() + } + + // The superseded frame's load arrives after its source was replaced. + act(() => layers[1].image.props.onLoad()) + + // The promoted layer is already pointed at the newest frame, not the superseded one, + // and the layer it replaces held a strictly older frame. + expect(latestNativeImageUri(layers[1].imageNative)).toBe(frameUri('final')) + expect(layers[0].wrapperNative.setNativeProps).toHaveBeenLastCalledWith({ + style: { opacity: 0 } + }) + expect(layers[1].wrapperNative.setNativeProps).toHaveBeenLastCalledWith({ + style: { opacity: 1 } + }) + + // The newest frame's own load is then a no-op: the transition is already settled. + for (const layer of layers) { + layer.wrapperNative.setNativeProps.mockClear() + } + act(() => layers[1].image.props.onLoad()) + expect(layers[0].wrapperNative.setNativeProps).not.toHaveBeenCalled() + expect(layers[1].wrapperNative.setNativeProps).not.toHaveBeenCalled() + }) + it('does no rendering or native work on a healthy connected boundary', async () => { const { renderer, diff --git a/mobile/src/transport/stable-logical-rpc-client.test.ts b/mobile/src/transport/stable-logical-rpc-client.test.ts index 2f4f8b04ae1..ffe06201ac0 100644 --- a/mobile/src/transport/stable-logical-rpc-client.test.ts +++ b/mobile/src/transport/stable-logical-rpc-client.test.ts @@ -128,8 +128,9 @@ describe('stable logical RPC client', () => { const replacement = new FakeSession('connected') const client = createStableLogicalRpcClient(oldSession, fromPath) const states: ConnectionState[] = [] + const onBinaryFrame = vi.fn() client.subscribe('browser.screencast', { worktree: 'id:wt-1', page: 'page-1' }, vi.fn(), { - onBinaryFrame: vi.fn() + onBinaryFrame }) client.onStateChange((state) => states.push(state)) @@ -142,6 +143,15 @@ describe('stable logical RPC client', () => { expect.any(Function), { onBinaryFrame: expect.any(Function) } ) + // Why invoke it: asserting the shape alone passes on a no-op, which is exactly the + // dropped-frame bug this migration path is meant to fix. + const replayedOpts = replacement.subscribe.mock.calls[0]![3] as { + onBinaryFrame?: (frame: unknown) => void + } + const frame = { opcode: 1, seq: 7 } + replayedOpts.onBinaryFrame?.(frame) + expect(onBinaryFrame).toHaveBeenCalledOnce() + expect(onBinaryFrame).toHaveBeenCalledWith(frame) expect(oldSession.close).toHaveBeenCalledOnce() expect(states).toEqual(['connected']) expect(client.getActivePath()).toBe(toPath)