diff --git a/mobile/src/mobile-web-shell/MobileWebShellScreen.test.tsx b/mobile/src/mobile-web-shell/MobileWebShellScreen.test.tsx index e3b70b0dcf0..19f5e68b50a 100644 --- a/mobile/src/mobile-web-shell/MobileWebShellScreen.test.tsx +++ b/mobile/src/mobile-web-shell/MobileWebShellScreen.test.tsx @@ -27,6 +27,12 @@ type ScreenDependencies = { posted: string[] /** Whether the view refuses what it is handed, which is a page the post never reached. */ postFails: boolean + /** Whether a post waits for the case to settle it, so a case can render while one is in flight. */ + holdPosts: boolean + /** The settlers for posts the view has taken and not answered. */ + heldPosts: (() => void)[] + /** The handle the view last attached, so a case can make the host lose and regain one. */ + handle: { postBridgeMessage: (json: string) => Promise } | null state: MobileWebShellSessionState /** Null for every case but the bridge's: with no client the hook builds no host at all. */ client: FakeRpcClient | null @@ -65,6 +71,9 @@ const dependencies = vi.hoisted((): ScreenDependencies => { viewRenders: 0, posted: [], postFails: false, + holdPosts: false, + heldPosts: [], + handle: null, state: { kind: 'checking' }, client: null } @@ -153,14 +162,19 @@ vi.mock('../../modules/orca-mobile-web-shell/src', async () => { // post rejected as a view that is gone, so no case could see a frame reach the page. const attach = props.ref React.useLayoutEffect(() => { - attach?.({ + const handle = { postBridgeMessage: (json: string) => { dependencies.posted.push(json) - return dependencies.postFails - ? Promise.reject(new Error('the view would not take it')) + if (dependencies.postFails) { + return Promise.reject(new Error('the view would not take it')) + } + return dependencies.holdPosts + ? new Promise((settle) => dependencies.heldPosts.push(() => settle())) : Promise.resolve() } - }) + } + dependencies.handle = handle + attach?.(handle) return () => { attach?.(null) } @@ -205,6 +219,7 @@ vi.mock('./use-mobile-web-shell-session', () => ({ import { clientFrame, createFakeRpcClient } from './bridge-host-test-fakes' import { BRIDGE_FAULT_GRANT, BRIDGE_NAVIGATE_BACK_NOTIFY } from './bridge/bridge-envelope' +import { BRIDGE_ROUTE_UPDATE_ACCEPT } from './bridge/bridge-route-update' import { MobileWebShellScreen } from './MobileWebShellScreen' /** The caller's native screen, as a component so `findAllByType` can name it without a host string. */ @@ -300,6 +315,9 @@ beforeEach(() => { dependencies.viewRenders = 0 dependencies.posted.length = 0 dependencies.postFails = false + dependencies.holdPosts = false + dependencies.heldPosts.length = 0 + dependencies.handle = null dependencies.client = null dependencies.routeGrants = DEFAULT_ROUTE_GRANTS dependencies.back.mockReset() @@ -555,6 +573,158 @@ describe('the hybrid shell screen', () => { warned.mockRestore() }) + /** + * A route whose frame the view refused is still owed to the page (round 4). + * + * The screen used to own the attempt: it recorded the route it had tried and cleared that record + * on a refusal, so the only thing that could try again was another render. A view that comes + * back — the native handle re-attaching under the same mounted page — is not one, so the pane + * the user asked for stayed on the shell's side forever. The host owns it now: the route stays + * pending until a post resolves true, and a regained handle is one of the moments it retries. + */ + it('delivers a pending route when the host regains a view handle', async () => { + dependencies.client = createFakeRpcClient() + dependencies.postFails = true + const warned = vi.spyOn(console, 'warn').mockImplementation(() => undefined) + const delivered: unknown[] = [] + dependencies.state = readyState('session-one') + const rendered: { tree: ReactTestRenderer | null } = { tree: null } + await act(async () => { + rendered.tree = create( + createElement(MobileWebShellScreen, { + hostId: 'host-1', + route: { pathname: '/h/host-1', params: { paneKey: 'pane-1' } }, + fallback: createElement(NativeFallback), + onRouteDelivered: (route) => delivered.push(route) + }) + ) + }) + const tree = rendered.tree + if (tree === null) { + throw new Error('screen did not render') + } + mounted.push(tree) + const probe = byName(tree, 'ShellViewProbe')[0] + await act(async () => { + probe?.props.onBridgeMessage({ + nativeEvent: { + json: clientFrame({ type: 'ready', accepts: [BRIDGE_ROUTE_UPDATE_ACCEPT] }) + } + }) + }) + expect(delivered).toEqual([]) + // The same page, a handle it may be posted on again. + dependencies.postFails = false + const handle = dependencies.handle + await act(async () => { + probe?.props.ref(null) + probe?.props.ref(handle) + }) + expect(delivered).toEqual([{ pathname: '/h/host-1', params: { paneKey: 'pane-1' } }]) + warned.mockRestore() + }) + + /** + * A render between the frame and its answer does not cancel the delivery (round 4). + * + * The screen's effect cancelled its own pending answer on cleanup, so any render while a post + * was in flight — a state change anywhere above, which is routine — dropped the report the + * switch spends to clear the param. The page had the route and the shell never heard. + */ + it('delivers once when the screen re-renders while the frame is in flight', async () => { + dependencies.client = createFakeRpcClient() + const delivered: { pathname: string; params?: Record }[] = [] + const screen = (params: Record) => + createElement(MobileWebShellScreen, { + hostId: 'host-1', + route: { pathname: '/h/host-1', params }, + fallback: createElement(NativeFallback), + onRouteDelivered: (route) => delivered.push(route) + }) + dependencies.state = readyState('session-one') + const rendered: { tree: ReactTestRenderer | null } = { tree: null } + await act(async () => { + rendered.tree = create(screen({ paneKey: '' })) + }) + const tree = rendered.tree + if (tree === null) { + throw new Error('screen did not render') + } + mounted.push(tree) + await act(async () => { + byName(tree, 'ShellViewProbe')[0]?.props.onBridgeMessage({ + nativeEvent: { + json: clientFrame({ type: 'ready', accepts: [BRIDGE_ROUTE_UPDATE_ACCEPT] }) + } + }) + }) + expect(delivered).toHaveLength(1) + dependencies.holdPosts = true + await act(async () => { + tree.update(screen({ paneKey: 'pane-1' })) + }) + expect(dependencies.heldPosts).toHaveLength(1) + await act(async () => { + tree.update(screen({ paneKey: 'pane-1' })) + }) + // The route did not move, so the render in flight costs no second frame. + expect(dependencies.heldPosts).toHaveLength(1) + await act(async () => { + dependencies.heldPosts[0]?.() + }) + expect(delivered.slice(1)).toEqual([{ pathname: '/h/host-1', params: { paneKey: 'pane-1' } }]) + }) + + /** + * The same pane, asked for twice, after the first frame was refused (round 4). + * + * The host moved its held route on the refused attempt, so the second tap read as a route that + * had not moved and was held rather than sent: the page never got the pane and the param was + * never spent. Movement is measured against what the page received, so the repeat tap lands. + */ + it('delivers a repeat tap for the pane whose frame never landed', async () => { + dependencies.client = createFakeRpcClient() + const warned = vi.spyOn(console, 'warn').mockImplementation(() => undefined) + const delivered: { pathname: string; params?: Record }[] = [] + const screen = (params: Record) => + createElement(MobileWebShellScreen, { + hostId: 'host-1', + route: { pathname: '/h/host-1', params }, + fallback: createElement(NativeFallback), + onRouteDelivered: (route) => delivered.push(route) + }) + dependencies.state = readyState('session-one') + const rendered: { tree: ReactTestRenderer | null } = { tree: null } + await act(async () => { + rendered.tree = create(screen({ paneKey: '' })) + }) + const tree = rendered.tree + if (tree === null) { + throw new Error('screen did not render') + } + mounted.push(tree) + await act(async () => { + byName(tree, 'ShellViewProbe')[0]?.props.onBridgeMessage({ + nativeEvent: { + json: clientFrame({ type: 'ready', accepts: [BRIDGE_ROUTE_UPDATE_ACCEPT] }) + } + }) + }) + expect(delivered).toHaveLength(1) + dependencies.postFails = true + await act(async () => { + tree.update(screen({ paneKey: 'pane-1' })) + }) + expect(delivered).toHaveLength(1) + // The tap was never spent, so the next one carries the same pane. + dependencies.postFails = false + await act(async () => { + tree.update(screen({ paneKey: 'pane-1' })) + }) + expect(delivered.slice(1)).toEqual([{ pathname: '/h/host-1', params: { paneKey: 'pane-1' } }]) + warned.mockRestore() + }) + it('ends that wait on the page asking for a session', async () => { dependencies.client = createFakeRpcClient() const tree = await render(readyState('session-one')) diff --git a/mobile/src/mobile-web-shell/MobileWebShellScreen.tsx b/mobile/src/mobile-web-shell/MobileWebShellScreen.tsx index 0ba8fab7256..66f2622c988 100644 --- a/mobile/src/mobile-web-shell/MobileWebShellScreen.tsx +++ b/mobile/src/mobile-web-shell/MobileWebShellScreen.tsx @@ -1,4 +1,4 @@ -import { useEffect, useRef, type ReactNode } from 'react' +import { useEffect, type ReactNode } from 'react' import { ActivityIndicator, Linking, Pressable, StyleSheet, Text, View } from 'react-native' import { useRouter } from 'expo-router' import { useSafeAreaInsets } from 'react-native-safe-area-context' @@ -20,7 +20,6 @@ import { } from './mobile-web-shell-dev-facts' import { cancelledShellNavigationTarget } from './cancelled-navigation-target' import { playPageHaptic } from './page-haptics' -import { shellScreenRouteKey } from './shell-screen-route' import { useMobileWebShellBridge } from './use-mobile-web-shell-bridge' import type { MobileWebShellRuntime } from './mobile-web-shell-runtime' import { useNativeDeviceVerbs } from '../platform/use-native-device-verbs' @@ -215,17 +214,17 @@ export function MobileWebShellScreen({ // the map as they are made. This re-seats that map on the store afterwards, for the key whose // write never persisted, and it runs on every ask because a document that reloads inside this // mount asks again. - onPageReady: (delivered) => { + onPageReady: () => { reportPageReady() void refreshStorage() - // Awaited, not read: the page asking and the frame answering it landing are two facts, and - // the second settles later. A route param spent on the first is spent on a frame a view - // that is gone refused. - void delivered.then((landed) => { - if (landed) { - onRouteDelivered?.(route) - } - }) + }, + // Registered once per host rather than per publish: the frame that carries a route may be one + // this screen never asked for — the `init` answering a reload, or a retry of one the view + // refused — and a render between the ask and the answer must not lose the report. The page + // asking and a frame reaching it are two facts, and a param spent on the first is spent on a + // frame a view that was gone refused. + onRouteDelivered: (delivered) => { + onRouteDelivered?.(delivered) }, // `document-load-failed` because that is what happens: the document loads and the page refuses // the session, so no tree is ever built. The refetch it costs is wasted on a route this shell @@ -262,40 +261,16 @@ export function MobileWebShellScreen({ onBinaryFramesDropped: reportDroppedBinaryFrames }) - // A route that moved under a screen that stayed mounted, which the key is what decides: the - // session switch keeps `paneKey` out of its key so a notification tap for another pane is a tab - // switch rather than a page reload, and this is how the page hears about it. Keyed on the page's - // own identity for a route, so a re-render holding an equal route publishes nothing. - const attemptedRouteKey = useRef(shellScreenRouteKey(route)) + // A route that moved under a screen that stayed mounted: the session switch keeps `paneKey` out + // of its key so a notification tap for another pane is a tab switch rather than a page reload, + // and this is how the page hears about it. Nothing is tracked here — which route the page has, + // and which one is still owed it, belong to the host, which outlives any one run of this effect. + // All this says is what the screen is on now: a route that did not move is dropped there, and a + // frame in flight when this re-runs is not disturbed by it. const publishRoute = bridge.publishRoute useEffect(() => { - const key = shellScreenRouteKey(route) - if (key === attemptedRouteKey.current) { - return - } - // Recorded only once a frame has gone out. A publish the hook refuses — no host yet, which is - // the gap between a ready session and its mounted bridge — leaves the key unrecorded, so the - // render that brings the host publishes the route the page never received. `publishRoute`'s - // identity changes with the inputs the host is built from, which is what re-runs this. - // Attempted now, recorded only once the frame has reached the page. A publish that was - // refused — no host yet, or a view that would not take it — leaves nothing recorded, so the - // next render that can carry it tries again. - attemptedRouteKey.current = key - let live = true - void publishRoute(route).then((landed) => { - if (!live) { - return - } - if (!landed) { - attemptedRouteKey.current = null - return - } - onRouteDelivered?.(route) - }) - return () => { - live = false - } - }, [onRouteDelivered, publishRoute, route]) + publishRoute(route) + }, [publishRoute, route]) // A profile read that rejected never becomes a host, so the session would otherwise sit in // `ready` behind an un-hidden view with nothing serving it and the page asking forever. diff --git a/mobile/src/mobile-web-shell/bridge-host-contract.ts b/mobile/src/mobile-web-shell/bridge-host-contract.ts index 1f43513aa68..ca5fd3294ae 100644 --- a/mobile/src/mobile-web-shell/bridge-host-contract.ts +++ b/mobile/src/mobile-web-shell/bridge-host-contract.ts @@ -180,7 +180,14 @@ export type BridgeHostOptions = { */ /** The page asked for a session. `delivered` settles true once the `init` answering that ask * reached the page, which is a later and different fact from the ask itself. */ - onPageReady: (delivered: Promise) => void + onPageReady: () => void + /** + * A frame carrying that route reached the page. Once per route, from whichever frame carried it — + * the first `init`, a re-sent one, or a retry of a frame the view had refused — so the caller may + * spend a one-shot param on it. Registered once per host, because a delivery outlives the render + * that asked for it. + */ + onRouteDelivered: (route: BridgeInitRoute) => void /** * The route this shell was built with is not one the protocol allows, so no honest `init` can be * sent and the page will never mount. Loud on purpose: the page's own refusal is a `console.warn` diff --git a/mobile/src/mobile-web-shell/bridge-host-init.test.ts b/mobile/src/mobile-web-shell/bridge-host-init.test.ts index 6636afc48d9..da00417fefb 100644 --- a/mobile/src/mobile-web-shell/bridge-host-init.test.ts +++ b/mobile/src/mobile-web-shell/bridge-host-init.test.ts @@ -146,7 +146,7 @@ describe('init and state', () => { } }) - it('tells the ready handler that no init reached the page for a refused route', async () => { + it('reports no delivery for a refused route, so its param is not spent', async () => { // The screen clears a one-shot param when the page has been handed the route (ruling 33.1), // and a refused route answers the `ready` without sending anything: the caller would spend a // notification tap the page never received. Unreachable from the session switch, which parses @@ -156,10 +156,10 @@ describe('init and state', () => { await Promise.resolve() expect(bridge.posted).toEqual([]) expect(bridge.pageReadyCount()).toBe(1) - expect(bridge.pageReadySentInit()).toEqual([false]) + expect(bridge.routeDeliveries()).toEqual([]) }) - it('tells it an init did reach the page for a route the protocol allows', async () => { + it('reports the delivery once an init reached the page', async () => { const bridge = harness({ route: { pathname: '/h/host-a' } }) bridge.host.receive(clientFrame({ type: 'ready' })) // Settled after the post, which is the whole point: readiness is the ask, this is the landing. @@ -167,10 +167,10 @@ describe('init and state', () => { await Promise.resolve() } expect(bridge.posted).toHaveLength(1) - expect(bridge.pageReadySentInit()).toEqual([true]) + expect(bridge.routeDeliveries()).toEqual([{ pathname: '/h/host-a' }]) }) - it('tells it nothing reached the page when the view refuses the frame', async () => { + it('reports no delivery when the view refuses the frame', async () => { const bridge = harness({ route: { pathname: '/h/host-a' }, post: () => Promise.reject(new Error('the view is gone')) @@ -179,7 +179,7 @@ describe('init and state', () => { for (let turn = 0; turn < 4; turn += 1) { await Promise.resolve() } - expect(bridge.pageReadySentInit()).toEqual([false]) + expect(bridge.routeDeliveries()).toEqual([]) expect(bridge.diagnostics.map((diagnostic) => diagnostic.kind)).toContain('post-failed') }) diff --git a/mobile/src/mobile-web-shell/bridge-host-route.ts b/mobile/src/mobile-web-shell/bridge-host-route.ts index 835f7a5e7b4..efcc2733072 100644 --- a/mobile/src/mobile-web-shell/bridge-host-route.ts +++ b/mobile/src/mobile-web-shell/bridge-host-route.ts @@ -3,7 +3,7 @@ import { type BridgeClientMessage, type BridgeInitRoute } from './bridge/bridge-envelope' -import { readBridgeRouteUpdate } from './bridge/bridge-route-update' +import { bridgeRouteMoved, readBridgeRouteUpdate } from './bridge/bridge-route-update' /** The screen one host is serving, which is the one field of `init` that moves under a live page. */ export type BridgeHostRoute = { @@ -14,15 +14,27 @@ export type BridgeHostRoute = { /** What the page's latest `ready` said it can be sent. Reset by each document's `ready`. */ readonly readReady: (message: Extract) => void /** - * Hands the page a rewritten param for the screen it is already on, and answers whether the - * frame reached it. The held route moves either way, so a page that reloads inside this mount is - * given the newest one on its next `ready` even when it is too old to be sent one in flight. + * Hands the page a rewritten param for the screen it is already on. The held route moves either + * way, so a page that reloads inside this mount is given the newest one on its next `ready` even + * when it is too old to be sent one in flight. Delivery is reported by `onDelivered`, not here: + * the frame may still be in flight when this returns, and whoever spends the param is not + * whoever asked for it. */ - readonly publish: (next: BridgeInitRoute, deliverable: boolean) => Promise + readonly publish: (next: BridgeInitRoute, deliverable: boolean) => void + /** + * Re-attempts the route the page has not received. Called on the moments a stranded frame gets + * another chance: a new `ready`, and a view handle the host has just regained. + */ + readonly retry: (deliverable: boolean) => void + /** Records that a frame carrying `sent` reached the page, which is what reports delivery. */ + readonly landed: (sent: BridgeInitRoute) => void } /** - * One host's route, parsed once and reassigned only by `publish`. + * One host's route: what the shell asked for, and what a frame has actually reached the page + * with. The two differ while a delivery is owed, which is the whole point of this module owning + * them — a post that the view refused leaves the route pending here rather than stranding it in a + * caller's effect, so the next `ready`, the next handle and the next tap all carry it. * * Parsed here against the same schema the page reads it with, rather than trusted. The producer * interpolates a host id into a pathname, so a host id carrying `?`, `#`, whitespace or a dot @@ -41,10 +53,33 @@ export function createBridgeHostRoute(args: { refused: boolean sendInit: () => Promise onRefused: (issue: string) => void + /** The page received this route. Fired once per route, from whichever frame carried it. */ + onDelivered: (route: BridgeInitRoute) => void }): BridgeHostRoute { const parsed = BridgeInitRouteSchema.safeParse(args.opened) let route = parsed.success && !args.refused ? parsed.data : null let accepts: readonly string[] = [] + /** What a frame has reached the page with. Null until one lands, so the first `init` delivers. */ + let delivered: BridgeInitRoute | null = null + /** One frame at a time: a render during a post must not put a second copy of it on the wire. */ + let inFlight = false + + function attempt(next: BridgeInitRoute, deliverable: boolean): void { + const update = readBridgeRouteUpdate({ held: route, delivered, next, accepts, deliverable }) + if (update.kind === 'refuse') { + args.onRefused(update.issue) + return + } + route = update.route + if (update.kind === 'hold' || inFlight) { + return + } + inFlight = true + void args.sendInit().finally(() => { + inFlight = false + }) + } + return { current: () => route, openIssue: () => (parsed.success ? 'unknown' : (parsed.error.issues[0]?.message ?? 'unknown')), @@ -52,16 +87,19 @@ export function createBridgeHostRoute(args: { accepts = message.accepts ?? [] }, publish: (next, deliverable) => { - const update = readBridgeRouteUpdate({ held: route, next, accepts, deliverable }) - if (update.kind === 'refuse') { - args.onRefused(update.issue) - return Promise.resolve(false) + attempt(next, deliverable) + }, + retry: (deliverable) => { + if (route !== null) { + attempt(route, deliverable) } - route = update.route - if (update.kind === 'hold') { - return Promise.resolve(false) + }, + landed: (sent) => { + if (!bridgeRouteMoved(delivered, sent)) { + return } - return args.sendInit() + delivered = sent + args.onDelivered(sent) } } } diff --git a/mobile/src/mobile-web-shell/bridge-host-test-harness.ts b/mobile/src/mobile-web-shell/bridge-host-test-harness.ts index 98331eff108..babdba1ace8 100644 --- a/mobile/src/mobile-web-shell/bridge-host-test-harness.ts +++ b/mobile/src/mobile-web-shell/bridge-host-test-harness.ts @@ -47,7 +47,8 @@ export type Harness = { pageReadyCount: () => number /** One entry per `ready` answered, saying whether its `init` reached the page. Filled as each * post settles, so a case reads it after awaiting the turn the post resolves on. */ - pageReadySentInit: () => readonly boolean[] + /** Every route a frame actually reached the page with, in the order they landed. */ + routeDeliveries: () => readonly BridgeInitRoute[] routeRefusals: string[] pageFaults: BridgeErrorCapture[] frames: () => BridgeHostMessage[] @@ -105,7 +106,7 @@ export function harness( const storageWrites: { key: string; value: string | null }[] = [] let pageReadies = 0 /** One entry per `ready` answered, saying whether an `init` actually went out for it. */ - const pageReadySentInit: boolean[] = [] + const routeDeliveries: BridgeInitRoute[] = [] const routeRefusals: string[] = [] const pageFaults: BridgeErrorCapture[] = [] const droppedBinaryFrames: number[] = [] @@ -126,10 +127,10 @@ export function harness( readStorage: options.readStorage ?? (() => ({ storage: options.storage ?? {}, storageOversize: [] })), onStorageWrite: (key, value) => storageWrites.push({ key, value }), - onPageReady: (delivered: Promise) => { + onPageReady: () => { pageReadies += 1 - void delivered.then((landed) => pageReadySentInit.push(landed)) }, + onRouteDelivered: (route) => routeDeliveries.push(route), onRouteRefused: (issue) => routeRefusals.push(issue), onNavigate: options.onNavigate ?? ((href) => navigations.push(href)), onExternalLink: (url) => externalLinks.push(url), @@ -185,7 +186,7 @@ export function harness( backPops, storageWrites, pageReadyCount: () => pageReadies, - pageReadySentInit: () => pageReadySentInit, + routeDeliveries: () => routeDeliveries, routeRefusals, pageFaults, frames, diff --git a/mobile/src/mobile-web-shell/bridge-host.ts b/mobile/src/mobile-web-shell/bridge-host.ts index 1c0e0527ef0..7b9856e07ae 100644 --- a/mobile/src/mobile-web-shell/bridge-host.ts +++ b/mobile/src/mobile-web-shell/bridge-host.ts @@ -40,10 +40,16 @@ export type BridgeHost = { * page too old to name it reads a second `init` as a replacement. A different pathname is a * different screen and is refused here — that is a remount, which is what the shell already does. * - * True only once the frame reached the page, because the caller's next move is to clear the - * param it just delivered: clearing one the page never received would spend the tap on nothing. + * Answers nothing: the host keeps the route pending until a frame carrying it reaches the page + * and reports that through `onRouteDelivered`. The caller's next move is to clear the param it + * delivered, and clearing one the page never received would spend the tap on nothing. */ - publishRoute: (next: BridgeInitRoute) => Promise + publishRoute: (next: BridgeInitRoute) => void + /** + * Another chance for a route whose frame never landed, for a caller that has just made delivery + * possible again: the view handle this host posts on coming back under the same page. + */ + retryPendingRoute: () => void dispose: () => void } @@ -73,7 +79,8 @@ export function createBridgeHost(options: BridgeHostOptions): BridgeHost { opened: options.route, refused: routeGrantsIssue !== null, sendInit: () => sendInit(), - onRefused: (issue) => options.onDiagnostic?.({ kind: 'route-update-refused', issue }) + onRefused: (issue) => options.onDiagnostic?.({ kind: 'route-update-refused', issue }), + onDelivered: (delivered) => options.onRouteDelivered(delivered) }) let closed = false // One document's turn at the bridge. `close` ends it and the next `ready` begins the next one; @@ -143,7 +150,7 @@ export function createBridgeHost(options: BridgeHostOptions): BridgeHost { return false } initSent = true - return postJson( + const landed = await postJson( JSON.stringify( createBridgeInitFrame({ sessionId, @@ -160,6 +167,10 @@ export function createBridgeHost(options: BridgeHostOptions): BridgeHost { }) ) ) + if (landed) { + routes.landed(route) + } + return landed } function sendReply(id: string, payload: RpcResponse): void { @@ -306,12 +317,12 @@ export function createBridgeHost(options: BridgeHostOptions): BridgeHost { if (message.type === 'ready') { serving = true routes.readReady(message) - const delivered = sendInit() + void sendInit() // Every time it is asked, not once: the page re-asks on a backoff, and the shell's wait ends - // on the first of those that lands rather than on a particular one. Carrying whether an - // the frame reached the page, which is not the same fact and settles later: a refused route - // answers the ask with nothing, and a view that is gone refuses what it was handed. - options.onPageReady(delivered) + // on the first of those that lands rather than on a particular one. Whether the frame reached + // the page is a different fact that settles later, and it is reported by `onRouteDelivered`: + // a refused route answers the ask with nothing, and a view that is gone refuses the frame. + options.onPageReady() return } if (!serving) { @@ -383,7 +394,12 @@ export function createBridgeHost(options: BridgeHostOptions): BridgeHost { } dispatch(read.message) }, - publishRoute: (next) => routes.publish(next, serving && initSent), + publishRoute: (next) => { + routes.publish(next, serving && initSent) + }, + retryPendingRoute: () => { + routes.retry(serving && initSent) + }, dispose } } diff --git a/mobile/src/mobile-web-shell/bridge/bridge-port-pair-test-harness.ts b/mobile/src/mobile-web-shell/bridge/bridge-port-pair-test-harness.ts index a75e5325eb3..d99b0d437f0 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-port-pair-test-harness.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-port-pair-test-harness.ts @@ -237,6 +237,7 @@ export function createBridgePortPair( onPageReady: () => { pageReadies += 1 }, + onRouteDelivered: () => {}, onRouteRefused: (issue) => routeRefusals.push(issue), onDiagnostic: (diagnostic) => hostDiagnostics.push(diagnostic) }) diff --git a/mobile/src/mobile-web-shell/bridge/bridge-route-update.ts b/mobile/src/mobile-web-shell/bridge/bridge-route-update.ts index 3d7d7565101..ad35784a9d6 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-route-update.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-route-update.ts @@ -59,6 +59,13 @@ export type BridgeRouteUpdate = */ export function readBridgeRouteUpdate(args: { held: BridgeInitRoute | null + /** + * The route a frame has actually reached the page with, which is what movement is measured + * against. Not `held`: a frame the view refused leaves the page on the older route while the + * host already holds the newer one, and measuring against `held` reads that owed route as one + * that did not move — so the retry, and the repeat tap, are both held and the page never gets it. + */ + delivered: BridgeInitRoute | null next: BridgeInitRoute /** What the page's last `ready` declared. Empty for every page built before this existed. */ accepts: readonly string[] @@ -73,7 +80,7 @@ export function readBridgeRouteUpdate(args: { if (parsed.data.pathname !== held.pathname) { return { kind: 'refuse', issue: 'not-this-screen' } } - const moved = bridgeRouteMoved(held, parsed.data) + const moved = bridgeRouteMoved(args.delivered, parsed.data) const sendable = moved && args.deliverable && args.accepts.includes(BRIDGE_ROUTE_UPDATE_ACCEPT) return { kind: sendable ? 'send' : 'hold', route: parsed.data } } diff --git a/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.test.ts b/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.test.ts index 33af64dd49f..44bb026ea8b 100644 --- a/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.test.ts +++ b/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.test.ts @@ -152,6 +152,7 @@ function Harness(props: { // absorb: rebuilding the host here would settle every pending request on each render. onPageFault: (error) => props.faults.push(error), onRouteRefused: () => {}, + onRouteDelivered: () => {}, onBinaryFramesDropped: (total) => props.probe.droppedBinaryFrames.push(total), onPageReady: () => { props.readies.push( diff --git a/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.ts b/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.ts index 42408ca9736..c92e6f3de18 100644 --- a/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.ts +++ b/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.ts @@ -48,11 +48,11 @@ export type MobileWebShellBridgeView = { readonly viewRef: (handle: OrcaMobileWebShellViewHandle | null) => void readonly onBridgeMessage: (event: MobileWebShellBridgeMessageEvent) => void /** - * Hands the mounted host a rewritten route for the screen it is already serving, and answers - * whether the frame reached the page. False whenever there is no host yet, which the caller has - * to know: a param it clears after a publish nobody made is a request the page never heard. + * Hands the mounted host a rewritten route for the screen it is already serving. Dropped when + * there is no host yet; the route the host is built from carries it instead, and either way the + * page is told through `onRouteDelivered` rather than through an answer here. */ - readonly publishRoute: (route: BridgeInitRoute) => Promise + readonly publishRoute: (route: BridgeInitRoute) => void } /** @@ -95,7 +95,9 @@ export function useMobileWebShellBridge(args: { /** The page could not render the generation on screen. Reported, never recovered from here. */ onPageFault: (error: BridgeErrorCapture) => void /** The page asked for a session. Reported so the screen can stop waiting for it. */ - onPageReady: (delivered: Promise) => void + onPageReady: () => void + /** A frame carrying that route reached the page, so a one-shot param on it may be spent. */ + onRouteDelivered: (route: BridgeInitRoute) => void /** This shell named a screen the protocol does not allow, so no session is served. */ onRouteRefused: (issue: string) => void /** Every screencast frame this host has dropped, so the shell can show the running total. */ @@ -133,6 +135,7 @@ export function useMobileWebShellBridge(args: { const readStorageRef = useRef(args.readStorage) const pageFaultRef = useRef(args.onPageFault) const pageReadyRef = useRef(args.onPageReady) + const routeDeliveredRef = useRef(args.onRouteDelivered) const routeRefusedRef = useRef(args.onRouteRefused) const binaryFramesDroppedRef = useRef(args.onBinaryFramesDropped) // Commit-phase and declared above the host's effect, so the host is built against what this @@ -151,6 +154,7 @@ export function useMobileWebShellBridge(args: { readStorageRef.current = args.readStorage pageFaultRef.current = args.onPageFault pageReadyRef.current = args.onPageReady + routeDeliveredRef.current = args.onRouteDelivered routeRefusedRef.current = args.onRouteRefused binaryFramesDroppedRef.current = args.onBinaryFramesDropped }, [ @@ -162,6 +166,7 @@ export function useMobileWebShellBridge(args: { args.onNavigateBack, args.onPageFault, args.onPageReady, + args.onRouteDelivered, args.onRouteRefused, args.onStorageWrite, args.readStorage, @@ -190,9 +195,12 @@ export function useMobileWebShellBridge(args: { onPageFault: (error) => { pageFaultRef.current(error) }, - onPageReady: (delivered) => { + onPageReady: () => { establishedSessionRef.current = sessionId - pageReadyRef.current(delivered) + pageReadyRef.current() + }, + onRouteDelivered: (delivered) => { + routeDeliveredRef.current(delivered) }, onRouteRefused: (issue) => { routeRefusedRef.current(issue) @@ -240,6 +248,13 @@ export function useMobileWebShellBridge(args: { viewRef: useCallback( (handle: OrcaMobileWebShellViewHandle | null) => { viewRef.current = handle === null || sessionId === null ? null : { sessionId, handle } + // A view the host can post on again is the moment a frame it could not send gets another + // chance. Nothing else would ask: the page is mounted, so there is no `ready` coming, and + // the screen re-renders only when something above it changes. + const mounted = hostRef.current + if (handle !== null && mounted !== null && mounted.sessionId === sessionId) { + mounted.host.retryPendingRoute() + } }, [sessionId] ), @@ -261,11 +276,11 @@ export function useMobileWebShellBridge(args: { // effect is a layout effect, so by the time a passive effect sees the new identity the host // behind it exists. publishRoute: useCallback( - async (route: BridgeInitRoute) => { + (route: BridgeInitRoute) => { const mounted = hostRef.current - return mounted !== null && mounted.sessionId === sessionId - ? mounted.host.publishRoute(route) - : false + if (mounted !== null && mounted.sessionId === sessionId) { + mounted.host.publishRoute(route) + } }, [buildId, client, sessionId, snapshot] )