From e7e950a55e0b35cade8ae0b1f3e9fbfb05482a32 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Fri, 4 Sep 2026 16:47:53 -0400 Subject: [PATCH] fix(mobile): tie the hosted page broker to the document lifecycle A hosted document is replaced without the shell session, its build or the view epoch moving: a native-route excursion deactivates the view to about:blank and reloads on return, the route error boundary reloads in place, and a re-attached view reloads its URL. The previous page's broker survived all three, and its subscription records kept holding every per-operation grant (one for workspace, account and source control), so the new document's subscribes were refused with rate_limited for the rest of the shell session and the host kept publishing to records nobody read. The shell owns the document lifecycle, so it now observes it: the native views report every document load start (Android through onPageStarted, iOS through didStartProvisionalNavigation, which is the only signal a page-initiated reload gives), and a load that displaces a document that finished loading bumps a page document epoch. The broker hook retires on that epoch exactly as it does on a view-epoch bump, before the incoming document is initialized. A first load and the duplicate load-start events for one navigation are not replacements and do not retire anything. Android also never destroyed its WebView: OnViewDestroys only dropped the registry entry, so every package swap, host switch and route exit leaked a renderer process, a JavaScript context, the message listener and both script handlers. Unmount now destroys it; deactivation still only parks it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- mobile/app/hybrid.tsx | 28 +- .../ExpoMobileWebShellModule.kt | 2 + .../mobilewebshell/MobileWebShellView.kt | 38 ++- .../ios/MobileWebShellView.swift | 8 + .../MobileWebHybridShellPresentation.tsx | 8 + .../hosted-webview-document-lifecycle.test.ts | 47 ++++ ...obile-native-shell-route-ownership.test.ts | 2 +- .../mobile-web-document-replacement.test.tsx | 264 ++++++++++++++++++ .../mobile-web-shell-load-failure.test.tsx | 1 + .../use-mobile-web-capability-broker.test.ts | 22 +- .../use-mobile-web-capability-broker.ts | 7 +- .../use-mobile-web-page-document.ts | 71 +++++ 12 files changed, 479 insertions(+), 19 deletions(-) create mode 100644 mobile/src/mobile-web/hosted-webview-document-lifecycle.test.ts create mode 100644 mobile/src/mobile-web/mobile-web-document-replacement.test.tsx create mode 100644 mobile/src/mobile-web/use-mobile-web-page-document.ts diff --git a/mobile/app/hybrid.tsx b/mobile/app/hybrid.tsx index 7f15ad67866..3101553ca63 100644 --- a/mobile/app/hybrid.tsx +++ b/mobile/app/hybrid.tsx @@ -13,6 +13,7 @@ import { useMobileWebCapabilityBroker, type MobileWebBrokerPageIdentity } from '../src/mobile-web/use-mobile-web-capability-broker' +import { useMobileWebPageDocument } from '../src/mobile-web/use-mobile-web-page-document' import { MOBILE_WEB_PRODUCTION_GRANTS } from '../src/mobile-web/mobile-web-production-grants' import { MobileWebHealthDeadline } from '../src/mobile-web/mobile-web-health-deadline' import { useMobileWebAlertSafePackageSession } from '../src/mobile-web/use-mobile-web-alert-safe-package-session' @@ -49,7 +50,6 @@ export default function HybridScreen() { const params = useLocalSearchParams<{ hostId?: string }>() const viewRef = useRef(null) const activeSessionIdRef = useRef(undefined) - const initializedSessionRef = useRef(undefined) const healthDeadlineRef = useRef(new MobileWebHealthDeadline(10_000)) const brokerRef = useRef(null) const postInitRef = useRef<() => Promise>(() => Promise.resolve()) @@ -60,7 +60,6 @@ export default function HybridScreen() { ) const { hosts, hostsLoading, hostLoadError, refreshHosts } = useMobileWebHostCatalog() const [selectedHostId, setSelectedHostId] = useState(params.hostId) - const [pageReadySessionId, setPageReadySessionId] = useState() const [brokerSessionId, setBrokerSessionId] = useState() const [hostedViewActive, setHostedViewActive] = useState(true) const selectHost = useCallback((hostId: string | undefined) => setSelectedHostId(hostId), []) @@ -140,14 +139,12 @@ export default function HybridScreen() { } }, [params.hostId]) - // A view-epoch bump replaces the document, so every page-scoped grant retires with it. - useEffect(() => { - initializedSessionRef.current = undefined - nativeRouteHandoffRef.current.clear() - setPageReadySessionId(undefined) - healthDeadlineRef.current.clear() - return () => healthDeadlineRef.current.clear() - }, [session?.sessionId, viewEpoch]) + const pageDocument = useMobileWebPageDocument({ + sessionId: session?.sessionId, + viewEpoch, + healthDeadlineRef, + routeHandoffRef: nativeRouteHandoffRef + }) const postToWeb = useCallback(async (message: MobileWebBridgeShellMessage) => { if (responseDropRef.current.shouldDrop(message)) { @@ -229,6 +226,7 @@ export default function HybridScreen() { sessionId: session?.sessionId, buildId: session?.buildId, viewEpoch, + documentEpoch: pageDocument.epoch, createBroker, onBrokerReady, onBrokerSessionChange: setBrokerSessionId @@ -243,7 +241,7 @@ export default function HybridScreen() { if (!current || activeSessionIdRef.current !== current.sessionId || !brokerRef.current) { return } - initializedSessionRef.current = current.sessionId + pageDocument.initializedSessionRef.current = current.sessionId healthDeadlineRef.current.arm(current.sessionId, (sessionId) => { if (activeSessionIdRef.current === sessionId) { void onHealthTimeout(sessionId) @@ -269,7 +267,7 @@ export default function HybridScreen() { useEffect(() => { brokerRef.current?.updateConnectionState(mobileWebBridgeConnectionState(state)) const current = session - if (!current || initializedSessionRef.current !== current.sessionId) { + if (!current || pageDocument.initializedSessionRef.current !== current.sessionId) { return } void postToWeb({ @@ -301,7 +299,7 @@ export default function HybridScreen() { if (parsed.value.type === 'ready') { // `ready` acknowledges init; echoing init here starves the health frame. if (activeSessionIdRef.current === current.sessionId) { - setPageReadySessionId(current.sessionId) + pageDocument.setReadySessionId(current.sessionId) } } else if (!backMessageHandled) { if (parsed.value.type === 'health') { @@ -356,7 +354,7 @@ export default function HybridScreen() { selectedHostId, connectionState: state, shellContext, - pageReadySessionId, + pageReadySessionId: pageDocument.readySessionId, brokerSessionId, getBroker, selectHost, @@ -394,7 +392,9 @@ export default function HybridScreen() { showWarning('That didn’t work. Try again.', 'recovery_action_failed') } onBridgeMessage={(message) => void handleBridgeMessage(message)} + onDocumentLoadStarted={pageDocument.onLoadStart} onPageLoaded={() => { + pageDocument.onLoaded() hardwareBackHandoff.resetPage() void postInit() }} diff --git a/mobile/packages/expo-mobile-web-shell/android/src/main/java/expo/modules/mobilewebshell/ExpoMobileWebShellModule.kt b/mobile/packages/expo-mobile-web-shell/android/src/main/java/expo/modules/mobilewebshell/ExpoMobileWebShellModule.kt index 3a860cab257..9fde3ec34e1 100644 --- a/mobile/packages/expo-mobile-web-shell/android/src/main/java/expo/modules/mobilewebshell/ExpoMobileWebShellModule.kt +++ b/mobile/packages/expo-mobile-web-shell/android/src/main/java/expo/modules/mobilewebshell/ExpoMobileWebShellModule.kt @@ -94,6 +94,8 @@ class ExpoMobileWebShellModule : Module() { OnViewDestroys { view -> sessionViews.entries.removeAll { it.value === view } + // Deregister first: nothing may reach a WebView whose renderer process is being torn down. + view.destroy() } AsyncFunction("activateSessionView") { view: MobileWebShellView, sessionId: String -> diff --git a/mobile/packages/expo-mobile-web-shell/android/src/main/java/expo/modules/mobilewebshell/MobileWebShellView.kt b/mobile/packages/expo-mobile-web-shell/android/src/main/java/expo/modules/mobilewebshell/MobileWebShellView.kt index f4e2f6b9162..3e8e4a3adb1 100644 --- a/mobile/packages/expo-mobile-web-shell/android/src/main/java/expo/modules/mobilewebshell/MobileWebShellView.kt +++ b/mobile/packages/expo-mobile-web-shell/android/src/main/java/expo/modules/mobilewebshell/MobileWebShellView.kt @@ -2,6 +2,7 @@ package expo.modules.mobilewebshell import android.annotation.SuppressLint import android.content.Context +import android.graphics.Bitmap import android.graphics.Color import android.net.Uri import android.view.View @@ -76,6 +77,7 @@ internal class MobileWebShellView( private var networkBlockerScriptHandler: ScriptHandler? = null private var debugProbeScriptHandler: ScriptHandler? = null private var bridgeMessageListenerAttached = false + private var destroyed = false private var webView: WebView init { @@ -125,6 +127,7 @@ internal class MobileWebShellView( } fun setSessionId(sessionId: String?) { + if (destroyed) return if (sessionId == null) { deactivateSessionView() return @@ -158,6 +161,7 @@ internal class MobileWebShellView( } fun deactivateSessionView() { + if (destroyed) return removeBridgeMessageListener() networkBlockerScriptHandler?.remove() networkBlockerScriptHandler = null @@ -176,6 +180,7 @@ internal class MobileWebShellView( require(message.toByteArray(Charsets.UTF_8).size <= MOBILE_WEB_MESSAGE_BYTE_LIMIT) { "mobile_web_bridge_message_too_large" } + if (destroyed) return webView.evaluateJavascript( """ (function(value){ @@ -196,14 +201,36 @@ internal class MobileWebShellView( if (activeSessionId == sessionId) postMessage(message) } + /** + * Detaching only parks the WebView — a deactivated view is reactivated in place — so the renderer + * process, the JavaScript context and both script handlers only go away here. Expo calls this once + * the view is unmounted, which is what a package swap, a host switch and a route exit all do. + */ + fun destroy() { + if (destroyed) return + destroyed = true + removeBridgeMessageListener() + networkBlockerScriptHandler?.remove() + networkBlockerScriptHandler = null + debugProbeScriptHandler?.remove() + debugProbeScriptHandler = null + activeSessionId = null + documentLoaded = false + webView.stopLoading() + webView.loadUrl("about:blank") + removeView(webView) + webView.destroy() + } + override fun onDetachedFromWindow() { removeBridgeMessageListener() - webView.stopLoading() + if (!destroyed) webView.stopLoading() super.onDetachedFromWindow() } override fun onAttachedToWindow() { super.onAttachedToWindow() + if (destroyed) return attachWebView() val sessionId = activeSessionId ?: return addBridgeMessageListener(sessionId) @@ -284,6 +311,15 @@ internal class MobileWebShellView( return !allowed } + override fun onPageStarted(view: WebView, url: String, favicon: Bitmap?) { + // The page can replace its own document (the route error boundary reloads on a failed chunk), + // and that is the only signal the shell gets. Every load start is reported so the shell can + // retire the outgoing page's grants before the new document initializes. + if (!isAllowedDocumentRequestUrl(Uri.parse(url))) return + documentLoaded = false + onLoadState(mapOf("state" to "loading")) + } + override fun onPageFinished(view: WebView, url: String) { if (isAllowedDocumentUrl(Uri.parse(url))) { documentLoaded = true diff --git a/mobile/packages/expo-mobile-web-shell/ios/MobileWebShellView.swift b/mobile/packages/expo-mobile-web-shell/ios/MobileWebShellView.swift index 077dc6721b9..2e0f9516098 100644 --- a/mobile/packages/expo-mobile-web-shell/ios/MobileWebShellView.swift +++ b/mobile/packages/expo-mobile-web-shell/ios/MobileWebShellView.swift @@ -566,6 +566,14 @@ final class MobileWebShellView: ExpoView, WKNavigationDelegate, WKUIDelegate, decisionHandler(allowed ? .allow : .cancel) } + func webView(_ webView: WKWebView, didStartProvisionalNavigation navigation: WKNavigation!) { + // The page can replace its own document (the route error boundary reloads on a failed chunk), + // and that is the only signal the shell gets. Every load start is reported so the shell can + // retire the outgoing page's grants before the new document initializes. + guard activeSessionId != nil else { return } + onLoadState(["state": "loading"]) + } + func webView(_ webView: WKWebView, didFinish navigation: WKNavigation!) { guard isAllowedDocumentUrl(webView.url) else { return } onLoadState(["state": "loaded"]) diff --git a/mobile/src/mobile-web/MobileWebHybridShellPresentation.tsx b/mobile/src/mobile-web/MobileWebHybridShellPresentation.tsx index 97a22e6ca81..e575b189218 100644 --- a/mobile/src/mobile-web/MobileWebHybridShellPresentation.tsx +++ b/mobile/src/mobile-web/MobileWebHybridShellPresentation.tsx @@ -35,6 +35,7 @@ type MobileWebHybridShellPresentationProps = { onClearCache: () => void | Promise onRecoveryFailure: () => void onBridgeMessage: (message: string) => void + onDocumentLoadStarted: () => void onPageLoaded: () => void onLoadFailed: (reason: string | undefined) => void onNavigationBlocked: () => void @@ -57,6 +58,7 @@ export function MobileWebHybridShellPresentation({ onClearCache, onRecoveryFailure, onBridgeMessage, + onDocumentLoadStarted, onPageLoaded, onLoadFailed, onNavigationBlocked, @@ -141,6 +143,12 @@ export function MobileWebHybridShellPresentation({ sessionId={hostedViewActive ? session.sessionId : null} onBridgeMessage={(event) => onBridgeMessage(event.nativeEvent.data)} onLoadState={(event) => { + // A load only ever starts because the document is being replaced, and the outgoing + // page's grants have to retire before the incoming one initializes. + if (event.nativeEvent.state === 'loading') { + onDocumentLoadStarted() + return + } if (event.nativeEvent.state === 'loaded') { onPageLoaded() return diff --git a/mobile/src/mobile-web/hosted-webview-document-lifecycle.test.ts b/mobile/src/mobile-web/hosted-webview-document-lifecycle.test.ts new file mode 100644 index 00000000000..b08b8cd9c9a --- /dev/null +++ b/mobile/src/mobile-web/hosted-webview-document-lifecycle.test.ts @@ -0,0 +1,47 @@ +import { readFileSync } from 'node:fs' +import { fileURLToPath } from 'node:url' +import { describe, expect, it } from 'vitest' + +const ANDROID_VIEW = source( + '../../packages/expo-mobile-web-shell/android/src/main/java/expo/modules/mobilewebshell/MobileWebShellView.kt' +) +const ANDROID_MODULE = source( + '../../packages/expo-mobile-web-shell/android/src/main/java/expo/modules/mobilewebshell/ExpoMobileWebShellModule.kt' +) +const IOS_VIEW = source('../../packages/expo-mobile-web-shell/ios/MobileWebShellView.swift') + +describe('hosted WebView document lifecycle', () => { + // The shell owns the document lifecycle, but a page can replace its own document — the route + // error boundary reloads in place — and the load start is the only signal that reaches the shell. + it('reports a document load the page itself starts, on both platforms', () => { + expect(block(ANDROID_VIEW, 'override fun onPageStarted')).toContain( + 'onLoadState(mapOf("state" to "loading"))' + ) + expect( + block(IOS_VIEW, 'func webView(_ webView: WKWebView, didStartProvisionalNavigation') + ).toContain('onLoadState(["state": "loading"])') + }) + + // Deactivation parks the WebView for a reactivation in place, so only an unmount may destroy it. + it('destroys the Android WebView on unmount and never on deactivation', () => { + expect(block(ANDROID_MODULE, 'OnViewDestroys')).toContain('view.destroy()') + expect(block(ANDROID_VIEW, 'fun destroy()')).toContain('webView.destroy()') + expect(block(ANDROID_VIEW, 'fun deactivateSessionView()')).not.toContain('webView.destroy()') + expect(block(ANDROID_VIEW, 'override fun onDetachedFromWindow()')).not.toContain( + 'webView.destroy()' + ) + }) +}) + +function source(relativePath: string): string { + return readFileSync(fileURLToPath(new URL(relativePath, import.meta.url)), 'utf8') +} + +/** The declaration through the brace that closes it, so a match cannot leak into the next member. */ +function block(text: string, header: string): string { + const start = text.indexOf(header) + expect(start, `missing ${header}`).toBeGreaterThanOrEqual(0) + const indent = text.slice(0, start).split('\n').at(-1) ?? '' + const end = text.indexOf(`\n${indent}}`, start) + return text.slice(start, end === -1 ? undefined : end) +} diff --git a/mobile/src/mobile-web/mobile-native-shell-route-ownership.test.ts b/mobile/src/mobile-web/mobile-native-shell-route-ownership.test.ts index 89c45ab2690..9f8ea12e163 100644 --- a/mobile/src/mobile-web/mobile-native-shell-route-ownership.test.ts +++ b/mobile/src/mobile-web/mobile-native-shell-route-ownership.test.ts @@ -106,7 +106,7 @@ describe('mobile native shell route ownership', () => { const readyBranch = hybridShell.match( /if \(parsed\.value\.type === 'ready'\) \{([\s\S]*?)\} else if/ )?.[1] - expect(readyBranch).toContain('setPageReadySessionId(current.sessionId)') + expect(readyBranch).toContain('setReadySessionId(current.sessionId)') expect(readyBranch).not.toContain('postInit') }) diff --git a/mobile/src/mobile-web/mobile-web-document-replacement.test.tsx b/mobile/src/mobile-web/mobile-web-document-replacement.test.tsx new file mode 100644 index 00000000000..f29f293518f --- /dev/null +++ b/mobile/src/mobile-web/mobile-web-document-replacement.test.tsx @@ -0,0 +1,264 @@ +import { readFileSync } from 'node:fs' +import { fileURLToPath } from 'node:url' +import { createElement, useRef, type FunctionComponent } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + MOBILE_WEB_BRIDGE_PROTOCOL_VERSION, + type MobileWebBridgePageMessage, + type MobileWebBridgeShellMessage +} from '../../../src/shared/mobile-web/bridge-contract' +import type { RpcClient } from '../transport/rpc-client' +import { MobileWebCapabilityBroker } from './mobile-web-capability-broker' +import { MobileWebHealthDeadline } from './mobile-web-health-deadline' +import { MobileWebHybridShellPresentation } from './MobileWebHybridShellPresentation' +import { MobileWebNativeRouteHandoff } from './mobile-web-native-route-handoff' +import { useMobileWebCapabilityBroker } from './use-mobile-web-capability-broker' +import { useMobileWebPageDocument } from './use-mobile-web-page-document' + +vi.mock('react-native', () => ({ + ActivityIndicator: 'ActivityIndicator', + Pressable: 'Pressable', + StyleSheet: { create: (styles: Record) => styles }, + Text: 'Text', + View: 'View' +})) +vi.mock('react-native-safe-area-context', () => ({ + useSafeAreaInsets: () => ({ top: 0, bottom: 0, left: 0, right: 0 }) +})) +vi.mock('lucide-react-native', () => ({ + ChevronLeft: 'ChevronLeft', + MonitorSmartphone: 'MonitorSmartphone' +})) +vi.mock('@orca/expo-mobile-web-shell', () => ({ MobileWebShellView: 'MobileWebShellView' })) + +const noop = (): void => {} +const SESSION_ID = 'S'.repeat(43) +const BUILD_ID = 'a'.repeat(64) + +// A native-route excursion, the route error boundary's reload and a re-attach all replace the +// document while the shell session, its build and the view epoch stand still. The previous page's +// broker used to survive that, and its records held every per-operation grant (one for workspace), +// so the new document was refused with rate_limited for the life of the shell session. +describe('hosted document replacement', () => { + let renderer: ReactTestRenderer | null = null + let harness: ReturnType + + beforeEach(() => { + harness = createHarness() + }) + + afterEach(() => { + act(() => renderer?.unmount()) + renderer = null + }) + + it('re-subscribes after a native-route excursion reloads the page', async () => { + await mount() + await loadDocument() + await handle(subscribeRequest('A', 'Z')) + expect(harness.subscribe).toHaveBeenCalledOnce() + + // The excursion deactivates the view (about:blank) and reactivates it on return; the page is + // never unmounted, so the shell only ever sees the reload the reactivation starts. + await setHostedViewActive(false) + await setHostedViewActive(true) + await loadDocument() + await handle(subscribeRequest('B', 'Y')) + + expect(harness.unsubscribe).toHaveBeenCalledOnce() + expect(harness.subscribe).toHaveBeenCalledTimes(2) + expect(errorFor(harness.messages, 'B')).toEqual([]) + }) + + it('re-subscribes after the page reloads itself in place', async () => { + await mount() + await loadDocument() + await handle(subscribeRequest('A', 'Z')) + + // What the route error boundary's recovery button does: same view, same session, new document. + // The duplicate `loading` is what the shell really posts — once for the reload it starts and + // once for the navigation the page started — and the pair is still a single new page. + await emitLoadState({ state: 'loading' }) + await emitLoadState({ state: 'loading' }) + await emitLoadState({ state: 'loaded' }) + await handle(subscribeRequest('B', 'Y')) + + expect(harness.unsubscribe).toHaveBeenCalledOnce() + expect(harness.created).toBe(2) + expect(errorFor(harness.messages, 'B')).toEqual([]) + }) + + it('never retires the broker that is about to serve the first document', async () => { + await mount() + const live = harness.brokerRef.current + + // The shell posts `loading` for the first load too, and more than once for the same load. A + // page that has not loaded yet has nothing to retire, and retiring here would drop the grants + // out from under the document that is loading. + await emitLoadState({ state: 'loading' }) + await emitLoadState({ state: 'loading' }) + await emitLoadState({ state: 'loaded' }) + await handle(subscribeRequest('A', 'Z')) + + expect(harness.unsubscribe).not.toHaveBeenCalled() + expect(harness.created).toBe(1) + expect(harness.brokerRef.current).toBe(live) + expect(errorFor(harness.messages, 'A')).toEqual([]) + }) + + it('is the wiring the hosted screen uses', () => { + // The harness above composes the two seams by hand; this pins the screen to the same pair. + const source = readFileSync( + fileURLToPath(new URL('../../app/hybrid.tsx', import.meta.url)), + 'utf8' + ) + + expect(source).toContain('documentEpoch: pageDocument.epoch') + expect(source).toContain('onDocumentLoadStarted={pageDocument.onLoadStart}') + expect(source).toContain('pageDocument.onLoaded()') + }) + + async function mount(): Promise { + await act(async () => { + renderer = create(createElement(harness.Shell, { hostedViewActive: true })) + }) + } + + async function setHostedViewActive(hostedViewActive: boolean): Promise { + await act(async () => { + renderer?.update(createElement(harness.Shell, { hostedViewActive })) + }) + } + + async function loadDocument(): Promise { + await emitLoadState({ state: 'loading' }) + await emitLoadState({ state: 'loaded' }) + } + + async function emitLoadState(nativeEvent: { state: string; reason?: string }): Promise { + const shell = renderer!.root.findByType('MobileWebShellView' as never) + await act(async () => { + shell.props.onLoadState({ nativeEvent }) + }) + } + + async function handle(message: MobileWebBridgePageMessage): Promise { + await act(async () => { + await harness.brokerRef.current?.handle(message) + }) + } +}) + +function createHarness() { + const messages: MobileWebBridgeShellMessage[] = [] + const unsubscribe = vi.fn() + const subscribe = vi.fn(() => unsubscribe) + const client = { + sendRequest: vi.fn(), + subscribe, + sendTerminalBinaryFrame: vi.fn(() => true) + } as unknown as RpcClient + const brokerRef: { current: MobileWebCapabilityBroker | null } = { current: null } + // Stable like hybrid.tsx's useCallback: a fresh identity per render would retire the broker on + // every render and hide whether the document epoch is doing the work. + const createBroker = (page: { sessionId: string; buildId: string }) => { + state.created += 1 + return new MobileWebCapabilityBroker({ + context: { shellSessionId: page.sessionId, buildId: page.buildId }, + getClient: () => client, + isConnected: () => true, + isActive: () => true, + postMessage: (message) => void messages.push(message), + nativeAuthority: { + clipboardAvailability: vi.fn(), + hapticFeedback: vi.fn(), + clipboardWrite: vi.fn(), + openExternal: vi.fn(), + terminalPreferences: vi.fn(), + terminalTextScaleUpdate: vi.fn() + }, + terminalClientId: 'device-token', + randomBytes: (length) => new Uint8Array(length).fill(1) + }) + } + const state = { + messages, + subscribe, + unsubscribe, + brokerRef, + created: 0, + Shell: undefined as unknown as FunctionComponent<{ hostedViewActive: boolean }> + } + state.Shell = ({ hostedViewActive }) => { + const healthDeadlineRef = useRef(new MobileWebHealthDeadline(10_000)) + const routeHandoffRef = useRef(new MobileWebNativeRouteHandoff()) + const pageDocument = useMobileWebPageDocument({ + sessionId: SESSION_ID, + viewEpoch: 0, + healthDeadlineRef, + routeHandoffRef + }) + useMobileWebCapabilityBroker({ + brokerRef, + sessionId: SESSION_ID, + buildId: BUILD_ID, + viewEpoch: 0, + documentEpoch: pageDocument.epoch, + createBroker, + onBrokerReady: noop, + onBrokerSessionChange: noop + }) + return createElement(MobileWebHybridShellPresentation, { + viewRef: { current: null }, + selectedHost: { id: 'host-1', name: 'Desk', publicKeyB64: 'k' } as never, + session: { sessionId: SESSION_ID, buildId: BUILD_ID } as never, + viewEpoch: 0, + packageLoading: false, + packageProgress: undefined, + packageWarning: undefined, + hostedViewActive, + onBack: noop, + onShowHosts: noop, + onRetryRecovery: noop, + onUsePrevious: noop, + onClearCache: noop, + onRecoveryFailure: noop, + onBridgeMessage: noop, + onDocumentLoadStarted: pageDocument.onLoadStart, + onPageLoaded: pageDocument.onLoaded, + onLoadFailed: noop, + onNavigationBlocked: noop, + onProcessTerminated: noop + }) + } + return state +} + +function subscribeRequest( + requestId: string, + subscriptionId: string +): Extract { + return { + version: MOBILE_WEB_BRIDGE_PROTOCOL_VERSION, + shellSessionId: SESSION_ID, + buildId: BUILD_ID, + type: 'request', + mode: 'subscription', + requestId: requestId.repeat(22), + subscriptionId: subscriptionId.repeat(22), + capability: 'workspace', + operation: 'subscribe', + payload: {} + } +} + +function errorFor(messages: MobileWebBridgeShellMessage[], requestId: string) { + return messages.flatMap((message) => + message.type === 'response' && + message.requestId === requestId.repeat(22) && + message.status === 'error' + ? [message.error] + : [] + ) +} diff --git a/mobile/src/mobile-web/mobile-web-shell-load-failure.test.tsx b/mobile/src/mobile-web/mobile-web-shell-load-failure.test.tsx index ac8e8a93a5e..28d002478db 100644 --- a/mobile/src/mobile-web/mobile-web-shell-load-failure.test.tsx +++ b/mobile/src/mobile-web/mobile-web-shell-load-failure.test.tsx @@ -57,6 +57,7 @@ describe('hosted shell document load failures', () => { onClearCache: noop, onRecoveryFailure: noop, onBridgeMessage: noop, + onDocumentLoadStarted: noop, onPageLoaded: loaded, onLoadFailed: (reason) => failures.push(reason), onNavigationBlocked: noop, diff --git a/mobile/src/mobile-web/use-mobile-web-capability-broker.test.ts b/mobile/src/mobile-web/use-mobile-web-capability-broker.test.ts index 2891ffa5f6e..8e582fdde07 100644 --- a/mobile/src/mobile-web/use-mobile-web-capability-broker.test.ts +++ b/mobile/src/mobile-web/use-mobile-web-capability-broker.test.ts @@ -57,6 +57,20 @@ describe('useMobileWebCapabilityBroker', () => { expect(errorFor(harness.messages, 'B')).toEqual([]) }) + it('retires the previous page subscriptions when the document is replaced in place', async () => { + await mount(0) + await handle(subscribeRequest('A', 'Z')) + + await act(async () => { + renderer?.update(createElement(harness.Harness, { viewEpoch: 0, documentEpoch: 1 })) + }) + await handle(subscribeRequest('B', 'Y')) + + expect(harness.unsubscribe).toHaveBeenCalledOnce() + expect(harness.subscribe).toHaveBeenCalledTimes(2) + expect(errorFor(harness.messages, 'B')).toEqual([]) + }) + it('retires the broker on demand so an in-place reload cannot inherit it', async () => { await mount(0) await handle(subscribeRequest('A', 'Z')) @@ -99,7 +113,10 @@ function createHarness() { created: 0, brokerSessionId: undefined as string | undefined, retireBroker: undefined as (() => void) | undefined, - Harness: undefined as unknown as FunctionComponent<{ viewEpoch: number }> + Harness: undefined as unknown as FunctionComponent<{ + viewEpoch: number + documentEpoch?: number + }> } const createBroker = (page: MobileWebBrokerPageIdentity): MobileWebCapabilityBroker => { state.created += 1 @@ -124,12 +141,13 @@ function createHarness() { const onBrokerSessionChange = (sessionId: string | undefined): void => { state.brokerSessionId = sessionId } - state.Harness = ({ viewEpoch }) => { + state.Harness = ({ viewEpoch, documentEpoch = 0 }) => { const lifecycle = useMobileWebCapabilityBroker({ brokerRef, sessionId: CONTEXT.shellSessionId, buildId: CONTEXT.buildId, viewEpoch, + documentEpoch, createBroker, onBrokerReady, onBrokerSessionChange diff --git a/mobile/src/mobile-web/use-mobile-web-capability-broker.ts b/mobile/src/mobile-web/use-mobile-web-capability-broker.ts index 9b9b99398fa..248acfee0b4 100644 --- a/mobile/src/mobile-web/use-mobile-web-capability-broker.ts +++ b/mobile/src/mobile-web/use-mobile-web-capability-broker.ts @@ -5,12 +5,15 @@ export type MobileWebBrokerPageIdentity = { sessionId: string; buildId: string } // A view-epoch bump loads a fresh document over the same shell session, so the previous page's // broker (subscriptions, terminal streams, speech authority, replay window, rate limiter) has to -// be retired before the new document can post against it. +// be retired before the new document can post against it. A document epoch is the same boundary +// without a native remount: the shell replaces the document in place on a native-route return, an +// in-page reload and a re-attach, and each of those pages is just as new. export function useMobileWebCapabilityBroker({ brokerRef, sessionId, buildId, viewEpoch, + documentEpoch, createBroker, onBrokerReady, onBrokerSessionChange @@ -19,6 +22,7 @@ export function useMobileWebCapabilityBroker({ sessionId: string | undefined buildId: string | undefined viewEpoch: number + documentEpoch: number createBroker: (page: MobileWebBrokerPageIdentity) => MobileWebCapabilityBroker | null onBrokerReady: () => void onBrokerSessionChange: (sessionId: string | undefined) => void @@ -52,6 +56,7 @@ export function useMobileWebCapabilityBroker({ brokerRef, buildId, createBroker, + documentEpoch, onBrokerReady, onBrokerSessionChange, retireBroker, diff --git a/mobile/src/mobile-web/use-mobile-web-page-document.ts b/mobile/src/mobile-web/use-mobile-web-page-document.ts new file mode 100644 index 00000000000..a6ce4b548ef --- /dev/null +++ b/mobile/src/mobile-web/use-mobile-web-page-document.ts @@ -0,0 +1,71 @@ +import { useCallback, useEffect, useRef, useState, type MutableRefObject } from 'react' +import type { MobileWebHealthDeadline } from './mobile-web-health-deadline' +import type { MobileWebNativeRouteHandoff } from './mobile-web-native-route-handoff' + +/** + * The shell owns the document lifecycle, so it also owns the page-scoped state that dies with a + * document: what was initialized, what reported ready, and the health deadline armed for it. + * + * A document is replaced without the shell session, its build or the view epoch moving at all — a + * native-route excursion deactivates the view to about:blank and reloads on return, the route error + * boundary reloads in place, and a re-attached view reloads its URL. Each of those mints a new page + * with new subscription ids while the previous page's broker survives, and its records keep holding + * every per-operation grant (one for workspace, account and source control), so the new document's + * subscribes are refused with `rate_limited` for the life of the shell session. The document epoch + * is that boundary: it retires the outgoing page's broker before the incoming one initializes. + */ +export function useMobileWebPageDocument({ + sessionId, + viewEpoch, + healthDeadlineRef, + routeHandoffRef +}: { + sessionId: string | undefined + viewEpoch: number + healthDeadlineRef: MutableRefObject + routeHandoffRef: MutableRefObject +}): { + epoch: number + initializedSessionRef: MutableRefObject + readySessionId: string | undefined + setReadySessionId: (sessionId: string | undefined) => void + onLoadStart: () => void + onLoaded: () => void +} { + const initializedSessionRef = useRef(undefined) + const loadedRef = useRef(false) + const [epoch, setEpoch] = useState(0) + const [readySessionId, setReadySessionId] = useState() + + useEffect(() => { + initializedSessionRef.current = undefined + loadedRef.current = false + routeHandoffRef.current.clear() + setReadySessionId(undefined) + healthDeadlineRef.current.clear() + return () => healthDeadlineRef.current.clear() + }, [epoch, healthDeadlineRef, routeHandoffRef, sessionId, viewEpoch]) + + const onLoadStart = useCallback(() => { + // Only a load that displaces a document that finished loading is a replacement. The shell posts + // `loading` for the first load too, and more than once per load, and neither is a new page. + if (!loadedRef.current) { + return + } + loadedRef.current = false + setEpoch((current) => current + 1) + }, []) + + const onLoaded = useCallback(() => { + loadedRef.current = true + }, []) + + return { + epoch, + initializedSessionRef, + readySessionId, + setReadySessionId, + onLoadStart, + onLoaded + } +}