diff --git a/config/scripts/mobile-web-app-render-harness.mjs b/config/scripts/mobile-web-app-render-harness.mjs index 090ae537128..ec4c9e89755 100644 --- a/config/scripts/mobile-web-app-render-harness.mjs +++ b/config/scripts/mobile-web-app-render-harness.mjs @@ -199,6 +199,19 @@ export async function readBridgeFaultGrant() { return match[1] } +/** The name the page posts its first frame under, read where the page and the shell both read it. */ +export async function readBridgePagePainted() { + const source = await readFile( + join(projectDir, 'mobile/src/mobile-web-shell/bridge/bridge-page-painted.ts'), + 'utf8' + ) + const match = /BRIDGE_PAGE_PAINTED = '([a-zA-Z]+)'/.exec(source) + if (!match) { + throw new Error('could not read BRIDGE_PAGE_PAINTED from bridge-page-painted.ts') + } + return match[1] +} + /** * The shell's half of the bridge, as the page's channel sees it. * diff --git a/config/scripts/mobile-web-app-render.test.mjs b/config/scripts/mobile-web-app-render.test.mjs index 3bc3377d876..1cd797d2600 100644 --- a/config/scripts/mobile-web-app-render.test.mjs +++ b/config/scripts/mobile-web-app-render.test.mjs @@ -11,6 +11,7 @@ import { parseCspDirectives, projectDir, readBridgeFaultGrant, + readBridgePagePainted, readBridgeProtocolVersion, readShellCsp } from './mobile-web-app-render-harness.mjs' @@ -58,10 +59,18 @@ let faultGrant = null const poisonedChunks = new Set() const POISON_MESSAGE = 'render check poisoned this route chunk' +/** + * Chunk paths the server holds until the check lets them go, so "the chunk has not arrived" is a + * state the check controls rather than a window it has to win a race against. + */ +const heldChunks = new Map() +let paintName = null + beforeAll(async () => { cspHeader = await readShellCsp() bridgeVersion = await readBridgeProtocolVersion() faultGrant = await readBridgeFaultGrant() + paintName = await readBridgePagePainted() if (!bundles) { return } @@ -78,7 +87,20 @@ beforeAll(async () => { transformChunk: (path, real) => poisonedChunks.has(path) ? `throw new Error(${JSON.stringify(POISON_MESSAGE)});\n${real.toString('utf8')}` - : real + : real, + handleRequest: (request, response, path) => { + const held = heldChunks.get(path) + if (!held) { + return false + } + held + .then(() => readFile(join(outDir, path.slice(1)))) + .then((real) => { + response.writeHead(200, { 'content-type': 'text/javascript' }) + response.end(real) + }) + return true + } }) server = served.server origin = served.origin @@ -113,7 +135,8 @@ async function openPage({ shellHost = SHELL_HOST, shellStorage = {}, shellGrants, - shellPageRoutes = null + shellPageRoutes = null, + shellAccepts = null } = {}) { const page = await browser.newPage({ viewport: { width: 390, height: 844 } }) if (shellRoute !== undefined) { @@ -128,7 +151,8 @@ async function openPage({ storage: shellStorage, faultGrant, grants: shellGrants ?? [faultGrant], - pageRoutes: shellPageRoutes + pageRoutes: shellPageRoutes, + accepts: shellAccepts }) } const errors = [] @@ -239,6 +263,14 @@ async function render(route, awaitText, { shellRoute = { pathname: route }, ...s } } +/** How many frames the double has heard under the paint name, which is what uncovers the view. */ +const paintReports = (page, name) => + page.evaluate( + (paint) => + (globalThis.__orcaRenderCheckNotifies ?? []).filter((frame) => frame.name === paint).length, + name + ) + /** The entry's state and what it painted, for a page that is never going to mount a route tree. */ async function renderWithoutTree({ shellRoute } = {}) { const { page, errors } = await openPage({ shellRoute }) @@ -541,6 +573,100 @@ describeRender('the Route A page in a real browser', () => { } }, 60_000) + it('reports its frame from the route screen, not from the router shell above it', async () => { + // The gap the shell's cover exists for. The entry's wrapper commits against the suspense + // fallback of a chunk still in flight, so a report hung there uncovers an empty body. + const chunk = routeChunks['./h/[hostId]/index.tsx'] + expect(chunk, Object.keys(routeChunks).join(' ')).toBeTruthy() + const path = `/assets/${chunk}` + let arrive = () => {} + heldChunks.set( + path, + new Promise((resolve) => { + arrive = resolve + }) + ) + try { + // The one case that advertises the report, because it is the only one asserting on it. + const opened = await openPage({ + shellRoute: { pathname: HOST_ROUTE }, + shellAccepts: [paintName] + }) + await opened.page.goto(`${origin}/`, { waitUntil: 'load' }) + await opened.page.waitForFunction( + () => document.documentElement.dataset.orcaWebEntry === 'mounted', + { timeout: 30_000, polling: 250 } + ) + // Mounted, and nothing drawn: the body is the fallback's, which is what the old seam + // reported on. + expect(await opened.page.evaluate(() => document.body.innerText)).not.toContain( + SHELL_HOST.name + ) + await opened.page.waitForTimeout(1_000) + expect(await paintReports(opened.page, paintName)).toBe(0) + + arrive() + await waitForRoute(opened, HOST_ROUTE, SHELL_HOST.name) + await opened.page.waitForFunction( + (name) => + (globalThis.__orcaRenderCheckNotifies ?? []).filter((frame) => frame.name === name) + .length > 0, + paintName, + { timeout: 30_000, polling: 250 } + ) + expect(opened.errors).toEqual([]) + await opened.page.close() + } finally { + arrive() + heldChunks.delete(path) + } + }, 120_000) + + it('says nothing for a redirect screen whose target is still behind its chunk', async () => { + // The `pr` route renders a `Redirect` into the source-control hub and nothing else. It commits, + // sends the document on, and stays mounted behind the target's fallback while that chunk loads. + const target = routeChunks['./h/[hostId]/source-control/[worktreeId].tsx'] + expect(target, Object.keys(routeChunks).join(' ')).toBeTruthy() + const path = `/assets/${target}` + let arrive = () => {} + heldChunks.set( + path, + new Promise((resolve) => { + arrive = resolve + }) + ) + try { + const route = `${HOST_ROUTE}/pr/render-check-tree` + const opened = await openPage({ + shellRoute: { pathname: route }, + shellAccepts: [paintName], + shellPageRoutes: [HOST_ROUTE_PATTERN, '/h/[hostId]/pr/[worktreeId]'] + }) + await opened.page.goto(`${origin}/`, { waitUntil: 'load' }) + await opened.page.waitForFunction( + () => location.pathname.includes('/source-control/'), + undefined, + { timeout: 30_000, polling: 250 } + ) + // The router has moved on and the hub is still arriving, so the document is showing nothing. + await opened.page.waitForTimeout(1_000) + expect(await paintReports(opened.page, paintName)).toBe(0) + + arrive() + await opened.page.waitForFunction( + (name) => + (globalThis.__orcaRenderCheckNotifies ?? []).filter((frame) => frame.name === name) + .length > 0, + paintName, + { timeout: 30_000, polling: 250 } + ) + await opened.page.close() + } finally { + arrive() + heldChunks.delete(path) + } + }, 120_000) + it('refuses a target the shell will not take, rather than opening it in the page', async () => { // The double grants only `fault`, so `notifyNavigate` answers false -- the shell-disposed and // older-shell cases reach the page the same way. Before C5.1 this left the host route and diff --git a/config/scripts/mobile-web-app-route-manifest.mjs b/config/scripts/mobile-web-app-route-manifest.mjs index 8da3082a492..8c553b5d386 100644 --- a/config/scripts/mobile-web-app-route-manifest.mjs +++ b/config/scripts/mobile-web-app-route-manifest.mjs @@ -111,11 +111,18 @@ routeContext.id = 'orca-mobile-web-app-routes'` * build rather than emitting a page that mounts with the export silently gone. */ export function renderMobileWebAppRouteManifest(routes) { - const entryLines = routes.map( - ({ key, module }) => - ` [${JSON.stringify(key)}]: { default: lazy(() => import(${JSON.stringify(module)})) }` - ) + const entryLines = routes.map(({ key, module }) => { + // The layout commits with the screen below it still behind its own chunk, so it is not what + // says the page has something to show. + // Optional because the closure builds call this with the page-route list, whose entries carry + // no key: that manifest is never loaded, since a route module imports nothing from it. + const resolved = key?.endsWith('/_layout.tsx') + ? `import(${JSON.stringify(module)})` + : `import(${JSON.stringify(module)}).then(withRouteScreenPaintReport)` + return ` [${JSON.stringify(key)}]: { default: lazy(() => ${resolved}) }` + }) return `import { lazy } from "react" +import { withRouteScreenPaintReport } from "./src/mobile-web-shell/bridge/page-first-paint" const modules = { ${entryLines.join(',\n')} } diff --git a/config/scripts/mobile-web-app-route-manifest.test.mjs b/config/scripts/mobile-web-app-route-manifest.test.mjs index 7a8e169e4d1..6fbf9d473a4 100644 --- a/config/scripts/mobile-web-app-route-manifest.test.mjs +++ b/config/scripts/mobile-web-app-route-manifest.test.mjs @@ -78,15 +78,32 @@ describe('route manifest', () => { { key: './h/index.tsx', module: '/app/h/index.tsx' }, { key: './h/_layout.tsx', module: '/app/h/_layout.tsx' } ]) - expect(source).toContain('["./h/index.tsx"]: { default: lazy(() => import("/app/h/index.tsx"))') + expect(source).toContain('["./h/index.tsx"]: { default: lazy(() => import("/app/h/index.tsx")') expect(source).toContain( - '["./h/_layout.tsx"]: { default: lazy(() => import("/app/h/_layout.tsx"))' + '["./h/_layout.tsx"]: { default: lazy(() => import("/app/h/_layout.tsx")' ) // A static import is what collapses the split back into one chunk. expect(source).not.toContain('import * as route') expect(source.match(/import\(/g)).toHaveLength(2) }) + it('reports the paint off the screen behind the chunk, not off the router above it', () => { + const source = renderMobileWebAppRouteManifest([ + { key: './h/index.tsx', module: '/app/h/index.tsx' }, + { key: './h/_layout.tsx', module: '/app/h/_layout.tsx' } + ]) + // The screen, wrapped where the chunk resolves: the wrapper above expo-router commits with the + // suspense fallback, so a report hung there lands while the body is still empty. + expect(source).toContain( + '["./h/index.tsx"]: { default: lazy(() => import("/app/h/index.tsx").then(withRouteScreenPaintReport))' + ) + // And never the layout, which commits with the screen below it still arriving. + expect(source).toContain( + '["./h/_layout.tsx"]: { default: lazy(() => import("/app/h/_layout.tsx")) }' + ) + expect(source).toContain('import { withRouteScreenPaintReport } from') + }) + it('leaves the RequireContext itself synchronous', () => { // expo-router calls keys() to build the route tree before anything renders, so the context // may not be a promise; only the screen behind each key is deferred. diff --git a/config/scripts/mobile-web-app-session-terminal-closure.test.mjs b/config/scripts/mobile-web-app-session-terminal-closure.test.mjs index b7b8ef4b70d..d416f1f7d07 100644 --- a/config/scripts/mobile-web-app-session-terminal-closure.test.mjs +++ b/config/scripts/mobile-web-app-session-terminal-closure.test.mjs @@ -354,8 +354,18 @@ const MERMAID_PACKAGE = 'node_modules/mermaid/' * * modules 4271 -> 4210 (-61) * local modules 1023 -> 1024 (+1) + * + * The page's paint report joins beside that one, for the same reason: + * `src/mobile-web-shell/bridge/bridge-page-painted.ts` holds the name the page posts and the name + * it declares in `ready`, so `bridge-client-notifications.ts` — which every screen's client is + * built from — imports it. One local module, nothing vendored; the seam that schedules the report + * is the web entry's and does not enter a route closure. Re-measured on this merged head rather + * than carried over from before the cut, with all five generators run first. + * + * modules 4210 -> 4211 (+1) + * local modules 1024 -> 1025 (+1) */ -const SESSION_ROUTE_MODULES = 4210 +const SESSION_ROUTE_MODULES = 4211 /** What the page enters this route through once the route is a switch with a `.web.tsx` sibling. */ const ROUTE_ENTRY = [ diff --git a/mobile/modules/orca-mobile-web-shell/ios/MobileWebShellView.swift b/mobile/modules/orca-mobile-web-shell/ios/MobileWebShellView.swift index 04b9e591f0a..0a2122f8183 100644 --- a/mobile/modules/orca-mobile-web-shell/ios/MobileWebShellView.swift +++ b/mobile/modules/orca-mobile-web-shell/ios/MobileWebShellView.swift @@ -213,6 +213,13 @@ final class OrcaMobileWebShellView: ExpoView, WKNavigationDelegate, WKUIDelegate webView.navigationDelegate = self webView.uiDelegate = self webView.allowsBackForwardNavigationGestures = false + // Transparent, as the Android view is. A WKWebView is opaque by default and paints white + // before its document does, so a dark app opening a page flashed white for the whole of the + // page's boot; with no surface of its own, what shows through is the shell's own frame, which + // is the one thing that knows the app's colours. + webView.isOpaque = false + webView.backgroundColor = .clear + webView.scrollView.backgroundColor = .clear webView.scrollView.contentInsetAdjustmentBehavior = .never webView.translatesAutoresizingMaskIntoConstraints = false addSubview(webView) diff --git a/mobile/src/mobile-web-shell/MobileWebShellScreen.test.tsx b/mobile/src/mobile-web-shell/MobileWebShellScreen.test.tsx index 0233d786491..6609b5f553d 100644 --- a/mobile/src/mobile-web-shell/MobileWebShellScreen.test.tsx +++ b/mobile/src/mobile-web-shell/MobileWebShellScreen.test.tsx @@ -1,89 +1,24 @@ -import { createElement } from 'react' -import { act, create, type ReactTestInstance, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest' -import type { FakeRpcClient } from './bridge-host-test-fakes' -import type { - MobileWebShellSessionState, - MobileWebShellUpdateNotice -} from './mobile-web-shell-session-contract' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -type ScreenDependencies = { - retry: Mock - reportShellFailure: Mock - reportDocumentLoaded: Mock - reportPageReady: Mock - /** The profile read rejected, which is the one state that has no host to build against. */ - snapshotUnreadable: boolean - storageRefreshes: number - openUrl: Mock - push: Mock - back: Mock - /** What the native stack answers: false is a page opened as the first screen on it. */ - canGoBack: boolean - pathname: string - pageRoutes: readonly string[] - routeGrants: readonly string[] - lifecycle: string[] - /** Every render of the shell view, which is one per render of the screen above it. */ - viewRenders: number - /** Every frame the shell posted to the page, raw. */ - posted: string[] - /** Whether the view refuses what it is handed, which is a page the post never reached. */ - postFails: boolean - state: MobileWebShellSessionState - /** Non-null when the generation on screen is a fallback from an update the shell refused. */ - updateNotice: MobileWebShellUpdateNotice | null - /** What the session reducer says about the page's handshake; true only for the fence's case. */ - pageReady: boolean - /** Null for every case but the bridge's: with no client the hook builds no host at all. */ - client: FakeRpcClient | null - /** The IME events the app's own keyboard seam subscribes to, by name. */ - keyboardListeners: Map void> -} - -const SNAPSHOT = vi.hoisted(() => ({ - host: { id: 'host-1', name: 'Host One', endpoint: 'ws://host-1', lastConnected: 3 } -})) - -const DEFAULT_ROUTE_GRANTS = vi.hoisted((): readonly string[] => [ - 'navigate', - 'storage', - 'externalLink', - 'native.clipboard.write' -]) - -const dependencies = vi.hoisted((): ScreenDependencies => { - // Before the module under test is imported, so its `__DEV__` guard is on and the developer facts - // are reachable at all — they are the one thing here that must never grow a secret. - Object.assign(globalThis, { __DEV__: true }) - return { - retry: vi.fn(), - reportShellFailure: vi.fn(), - reportDocumentLoaded: vi.fn(), - reportPageReady: vi.fn(), - snapshotUnreadable: false, - storageRefreshes: 0, - openUrl: vi.fn(), - push: vi.fn(), - back: vi.fn(), - canGoBack: true, - pathname: '/h/host-1', - pageRoutes: ['/h/[hostId]'], - routeGrants: DEFAULT_ROUTE_GRANTS, - lifecycle: [], - viewRenders: 0, - posted: [], - postFails: false, - state: { kind: 'checking' }, - updateNotice: null, - pageReady: false, - client: null, - keyboardListeners: new Map() - } -}) +const harness = await vi.hoisted(async () => await import('./mobile-web-shell-screen-test-harness')) +const dependencies = vi.hoisted(() => harness.createScreenDependencies()) +const SNAPSHOT = harness.SCREEN_SNAPSHOT vi.mock('react-native', () => ({ ActivityIndicator: 'ActivityIndicator', + Easing: { in: (fn: unknown) => fn, quad: 'quad' }, + // Enough of it for the cover to mount, fade and unmount. What the fade looks like is not this + // test's business; that the cover is up until the page paints is, and that is the `visible` prop. + Animated: { + View: 'Animated.View', + Value: class { + setValue(): void {} + }, + timing: () => ({ + start: (done?: (result: { finished: boolean }) => void) => done?.({ finished: true }), + stop: () => {} + }) + }, Keyboard: { addListener: ( name: string, @@ -96,7 +31,11 @@ vi.mock('react-native', () => ({ Linking: { openURL: dependencies.openUrl }, Platform: { OS: 'ios' }, Pressable: 'Pressable', - StyleSheet: { create: (styles: unknown) => styles }, + StyleSheet: { + create: (styles: unknown) => styles, + // The real values, so a case that reads them off the cover reads something. + absoluteFillObject: { position: 'absolute', top: 0, left: 0, right: 0, bottom: 0 } + }, Text: 'Text', View: 'View' })) @@ -221,13 +160,32 @@ vi.mock('./use-mobile-web-shell-session', () => ({ updateNotice: dependencies.updateNotice, retry: dependencies.retry, reportShellFailure: dependencies.reportShellFailure, + reportDocumentStarted: dependencies.reportDocumentStarted, reportDocumentLoaded: dependencies.reportDocumentLoaded, reportPageReady: dependencies.reportPageReady, - pageReady: dependencies.pageReady + reportPagePainted: dependencies.reportPagePainted, + pageReady: dependencies.pageReady, + pageFrame: dependencies.pageFrame }) })) +import { createElement } from 'react' +import { act, create } from 'react-test-renderer' import { bridgeId, clientFrame, createFakeRpcClient } from './bridge-host-test-fakes' +import { + byName, + DEFAULT_ROUTE_GRANTS, + trackRenderedScreen, + NativeFallback, + SCREEN_BUILD_ID as BUILD_ID, + SCREEN_DIRECTORY as DIRECTORY, + hostParentOf, + readyState, + renderScreen as mountScreen, + textOf, + updateScreen as reRenderScreen +} from './mobile-web-shell-screen-test-harness' +import { BRIDGE_PAGE_PAINTED } from './bridge/bridge-page-painted' import { BRIDGE_FAULT_GRANT, BRIDGE_NAVIGATE_BACK_NOTIFY, @@ -235,114 +193,24 @@ import { } from './bridge/bridge-envelope' import { BRIDGE_ROUTE_UPDATE_ACCEPT } from './bridge/bridge-route-update' import { MobileWebShellScreen } from './MobileWebShellScreen' +import type { MobileWebShellSessionState } from './mobile-web-shell-session-contract' +import type { ReactTestInstance, ReactTestRenderer } from 'react-test-renderer' -/** The caller's native screen, as a component so `findAllByType` can name it without a host string. */ -function NativeFallback(): null { - return null -} +const renderScreen = (state: MobileWebShellSessionState): Promise => + mountScreen(MobileWebShellScreen, dependencies, state) -const BUILD_ID = 'a1b2c3d4e5f6'.repeat(5) + 'abcd' -const DIRECTORY = '/var/mobile/Containers/Data/Caches/mobile-web/deadbeef/generations/a1b2' +const updateScreen = (tree: ReactTestRenderer, state: MobileWebShellSessionState): Promise => + reRenderScreen(MobileWebShellScreen, dependencies, tree, state) -async function render(state: MobileWebShellSessionState): Promise { - dependencies.state = state - const rendered: { tree: ReactTestRenderer | null } = { tree: null } - await act(async () => { - rendered.tree = create( - createElement(MobileWebShellScreen, { - hostId: 'host-1', - route: { pathname: '/h/host-1' }, - fallback: createElement(NativeFallback) - }) - ) - }) - if (rendered.tree === null) { - throw new Error('screen did not render') - } - mounted.push(rendered.tree) - return rendered.tree -} +afterEach(harness.unmountRenderedScreens) -/** Unmounted between cases: the shell's stack latch is one per stack, so a screen left mounted is - * a screen still holding whatever pop it took. */ -const mounted: ReactTestRenderer[] = [] - -function unmountRenderedScreens(): void { - act(() => { - for (const tree of mounted.splice(0)) { - tree.unmount() - } - }) -} - -function readyState(sessionId: string): MobileWebShellSessionState { - return { - kind: 'ready', - generationDirectory: DIRECTORY, - sessionId, - buildId: BUILD_ID, - totalBytes: 4096, - elapsedMs: 811 - } -} - -async function update(tree: ReactTestRenderer, state: MobileWebShellSessionState): Promise { - dependencies.state = state - await act(async () => { - tree.update( - createElement(MobileWebShellScreen, { - hostId: 'host-1', - route: { pathname: '/h/host-1' }, - fallback: createElement(NativeFallback) - }) - ) - }) -} - -/** Host elements are matched by name, not by `findAllByType`: React's `ElementType` does not admit - * an arbitrary React Native host name, so the typed form is a predicate. */ -function byName(tree: ReactTestRenderer, name: string): ReactTestInstance[] { - return tree.root.findAll((node) => String(node.type) === name) -} - -function textOf(tree: ReactTestRenderer): string { - return byName(tree, 'Text') - .map((node) => node.children.filter((child) => typeof child === 'string').join('')) - .join('\n') -} - -afterEach(unmountRenderedScreens) - -/** - * File-level, not per describe: every block here shares one mutable `dependencies`, so a reset - * scoped to one of them leaves whatever the others set. `routeGrants` is reset for that reason — - * a case that grants the screencast lane would otherwise hand it to every case that follows. - */ beforeEach(() => { - dependencies.retry.mockReset() - dependencies.reportShellFailure.mockReset() - dependencies.reportDocumentLoaded.mockReset() - dependencies.reportPageReady.mockReset() - dependencies.snapshotUnreadable = false - dependencies.storageRefreshes = 0 - dependencies.lifecycle.length = 0 - dependencies.viewRenders = 0 - dependencies.posted.length = 0 - dependencies.postFails = false - dependencies.client = null - dependencies.pageReady = false - dependencies.routeGrants = DEFAULT_ROUTE_GRANTS - dependencies.back.mockReset() - dependencies.openUrl.mockReset() - dependencies.openUrl.mockImplementation(() => Promise.resolve(true)) - dependencies.canGoBack = true - dependencies.pathname = '/h/host-1' - dependencies.updateNotice = null + harness.resetScreenDependencies(dependencies) }) describe('the hybrid shell screen', () => { it('renders the update wall for a bundle verdict, with no shell view', async () => { - const tree = await render({ + const tree = await renderScreen({ kind: 'wall', verdict: { kind: 'blocked', reason: 'bundle-unavailable' } }) @@ -351,7 +219,7 @@ describe('the hybrid shell screen', () => { }) it('renders the refetch wall a cached generation older than the host earns', async () => { - const tree = await render({ + const tree = await renderScreen({ kind: 'wall', verdict: { kind: 'blocked', @@ -365,7 +233,7 @@ describe('the hybrid shell screen', () => { }) it('offers Try again on a failure a retry can clear', async () => { - const tree = await render({ + const tree = await renderScreen({ kind: 'failed', reason: 'document-load-failed', retriedOnce: true @@ -380,7 +248,7 @@ describe('the hybrid shell screen', () => { }) it('offers no retry when the device cannot isolate a WebView', async () => { - const tree = await render({ + const tree = await renderScreen({ kind: 'failed', reason: 'isolation-unavailable', retriedOnce: false @@ -390,7 +258,7 @@ describe('the hybrid shell screen', () => { }) it('offers no retry for a status that could not be read, since the gate is settled', async () => { - const tree = await render({ + const tree = await renderScreen({ kind: 'failed', reason: 'status-unreadable', retriedOnce: false @@ -400,13 +268,13 @@ describe('the hybrid shell screen', () => { }) it('names what is missing when the host is unreachable and nothing is cached', async () => { - expect(textOf(await render({ kind: 'offline' }))).toContain( + expect(textOf(await renderScreen({ kind: 'offline' }))).toContain( 'Connect to this host to download the workspace' ) }) it('counts assets and bytes while downloading', async () => { - const tree = await render({ + const tree = await renderScreen({ kind: 'fetching', completedAssets: 2, totalAssets: 4, @@ -418,14 +286,14 @@ describe('the hybrid shell screen', () => { }) it('hands the shell view the generation path and the session id', async () => { - const tree = await render(readyState('session-one')) + const tree = await renderScreen(readyState('session-one')) const view = byName(tree, 'ShellViewProbe')[0] expect(view.props.generationDirectory).toBe(DIRECTORY) expect(view.props.sessionId).toBe('session-one') }) it('opens the bridge channel on a ready session and hands it a receiver', async () => { - const tree = await render(readyState('session-one')) + const tree = await renderScreen(readyState('session-one')) const view = byName(tree, 'ShellViewProbe')[0] expect(view.props.bridgeEnabled).toBe(true) expect(typeof view.props.onBridgeMessage).toBe('function') @@ -436,8 +304,8 @@ describe('the hybrid shell screen', () => { }) it('rebuilds the view rather than updating it when the session id changes', async () => { - const tree = await render(readyState('session-one')) - await update(tree, readyState('session-two')) + const tree = await renderScreen(readyState('session-one')) + await updateScreen(tree, readyState('session-two')) expect(dependencies.lifecycle).toEqual([ 'mount:session-one', 'unmount:session-one', @@ -446,7 +314,7 @@ describe('the hybrid shell screen', () => { }) it('forwards a failure the native view reports and drops a payload it cannot read', async () => { - const tree = await render(readyState('session-one')) + const tree = await renderScreen(readyState('session-one')) const view = byName(tree, 'ShellViewProbe')[0] await act(async () => { view.props.onLoadState({ nativeEvent: { state: 'ready' } }) @@ -457,7 +325,7 @@ describe('the hybrid shell screen', () => { }) it('starts the wait for the page when the native view says the document finished', async () => { - const tree = await render(readyState('session-one')) + const tree = await renderScreen(readyState('session-one')) const view = byName(tree, 'ShellViewProbe')[0] await act(async () => { view.props.onLoadState({ nativeEvent: { state: 'loading' } }) @@ -467,6 +335,8 @@ describe('the hybrid shell screen', () => { // Once, for the one finished document, and never for the failure: a view that reported a // failure has nothing left to wait for. expect(dependencies.reportDocumentLoaded).toHaveBeenCalledTimes(1) + // The document that started is what drops the previous one's paint, so it is reported too. + expect(dependencies.reportDocumentStarted).toHaveBeenCalledTimes(1) }) it('fails the session when this host could not be read from the app store', async () => { @@ -474,14 +344,14 @@ describe('the hybrid shell screen', () => { // page re-posting `ready` on its backoff for as long as the screen is open. dependencies.snapshotUnreadable = true const warned = vi.spyOn(console, 'warn').mockImplementation(() => undefined) - await render(readyState('session-one')) + await renderScreen(readyState('session-one')) expect(dependencies.reportShellFailure.mock.calls).toEqual([['document-load-failed']]) warned.mockRestore() }) it('re-reads the app store on every ask, so the next init is not the first one again', async () => { dependencies.client = createFakeRpcClient() - const tree = await render(readyState('session-one')) + const tree = await renderScreen(readyState('session-one')) const view = byName(tree, 'ShellViewProbe')[0] await act(async () => { view.props.onBridgeMessage({ nativeEvent: { json: clientFrame({ type: 'ready' }) } }) @@ -519,7 +389,7 @@ describe('the hybrid shell screen', () => { if (tree === null) { throw new Error('screen did not render') } - mounted.push(tree) + trackRenderedScreen(tree) return { tree, // Read with the page's own reader rather than parsed loose: a frame this refuses is one the @@ -591,7 +461,7 @@ describe('the hybrid shell screen', () => { it('ends that wait on the page asking for a session', async () => { dependencies.client = createFakeRpcClient() - const tree = await render(readyState('session-one')) + const tree = await renderScreen(readyState('session-one')) await act(async () => { byName(tree, 'ShellViewProbe')[0].props.onBridgeMessage({ nativeEvent: { json: clientFrame({ type: 'ready' }) } @@ -610,7 +480,7 @@ describe('the hybrid shell screen', () => { const client = createFakeRpcClient() dependencies.client = client dependencies.pageReady = true - const tree = await render(readyState('session-one')) + const tree = await renderScreen(readyState('session-one')) // No `ready` first, which is exactly what a page that was never told cannot send. await act(async () => { byName(tree, 'ShellViewProbe')[0].props.onBridgeMessage({ @@ -625,7 +495,7 @@ describe('the hybrid shell screen', () => { it('fails the session on a page fault, so a blank page becomes the failure screen', async () => { dependencies.client = createFakeRpcClient() const warned = vi.spyOn(console, 'warn').mockImplementation(() => undefined) - const tree = await render(readyState('session-one')) + const tree = await renderScreen(readyState('session-one')) await act(async () => { // The page asks for its session first, which is what earns it the `fault` grant: a host that // has told a page nothing refuses the name. @@ -646,7 +516,9 @@ describe('the hybrid shell screen', () => { warned.mockRestore() // The reducer's answer to that reason, rendered: this is what the page's blank turns into. expect( - textOf(await render({ kind: 'failed', reason: 'document-load-failed', retriedOnce: true })) + textOf( + await renderScreen({ kind: 'failed', reason: 'document-load-failed', retriedOnce: true }) + ) ).toContain('The downloaded workspace could not be opened.') }) @@ -658,7 +530,7 @@ describe('the hybrid shell screen', () => { // rejection in the window between. dependencies.openUrl.mockImplementation(() => Promise.reject(failure)) const warned = vi.spyOn(console, 'warn').mockImplementation(() => undefined) - const tree = await render(readyState('session-one')) + const tree = await renderScreen(readyState('session-one')) await act(async () => { byName(tree, 'ShellViewProbe')[0].props.onBridgeMessage({ nativeEvent: { json: clientFrame({ type: 'ready' }) } @@ -684,7 +556,7 @@ describe('the hybrid shell screen', () => { it('pops its own stack when the page hands its back button over', async () => { dependencies.client = createFakeRpcClient() - const tree = await render(readyState('session-one')) + const tree = await renderScreen(readyState('session-one')) await act(async () => { byName(tree, 'ShellViewProbe')[0].props.onBridgeMessage({ nativeEvent: { json: clientFrame({ type: 'ready' }) } @@ -701,7 +573,7 @@ describe('the hybrid shell screen', () => { dependencies.client = createFakeRpcClient() dependencies.canGoBack = false const warned = vi.spyOn(console, 'warn').mockImplementation(() => undefined) - const tree = await render(readyState('session-one')) + const tree = await renderScreen(readyState('session-one')) await act(async () => { byName(tree, 'ShellViewProbe')[0].props.onBridgeMessage({ nativeEvent: { json: clientFrame({ type: 'ready' }) } @@ -720,7 +592,7 @@ describe('the hybrid shell screen', () => { }) it('shows a build id prefix and never the whole one, the cache path, or the host id', async () => { - const tree = await render(readyState('session-one')) + const tree = await renderScreen(readyState('session-one')) const text = textOf(tree) expect(text).toContain(BUILD_ID.slice(0, 12)) expect(text).toContain('4096 B') @@ -733,7 +605,7 @@ describe('the hybrid shell screen', () => { describe('the route the shell was not asked to render', () => { it('hands the screen back to the caller rather than painting anything of its own', async () => { - const tree = await render({ kind: 'native-route' }) + const tree = await renderScreen({ kind: 'native-route' }) expect(tree.root.findAllByType(NativeFallback)).toHaveLength(1) expect(byName(tree, 'ShellViewProbe')).toEqual([]) expect(byName(tree, 'ActivityIndicator')).toEqual([]) @@ -789,11 +661,11 @@ describe('the dropped-frame count on the dev facts line', () => { it('shows the running total and resets it when the host is rebuilt', async () => { dependencies.client = createFakeRpcClient() dependencies.routeGrants = ['navigate', 'screencastBinary'] - const tree = await render(readyState('session-one')) + const tree = await renderScreen(readyState('session-one')) await openBinaryStream(tree) await drop(2) expect(devFactsText(tree)).toContain('2 frames dropped') - await update(tree, readyState('session-two')) + await updateScreen(tree, readyState('session-two')) expect(devFactsText(tree)).not.toContain('dropped') }) @@ -807,7 +679,7 @@ describe('the dropped-frame count on the dev facts line', () => { try { dependencies.client = createFakeRpcClient() dependencies.routeGrants = ['navigate', 'screencastBinary'] - const tree = await render(readyState('session-one')) + const tree = await renderScreen(readyState('session-one')) await openBinaryStream(tree) expect(devFactsText(tree)).toBeNull() const before = dependencies.viewRenders @@ -836,7 +708,7 @@ describe('a refused update is said beside the page, not in front of it', () => { it('serves the page and says the update did not happen, promising no retry', async () => { dependencies.updateNotice = 'update-failed' - const tree = await render(readyState('session-one')) + const tree = await renderScreen(readyState('session-one')) expect(byName(tree, 'ShellViewProbe')).toHaveLength(1) const text = textOf(tree) expect(text).toContain("Couldn't update the workspace from this host") @@ -848,21 +720,21 @@ describe('a refused update is said beside the page, not in front of it', () => { // The banner is inserted into a screen already on screen. Assertive because the shell passes // the failure tone: what it reports is an update that did not happen. dependencies.updateNotice = 'update-failed' - const tree = await render(readyState('session-one')) + const tree = await renderScreen(readyState('session-one')) const alert = byName(tree, 'View').find((node) => node.props.accessibilityRole === 'alert') expect(alert).toBeDefined() expect(alert?.props.accessibilityLiveRegion).toBe('assertive') }) it('says nothing when the generation on screen is the one the host serves', async () => { - const tree = await render(readyState('session-one')) + const tree = await renderScreen(readyState('session-one')) expect(dismissControl(tree)).toBeUndefined() expect(textOf(tree)).not.toContain("Couldn't update") }) it('keeps the same document mounted when the notice is dismissed', async () => { dependencies.updateNotice = 'update-failed' - const tree = await render(readyState('session-one')) + const tree = await renderScreen(readyState('session-one')) dependencies.lifecycle.length = 0 await act(async () => { dismissControl(tree)?.props.onPress() @@ -877,13 +749,13 @@ describe('a refused update is said beside the page, not in front of it', () => { it('shows a later refusal rather than staying dismissed for the rest of the host', async () => { dependencies.updateNotice = 'update-failed' - const tree = await render(readyState('session-one')) + const tree = await renderScreen(readyState('session-one')) await act(async () => { dismissControl(tree)?.props.onPress() }) // The next flow refused too, and opened its own fallback: a new document, so the tap on the // one before it is not an answer about this one. - await update(tree, readyState('session-two')) + await updateScreen(tree, readyState('session-two')) expect(dismissControl(tree)).toBeDefined() }) }) @@ -907,7 +779,7 @@ describe('what one case mutates does not reach the next', () => { // Edge-to-edge makes the manifest's `adjustResize` inert, so the window never shrinks and the // page's `visualViewport` reads full height with the IME up: it lays its live input row out // under the keys. The shell owns the window, so it takes the strip off the view instead. - const tree = await render(readyState('session-keyboard')) + const tree = await renderScreen(readyState('session-keyboard')) const root = tree.root.find((node) => node.props.testID === 'mobile-web-shell-ready') expect(root.props.style[1]).toEqual({ paddingTop: 44, paddingBottom: 8 }) @@ -922,3 +794,84 @@ describe('what one case mutates does not reach the next', () => { expect(root.props.style[1]).toEqual({ paddingTop: 44, paddingBottom: 8 }) }) }) + +/** + * What is on screen between the generation being mounted and the page having a frame. + * + * Before this the answer was nothing: the shell tore its own frame down at `ready` and the WebView + * draws nothing until its document paints, so the surface behind it was the whole picture for the + * length of the page's boot — measured at 1.42 s on a cached generation. + */ +describe('the frame under a page that has not painted', () => { + it('keeps the shell frame over a mounted view, with the view underneath it', async () => { + dependencies.pageFrame = 'unpainted' + const tree = await renderScreen(readyState('session-a')) + expect( + tree.root.findAll((node) => node.props.testID === 'mobile-web-shell-cover') + ).toHaveLength(1) + // Over, not instead of: the document is loading the whole time the cover is up. + expect(byName(tree, 'ShellViewProbe')).toHaveLength(1) + expect(textOf(tree)).toContain('Opening workspace') + }) + + it('carries the same label the screen was already painting while it opened the generation', async () => { + const opening = await renderScreen({ kind: 'activating' }) + expect(textOf(opening)).toContain('Opening workspace') + dependencies.pageFrame = 'unpainted' + await updateScreen(opening, readyState('session-a')) + // The frame does not change when the state does, which is what makes the handover invisible. + expect(textOf(opening)).toContain('Opening workspace') + }) + + it('takes the frame down once the page reports one of its own', async () => { + dependencies.pageFrame = 'unpainted' + const tree = await renderScreen(readyState('session-a')) + dependencies.pageFrame = 'painted' + await updateScreen(tree, readyState('session-a')) + expect(tree.root.findAll((node) => node.props.testID === 'mobile-web-shell-cover')).toEqual([]) + }) + + it('covers the same box the view gets, which is what the keyboard strip shortens', async () => { + // Both are children of the padded root: the view is `flex: 1` and the cover is an absolute + // fill, so Yoga lays each of them out against the same content box. The keyboard takes its + // strip off that box, so it takes it off both, and the cover cannot leave a gap the view fills. + dependencies.pageFrame = 'unpainted' + const tree = await renderScreen(readyState('session-keyboard-cover')) + const root = tree.root.find((node) => node.props.testID === 'mobile-web-shell-ready') + await act(async () => { + dependencies.keyboardListeners.get('keyboardWillShow')?.({ endCoordinates: { height: 336 } }) + }) + expect(root.props.style[1]).toEqual({ paddingTop: 44, paddingBottom: 336 }) + const cover = tree.root.find((node) => node.props.testID === 'mobile-web-shell-cover') + expect(cover.props.style[0]).toMatchObject({ + position: 'absolute', + top: 0, + left: 0, + right: 0, + bottom: 0 + }) + // Both sit directly in that root with no box in between, so neither carries a padding of its + // own for the two to disagree about. + expect(hostParentOf(cover)).toBe('mobile-web-shell-ready') + expect(hostParentOf(byName(tree, 'ShellViewProbe')[0])).toBe('mobile-web-shell-ready') + }) + + it('hands the page report to the session', async () => { + dependencies.pageFrame = 'unpainted' + dependencies.client = createFakeRpcClient() + const tree = await renderScreen(readyState('session-a')) + const probe = byName(tree, 'ShellViewProbe')[0] + await act(async () => { + probe.props.onBridgeMessage({ + nativeEvent: { json: clientFrame({ type: 'ready', reports: [BRIDGE_PAGE_PAINTED] }) } + }) + }) + expect(dependencies.reportPageReady).toHaveBeenCalledWith([BRIDGE_PAGE_PAINTED]) + await act(async () => { + probe.props.onBridgeMessage({ + nativeEvent: { json: clientFrame({ type: 'notify', name: BRIDGE_PAGE_PAINTED }) } + }) + }) + expect(dependencies.reportPagePainted).toHaveBeenCalledTimes(1) + }) +}) diff --git a/mobile/src/mobile-web-shell/MobileWebShellScreen.tsx b/mobile/src/mobile-web-shell/MobileWebShellScreen.tsx index e3f4b7dd886..7c0298f8d64 100644 --- a/mobile/src/mobile-web-shell/MobileWebShellScreen.tsx +++ b/mobile/src/mobile-web-shell/MobileWebShellScreen.tsx @@ -39,6 +39,7 @@ import { useNativeDeviceVerbs } from '../platform/use-native-device-verbs' import { useShellStackPop } from './use-shell-stack-pop' import { useMobileWebShellSession } from './use-mobile-web-shell-session' import { usePageHostSnapshot } from './use-page-host-snapshot' +import { SHELL_OPENING_LABEL, ShellPageCover, ShellWaitingFrame } from './ShellWaitingFrame' function failureMessage(reason: MobileWebShellFailureCause): string { switch (reason) { @@ -72,8 +73,7 @@ function Centered({ children }: { children: ReactNode }) { function Waiting({ label }: { label: string }) { return ( - - {label} + ) } @@ -197,9 +197,12 @@ export function MobileWebShellScreen({ updateNotice, retry, reportShellFailure, + reportDocumentStarted, reportDocumentLoaded, reportPageReady, - pageReady + reportPagePainted, + pageReady, + pageFrame } = useMobileWebShellSession({ hostId, routePathname: route.pathname, runtime }) // Which mount the notice was dismissed on, not whether it was: a later refusal opens its own // generation under a new session id, so it is not silenced by a tap on the one before it. @@ -247,10 +250,13 @@ 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: () => { - reportPageReady() + onPageReady: (reports) => { + reportPageReady(reports) void refreshStorage() }, + // The one thing that says the page is something to look at. The cover below stays up until it + // lands, for a page that declared it would send one. + onPagePainted: reportPagePainted, onRouteParamClear: (param, value) => { onRouteParamClear?.(param, value) }, @@ -334,7 +340,7 @@ export function MobileWebShellScreen({ return } if (state.kind !== 'ready') { - return + return } return ( + ) diff --git a/mobile/src/mobile-web-shell/ShellWaitingFrame.test.tsx b/mobile/src/mobile-web-shell/ShellWaitingFrame.test.tsx new file mode 100644 index 00000000000..ec6d2ae1680 --- /dev/null +++ b/mobile/src/mobile-web-shell/ShellWaitingFrame.test.tsx @@ -0,0 +1,103 @@ +import { createElement } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { describe, expect, it, vi } from 'vitest' +import { colors } from '../theme/mobile-theme' + +const fades = vi.hoisted(() => ({ stops: 0, finished: true })) + +vi.mock('react-native', () => ({ + ActivityIndicator: 'ActivityIndicator', + Easing: { in: (fn: unknown) => fn, quad: 'quad' }, + Animated: { + View: 'Animated.View', + Value: class { + setValue(): void {} + }, + timing: () => ({ + start: (done?: (result: { finished: boolean }) => void) => { + done?.({ finished: fades.finished }) + }, + stop: () => { + fades.stops += 1 + } + }) + }, + StyleSheet: { + create: (styles: unknown) => styles, + absoluteFillObject: { position: 'absolute', top: 0, left: 0, right: 0, bottom: 0 } + }, + Text: 'Text' +})) + +const { ShellPageCover } = await import('./ShellWaitingFrame') + +function render(visible: boolean): ReactTestRenderer { + let tree: ReactTestRenderer | null = null + act(() => { + tree = create(createElement(ShellPageCover, { label: 'Opening workspace', visible })) + }) + if (tree === null) { + throw new Error('the cover never rendered') + } + return tree +} + +function cover(tree: ReactTestRenderer) { + return tree.root.findAllByProps({ testID: 'mobile-web-shell-cover' })[0] ?? null +} + +/** The colour the cover fills with, read out of its style prop rather than assumed of its shape. */ +function coverBackground(tree: ReactTestRenderer): unknown { + const style: unknown = cover(tree)?.props.style + const base: unknown = Array.isArray(style) ? style[0] : null + return typeof base === 'object' && base !== null && 'backgroundColor' in base + ? base.backgroundColor + : null +} + +describe('the frame the shell keeps over an unpainted page', () => { + it('is up while the page has not painted', () => { + const tree = render(true) + expect(cover(tree)).not.toBeNull() + }) + + it('paints the app surface and never black, so an empty view is never a hole', () => { + // The whole defect in one assertion: what shows while the WebView draws nothing is this. + const background = coverBackground(render(true)) + expect(background).toBe(colors.bgBase) + expect(background).not.toBe('#000000') + }) + + it('never takes a touch, so a report that never lands leaves a usable page under it', () => { + expect(cover(render(true))?.props.pointerEvents).toBe('none') + }) + + it('goes once the page reports a frame', () => { + const tree = render(true) + act(() => { + tree.update(createElement(ShellPageCover, { label: 'Opening workspace', visible: false })) + }) + expect(cover(tree)).toBeNull() + }) + + it('stays out of the way when a fade is cut short rather than staying opaque', () => { + // A platform that stops the fade reports `finished: false`; the cover keeps its element and + // its `pointerEvents: none`, which is a transparent inert layer rather than an opaque one. + fades.finished = false + const tree = render(true) + act(() => { + tree.update(createElement(ShellPageCover, { label: 'Opening workspace', visible: false })) + }) + expect(cover(tree)?.props.pointerEvents).toBe('none') + fades.finished = true + }) + + it('stops a fade in flight when the view unmounts', () => { + const before = fades.stops + const tree = render(false) + act(() => { + tree.unmount() + }) + expect(fades.stops).toBeGreaterThan(before) + }) +}) diff --git a/mobile/src/mobile-web-shell/ShellWaitingFrame.tsx b/mobile/src/mobile-web-shell/ShellWaitingFrame.tsx new file mode 100644 index 00000000000..ea7c5f06603 --- /dev/null +++ b/mobile/src/mobile-web-shell/ShellWaitingFrame.tsx @@ -0,0 +1,87 @@ +import { useEffect, useRef, useState } from 'react' +import { ActivityIndicator, Animated, Easing, StyleSheet, Text } from 'react-native' +import { colors, spacing, typography } from '../theme/mobile-theme' + +/** What the shell calls the wait from the bytes being on disk to the page having a frame. */ +export const SHELL_OPENING_LABEL = 'Opening workspace' + +/** + * Eased in, so the cover holds near full opacity through the handover: the page reports its own + * renderer's paint, and the compositor needs a frame or two more to put that on the app's surface. + * A linear fade left two frames of bare surface between the two on an emulator. + */ +export const SHELL_PAGE_COVER_FADE_MS = 220 + +/** The shell's neutral frame: one spinner and what it is waiting on. */ +export function ShellWaitingFrame({ label }: { label: string }) { + return ( + <> + + {label} + + ) +} + +/** + * That same frame, held over a mounted view until the page reports one of its own: a WebView that + * has not painted draws nothing, so uncovering at `ready` shows the surface behind it and nothing + * else for the whole of the page's boot. Never interactive even while opaque, so a report that + * never arrives strands a spinner over a usable page rather than a dead one. + */ +export function ShellPageCover({ label, visible }: { label: string; visible: boolean }) { + const opacity = useRef(new Animated.Value(1)).current + const [mounted, setMounted] = useState(visible) + + useEffect(() => { + if (visible) { + opacity.setValue(1) + setMounted(true) + return + } + const fade = Animated.timing(opacity, { + toValue: 0, + duration: SHELL_PAGE_COVER_FADE_MS, + easing: Easing.in(Easing.quad), + useNativeDriver: true + }) + // Unmounted on the callback rather than on a timer, so a fade the platform cut short does not + // leave an opaque cover behind; one that never calls back leaves a transparent inert one. + fade.start(({ finished }) => { + if (finished) { + setMounted(false) + } + }) + return () => { + fade.stop() + } + }, [opacity, visible]) + + if (!mounted) { + return null + } + return ( + + + + ) +} + +const styles = StyleSheet.create({ + cover: { + ...StyleSheet.absoluteFillObject, + alignItems: 'center', + justifyContent: 'center', + backgroundColor: colors.bgBase, + paddingHorizontal: spacing.lg + }, + label: { + fontSize: typography.bodySize, + color: colors.textSecondary, + marginTop: spacing.md, + textAlign: 'center' + } +}) diff --git a/mobile/src/mobile-web-shell/bridge-host-contract.ts b/mobile/src/mobile-web-shell/bridge-host-contract.ts index 08400ba3fa9..e6d1549bb29 100644 --- a/mobile/src/mobile-web-shell/bridge-host-contract.ts +++ b/mobile/src/mobile-web-shell/bridge-host-contract.ts @@ -186,7 +186,13 @@ export type BridgeHostOptions = { * the same reason as the fault: the shell bounds the wait for it, and a host built without this * would leave a document that never spoke looking exactly like one still starting up. */ - onPageReady: () => void + onPageReady: (reports: readonly string[]) => void + /** + * The page has a frame on screen. Only pages whose `ready` listed `BRIDGE_PAGE_PAINTED` post it, + * which is why `onPageReady` carries that list: a caller covering the view until this arrives + * has to know whether it is coming, and a page served from an older desktop never sends one. + */ + onPagePainted: () => void /** * The page applied a one-shot route param and is asking for it to be erased (ruling 34), naming * the value it applied. The holder of that param compares before it clears: a tap that has moved 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 862a2f250a9..810de881d64 100644 --- a/mobile/src/mobile-web-shell/bridge-host-init.test.ts +++ b/mobile/src/mobile-web-shell/bridge-host-init.test.ts @@ -8,6 +8,7 @@ import { } from './bridge/bridge-caps' import { BRIDGE_FAULT_GRANT } from './bridge/bridge-envelope' import { BRIDGE_PAGE_CLIENT_IDENTITY_ACCEPT } from './bridge/bridge-page-client-identity' +import { BRIDGE_PAGE_PAINTED } from './bridge/bridge-page-painted' import { BRIDGE_ROUTE_PARAM_CLEAR } from './bridge/bridge-route-update' import { routeViewOf } from './page-route-policy' @@ -28,7 +29,7 @@ describe('init and state', () => { sessionId: 'session-a', buildId: 'build-a', // What this shell takes from the page, which is the page's own check before it posts one. - accepts: [BRIDGE_ROUTE_PARAM_CLEAR, BRIDGE_PAGE_CLIENT_IDENTITY_ACCEPT], + accepts: [BRIDGE_ROUTE_PARAM_CLEAR, BRIDGE_PAGE_CLIENT_IDENTITY_ACCEPT, BRIDGE_PAGE_PAINTED], connection: { state: 'reconnecting', reconnectAttempt: 3, diff --git a/mobile/src/mobile-web-shell/bridge-host-notifications.test.ts b/mobile/src/mobile-web-shell/bridge-host-notifications.test.ts index 10cb4f2b609..68896414d78 100644 --- a/mobile/src/mobile-web-shell/bridge-host-notifications.test.ts +++ b/mobile/src/mobile-web-shell/bridge-host-notifications.test.ts @@ -7,6 +7,7 @@ import { BRIDGE_NAVIGATE_BACK_NOTIFY } from './bridge/bridge-envelope' import { BRIDGE_PAGE_CLIENT_IDENTITY_ACCEPT } from './bridge/bridge-page-client-identity' +import { BRIDGE_PAGE_PAINTED } from './bridge/bridge-page-painted' import { BRIDGE_ROUTE_PARAM_CLEAR } from './bridge/bridge-route-update' import { BRIDGE_HAPTICS_GRANT, @@ -465,9 +466,45 @@ describe('the page erasing a one-shot route param', () => { const bridge = harness({ route: { pathname: '/h/host-a/session/wt-1' } }) bridge.host.receive(clientFrame({ type: 'ready' })) const init = bridge.last() + // Written out rather than compared against `BRIDGE_SHELL_ACCEPTS`: a list that pins itself + // pins nothing, and this is the frame an older page reads to decide what it may post. expect(init.type === 'init' && init.accepts).toEqual([ BRIDGE_ROUTE_PARAM_CLEAR, - BRIDGE_PAGE_CLIENT_IDENTITY_ACCEPT + BRIDGE_PAGE_CLIENT_IDENTITY_ACCEPT, + BRIDGE_PAGE_PAINTED ]) }) }) + +/** + * The page's word about its own document, which is the only thing that says the view is worth + * uncovering: a document commit is the WebView's, and `ready` is posted before a tree is built. + */ +describe('the page reporting its first frame', () => { + it('hands the report to the session and asks the client for nothing', () => { + const bridge = harness() + bridge.host.receive(clientFrame({ type: 'ready', reports: [BRIDGE_PAGE_PAINTED] })) + bridge.host.receive(clientFrame({ type: 'notify', name: BRIDGE_PAGE_PAINTED })) + expect(bridge.pagePaintCount()).toBe(1) + expect(bridge.client.requests).toHaveLength(0) + expect(bridge.client.foregroundCalls).toHaveLength(0) + }) + + it('forwards what each ready declared, including a name this shell does not implement', () => { + const bridge = harness() + bridge.host.receive(clientFrame({ type: 'ready', reports: [BRIDGE_PAGE_PAINTED, 'weather'] })) + bridge.host.receive(clientFrame({ type: 'ready' })) + expect(bridge.pageReports()).toEqual([[BRIDGE_PAGE_PAINTED, 'weather'], []]) + }) + + it('refuses a report from a document nothing has answered', () => { + const bridge = harness() + bridge.host.receive(clientFrame({ type: 'notify', name: BRIDGE_PAGE_PAINTED })) + expect(bridge.pagePaintCount()).toBe(0) + expect(bridge.diagnostics).toContainEqual({ + kind: 'notify-refused', + name: BRIDGE_PAGE_PAINTED, + why: 'before-ready' + }) + }) +}) 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 94b9fedf718..ef06ef6b64e 100644 --- a/mobile/src/mobile-web-shell/bridge-host-test-harness.ts +++ b/mobile/src/mobile-web-shell/bridge-host-test-harness.ts @@ -45,6 +45,9 @@ export type Harness = { backPops: BridgeNavigateBackOutcome[] storageWrites: { key: string; value: string | null }[] pageReadyCount: () => number + pagePaintCount: () => number + /** What each answered `ready` declared it reports, in order. */ + pageReports: () => readonly (readonly string[])[] /** 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. */ /** Every clear the page asked for, in order. */ @@ -109,6 +112,9 @@ export function harness( const backPops: BridgeNavigateBackOutcome[] = [] const storageWrites: { key: string; value: string | null }[] = [] let pageReadies = 0 + let pagePaints = 0 + /** What each answered `ready` declared it reports, in order. */ + const pageReports: (readonly string[])[] = [] /** One entry per `ready` answered, saying whether an `init` actually went out for it. */ const routeParamClears: { param: string; value: string }[] = [] const routeRefusals: string[] = [] @@ -133,8 +139,12 @@ export function harness( readStorage: options.readStorage ?? (() => ({ storage: options.storage ?? {}, storageOversize: [] })), onStorageWrite: (key, value) => storageWrites.push({ key, value }), - onPageReady: () => { + onPageReady: (reports) => { pageReadies += 1 + pageReports.push(reports) + }, + onPagePainted: () => { + pagePaints += 1 }, onRouteParamClear: (param, value) => routeParamClears.push({ param, value }), onRouteRefused: (issue) => routeRefusals.push(issue), @@ -192,6 +202,8 @@ export function harness( backPops, storageWrites, pageReadyCount: () => pageReadies, + pagePaintCount: () => pagePaints, + pageReports: () => pageReports, routeParamClears: () => routeParamClears, routeRefusals, pageFaults, diff --git a/mobile/src/mobile-web-shell/bridge-host.ts b/mobile/src/mobile-web-shell/bridge-host.ts index 797a2f4548e..8bfde7c6298 100644 --- a/mobile/src/mobile-web-shell/bridge-host.ts +++ b/mobile/src/mobile-web-shell/bridge-host.ts @@ -15,9 +15,9 @@ import { type BridgeConnectionSnapshot, type BridgeInitRoute } from './bridge/bridge-envelope' -import { BRIDGE_PAGE_CLIENT_IDENTITY_ACCEPT } from './bridge/bridge-page-client-identity' +import { BRIDGE_PAGE_PAINTED } from './bridge/bridge-page-painted' import { BridgePageRouteGrantsSchema } from './bridge/bridge-page-route-grants' -import { createBridgeInitFrame } from './bridge/bridge-init-frame' +import { BRIDGE_SHELL_ACCEPTS, createBridgeInitFrame } from './bridge/bridge-init-frame' import { BRIDGE_HAPTICS_NOTIFY } from './bridge/bridge-haptics-notify' import { bridgeNotifyRefusal } from './bridge/bridge-notify-grants' import { splitBridgeReply } from './bridge/bridge-reply-chunking' @@ -159,9 +159,7 @@ export function createBridgeHost(options: BridgeHostOptions): BridgeHost { ? { pageRouteGrants: parsedRouteGrants.data } : {}), granted, - // Both are additive names on an optional list, so no version moves: a page that knows - // neither posts neither, and one told nothing claims no identity and sends none. - accepts: [BRIDGE_ROUTE_PARAM_CLEAR, BRIDGE_PAGE_CLIENT_IDENTITY_ACCEPT], + accepts: BRIDGE_SHELL_ACCEPTS, host, ...options.readStorage() }) @@ -221,6 +219,11 @@ export function createBridgeHost(options: BridgeHostOptions): BridgeHost { options.onPageFault(message.error) return } + if (message.name === BRIDGE_PAGE_PAINTED) { + // Local, like `navigate`: nothing about the page's own frame reaches the desktop. + options.onPagePainted() + return + } if (message.name === 'foreground') { if (message.reason === undefined) { client.notifyForeground() @@ -326,7 +329,10 @@ export function createBridgeHost(options: BridgeHostOptions): BridgeHost { // with the route the shell holds now. That is the whole repair path for a frame that never // arrived (ruling 34) — nothing here waits on one, and nothing retries one. sendInit() - options.onPageReady() + // Forwarded verbatim, including a name this shell has never implemented: what each report + // means is the caller's, and this host's job is that the list belongs to the document that + // just spoke rather than to the one before it. + options.onPageReady(message.reports ?? []) return } if (!serving) { diff --git a/mobile/src/mobile-web-shell/bridge/bridge-client-notifications.ts b/mobile/src/mobile-web-shell/bridge/bridge-client-notifications.ts index 0e1cdf18960..2fb1c5557d6 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-client-notifications.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-client-notifications.ts @@ -13,17 +13,18 @@ import { type BridgeHapticsKind } from './bridge-haptics-notify' import { captureBridgeError } from './bridge-error-capture' +import { BRIDGE_PAGE_PAINTED } from './bridge-page-painted' /** * Everything the page posts and hears nothing back about. * - * Six of the seven post through one guard, but only two reach its throw, and it is not the guard + * Seven of the eight post through one guard, but only three reach its throw, and it is not the guard * `sendRequest` uses. A call before `init` is a mount-order bug and throws; a call after `close` is * an unmounting screen posting one more nudge on its way out, which the native clients answer * inertly rather than by throwing into a teardown path nobody wrote a catch for. Nothing here * returns a promise, so nothing here can be awaited into a rejection either. * - * Only the two ungated notifies reach that throw. A grant is read off the session, so before `init` + * Only the three ungated notifies reach that throw. A grant is read off the session, so before `init` * there is no grant either and `navigate`, `navigate-back`, `externalLink`, `storage` and the * haptic answer false without asking: that is the same false they answer a shell that withheld the * grant, and every caller already handles it — `useRouteHandoff` pushes or goes back inside the @@ -41,6 +42,8 @@ export type BridgeClientNotificationDeps = { isClosed: () => boolean /** What `init.grants.native` named. A grant the shell did not give is a frame it would refuse. */ hasGrant: (name: string) => boolean + /** What `init.accepts` named. The other half of the same read: a capability rather than a grant. */ + shellAccepts: (name: string) => boolean } export type BridgeClientNotifications = { @@ -55,6 +58,7 @@ export type BridgeClientNotifications = { notifyStorageWrite: (key: string, value: string | null) => boolean notifyHaptics: (kind: BridgeHapticsKind) => boolean notifyPageFault: (error: unknown) => boolean + notifyPagePainted: () => void } export function createBridgeClientNotifications( @@ -133,6 +137,14 @@ export function createBridgeClientNotifications( name: BRIDGE_FAULT_GRANT, error: captureBridgeError(error) }) + }, + // Gated on the shell saying it takes one, not on a grant: `notify` is a closed union, so an + // older shell answers an unknown name with an error frame per mount. Answering nothing, because + // a page that has painted has nothing else to do about a shell that will not hear it. + notifyPagePainted: () => { + if (deps.shellAccepts(BRIDGE_PAGE_PAINTED)) { + post({ v: BRIDGE_PROTOCOL_VERSION, type: 'notify', name: BRIDGE_PAGE_PAINTED }) + } } } } diff --git a/mobile/src/mobile-web-shell/bridge/bridge-envelope.test.ts b/mobile/src/mobile-web-shell/bridge/bridge-envelope.test.ts index e46c687569a..bfbb69aa1b0 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-envelope.test.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-envelope.test.ts @@ -34,6 +34,7 @@ import { type BridgeHostMessage, type BridgeReplyPayload } from './bridge-envelope' +import { BRIDGE_PAGE_PAINTED } from './bridge-page-painted' const ID = 'AAAAAAAAAAAAAAAAAAAAAA' const CONNECTION = { @@ -87,6 +88,14 @@ function client(fields: Record): Record { describe('client messages', () => { const accepted = [ ['ready', { type: 'ready' }], + ['ready naming what it reports', { type: 'ready', reports: [BRIDGE_PAGE_PAINTED] }], + // A shell with no row for the name reads a report it will never wait on, which is what an + // additive field has to look like in the older direction. + [ + 'ready naming a report this shell does not implement', + { type: 'ready', reports: ['weather'] } + ], + ['a page painted notify', { type: 'notify', name: BRIDGE_PAGE_PAINTED }], ['request without params', { type: 'request', id: ID, method: 'status.get' }], ['request with params', { type: 'request', id: ID, method: 'status.get', params: { a: 1 } }], [ diff --git a/mobile/src/mobile-web-shell/bridge/bridge-envelope.ts b/mobile/src/mobile-web-shell/bridge/bridge-envelope.ts index 704fc07c131..036ee28352e 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-envelope.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-envelope.ts @@ -161,6 +161,23 @@ const BridgeClientMessageSchema = z.discriminatedUnion('type', [ * capability this shell has never implemented has to look like. */ accepts: z + .array(z.string().min(1).max(BRIDGE_MAX_PAGE_ACCEPT_CHARS)) + .max(BRIDGE_MAX_PAGE_ACCEPTS) + .optional(), + /** + * What this page will post that the shell may have to wait for, which today is + * `BRIDGE_PAGE_PAINTED` and nothing else. + * + * `accepts` runs the other way and cannot stand in for this: it says what may be sent *to* the + * page. A shell waiting on a frame has to know the page will send one, because the generation + * is served by a desktop that updates independently of the installed shell — an undeclared + * wait would hide a working page built before the frame existed. + * + * Optional and additive in both directions, on the same bounds as `accepts`: a page that + * declares none is waited for by nothing, and an unknown name is a report this shell does not + * act on. + */ + reports: z .array(z.string().min(1).max(BRIDGE_MAX_PAGE_ACCEPT_CHARS)) .max(BRIDGE_MAX_PAGE_ACCEPTS) .optional() diff --git a/mobile/src/mobile-web-shell/bridge/bridge-init-frame.ts b/mobile/src/mobile-web-shell/bridge/bridge-init-frame.ts index a6f301e1adb..999519beffb 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-init-frame.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-init-frame.ts @@ -8,6 +8,9 @@ import { type BridgeInitHost, type BridgeInitRoute } from './bridge-envelope' +import { BRIDGE_PAGE_CLIENT_IDENTITY_ACCEPT } from './bridge-page-client-identity' +import { BRIDGE_PAGE_PAINTED } from './bridge-page-painted' +import { BRIDGE_ROUTE_PARAM_CLEAR } from './bridge-route-update' /** * Every grant this app implements, which is the ceiling a session's own list is drawn from. A page @@ -25,6 +28,17 @@ export const BRIDGE_NATIVE_GRANTS: readonly string[] = [ ...MOBILE_WEB_SHELL_GRANTS ] +/** + * What this shell accepts from a page beyond the frames every shell has always taken. Additive + * names on an optional list, so no version moves: a page that knows none posts none, one told + * nothing claims no identity, and one told nothing reports no paint. + */ +export const BRIDGE_SHELL_ACCEPTS: readonly string[] = [ + BRIDGE_ROUTE_PARAM_CLEAR, + BRIDGE_PAGE_CLIENT_IDENTITY_ACCEPT, + BRIDGE_PAGE_PAINTED +] + /** The one frame that starts a session, built in one place so its caps and its grants agree. */ export function createBridgeInitFrame(args: { sessionId: string diff --git a/mobile/src/mobile-web-shell/bridge/bridge-notify-envelope.ts b/mobile/src/mobile-web-shell/bridge/bridge-notify-envelope.ts index cbe5adf007b..ea6d6cededa 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-notify-envelope.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-notify-envelope.ts @@ -1,6 +1,7 @@ import { z } from 'zod' import { BridgeErrorCaptureSchema } from './bridge-error-capture' import { BRIDGE_HAPTICS_NOTIFY_FIELDS } from './bridge-haptics-notify' +import { BRIDGE_PAGE_PAINTED } from './bridge-page-painted' import { BRIDGE_CLEARABLE_ROUTE_PARAMS, BRIDGE_ROUTE_PARAM_CLEAR } from './bridge-route-update' import { isPageStorageKey, @@ -101,6 +102,14 @@ export const BridgeNotifySchema = z.discriminatedUnion('name', [ name: z.literal(BRIDGE_ROUTE_PARAM_CLEAR), param: z.enum(BRIDGE_CLEARABLE_ROUTE_PARAMS), value: z.string().min(1).max(BRIDGE_MAX_ROUTE_PARAM_CHARS) + }), + // Ungranted, and carrying nothing: the page is reporting on its own document, which no grant + // gates. The shell waits for it only from a page whose `ready` listed it, so a name an older + // shell refuses is one a newer page was never waited on for. + z.object({ + v: versionSchema, + type: z.literal('notify'), + name: z.literal(BRIDGE_PAGE_PAINTED) }) ]) diff --git a/mobile/src/mobile-web-shell/bridge/bridge-notify-grants.test.ts b/mobile/src/mobile-web-shell/bridge/bridge-notify-grants.test.ts index 1675cccd3dd..91cd2406aae 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-notify-grants.test.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-notify-grants.test.ts @@ -5,6 +5,7 @@ import { BRIDGE_NAVIGATE_BACK_NOTIFY } from './bridge-envelope' import { BRIDGE_HAPTICS_GRANT, BRIDGE_HAPTICS_NOTIFY } from './bridge-haptics-notify' +import { BRIDGE_PAGE_PAINTED } from './bridge-page-painted' import { bridgeNotifyRefusal, type BridgeNotifyName } from './bridge-notify-grants' const GRANTED = [BRIDGE_FAULT_GRANT] @@ -180,3 +181,24 @@ describe('a grant table missing a row', () => { expect(Object.keys(incomplete)).toHaveLength(7) }) }) + +/** + * The page reporting on its own document. + * + * Ungranted for the same reason the param clear is: nothing here reaches the host or the device, + * and the shell acts on it only for a page whose `ready` declared it. It is still refused before + * `init`, because a frame from a document nothing has answered is not this document's word. + */ +describe('the page reporting its first frame', () => { + it('needs no grant once the session is open', () => { + expect( + bridgeNotifyRefusal({ name: BRIDGE_PAGE_PAINTED, initSent: true, granted: [] }) + ).toBeNull() + }) + + it('is refused before the page has been told anything', () => { + expect( + bridgeNotifyRefusal({ name: BRIDGE_PAGE_PAINTED, initSent: false, granted: GRANTED }) + ).toBe('before-ready') + }) +}) diff --git a/mobile/src/mobile-web-shell/bridge/bridge-notify-grants.ts b/mobile/src/mobile-web-shell/bridge/bridge-notify-grants.ts index 3cf02742a9f..6907fdf4a39 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-notify-grants.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-notify-grants.ts @@ -5,6 +5,7 @@ import { type BridgeClientMessage } from './bridge-envelope' import { BRIDGE_HAPTICS_GRANT, BRIDGE_HAPTICS_NOTIFY } from './bridge-haptics-notify' +import { BRIDGE_PAGE_PAINTED } from './bridge-page-painted' import { BRIDGE_ROUTE_PARAM_CLEAR } from './bridge-route-update' /** Every `notify` name the envelope accepts, so the table below cannot be asked about another. */ @@ -35,6 +36,9 @@ const BRIDGE_NOTIFY_GRANTS: Readonly> = // The protocol's own as well: it spends a request this shell handed the page, on a param closed // to the one the shell hands over, so there is nothing here for a grant to gate. [BRIDGE_ROUTE_PARAM_CLEAR]: null, + // The page reporting on its own document. Nothing here reaches the host or the device, and the + // shell acts on it only for a page that declared it in `ready.reports`. + [BRIDGE_PAGE_PAINTED]: null, navigate: 'navigate', [BRIDGE_NAVIGATE_BACK_NOTIFY]: 'navigate', storage: 'storage', diff --git a/mobile/src/mobile-web-shell/bridge/bridge-page-painted.ts b/mobile/src/mobile-web-shell/bridge/bridge-page-painted.ts new file mode 100644 index 00000000000..effa3c07a05 --- /dev/null +++ b/mobile/src/mobile-web-shell/bridge/bridge-page-painted.ts @@ -0,0 +1,6 @@ +/** + * The page's first frame, in all three places it is named: `ready.reports`, the notify, and + * `init.accepts`. Negotiated both ways, because the notify union is closed — a report to a shell + * that never advertised it is an error frame per mount on every shell already installed. + */ +export const BRIDGE_PAGE_PAINTED = 'painted' diff --git a/mobile/src/mobile-web-shell/bridge/bridge-port-pair-test-harness.test.ts b/mobile/src/mobile-web-shell/bridge/bridge-port-pair-test-harness.test.ts index e3a865edcd4..b46bd2f6df4 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-port-pair-test-harness.test.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-port-pair-test-harness.test.ts @@ -3,6 +3,7 @@ import type { RpcClient } from '../../transport/rpc-client' import type { RpcResponse } from '../../transport/types' import { createBridgePortPair, createFakeBridgePortPair } from './bridge-port-pair-test-harness' import { BRIDGE_PAGE_CLIENT_IDENTITY_ACCEPT } from './bridge-page-client-identity' +import { BRIDGE_PAGE_PAINTED } from './bridge-page-painted' import { BRIDGE_ROUTE_PARAM_CLEAR } from './bridge-route-update' /** Every member of the contract, none of them a fake anything: the pair must carry a plain client. */ @@ -43,7 +44,7 @@ describe('the bridge port pair', () => { host: expect.objectContaining({ id: expect.any(String) }), storage: expect.any(Object), storageOversize: [], - accepts: [BRIDGE_ROUTE_PARAM_CLEAR, BRIDGE_PAGE_CLIENT_IDENTITY_ACCEPT] + accepts: [BRIDGE_ROUTE_PARAM_CLEAR, BRIDGE_PAGE_CLIENT_IDENTITY_ACCEPT, BRIDGE_PAGE_PAINTED] }) }) 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 095a13edcfa..c797845241d 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 @@ -55,6 +55,9 @@ export type BridgePortPair = { pageFaults: BridgeErrorCapture[] /** How many times the page asked for a session; it re-asks on a backoff until one lands. */ readonly pageReadyCount: () => number + readonly pagePaintCount: () => number + /** What each answered `ready` declared it reports, in order. */ + readonly pageReports: () => readonly (readonly string[])[] /** Every clear the page asked the shell for, in order. */ readonly routeParamClears: () => readonly { param: string; value: string }[] /** Why the host refused to open a session at all, if it did. */ @@ -200,6 +203,9 @@ export function createBridgePortPair( const storageWrites: { key: string; value: string | null }[] = [] const pageFaults: BridgeErrorCapture[] = [] let pageReadies = 0 + let pagePaints = 0 + /** What each answered `ready` declared it reports, in order. */ + const pageReports: (readonly string[])[] = [] const routeParamClears: { param: string; value: string }[] = [] const routeRefusals: string[] = [] let receiveOnPage: ((json: string) => void) | null = null @@ -242,8 +248,12 @@ export function createBridgePortPair( }), onStorageWrite: (key, value) => storageWrites.push({ key, value }), onPageFault: (error) => pageFaults.push(error), - onPageReady: () => { + onPageReady: (reports) => { pageReadies += 1 + pageReports.push(reports) + }, + onPagePainted: () => { + pagePaints += 1 }, onRouteParamClear: (param, value) => routeParamClears.push({ param, value }), onRouteRefused: (issue) => routeRefusals.push(issue), @@ -280,6 +290,8 @@ export function createBridgePortPair( storageWrites, pageFaults, pageReadyCount: () => pageReadies, + pagePaintCount: () => pagePaints, + pageReports: () => pageReports, routeParamClears: () => routeParamClears, routeRefusals, async flush(): Promise { diff --git a/mobile/src/mobile-web-shell/bridge/bridge-rpc-client-frames.test.ts b/mobile/src/mobile-web-shell/bridge/bridge-rpc-client-frames.test.ts index 91dfce4082d..222d2a08246 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-rpc-client-frames.test.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-rpc-client-frames.test.ts @@ -12,6 +12,7 @@ import { BRIDGE_ACK_INTERVAL_FRAMES } from './bridge-client-subscriptions' import { BRIDGE_PROTOCOL_VERSION, type BridgeHostMessage } from './bridge-envelope' +import { BRIDGE_PAGE_PAINTED } from './bridge-page-painted' import { BRIDGE_ROUTE_UPDATE_ACCEPT } from './bridge-route-update' import { BRIDGE_READY_RETRY_MAX_MS, @@ -40,10 +41,15 @@ afterEach(() => { }) describe('bridge client handshake', () => { - it('asks for a session as soon as it exists, naming what it can be sent', () => { + it('asks for a session as soon as it exists, naming what it can be sent and what it reports', () => { const page = createPageClient() expect(page.frames()).toEqual([ - { v: BRIDGE_PROTOCOL_VERSION, type: 'ready', accepts: [BRIDGE_ROUTE_UPDATE_ACCEPT] } + { + v: BRIDGE_PROTOCOL_VERSION, + type: 'ready', + accepts: [BRIDGE_ROUTE_UPDATE_ACCEPT], + reports: [BRIDGE_PAGE_PAINTED] + } ]) }) diff --git a/mobile/src/mobile-web-shell/bridge/bridge-rpc-client.ts b/mobile/src/mobile-web-shell/bridge/bridge-rpc-client.ts index 95b9fba3c63..a1014131a28 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-rpc-client.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-rpc-client.ts @@ -24,6 +24,7 @@ import { import type { BridgeHapticsKind } from './bridge-haptics-notify' import { createBridgeInboundFrameReader } from './bridge-client-inbound-frames' import { createBridgeClientNotifications } from './bridge-client-notifications' +import { BRIDGE_PAGE_PAINTED } from './bridge-page-painted' import { BridgeClientRequests } from './bridge-client-requests' import { BridgeClientSubscriptions } from './bridge-client-subscriptions' import { isBridgeNativeMethod, type BridgeNativeVerb } from './bridge-native-verbs' @@ -106,6 +107,12 @@ export type BridgeRpcClient = RpcClient & { * runtime, so it has no `RpcOperation` and no entry in the desktop's method catalog. */ callNativeVerb: (verb: BridgeNativeVerb, params: unknown) => Promise + /** + * Tells the shell this document has a frame on screen, which is the only thing that does: the + * shell sees a document commit and a page say `ready`, and neither of those is a painted tree. + * Declared in `ready.reports`, so a shell waiting for it is one this page will answer. + */ + notifyPagePainted: () => void /** Writes one allowlisted key into the app's store. False when the shell granted no `storage`. */ notifyStorageWrite: (key: string, value: string | null) => boolean /** @@ -212,7 +219,14 @@ export function createBridgeRpcClient(options: BridgeRpcClientOptions): BridgeRp const handshake = createBridgeInitHandshake(() => { // Declared on every ask, because the shell reads it off whichever `ready` it answers: this // page build knows how to take a second `init` for the session it already holds. - sendFrame({ v: BRIDGE_PROTOCOL_VERSION, type: 'ready', accepts: [BRIDGE_ROUTE_UPDATE_ACCEPT] }) + // `reports` runs the other way from `accepts`: it is what a shell may wait for this page to + // post, and the shell holds a frame over the view until the one below arrives. + sendFrame({ + v: BRIDGE_PROTOCOL_VERSION, + type: 'ready', + accepts: [BRIDGE_ROUTE_UPDATE_ACCEPT], + reports: [BRIDGE_PAGE_PAINTED] + }) }) /** @@ -388,7 +402,8 @@ export function createBridgeRpcClient(options: BridgeRpcClientOptions): BridgeRp send: posted, requireSession, isClosed: () => closed, - hasGrant: (name) => shellSession.current()?.grants.native.includes(name) === true + hasGrant: (name) => shellSession.current()?.grants.native.includes(name) === true, + shellAccepts: (name) => shellSession.current()?.accepts.includes(name) === true }) const unsubscribeFromMessages = options.onMessage(receive) @@ -434,6 +449,7 @@ export function createBridgeRpcClient(options: BridgeRpcClientOptions): BridgeRp notifyStorageWrite: notifications.notifyStorageWrite, notifyHaptics: notifications.notifyHaptics, notifyPageFault: notifications.notifyPageFault, + notifyPagePainted: notifications.notifyPagePainted, close, onReady: shellSession.onReady, onRouteUpdate: shellSession.onRouteUpdate, diff --git a/mobile/src/mobile-web-shell/bridge/page-first-paint-through-the-bridge.test.ts b/mobile/src/mobile-web-shell/bridge/page-first-paint-through-the-bridge.test.ts new file mode 100644 index 00000000000..86d5189db5e --- /dev/null +++ b/mobile/src/mobile-web-shell/bridge/page-first-paint-through-the-bridge.test.ts @@ -0,0 +1,107 @@ +/** + * The paint report end to end, and the three pairings it has to survive: a shell that never + * advertised it, a page that never declared it, and two halves that did both. + * + * Driven through the real port pair rather than a mocked host, because what is worth proving is + * that each half reads the other's word off a real frame rather than assuming it. + */ +import { describe, expect, it } from 'vitest' +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { createFakeRpcClient } from '../bridge-host-test-fakes' +import { createFakeBridgePortPair } from './bridge-port-pair-test-harness' +import { BRIDGE_PAGE_PAINTED } from './bridge-page-painted' +import { createRouteScreenPaintReporter } from './page-first-paint' + +/** One `init` frame with one name taken out of `accepts`, which is what an older shell sends. */ +function stripAccept(json: string, name: string): string { + const frame: unknown = JSON.parse(json) + if (typeof frame !== 'object' || frame === null || !('type' in frame)) { + return json + } + const record: Record = { ...frame } + if (record.type !== 'init' || !Array.isArray(record.accepts)) { + return json + } + record.accepts = record.accepts.filter((entry) => entry !== name) + return JSON.stringify(record) +} + +describe('the page telling the shell it has a frame', () => { + it('new shell, new page: declares on ready and posts after the first paint', async () => { + const pair = createFakeBridgePortPair({ rpc: createFakeRpcClient() }) + await pair.flush() + expect(pair.pageReports()).toEqual([[BRIDGE_PAGE_PAINTED]]) + expect(pair.pagePaintCount()).toBe(0) + + // The two frames the entry waits out, drained by hand so "after the paint" is a step. + const frames: (() => void)[] = [] + createRouteScreenPaintReporter( + { + requestFrame: (callback) => frames.push(callback), + cancelFrame: () => undefined + }, + () => { + pair.client.notifyPagePainted() + } + )() + while (frames.length > 0) { + frames.shift()?.() + } + await pair.flush() + expect(pair.pagePaintCount()).toBe(1) + }) + + it('old shell, new page: posts nothing, so no shell is left refusing a frame', async () => { + // Stands for every shell installed before this change: `notify` is a closed union there, so an + // unasked-for report is an error frame per mount rather than a dropped one. + const pair = createFakeBridgePortPair({ + rpc: createFakeRpcClient(), + rewriteToPage: (json) => stripAccept(json, BRIDGE_PAGE_PAINTED) + }) + await pair.flush() + expect(pair.client.getShellSession()?.accepts).not.toContain(BRIDGE_PAGE_PAINTED) + const framesToShell = pair.toShell.length + pair.client.notifyPagePainted() + await pair.flush() + expect(pair.toShell).toHaveLength(framesToShell) + expect(pair.pagePaintCount()).toBe(0) + expect(pair.hostDiagnostics).not.toContainEqual( + expect.objectContaining({ refusal: 'unrecognised-message' }) + ) + }) + + it('advertises the report in init, which is what the page reads before posting one', async () => { + const pair = createFakeBridgePortPair({ rpc: createFakeRpcClient() }) + await pair.flush() + expect(pair.client.getShellSession()?.accepts).toContain(BRIDGE_PAGE_PAINTED) + }) + + it('is reported by the page entry, from the effect that runs after the tree commits', () => { + // The one call site, pinned: every test above drives the client directly, so a deleted line in + // the entry would leave a shell covering a page that has painted and will never say so. + const entry = readFileSync( + join(import.meta.dirname, '..', '..', '..', 'web-entry', 'index.tsx'), + 'utf8' + ) + expect(entry).toContain('createRouteScreenPaintReporter(') + expect(entry).toContain('client.notifyPagePainted()') + // Handed to the route screen rather than called from the wrapper's own effect, which commits + // while the route's chunk is still arriving and the body is empty. + expect(entry).toContain('RouteScreenPaintProvider') + expect(entry).not.toMatch( + /stampPageMountState\(target, 'mounted'\)\s*\n\s*reportRouteScreenPaint/ + ) + }) + + it('costs the shell nothing to hear: no request, no subscription, no reply', async () => { + const rpc = createFakeRpcClient() + const pair = createFakeBridgePortPair({ rpc }) + await pair.flush() + const framesToPage = pair.toPage.length + pair.client.notifyPagePainted() + await pair.flush() + expect(rpc.requests).toHaveLength(0) + expect(pair.toPage).toHaveLength(framesToPage) + }) +}) diff --git a/mobile/src/mobile-web-shell/bridge/page-first-paint.test.tsx b/mobile/src/mobile-web-shell/bridge/page-first-paint.test.tsx new file mode 100644 index 00000000000..a397729a9e8 --- /dev/null +++ b/mobile/src/mobile-web-shell/bridge/page-first-paint.test.tsx @@ -0,0 +1,216 @@ +import { Suspense, lazy, useEffect, type ComponentType, type PropsWithChildren } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { describe, expect, it } from 'vitest' +import { + RouteScreenPaintProvider, + createRouteScreenPaintReporter, + reportAfterFirstPaint, + withRouteScreenPaintReport +} from './page-first-paint' + +/** Frames the caller drains by hand, so "one frame later" is a step rather than a wait. */ +function frames() { + const queued = new Map void>() + let nextHandle = 0 + return { + scheduler: { + requestFrame: (callback: () => void) => { + nextHandle += 1 + queued.set(nextHandle, callback) + return nextHandle + }, + cancelFrame: (handle: number) => { + queued.delete(handle) + } + }, + tick: () => { + const [handle, callback] = queued.entries().next().value ?? [] + if (handle !== undefined) { + queued.delete(handle) + callback?.() + } + }, + pending: () => queued.size + } +} + +describe('when the page says it has a frame', () => { + it('waits for a frame boundary past the commit, never the same one', () => { + // One frame is the frame that paints the commit, and a callback inside it can still run ahead + // of the paint. Reporting there would uncover the view over a tree nothing has drawn. + const clock = frames() + let reported = 0 + reportAfterFirstPaint(clock.scheduler, () => { + reported += 1 + }) + expect(reported).toBe(0) + clock.tick() + expect(reported).toBe(0) + clock.tick() + expect(reported).toBe(1) + }) + + it('reports once and schedules nothing after it', () => { + const clock = frames() + reportAfterFirstPaint(clock.scheduler, () => {}) + clock.tick() + clock.tick() + expect(clock.pending()).toBe(0) + }) +}) + +/** A route chunk the case releases by hand, so "still arriving" is a state and not a race. */ +function deferredRouteChunk() { + let arrive: (() => void) | null = null + const chunk = new Promise<{ default: ComponentType> }>((resolve) => { + arrive = () => { + resolve({ default: () => null }) + } + }) + return { + chunk, + arrive: () => { + arrive?.() + } + } +} + +describe('a route that leaves before its frame lands', () => { + it('takes its report back, so the shell never uncovers on a screen that went away', async () => { + const clock = frames() + let posted = 0 + const report = createRouteScreenPaintReporter(clock.scheduler, () => { + posted += 1 + }) + const ScreenA = lazy(async () => withRouteScreenPaintReport({ default: () => null })) + + let tree: ReactTestRenderer | null = null + await act(async () => { + tree = create( + + + + + + ) + }) + // Committed and owed two frames; one has passed. + clock.tick() + await act(async () => { + tree?.unmount() + }) + clock.tick() + clock.tick() + expect(posted).toBe(0) + expect(clock.pending()).toBe(0) + }) + + it('hands the report to the screen that arrived while the last one was still owed a frame', () => { + const clock = frames() + let posted = 0 + const report = createRouteScreenPaintReporter(clock.scheduler, () => { + posted += 1 + }) + report() + // One of the first screen's two frames has passed. + clock.tick() + // The replacement commits, and the screen it replaces stays mounted behind it. + report() + clock.tick() + // The frame that just ran was the replacement's first, not the one the screen behind it was + // still owed: that frame would report a document the view is no longer showing. + expect(posted).toBe(0) + clock.tick() + expect(posted).toBe(1) + clock.tick() + // And nothing is left over to report a second time. + expect(posted).toBe(1) + expect(clock.pending()).toBe(0) + }) + + it('leaves the next screen free to report, because a frame taken back was never spent', () => { + const clock = frames() + let posted = 0 + const report = createRouteScreenPaintReporter(clock.scheduler, () => { + posted += 1 + }) + report()() + const second = report() + clock.tick() + clock.tick() + expect(posted).toBe(1) + // And that one is spent: a screen leaving after the shell uncovered has nothing to undo. + second() + report() + clock.tick() + clock.tick() + expect(posted).toBe(1) + }) +}) + +describe('which commit the page reports its frame from', () => { + it('says nothing while the route chunk is still arriving', async () => { + const route = deferredRouteChunk() + const Screen = lazy(() => route.chunk.then(withRouteScreenPaintReport)) + let reports = 0 + let commitsAboveTheRouter = 0 + // Shaped like the page: the entry's wrapper sits above expo-router, which puts every screen + // behind a suspense boundary of its own. + function WrapperAboveTheRouter({ children }: PropsWithChildren) { + useEffect(() => { + commitsAboveTheRouter += 1 + }, []) + return children + } + await act(async () => { + create( + { + reports += 1 + return () => undefined + }} + > + + + + + + + ) + }) + // The gap this seam exists for: the wrapper has committed, against a fallback that drew + // nothing, and a report hung there would uncover the shell's view over an empty body. + expect(commitsAboveTheRouter).toBe(1) + expect(reports).toBe(0) + + await act(async () => { + route.arrive() + }) + expect(reports).toBe(1) + }) + + it('reports the screen that arrived and not the one that replaced it', async () => { + const route = deferredRouteChunk() + const Screen = lazy(() => route.chunk.then(withRouteScreenPaintReport)) + let reports = 0 + await act(async () => { + create( + { + reports += 1 + return () => undefined + }} + > + + + + + ) + }) + await act(async () => { + route.arrive() + }) + // Once per screen that commits: the shell latches the first, and a re-render is not a new one. + expect(reports).toBe(1) + }) +}) diff --git a/mobile/src/mobile-web-shell/bridge/page-first-paint.tsx b/mobile/src/mobile-web-shell/bridge/page-first-paint.tsx new file mode 100644 index 00000000000..9ac1e11a0b0 --- /dev/null +++ b/mobile/src/mobile-web-shell/bridge/page-first-paint.tsx @@ -0,0 +1,111 @@ +import { + createContext, + useContext, + useEffect, + type ComponentType, + type PropsWithChildren, + type ReactElement +} from 'react' + +/** How the page schedules one frame. `requestAnimationFrame` on a document, a fake in a test. */ +export type PageFrameScheduler = { + readonly requestFrame: (callback: () => void) => number + readonly cancelFrame: (handle: number) => void +} + +/** + * Calls `report` once the browser has painted the commit this was scheduled from, and answers with + * the cancel for the frame still owed. Two frames, not one: an effect runs with the DOM mutated and + * the frame not yet painted, so the first callback scheduled from it can still run ahead of that + * paint. Being early uncovers the view over a tree nothing has drawn. + */ +export function reportAfterFirstPaint( + scheduler: PageFrameScheduler, + report: () => void +): () => boolean { + let settled = false + let handle = scheduler.requestFrame(() => { + handle = scheduler.requestFrame(() => { + settled = true + report() + }) + }) + // Answers whether it took a report back, which is the only thing that frees a caller's latch: a + // report already delivered is not one the next screen gets to spend again. + return () => { + if (settled) { + return false + } + settled = true + scheduler.cancelFrame(handle) + return true + } +} + +/** + * How a route screen says it committed. Defaulted to nothing: these screens also render natively + * and in unit trees, where no shell is holding a frame over them. + */ +const RouteScreenPaintContext = createContext(() => () => undefined) + +/** Reports one screen's commit and answers with the take-back for the frame it is still owed. */ +export type RouteScreenPaintReporter = () => () => void + +/** + * The page's one reporter. Once per document, because the shell latches the first frame — but only + * a report actually posted spends that one, so a screen unmounted or replaced before its frame + * landed leaves the cover up for whichever screen the document settles on. + */ +export function createRouteScreenPaintReporter( + scheduler: PageFrameScheduler, + post: () => void +): RouteScreenPaintReporter { + let posted = false + let owed: (() => boolean) | null = null + return () => { + if (posted) { + return () => undefined + } + // The newest commit is the one the view is about to show, so it takes over the frame an + // earlier screen is still owed. + owed?.() + const cancel = reportAfterFirstPaint(scheduler, () => { + owed = null + posted = true + post() + }) + owed = cancel + return () => { + cancel() + } + } +} + +export function RouteScreenPaintProvider({ + report, + children +}: PropsWithChildren<{ report: RouteScreenPaintReporter }>): ReactElement { + return ( + {children} + ) +} + +/** + * The screen behind a deferred route, reporting the commit that drew it. Applied where the route + * manifest resolves the chunk, because the wrapper above the router commits with the suspense + * fallback while that chunk is still arriving, and a report scheduled there uncovers the shell's + * view over an empty body. + */ +export function withRouteScreenPaintReport(module: { + readonly default: ComponentType> +}): { default: ComponentType> } { + const Screen = module.default + function RouteScreenPaintReport(props: Record): ReactElement { + const report = useContext(RouteScreenPaintContext) + // Returned as the cleanup: a route swapped out before its frame lands would otherwise uncover + // the view on the way out, over the fallback of whatever replaced it. + useEffect(() => report(), [report]) + return + } + return { default: RouteScreenPaintReport } +} diff --git a/mobile/src/mobile-web-shell/mobile-web-shell-cached-generation.ts b/mobile/src/mobile-web-shell/mobile-web-shell-cached-generation.ts new file mode 100644 index 00000000000..97cac49c4a8 --- /dev/null +++ b/mobile/src/mobile-web-shell/mobile-web-shell-cached-generation.ts @@ -0,0 +1,68 @@ +import type { + CachedGeneration, + MobileWebShellBlockedVerdict, + MobileWebShellSession, + MobileWebShellSessionEffect, + MobileWebShellStep +} from './mobile-web-shell-session-contract' +import { NATIVE_ROUTE } from './mobile-web-shell-gates' +import { matchesRoutePattern, routeViewOf } from './page-route-policy' +import { step } from './mobile-web-shell-session-step' + +/** + * Putting a generation that is already on disk on screen, and deciding whether this route is one + * that bundle carries. The only judge available when a newer manifest is absent or refused, so it + * is the reducer's cache path and its refused-update path both. + */ + +export function rendersRoute(pageRoutes: readonly string[], pathname: string): boolean { + return pageRoutes.some((pattern) => matchesRoutePattern(pathname, pattern)) +} + +export function openCached( + session: MobileWebShellSession, + generation: CachedGeneration, + patch: Partial = {}, + andThen: readonly MobileWebShellSessionEffect[] = [] +): MobileWebShellStep { + return step(session, { ...patch, state: { kind: 'activating' } }, [ + { + kind: 'open-generation', + directory: generation.directory, + buildId: generation.buildId, + totalBytes: generation.totalBytes + }, + ...andThen + ]) +} + +/** + * Opens a generation already on disk under its own route list, or leaves the route native when that + * list does not carry it. The only judge available when the newer manifest is absent or refused: + * opening under a bundle this shell is not running would grant the page what other bytes declared. + * + * The route question comes first and `wall` is asked only on the served branch, the order + * `onManifestRead` takes: a route this bundle never claimed is not a screen to refuse, and a + * generation cached before routes were listed claims none at all. `patch` belongs to either answer; + * `served` is what only an opened page gets, so the native one carries no notice about an update + * for a screen it is not showing. + */ +export function openByOwnRoutes( + session: MobileWebShellSession, + generation: CachedGeneration, + options: { + patch?: Partial + served?: Partial + wall?: MobileWebShellBlockedVerdict | null + } = {} +): MobileWebShellStep { + const { patch = {}, served = {}, wall = null } = options + const view = routeViewOf(generation.routes, session.routePathname) + if (!rendersRoute(view.pageRoutes, session.routePathname)) { + return step(session, { ...patch, ...view, state: NATIVE_ROUTE }) + } + if (wall !== null) { + return step(session, { ...patch, ...view, state: { kind: 'wall', verdict: wall } }) + } + return openCached(session, generation, { ...patch, ...served, ...view }) +} diff --git a/mobile/src/mobile-web-shell/mobile-web-shell-screen-test-harness.tsx b/mobile/src/mobile-web-shell/mobile-web-shell-screen-test-harness.tsx new file mode 100644 index 00000000000..bc29938c1cb --- /dev/null +++ b/mobile/src/mobile-web-shell/mobile-web-shell-screen-test-harness.tsx @@ -0,0 +1,234 @@ +import { createElement, type ComponentType, type ReactElement } from 'react' +import { act, create, type ReactTestInstance, type ReactTestRenderer } from 'react-test-renderer' +import { vi, type Mock } from 'vitest' +import type { FakeRpcClient } from './bridge-host-test-fakes' +import type { + MobileWebShellSessionState, + MobileWebShellUpdateNotice +} from './mobile-web-shell-session-contract' +import type { ShellPageFrame } from './shell-page-frame' + +/** + * Everything the screen's cases mock away, as one mutable record, plus the helpers that mount it. + * Separate from the cases because the `vi.mock` factories that read it must stay in the test file + * while nothing else here has to, and the file was over `max-lines` with both. + */ +export type ScreenDependencies = { + retry: Mock + reportShellFailure: Mock + reportDocumentStarted: Mock + reportDocumentLoaded: Mock + reportPageReady: Mock + reportPagePainted: Mock + /** The profile read rejected, which is the one state that has no host to build against. */ + snapshotUnreadable: boolean + storageRefreshes: number + openUrl: Mock + push: Mock + back: Mock + /** What the native stack answers: false is a page opened as the first screen on it. */ + canGoBack: boolean + pathname: string + pageRoutes: readonly string[] + routeGrants: readonly string[] + lifecycle: string[] + /** Every render of the shell view, which is one per render of the screen above it. */ + viewRenders: number + /** Every frame the shell posted to the page, raw. */ + posted: string[] + /** Whether the view refuses what it is handed, which is a page the post never reached. */ + postFails: boolean + state: MobileWebShellSessionState + /** Non-null when the generation on screen is a fallback from an update the shell refused. */ + updateNotice: MobileWebShellUpdateNotice | null + /** What the session reducer says about the page's handshake; true only for the fence's case. */ + pageReady: boolean + /** How far the document on screen has got, which is what decides whether the cover is up. */ + pageFrame: ShellPageFrame + /** Null for every case but the bridge's: with no client the hook builds no host at all. */ + client: FakeRpcClient | null + /** The IME events the app's own keyboard seam subscribes to, by name. */ + keyboardListeners: Map void> +} + +export const SCREEN_SNAPSHOT = { + host: { id: 'host-1', name: 'Host One', endpoint: 'ws://host-1', lastConnected: 3 } +} + +export const DEFAULT_ROUTE_GRANTS: readonly string[] = [ + 'navigate', + 'storage', + 'externalLink', + 'native.clipboard.write' +] + +export const SCREEN_BUILD_ID = 'a1b2c3d4e5f6'.repeat(5) + 'abcd' +export const SCREEN_DIRECTORY = + '/var/mobile/Containers/Data/Caches/mobile-web/deadbeef/generations/a1b2' + +/** Called from the test file's `vi.hoisted`, so `__DEV__` is on before the screen is imported and + * the developer facts are reachable at all — they are the one thing that must never grow a secret. */ +export function createScreenDependencies(): ScreenDependencies { + Object.assign(globalThis, { __DEV__: true }) + return { + retry: vi.fn(), + reportShellFailure: vi.fn(), + reportDocumentStarted: vi.fn(), + reportDocumentLoaded: vi.fn(), + reportPageReady: vi.fn(), + reportPagePainted: vi.fn(), + snapshotUnreadable: false, + storageRefreshes: 0, + openUrl: vi.fn(), + push: vi.fn(), + back: vi.fn(), + canGoBack: true, + pathname: '/h/host-1', + pageRoutes: ['/h/[hostId]'], + routeGrants: DEFAULT_ROUTE_GRANTS, + lifecycle: [], + viewRenders: 0, + posted: [], + postFails: false, + state: { kind: 'checking' }, + updateNotice: null, + pageReady: false, + pageFrame: 'pending', + client: null, + keyboardListeners: new Map() + } +} + +/** + * File-level, not per describe: every block shares one mutable record, so a reset scoped to one of + * them leaves whatever the others set. `routeGrants` is reset for that reason — a case that grants + * the screencast lane would otherwise hand it to every case that follows. + */ +export function resetScreenDependencies(dependencies: ScreenDependencies): void { + dependencies.retry.mockReset() + dependencies.reportShellFailure.mockReset() + dependencies.reportDocumentStarted.mockReset() + dependencies.reportDocumentLoaded.mockReset() + dependencies.reportPageReady.mockReset() + dependencies.reportPagePainted.mockReset() + dependencies.snapshotUnreadable = false + dependencies.storageRefreshes = 0 + dependencies.lifecycle.length = 0 + dependencies.viewRenders = 0 + dependencies.posted.length = 0 + dependencies.postFails = false + dependencies.client = null + dependencies.pageReady = false + dependencies.pageFrame = 'pending' + dependencies.routeGrants = DEFAULT_ROUTE_GRANTS + dependencies.back.mockReset() + dependencies.openUrl.mockReset() + dependencies.openUrl.mockImplementation(() => Promise.resolve(true)) + dependencies.canGoBack = true + dependencies.pathname = '/h/host-1' + dependencies.updateNotice = null +} + +/** The caller's native screen, as a component so `findAllByType` can name it without a host string. */ +export function NativeFallback(): null { + return null +} + +export function readyState(sessionId: string): MobileWebShellSessionState { + return { + kind: 'ready', + generationDirectory: SCREEN_DIRECTORY, + sessionId, + buildId: SCREEN_BUILD_ID, + totalBytes: 4096, + elapsedMs: 811 + } +} + +/** Unmounted between cases: the shell's stack latch is one per stack, so a screen left mounted is + * a screen still holding whatever pop it took. */ +const mounted: ReactTestRenderer[] = [] + +/** Registers a tree the caller mounted itself, so the teardown below reaches it too. */ +export function trackRenderedScreen(tree: ReactTestRenderer): void { + mounted.push(tree) +} + +export function unmountRenderedScreens(): void { + act(() => { + for (const tree of mounted.splice(0)) { + tree.unmount() + } + }) +} + +/** + * The screen is passed in rather than imported: this module is loaded from `vi.hoisted`, before the + * mocks are registered, so importing the module under test here would load it unmocked. + */ +export type ScreenComponent = ComponentType<{ + hostId: string + route: { pathname: string } + fallback: ReactElement +}> + +function element(Screen: ScreenComponent): ReactElement { + return createElement(Screen, { + hostId: 'host-1', + route: { pathname: '/h/host-1' }, + fallback: createElement(NativeFallback) + }) +} + +export async function renderScreen( + Screen: ScreenComponent, + dependencies: ScreenDependencies, + state: MobileWebShellSessionState +): Promise { + dependencies.state = state + const rendered: { tree: ReactTestRenderer | null } = { tree: null } + await act(async () => { + rendered.tree = create(element(Screen)) + }) + if (rendered.tree === null) { + throw new Error('screen did not render') + } + mounted.push(rendered.tree) + return rendered.tree +} + +export async function updateScreen( + Screen: ScreenComponent, + dependencies: ScreenDependencies, + tree: ReactTestRenderer, + state: MobileWebShellSessionState +): Promise { + dependencies.state = state + await act(async () => { + tree.update(element(Screen)) + }) +} + +/** Host elements are matched by name, not by `findAllByType`: React's `ElementType` does not admit + * an arbitrary React Native host name, so the typed form is a predicate. */ +export function byName(tree: ReactTestRenderer, name: string): ReactTestInstance[] { + return tree.root.findAll((node) => String(node.type) === name) +} + +/** The nearest laid-out ancestor of a node, which is the box its own box is measured against. */ +export function hostParentOf(node: ReactTestInstance): string | null { + let current: ReactTestInstance | null = node.parent + while (current !== null) { + if (typeof current.type === 'string') { + return current.props.testID ?? current.type + } + current = current.parent + } + return null +} + +export function textOf(tree: ReactTestRenderer): string { + return byName(tree, 'Text') + .map((node) => node.children.filter((child) => typeof child === 'string').join('')) + .join('\n') +} diff --git a/mobile/src/mobile-web-shell/mobile-web-shell-session-contract.ts b/mobile/src/mobile-web-shell/mobile-web-shell-session-contract.ts index 466a925877d..ac06f045acd 100644 --- a/mobile/src/mobile-web-shell/mobile-web-shell-session-contract.ts +++ b/mobile/src/mobile-web-shell/mobile-web-shell-session-contract.ts @@ -199,11 +199,19 @@ export type MobileWebShellSessionEvent = } | { readonly type: 'shell-failed'; readonly reason: MobileWebShellFailureReason } | { readonly type: 'retry-pressed' } + /** The native view began a document. Unstamped, like the view's failure and for the same + * reason: the view exists only under the generation on screen. */ + | { readonly type: 'document-started' } /** The native view finished a document. Unstamped, like the view's failure and for the same * reason: the view exists only under the generation on screen. */ | { readonly type: 'document-loaded' } - /** The page said `ready` over the bridge, which is the only proof its code ran at all. */ - | { readonly type: 'page-ready' } + /** + * The page said `ready` over the bridge, which is the only proof its code ran at all, carrying + * what that `ready` declared it reports. + */ + | { readonly type: 'page-ready'; readonly reports: readonly string[] } + /** The page has a frame on screen. Only a page that declared it ever sends one. */ + | { readonly type: 'page-painted' } | { readonly type: 'page-ready-deadline'; readonly flow: number } /** Latches live beside the state because both outlive the state they were set in: `retriedOnce` @@ -227,6 +235,11 @@ export type MobileWebShellSession = { /** Whether the document on screen has spoken over the bridge. Cleared by every new document, * because each one has to prove itself: the last one's word says nothing about this one. */ readonly pageReady: boolean + /** Whether this document said it would report its first paint. Cleared with `pageReady`, and + * false for every page built before the report existed. */ + readonly pageReportsPaint: boolean + /** Whether this document has reported a frame on screen. Cleared with `pageReady`. */ + readonly pagePainted: boolean /** The gates the current step was taken on; null until the first one arrives. */ readonly gates: MobileWebShellGates | null readonly cached: CachedGeneration | null diff --git a/mobile/src/mobile-web-shell/mobile-web-shell-session-step.ts b/mobile/src/mobile-web-shell/mobile-web-shell-session-step.ts new file mode 100644 index 00000000000..6fa55c5e415 --- /dev/null +++ b/mobile/src/mobile-web-shell/mobile-web-shell-session-step.ts @@ -0,0 +1,18 @@ +import type { + MobileWebShellSession, + MobileWebShellSessionEffect, + MobileWebShellStep +} from './mobile-web-shell-session-contract' + +/** + * One transition, built in one place: the session the patch produces and the effects it owes. + * Shared by the reducer and by the cached-generation opener beside it, so a step means the same + * thing wherever it is taken. + */ +export function step( + session: MobileWebShellSession, + patch: Partial, + effects: readonly MobileWebShellSessionEffect[] = [] +): MobileWebShellStep { + return { session: { ...session, ...patch }, effects } +} diff --git a/mobile/src/mobile-web-shell/mobile-web-shell-session-test-fixtures.ts b/mobile/src/mobile-web-shell/mobile-web-shell-session-test-fixtures.ts index 57d3654af6d..1f64aff79ff 100644 --- a/mobile/src/mobile-web-shell/mobile-web-shell-session-test-fixtures.ts +++ b/mobile/src/mobile-web-shell/mobile-web-shell-session-test-fixtures.ts @@ -106,8 +106,10 @@ export function stamp(flow: number, event: PendingEvent): MobileWebShellSessionE case 'gates-changed': case 'shell-failed': case 'retry-pressed': + case 'document-started': case 'document-loaded': case 'page-ready': + case 'page-painted': return event case 'cache-read': case 'manifest-read': diff --git a/mobile/src/mobile-web-shell/mobile-web-shell-session.test.ts b/mobile/src/mobile-web-shell/mobile-web-shell-session.test.ts index fcfa3243f96..5719f02ffd7 100644 --- a/mobile/src/mobile-web-shell/mobile-web-shell-session.test.ts +++ b/mobile/src/mobile-web-shell/mobile-web-shell-session.test.ts @@ -11,6 +11,8 @@ import { run, started } from './mobile-web-shell-session-test-fixtures' +import { BRIDGE_PAGE_PAINTED } from './bridge/bridge-page-painted' +import { shellPageFrame } from './shell-page-frame' describe('the gates decide whether a step is taken at all', () => { it('waits while a connection is still being made', () => { @@ -685,7 +687,11 @@ describe('the page has to speak for the document that loaded', () => { }) it('arms nothing when the page spoke first, because there is nothing left to wait for', () => { - const step = run(readySession().session, { type: 'page-ready' }, { type: 'document-loaded' }) + const step = run( + readySession().session, + { type: 'page-ready', reports: [] }, + { type: 'document-loaded' } + ) expect(step.effects).toEqual([]) expect(step.session.pageReady).toBe(true) }) @@ -709,7 +715,7 @@ describe('the page has to speak for the document that loaded', () => { const step = run( readySession().session, { type: 'document-loaded' }, - { type: 'page-ready' }, + { type: 'page-ready', reports: [] }, { type: 'page-ready-deadline' } ) expect(step.effects).toEqual([]) @@ -728,7 +734,7 @@ describe('the page has to speak for the document that loaded', () => { }) it('makes the second document prove itself, rather than riding the first one word', () => { - const spoken = run(readySession().session, { type: 'page-ready' }) + const spoken = run(readySession().session, { type: 'page-ready', reports: [] }) const remounted = run(spoken.session, { type: 'remounted', sessionId: 'session-two' }) expect(remounted.session.pageReady).toBe(false) expect(run(remounted.session, { type: 'document-loaded' }).effects).toEqual([ @@ -737,7 +743,11 @@ describe('the page has to speak for the document that loaded', () => { }) it('leaves a remounted document its own wait when the first one expires late', () => { - const first = run(readySession().session, { type: 'document-loaded' }, { type: 'page-ready' }) + const first = run( + readySession().session, + { type: 'document-loaded' }, + { type: 'page-ready', reports: [] } + ) const armed = first.session.flow const second = run( first.session, @@ -753,7 +763,7 @@ describe('the page has to speak for the document that loaded', () => { }) it('makes a freshly activated generation prove itself too', () => { - const spoken = run(readySession().session, { type: 'page-ready' }) + const spoken = run(readySession().session, { type: 'page-ready', reports: [] }) const reactivated = run(spoken.session, { type: 'activated', generationDirectory: CACHED.directory, @@ -765,3 +775,123 @@ describe('the page has to speak for the document that loaded', () => { expect(reactivated.session.pageReady).toBe(false) }) }) + +/** + * A commit is not a paint, and `ready` is posted before the page has built anything: the only + * thing that says the view is worth showing is the page saying so. + */ +describe('the page reporting a frame on screen', () => { + it('records what the ready declared and keeps the frame covered until the report', () => { + const spoken = run(readySession().session, { + type: 'page-ready', + reports: [BRIDGE_PAGE_PAINTED] + }) + expect(spoken.session.pageReportsPaint).toBe(true) + expect(spoken.session.pagePainted).toBe(false) + expect(shellPageFrame(spoken.session)).toBe('unpainted') + const painted = run(spoken.session, { type: 'page-painted' }) + expect(painted.session.pagePainted).toBe(true) + expect(shellPageFrame(painted.session)).toBe('painted') + }) + + it('new shell, old page: ignores an undeclared report and uncovers on ready', () => { + const spoken = run(readySession().session, { type: 'page-ready', reports: [] }) + const step = run(spoken.session, { type: 'page-painted' }) + expect(step.session.pagePainted).toBe(false) + // Uncovered anyway: `ready` is the newest word a page built before the report can say. + expect(shellPageFrame(step.session)).toBe('painted') + }) + + it('re-reads the declaration on every ask, because a reload asks again', () => { + const declared = run(readySession().session, { + type: 'page-ready', + reports: [BRIDGE_PAGE_PAINTED] + }) + const reloaded = run(declared.session, { type: 'page-ready', reports: [] }) + expect(reloaded.session.pageReportsPaint).toBe(false) + }) + + it('makes a remounted document report its own frame', () => { + const painted = run( + readySession().session, + { type: 'page-ready', reports: [BRIDGE_PAGE_PAINTED] }, + { type: 'page-painted' } + ) + const remounted = run(painted.session, { type: 'remounted', sessionId: 'session-two' }) + expect(remounted.session.pagePainted).toBe(false) + expect(remounted.session.pageReportsPaint).toBe(false) + expect(shellPageFrame(remounted.session)).toBe('unpainted') + }) + + it('makes a freshly activated generation report its own frame', () => { + const painted = run( + readySession().session, + { type: 'page-ready', reports: [BRIDGE_PAGE_PAINTED] }, + { type: 'page-painted' } + ) + const reactivated = run(painted.session, { + type: 'activated', + generationDirectory: CACHED.directory, + sessionId: 'session-three', + buildId: MANIFEST.buildId, + totalBytes: MANIFEST.totalBytes, + elapsedMs: 9 + }) + expect(reactivated.session.pagePainted).toBe(false) + expect(reactivated.session.pageReportsPaint).toBe(false) + }) + + it('makes the document that replaced a painted one inside this mount report its own frame', () => { + const painted = run( + readySession().session, + { type: 'page-ready', reports: [BRIDGE_PAGE_PAINTED] }, + { type: 'page-painted' } + ) + expect(shellPageFrame(painted.session)).toBe('painted') + // No new session and no new generation: the view reloaded under the one already on screen. + const restarted = run(painted.session, { type: 'document-started' }) + expect(restarted.session.pagePainted).toBe(false) + const reasked = run(restarted.session, { + type: 'page-ready', + reports: [BRIDGE_PAGE_PAINTED] + }) + expect(shellPageFrame(reasked.session)).toBe('unpainted') + expect(shellPageFrame(run(reasked.session, { type: 'page-painted' }).session)).toBe('painted') + }) + + it('retires the wait the document it replaced armed', () => { + const loaded = run(readySession().session, { type: 'document-loaded' }) + expect(loaded.effects).toEqual([{ kind: 'await-page-ready' }]) + const armed = loaded.session.flow + const spoken = run(loaded.session, { type: 'page-ready', reports: [BRIDGE_PAGE_PAINTED] }) + const restarted = run(spoken.session, { type: 'document-started' }) + // The replacement is still loading and has said nothing, which is exactly what the retired + // document's deadline reads as a document that never loaded. + const expired = run(restarted.session, { type: 'page-ready-deadline', flow: armed }) + expect(expired.session.state.kind).toBe('ready') + // And the replacement arms a wait of its own, so a document that really never speaks still + // takes the session down. + const reloaded = run(restarted.session, { type: 'document-loaded' }) + expect(reloaded.effects).toEqual([{ kind: 'await-page-ready' }]) + }) + + it('keeps the frame of a document that repeats its own handshake', () => { + const painted = run( + readySession().session, + { type: 'page-ready', reports: [BRIDGE_PAGE_PAINTED] }, + { type: 'page-painted' }, + { type: 'page-ready', reports: [BRIDGE_PAGE_PAINTED] } + ) + // The document never restarted, so its frame is still the one on screen. + expect(painted.session.pagePainted).toBe(true) + expect(shellPageFrame(painted.session)).toBe('painted') + }) + + it('records nothing from a page whose generation is no longer on screen', () => { + const failed = run(readySession().session, { + type: 'shell-failed', + reason: 'render-process-gone' + }) + expect(run(failed.session, { type: 'page-painted' }).session.pagePainted).toBe(false) + }) +}) diff --git a/mobile/src/mobile-web-shell/mobile-web-shell-session.ts b/mobile/src/mobile-web-shell/mobile-web-shell-session.ts index f09d66486b3..e1527c946c0 100644 --- a/mobile/src/mobile-web-shell/mobile-web-shell-session.ts +++ b/mobile/src/mobile-web-shell/mobile-web-shell-session.ts @@ -2,7 +2,6 @@ import type { MobileWebShellFailureReason } from '../../modules/orca-mobile-web- import { evaluateMobileWebBundleCompat } from '../transport/mobile-web-bundle-compat' import type { CachedGeneration, - MobileWebShellBlockedVerdict, MobileWebShellGates, MobileWebShellManifestFacts, MobileWebShellReadFailure, @@ -20,11 +19,10 @@ import { gateVerdict, NATIVE_ROUTE } from './mobile-web-shell-gates' -import { matchesRoutePattern, routeViewOf } from './page-route-policy' - -function rendersRoute(pageRoutes: readonly string[], pathname: string): boolean { - return pageRoutes.some((pattern) => matchesRoutePattern(pathname, pattern)) -} +import { routeViewOf } from './page-route-policy' +import { CLEAR_PAGE_DOCUMENT_STATE, pageDocumentStatePatch } from './page-document-state' +import { openByOwnRoutes, openCached, rendersRoute } from './mobile-web-shell-cached-generation' +import { step } from './mobile-web-shell-session-step' export function createMobileWebShellSession(routePathname: string): MobileWebShellSession { return { @@ -36,6 +34,8 @@ export function createMobileWebShellSession(routePathname: string): MobileWebShe retriedOnce: false, remountedOnce: false, pageReady: false, + pageReportsPaint: false, + pagePainted: false, gates: null, cached: null, updateNotice: null, @@ -43,14 +43,6 @@ export function createMobileWebShellSession(routePathname: string): MobileWebShe } } -function step( - session: MobileWebShellSession, - patch: Partial, - effects: readonly MobileWebShellSessionEffect[] = [] -): MobileWebShellStep { - return { session: { ...session, ...patch }, effects } -} - /** * The step the gate takes, and every entry into the flow goes through it. * @@ -77,54 +69,6 @@ function startFlow( /** Puts a generation that is already on disk on screen. The only producer of `open-generation`. * `andThen` is the disk work that opening one may owe, which runs after the view has its bytes. */ -function openCached( - session: MobileWebShellSession, - generation: CachedGeneration, - patch: Partial = {}, - andThen: readonly MobileWebShellSessionEffect[] = [] -): MobileWebShellStep { - return step(session, { ...patch, state: { kind: 'activating' } }, [ - { - kind: 'open-generation', - directory: generation.directory, - buildId: generation.buildId, - totalBytes: generation.totalBytes - }, - ...andThen - ]) -} - -/** - * Opens a generation already on disk under its own route list, or leaves the route native when that - * list does not carry it. The only judge available when the newer manifest is absent or refused: - * opening under a bundle this shell is not running would grant the page what other bytes declared. - * - * The route question comes first and `wall` is asked only on the served branch, the order - * `onManifestRead` takes: a route this bundle never claimed is not a screen to refuse, and a - * generation cached before routes were listed claims none at all. `patch` belongs to either answer; - * `served` is what only an opened page gets, so the native one carries no notice about an update - * for a screen it is not showing. - */ -function openByOwnRoutes( - session: MobileWebShellSession, - generation: CachedGeneration, - options: { - patch?: Partial - served?: Partial - wall?: MobileWebShellBlockedVerdict | null - } = {} -): MobileWebShellStep { - const { patch = {}, served = {}, wall = null } = options - const view = routeViewOf(generation.routes, session.routePathname) - if (!rendersRoute(view.pageRoutes, session.routePathname)) { - return step(session, { ...patch, ...view, state: NATIVE_ROUTE }) - } - if (wall !== null) { - return step(session, { ...patch, ...view, state: { kind: 'wall', verdict: wall } }) - } - return openCached(session, generation, { ...patch, ...served, ...view }) -} - function onCacheRead( session: MobileWebShellSession, generation: CachedGeneration | null @@ -347,7 +291,7 @@ export function reduceMobileWebShellSession( : step(session, {}) case 'activated': return step(session, { - pageReady: false, + ...CLEAR_PAGE_DOCUMENT_STATE, state: { kind: 'ready', generationDirectory: event.generationDirectory, @@ -364,7 +308,7 @@ export function reduceMobileWebShellSession( // healthy page that is still inside its own, and take a working workspace off screen. return session.state.kind === 'ready' ? step(session, { - pageReady: false, + ...CLEAR_PAGE_DOCUMENT_STATE, flow: session.flow + 1, state: { ...session.state, sessionId: event.sessionId } }) @@ -373,6 +317,14 @@ export function reduceMobileWebShellSession( return onDownloadFailed(session, event.failure) case 'shell-failed': return onShellFailed(session, event.reason) + case 'document-started': + // A replacement document inherits nothing: what the last one declared and painted says + // nothing about this one, and leaving its paint latched uncovers the view over a blank tree. + // The flow goes with it for the same reason `remounted` moves it — the retired document's + // readiness wait would otherwise expire onto a replacement that is still loading. + return session.state.kind === 'ready' + ? step(session, { ...CLEAR_PAGE_DOCUMENT_STATE, flow: session.flow + 1 }) + : step(session, {}) case 'document-loaded': // Nothing to wait on outside `ready`, and nothing to wait for once the page has spoken: the // two orders this can arrive in are a race, and the latch is what makes either one fine. @@ -380,7 +332,8 @@ export function reduceMobileWebShellSession( ? step(session, {}, [{ kind: 'await-page-ready' }]) : step(session, {}) case 'page-ready': - return session.state.kind === 'ready' ? step(session, { pageReady: true }) : step(session, {}) + case 'page-painted': + return step(session, pageDocumentStatePatch(session, event)) case 'page-ready-deadline': // A document that finished and never said a word is a document that did not load, whatever // the WebView reported: `document-load-failed` is what drops the generation and fetches once. diff --git a/mobile/src/mobile-web-shell/page-document-state.ts b/mobile/src/mobile-web-shell/page-document-state.ts new file mode 100644 index 00000000000..744eea90b27 --- /dev/null +++ b/mobile/src/mobile-web-shell/page-document-state.ts @@ -0,0 +1,39 @@ +import { BRIDGE_PAGE_PAINTED } from './bridge/bridge-page-painted' +import type { + MobileWebShellSession, + MobileWebShellSessionEvent +} from './mobile-web-shell-session-contract' + +/** Everything one document told the shell about itself, which the next one has to say again. */ +export const CLEAR_PAGE_DOCUMENT_STATE = { + pageReady: false, + pageReportsPaint: false, + pagePainted: false +} as const + +/** The two things a document reports about itself, as the reducer receives them. */ +export type PageDocumentEvent = Extract< + MobileWebShellSessionEvent, + { type: 'page-ready' } | { type: 'page-painted' } +> + +/** + * What one of those events leaves on the session. Nothing outside `ready` changes anything: the + * view exists only under the generation on screen. + */ +export function pageDocumentStatePatch( + session: Pick, + event: PageDocumentEvent +): Partial { + if (session.state.kind !== 'ready') { + return {} + } + if (event.type === 'page-ready') { + // Re-read on every ask rather than latched: a document that reloads inside this mount asks + // again, and it is the newest ask that says whether a paint report is coming. + return { pageReady: true, pageReportsPaint: event.reports.includes(BRIDGE_PAGE_PAINTED) } + } + // Kept off a page that never said it would report: acting on an unasked-for frame would make + // the wait depend on a name arriving instead of on a claim the page made. + return session.pageReportsPaint ? { pagePainted: true } : {} +} diff --git a/mobile/src/mobile-web-shell/shell-page-frame.test.ts b/mobile/src/mobile-web-shell/shell-page-frame.test.ts new file mode 100644 index 00000000000..677f1d93c7d --- /dev/null +++ b/mobile/src/mobile-web-shell/shell-page-frame.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, it } from 'vitest' +import { BRIDGE_PAGE_PAINTED } from './bridge/bridge-page-painted' +import type { MobileWebShellSessionState } from './mobile-web-shell-session-contract' +import { shellPageFrame } from './shell-page-frame' + +const READY: MobileWebShellSessionState = { + kind: 'ready', + generationDirectory: '/cache/gen', + sessionId: 'session-a', + buildId: 'build-a', + totalBytes: 1, + elapsedMs: 1 +} + +function frame(patch: { + state?: MobileWebShellSessionState + pageReady?: boolean + pageReportsPaint?: boolean + pagePainted?: boolean +}) { + return shellPageFrame({ + state: patch.state ?? READY, + pageReady: patch.pageReady ?? false, + pageReportsPaint: patch.pageReportsPaint ?? false, + pagePainted: patch.pagePainted ?? false + }) +} + +describe('how long the shell keeps its own frame up', () => { + it('has nothing to cover before a generation is on screen', () => { + for (const state of [ + { kind: 'checking' }, + { kind: 'activating' }, + { kind: 'offline' }, + { kind: 'native-route' } + ] as const satisfies readonly MobileWebShellSessionState[]) { + expect(frame({ state }), state.kind).toBe('pending') + } + }) + + it('covers a mounted view that has said nothing, which is the whole of the page boot', () => { + expect(frame({})).toBe('unpainted') + }) + + it('keeps covering a page that has handshaken and not yet painted', () => { + // The gap this exists for: `ready` is posted before the tree is built, so the view is mounted, + // empty and showing the surface behind it for every frame between the two. + expect(frame({ pageReady: true, pageReportsPaint: true })).toBe('unpainted') + }) + + it('new shell, new page: uncovers on the page reporting a frame', () => { + expect(frame({ pageReady: true, pageReportsPaint: true, pagePainted: true })).toBe('painted') + }) + + it('new shell, old page: uncovers on ready, the newest word that page will ever say', () => { + // A generation served by a desktop built before the report exists. Waiting on a frame it + // cannot send would hide a working workspace for the life of the document. + expect(frame({ pageReady: true, pageReportsPaint: false })).toBe('painted') + }) + + it('reads the declaration off the list the page sent, not off the shell', () => { + // The name is what the page puts in `ready.reports`, so the frame follows that list and not a + // flag the shell set: a list without it is a page whose newest word is `ready`. + const declared = [BRIDGE_PAGE_PAINTED].includes(BRIDGE_PAGE_PAINTED) + expect(frame({ pageReady: true, pageReportsPaint: declared })).toBe('unpainted') + expect( + frame({ pageReady: true, pageReportsPaint: ['other'].includes(BRIDGE_PAGE_PAINTED) }) + ).toBe('painted') + }) +}) diff --git a/mobile/src/mobile-web-shell/shell-page-frame.ts b/mobile/src/mobile-web-shell/shell-page-frame.ts new file mode 100644 index 00000000000..02d870a0f48 --- /dev/null +++ b/mobile/src/mobile-web-shell/shell-page-frame.ts @@ -0,0 +1,25 @@ +import type { MobileWebShellSession } from './mobile-web-shell-session-contract' + +/** + * How far the shell's own frame has to stay up. `unpainted` is the state that was missing: the + * view is mounted and the WebView draws nothing until its document paints, so what shows is the + * surface behind it with nothing on it, for the whole of a cached generation's boot. + */ +export type ShellPageFrame = 'pending' | 'unpainted' | 'painted' + +/** + * Bounded by the page's declaration, never by a timer. A page that declared none is one served by + * a desktop older than the report, and `ready` is the newest thing it will ever say: waiting on a + * word it cannot speak would hide a working workspace for the life of the document. + */ +export function shellPageFrame( + session: Pick +): ShellPageFrame { + if (session.state.kind !== 'ready') { + return 'pending' + } + if (session.pagePainted) { + return 'painted' + } + return session.pageReportsPaint || !session.pageReady ? 'unpainted' : 'painted' +} diff --git a/mobile/src/mobile-web-shell/shell-view-surface.test.ts b/mobile/src/mobile-web-shell/shell-view-surface.test.ts new file mode 100644 index 00000000000..7d8817147ca --- /dev/null +++ b/mobile/src/mobile-web-shell/shell-view-surface.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from 'vitest' +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { colors } from '../theme/mobile-theme' + +/** + * What the WebView paints before its document does, read off both shells. + * + * Neither can be proven from a JVM or a Swift unit test: it is a property of a live view. What is + * checkable is that neither shell gives the view a surface of its own, which is what makes the + * frame behind it — the screen's own, in the app's own colours — the thing on screen. + * + * The two defaults differ and both are wrong. Android's WebView paints nothing under a transparent + * background, which is fine; a WKWebView is opaque by default and paints white, so a dark app + * opening a page flashed white for the whole of the page's boot. + */ +const SHELL = join(import.meta.dirname, '..', '..', 'modules', 'orca-mobile-web-shell') + +function source(relative: string): string { + return readFileSync(join(SHELL, relative), 'utf8') +} + +describe('what a mounted view paints before the page does', () => { + it('gives the Android view no surface of its own', () => { + const kotlin = source( + 'android/src/main/java/expo/modules/orcamobilewebshell/MobileWebShellView.kt' + ) + expect(kotlin).toContain('view.setBackgroundColor(Color.TRANSPARENT)') + }) + + it('gives the iOS view none either, which is not its default', () => { + const swift = source('ios/MobileWebShellView.swift') + expect(swift).toContain('webView.isOpaque = false') + expect(swift).toContain('webView.backgroundColor = .clear') + expect(swift).toContain('webView.scrollView.backgroundColor = .clear') + }) + + it('leaves the app surface as the one colour behind a page, and it is not black', () => { + // What shows through both: the screen's own root, which is where the token is read. + const screen = readFileSync(join(import.meta.dirname, 'MobileWebShellScreen.tsx'), 'utf8') + expect(screen).toContain('backgroundColor: colors.bgBase') + expect(colors.bgBase).not.toBe('#000000') + }) +}) 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 e016e5ec555..23723f279dd 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 @@ -15,6 +15,7 @@ import { type BridgeHapticsKind } from './bridge/bridge-haptics-notify' import { BRIDGE_SCREENCAST_BINARY_GRANT } from './bridge/bridge-screencast-grant' +import { BRIDGE_PAGE_PAINTED } from './bridge/bridge-page-painted' import { BRIDGE_FAULT_GRANT, BRIDGE_NAVIGATE_BACK_NOTIFY, @@ -60,6 +61,8 @@ type Probe = { storageWrites: { key: string; value: string | null }[] /** The running total after each dropped screencast frame, as the screen receives it. */ droppedBinaryFrames: number[] + /** One per paint the page reported, which is what lifts the screen's cover. */ + paints: number } /** What the page cannot read for itself, as the screen hands it over. */ @@ -174,6 +177,9 @@ function Harness(props: { onPageReady: () => { setHandshook(sessionId) props.readies.push(sessionId ?? props.session.kind) + }, + onPagePainted: () => { + props.probe.paints += 1 } }) props.probe.view = view @@ -221,6 +227,7 @@ async function mount(session: MobileWebShellSessionState): Promise { haptics: [], backPops: 0, droppedBinaryFrames: [], + paints: 0, storageWrites: [] } const faults: BridgeErrorCapture[] = [] @@ -311,6 +318,14 @@ describe('the bridge channel', () => { expect(mounted.probe.navigations).toEqual(['/h/host-1/session/wt-1']) }) + it("hands the page's first paint to the caller that owns the cover over the view", async () => { + const mounted = await mount(readyState('session-one')) + await mounted.deliver(clientFrame({ type: 'ready', reports: [BRIDGE_PAGE_PAINTED] })) + expect(mounted.probe.paints).toBe(0) + await mounted.deliver(clientFrame({ type: 'notify', name: BRIDGE_PAGE_PAINTED })) + expect(mounted.probe.paints).toBe(1) + }) + it('hands a URL the page asked for to the caller that can leave the app', async () => { const mounted = await mount(readyState('session-one')) await mounted.deliver(clientFrame({ type: 'ready' })) @@ -565,6 +580,7 @@ describe('the callbacks a render passes', () => { haptics: [], backPops: 0, droppedBinaryFrames: [], + paints: 0, storageWrites: [] } // One session throughout, so the host is never rebuilt: only the ref refresh can carry the @@ -627,6 +643,7 @@ describe('client changes', () => { haptics: [], backPops: 0, droppedBinaryFrames: [], + paints: 0, storageWrites: [] } const render = (deliver: readonly string[]): ReactElement => @@ -661,6 +678,7 @@ function newProbe(): Probe { haptics: [], backPops: 0, droppedBinaryFrames: [], + paints: 0, storageWrites: [] } } 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 39b43be0ccc..aeee222b6e9 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 @@ -97,8 +97,11 @@ export type MobileWebShellBridgeArgs = { onStorageWrite: (key: string, value: string | null) => void /** 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: () => void + /** The page asked for a session, and what it declared it reports. Reported so the screen can + * stop waiting for it, and so it knows whether a paint report is coming. */ + onPageReady: (reports: readonly string[]) => void + /** The page has a frame on screen, from a page that said it would report one. */ + onPagePainted: () => void /** The page applied a one-shot route param and asks for it to be erased (ruling 34). */ onRouteParamClear: (param: BridgeClearableRouteParam, value: string) => void /** This shell named a screen the protocol does not allow, so no session is served. */ @@ -187,7 +190,8 @@ export function useMobileWebShellBridge(args: MobileWebShellBridgeArgs): MobileW onRouteParamClear: (param, value) => argsRef.current.onRouteParamClear(param, value), onRouteRefused: (issue) => argsRef.current.onRouteRefused(issue), onBinaryFramesDropped: (total) => argsRef.current.onBinaryFramesDropped(total), - onPageReady: () => argsRef.current.onPageReady() + onPageReady: (reports) => argsRef.current.onPageReady(reports), + onPagePainted: () => argsRef.current.onPagePainted() }) hostRef.current = { sessionId, host } // The count belongs to this host, so a rebuild starts it over. Without this the screen keeps diff --git a/mobile/src/mobile-web-shell/use-mobile-web-shell-session.test.ts b/mobile/src/mobile-web-shell/use-mobile-web-shell-session.test.ts index f11d089a30d..cf488485427 100644 --- a/mobile/src/mobile-web-shell/use-mobile-web-shell-session.test.ts +++ b/mobile/src/mobile-web-shell/use-mobile-web-shell-session.test.ts @@ -207,7 +207,7 @@ type Mounted = { /** Whether the page had spoken, as every render of the hook reported it. */ handshakes: () => readonly boolean[] documentLoaded: () => void - pageReady: () => void + pageReady: (reports?: readonly string[]) => void timers: ReturnType } @@ -216,7 +216,7 @@ async function mount(store: GenerationStore): Promise { const handle: { retry: () => void documentLoaded: () => void - pageReady: () => void + pageReady: (reports: readonly string[]) => void states: MobileWebShellSessionState[] handshakes: boolean[] } = { @@ -259,7 +259,7 @@ async function mount(store: GenerationStore): Promise { states: () => handle.states, handshakes: () => handle.handshakes, documentLoaded: () => handle.documentLoaded(), - pageReady: () => handle.pageReady(), + pageReady: (reports: readonly string[] = []) => handle.pageReady(reports), timers } } diff --git a/mobile/src/mobile-web-shell/use-mobile-web-shell-session.ts b/mobile/src/mobile-web-shell/use-mobile-web-shell-session.ts index b56ff0c8165..eb49d8d1a65 100644 --- a/mobile/src/mobile-web-shell/use-mobile-web-shell-session.ts +++ b/mobile/src/mobile-web-shell/use-mobile-web-shell-session.ts @@ -11,6 +11,7 @@ import { type MobileWebShellRuntime } from './mobile-web-shell-runtime' import { readMobileWebShellReachability } from './mobile-web-shell-reachability' +import { shellPageFrame, type ShellPageFrame } from './shell-page-frame' import { createMobileWebShellSession, reduceMobileWebShellSession @@ -34,10 +35,15 @@ export type MobileWebShellSessionView = { readonly retry: () => void /** B3's failure reasons, forwarded verbatim; the reducer owns what each one means. */ readonly reportShellFailure: (reason: MobileWebShellFailureReason) => void + /** The native view began a document; drops what the document it replaces said about itself. */ + readonly reportDocumentStarted: () => void /** The native view finished a document; starts the wait for the page's first word. */ readonly reportDocumentLoaded: () => void - /** The page spoke over the bridge; ends that wait, whichever of the two arrived first. */ - readonly reportPageReady: () => void + /** The page spoke over the bridge; ends that wait, whichever of the two arrived first. Carries + * what that `ready` declared it reports, which is what says whether a paint is coming. */ + readonly reportPageReady: (reports: readonly string[]) => void + /** The page has a frame on screen. Ignored for a page that never said it would report one. */ + readonly reportPagePainted: () => void /** * Whether the page has handshaken on this session, which the bridge host is rebuilt against. * @@ -45,6 +51,9 @@ export type MobileWebShellSessionView = { * `page-ready` changes nothing else, so no other value would re-render to carry it out. */ readonly pageReady: boolean + /** How far this document has got towards being something to show. Projected for the same + * reason as `pageReady`: `page-painted` moves nothing else. */ + readonly pageFrame: ShellPageFrame } /** @@ -73,6 +82,7 @@ export function useMobileWebShellSession(args: { const sessionRef = useRef(createMobileWebShellSession(routePathname)) const [state, setState] = useState(sessionRef.current.state) const [pageReady, setPageReady] = useState(sessionRef.current.pageReady) + const [pageFrame, setPageFrame] = useState(() => shellPageFrame(sessionRef.current)) const hostKey = useMemo(() => deriveHostCacheKey(hostId), [hostId]) const startedAtRef = useRef(runtime.now()) // Bumped by anything that invalidates work in flight; every dispatch out of an effect checks it. @@ -94,6 +104,7 @@ export function useMobileWebShellSession(args: { sessionRef.current = stepped.session setState(stepped.session.state) setPageReady(stepped.session.pageReady) + setPageFrame(shellPageFrame(stepped.session)) for (const effect of stepped.effects) { // Every effect of a step belongs to the flow that step produced, and its result carries that // number back, so a flow the session has since restarted reports into nothing. @@ -192,6 +203,7 @@ export function useMobileWebShellSession(args: { startedAtRef.current = runtime.now() setState(sessionRef.current.state) setPageReady(sessionRef.current.pageReady) + setPageFrame(shellPageFrame(sessionRef.current)) return invalidate }, [hostId, invalidate, routePathname, runtime]) @@ -236,24 +248,38 @@ export function useMobileWebShellSession(args: { [dispatch] ) + const reportDocumentStarted = useCallback(() => { + dispatch(epochRef.current, { type: 'document-started' }) + }, [dispatch]) + const reportDocumentLoaded = useCallback(() => { dispatch(epochRef.current, { type: 'document-loaded' }) }, [dispatch]) - const reportPageReady = useCallback(() => { - dispatch(epochRef.current, { type: 'page-ready' }) + const reportPageReady = useCallback( + (reports: readonly string[]) => { + dispatch(epochRef.current, { type: 'page-ready', reports }) + }, + [dispatch] + ) + + const reportPagePainted = useCallback(() => { + dispatch(epochRef.current, { type: 'page-painted' }) }, [dispatch]) return { state, pageReady, + pageFrame, pageRoutes: sessionRef.current.pageRoutes, pageRouteGrants: sessionRef.current.pageRouteGrants, routeGrants: sessionRef.current.routeGrants, updateNotice: sessionRef.current.updateNotice, retry, reportShellFailure, + reportDocumentStarted, reportDocumentLoaded, - reportPageReady + reportPageReady, + reportPagePainted } } diff --git a/mobile/web-entry/index.tsx b/mobile/web-entry/index.tsx index c561b85aabb..2c346e0dbf2 100644 --- a/mobile/web-entry/index.tsx +++ b/mobile/web-entry/index.tsx @@ -12,6 +12,10 @@ import { type PageMountTarget } from '../src/mobile-web-shell/bridge/page-bootstrap' import { publishPageStorage } from '../src/mobile-web-shell/bridge/page-async-storage' +import { + RouteScreenPaintProvider, + createRouteScreenPaintReporter +} from '../src/mobile-web-shell/bridge/page-first-paint' import { PageFaultBoundary } from '../src/mobile-web-shell/bridge/page-fault-boundary' import { publishPageHostProfile } from '../src/mobile-web-shell/bridge/page-host-profile' import { publishExternalLinkOpener } from '../src/platform/external-link.web' @@ -30,13 +34,33 @@ import routeContext from './route-manifest' // and that is the boundary below's, not suspense's. // A factory because the client is not in scope until `init` lands, and ExpoRoot takes a component. function createRootProviders(client: BridgeRpcClient, target: PageMountTarget) { + // A commit is not a paint, and an unpainted WebView shows the surface behind it and nothing else, + // so the shell keeps its own frame over this document until the second frame lands. + const reportRouteScreenPaint = createRouteScreenPaintReporter( + { + requestFrame: (callback) => requestAnimationFrame(callback), + cancelFrame: (handle) => { + cancelAnimationFrame(handle) + } + }, + () => { + client.notifyPagePainted() + } + ) return function RootProviders({ children }: PropsWithChildren) { // Effects run child-first, so 'mounted' lands only after the router tree below this wrapper - // has committed. The tree is rendered once, with a ready client, so there is one such commit. + // has committed. That commit can be the suspense fallback of a route chunk still arriving, + // which is why the paint is reported from the screen and not from here. useEffect(() => { stampPageMountState(target, 'mounted') }, []) - return {children} + return ( + + + {children} + + + ) } }