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-connection-diagnostics-route.test.tsx b/mobile/src/mobile-web/hosted-connection-diagnostics-route.test.tsx index 9c485fac89f..ac5bb7134df 100644 --- a/mobile/src/mobile-web/hosted-connection-diagnostics-route.test.tsx +++ b/mobile/src/mobile-web/hosted-connection-diagnostics-route.test.tsx @@ -7,6 +7,8 @@ import { handleMobileWebBrokerMessage } from './mobile-web-broker-message-handof import { MobileWebNativeRouteHandoff } from './mobile-web-native-route-handoff' import { MOBILE_WEB_PRODUCTION_NAVIGATION_GRANTS } from './mobile-web-production-navigation-grants' import { useMobileWebNavigationAuthority } from './use-mobile-web-navigation-authority' +import { leaveHostRoute } from '../host-route-exit' +import { removeHostAndCloseClient } from '../transport/host-removal-lifecycle' import type { MobileWebShellViewRef } from '@orca/expo-mobile-web-shell' import type { MobileWebNavigationAuthority } from './mobile-web-navigation-operations' @@ -73,6 +75,21 @@ describe('hosted network diagnostics route', () => { expect(deactivateSessionView).toHaveBeenCalledWith() }) + it('leaves the hosted route before the unpair deletes the package cache it serves', async () => { + const order: string[] = [] + vi.mocked(leaveHostRoute).mockImplementation(() => { + order.push('leave-route') + }) + vi.mocked(removeHostAndCloseClient).mockImplementation(async () => { + order.push('remove-host') + }) + const authority = renderNavigationAuthority(new MobileWebNativeRouteHandoff()) + + await authority.current?.removeHost() + + expect(order).toEqual(['leave-route', 'remove-host']) + }) + it('leaves the host picker exit on the page-local route path', () => { const routeHandoff = new MobileWebNativeRouteHandoff() const authority = renderNavigationAuthority(routeHandoff) 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-capability-broker.ts b/mobile/src/mobile-web/mobile-web-capability-broker.ts index 7143799b2c8..2cdee3e02fa 100644 --- a/mobile/src/mobile-web/mobile-web-capability-broker.ts +++ b/mobile/src/mobile-web/mobile-web-capability-broker.ts @@ -14,6 +14,7 @@ import { MobileWebCommitMessageGeneration } from './mobile-web-commit-message-ge import { MobileWebCapabilitySubscriptions } from './mobile-web-capability-subscriptions' import { MOBILE_WEB_PRODUCTION_GRANT_INDEX } from './mobile-web-production-grants' import { MobileWebTerminalStreams } from './mobile-web-terminal-streams' +import { MOBILE_WEB_TERMINAL_CLIENT_CLOSURE } from './mobile-web-terminal-stream-retirement' import { MobileWebSpeechAuthority } from './mobile-web-speech-authority' import { executeMobileWebCapabilityRequest } from './mobile-web-capability-execution' import { MobileWebCapabilityAuthorities } from './mobile-web-capability-authorities' @@ -74,8 +75,8 @@ export class MobileWebCapabilityBroker { onFlowMetrics: options.onTerminalFlowMetrics, onResync: options.onTerminalResync, workspaceAuthority: this.authorities.workspace, - postEvent: (subscriptionId, sequence, event) => - this.messages.event(subscriptionId, sequence, event) + postEvent: this.messages.event.bind(this.messages), + postClosed: mobileWebSubscriptionClosedPoster(this.messages) }) } @@ -108,7 +109,7 @@ export class MobileWebCapabilityBroker { // The page document outlives the swap, so every live subscription needs a terminal frame; a // silent teardown leaves it waiting on a feed the new client will never resume. this.subscriptions.closeAll({ code: 'unavailable', retryable: true }) - this.terminalStreams.dispose(null) + this.terminalStreams.dispose(null, MOBILE_WEB_TERMINAL_CLIENT_CLOSURE) this.speechAuthority.replaceClient() for (const [requestId, pending] of this.pending) { if (survivesCancellation(pending)) { @@ -244,8 +245,7 @@ export class MobileWebCapabilityBroker { sourceControlSubscriptions: this.subscriptions.sourceControl, sourceControlBranchCompare: this.authorities.sourceControlBranchCompare, speechAuthority: this.speechAuthority, - postSpeechEvent: (subscriptionId, sequence, event) => - this.messages.event(subscriptionId, sequence, event), + postSpeechEvent: this.messages.event.bind(this.messages), postSpeechClosed: mobileWebSubscriptionClosedPoster(this.messages), workspaceSubscriptions: this.subscriptions.workspace, terminalStreams: this.terminalStreams, 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-package-refresh-backoff.ts b/mobile/src/mobile-web/mobile-web-package-refresh-backoff.ts new file mode 100644 index 00000000000..c14ffe44979 --- /dev/null +++ b/mobile/src/mobile-web/mobile-web-package-refresh-backoff.ts @@ -0,0 +1,48 @@ +import { withReconnectJitter } from '../../../src/shared/reconnect-jitter' + +// Why: the refresh effect restarts from zero on every client swap and the staged bytes are dropped +// with it, so a flapping link re-downloads the whole bundle back to back on cellular data. The delay +// escalates per attempt that never reached a completed download and stops at a ceiling; a user-driven +// retry starts the ladder over. +const REFRESH_RETRY_BASE_DELAY_MS = 1_000 +const REFRESH_RETRY_MAX_DELAY_MS = 30_000 + +export function mobileWebPackageRefreshDelayMs( + unfinishedAttempts: number, + random: () => number = Math.random +): number { + if (unfinishedAttempts <= 0) { + return 0 + } + return withReconnectJitter( + Math.min( + REFRESH_RETRY_BASE_DELAY_MS * 2 ** (unfinishedAttempts - 1), + REFRESH_RETRY_MAX_DELAY_MS + ), + random + ) +} + +/** Resolves false when the wait was abandoned, so the caller drops the attempt instead of starting. */ +export function waitBeforeMobileWebPackageRefresh( + delayMs: number, + signal: AbortSignal +): Promise { + if (signal.aborted) { + return Promise.resolve(false) + } + if (delayMs <= 0) { + return Promise.resolve(true) + } + return new Promise((resolve) => { + const onAbort = (): void => { + clearTimeout(timer) + resolve(false) + } + const timer = setTimeout(() => { + signal.removeEventListener('abort', onAbort) + resolve(true) + }, delayMs) + signal.addEventListener('abort', onAbort, { once: true }) + }) +} 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/mobile-web-terminal-lease-streams.ts b/mobile/src/mobile-web/mobile-web-terminal-lease-streams.ts index 500e61e3a04..38e8e50cb2c 100644 --- a/mobile/src/mobile-web/mobile-web-terminal-lease-streams.ts +++ b/mobile/src/mobile-web/mobile-web-terminal-lease-streams.ts @@ -5,6 +5,14 @@ import { } from '../../../src/shared/mobile-web/terminal-stream-contract' import type { RpcClient } from '../transport/rpc-client' import { MobileWebBrokerError } from './mobile-web-broker-error' +import type { + MobileWebPostSubscriptionClosed, + MobileWebSubscriptionClosure +} from './mobile-web-subscription-closure' +import { + MOBILE_WEB_TERMINAL_AUTHORITY_CLOSURE, + MOBILE_WEB_TERMINAL_DELIVERY_CLOSURE +} from './mobile-web-terminal-stream-retirement' import type { MobileWebWorkspaceAuthority } from './mobile-web-workspace-authority' type LeaseSubscribeRequest = Extract @@ -34,6 +42,7 @@ export class MobileWebTerminalLeaseStreams { sequence: number, event: MobileWebTerminalEvent ) => Promise + postClosed: MobileWebPostSubscriptionClosed } ) {} @@ -85,12 +94,12 @@ export class MobileWebTerminalLeaseStreams { return this.records.has(streamId) } - cancel(subscriptionId: string): string | null { + cancel(subscriptionId: string, closure?: MobileWebSubscriptionClosure): string | null { const record = this.records.get(subscriptionId) if (!record) { return null } - this.retire(record) + this.retire(record, closure) return record.requestId } @@ -102,9 +111,9 @@ export class MobileWebTerminalLeaseStreams { } } - dispose(): void { + dispose(closure?: MobileWebSubscriptionClosure): void { for (const subscriptionId of this.records.keys()) { - this.cancel(subscriptionId) + this.cancel(subscriptionId, closure) } } @@ -113,10 +122,14 @@ export class MobileWebTerminalLeaseStreams { } private receive(subscriptionId: string, record: LeaseRecord, value: unknown): void { - if (!this.isLive(subscriptionId, record) || !this.isAuthorized(record)) { + if (!this.isLive(subscriptionId, record)) { this.retire(record) return } + if (!this.isAuthorized(record)) { + this.retire(record, MOBILE_WEB_TERMINAL_AUTHORITY_CLOSURE) + return + } if (!isRecord(value) || typeof value.type !== 'string') { this.post( record, @@ -183,7 +196,7 @@ export class MobileWebTerminalLeaseStreams { this.retire(record) } }) - .catch(() => this.retire(record)) + .catch(() => this.retire(record, MOBILE_WEB_TERMINAL_DELIVERY_CLOSURE)) } private isLive(subscriptionId: string, record: LeaseRecord): boolean { @@ -201,7 +214,7 @@ export class MobileWebTerminalLeaseStreams { } } - private retire(record: LeaseRecord): void { + private retire(record: LeaseRecord, closure?: MobileWebSubscriptionClosure): void { if (!record.active) { return } @@ -212,6 +225,9 @@ export class MobileWebTerminalLeaseStreams { } catch { // The authenticated host subscription is already gone. } + if (closure) { + this.options.postClosed(record.pageStreamId, closure) + } } } diff --git a/mobile/src/mobile-web/mobile-web-terminal-stream-retirement.ts b/mobile/src/mobile-web/mobile-web-terminal-stream-retirement.ts new file mode 100644 index 00000000000..39c54717dff --- /dev/null +++ b/mobile/src/mobile-web/mobile-web-terminal-stream-retirement.ts @@ -0,0 +1,49 @@ +import type { RpcClient } from '../transport/rpc-client' +import type { MobileWebTerminalStreamRecord } from './mobile-web-terminal-flow-control' +import type { + MobileWebPostSubscriptionClosed, + MobileWebSubscriptionClosure +} from './mobile-web-subscription-closure' +import { safeUnsubscribeMobileWebTerminal } from './mobile-web-terminal-host-transport' +import type { MobileWebTerminalStreamRegistry } from './mobile-web-terminal-stream-registry' + +/** The page cannot observe a shell-side retirement: its terminal keeps a live entry, freezes on the + * last frame it got and never re-subscribes. So every retirement the page did not ask for carries a + * closure, and every retirement drops the host stream first — a retired record is unreachable from + * the registry, so no later sweep can unsubscribe it. */ +export const MOBILE_WEB_TERMINAL_AUTHORITY_CLOSURE: MobileWebSubscriptionClosure = { + code: 'not_found', + retryable: false +} +export const MOBILE_WEB_TERMINAL_DELIVERY_CLOSURE: MobileWebSubscriptionClosure = { + code: 'unavailable', + retryable: true +} +/** A client swap keeps the page and its grants, so re-subscribing on the new transport works. */ +export const MOBILE_WEB_TERMINAL_CLIENT_CLOSURE: MobileWebSubscriptionClosure = { + code: 'cancelled', + retryable: true +} + +export class MobileWebTerminalStreamRetirement { + constructor( + private readonly options: { + registry: MobileWebTerminalStreamRegistry + postClosed: MobileWebPostSubscriptionClosed + } + ) {} + + retire( + record: MobileWebTerminalStreamRecord, + client: RpcClient | null, + closure?: MobileWebSubscriptionClosure + ): void { + if (client) { + safeUnsubscribeMobileWebTerminal(client, record) + } + this.options.registry.retire(record) + if (closure) { + this.options.postClosed(record.subscriptionId, closure) + } + } +} diff --git a/mobile/src/mobile-web/mobile-web-terminal-streams.test.ts b/mobile/src/mobile-web/mobile-web-terminal-streams.test.ts index dbc13cd4232..43ede100bc7 100644 --- a/mobile/src/mobile-web/mobile-web-terminal-streams.test.ts +++ b/mobile/src/mobile-web/mobile-web-terminal-streams.test.ts @@ -10,7 +10,9 @@ import { TerminalStreamOpcode, type TerminalStreamFrame } from '../transport/terminal-stream-protocol' +import type { MobileWebSubscriptionClosure } from './mobile-web-subscription-closure' import { MobileWebTerminalStreams } from './mobile-web-terminal-streams' +import { MOBILE_WEB_TERMINAL_CLIENT_CLOSURE } from './mobile-web-terminal-stream-retirement' import { prepareMobileWebClipboardPaste, prepareMobileWebImageAttachment @@ -536,10 +538,159 @@ describe('MobileWebTerminalStreams', () => { expect(harness.streams.cancel(SUBSCRIPTION_ID, harness.client)).toBeNull() expect(harness.leaseUnsubscribe).toHaveBeenCalledOnce() }) + + // Every shell-side retirement below used to be silent: the page kept a live terminal entry with + // no error and no resubscribe, and some of them left the host PTY publishing into nothing. + it('releases the host stream when the page session stops being active', async () => { + let active = true + const harness = createHarness({ isActive: () => active }) + const hostStreamId = await subscribeHostStream(harness, 'request-inactive') + + active = false + harness.emitMultiplex({ type: 'subscribed', streamId: hostStreamId }) + + expect(harness.sentFrames.at(-1)).toMatchObject({ + opcode: TerminalStreamOpcode.Unsubscribe, + streamId: hostStreamId + }) + // The shell drops every frame for an inactive page, so a closure would go nowhere. + expect(harness.closures).toEqual([]) + expect(harness.streams.countForOperation('terminal.subscribe')).toBe(0) + }) + + it('closes the page terminal when the workspace binding disappears under a host frame', async () => { + const harness = createHarness() + const hostStreamId = await subscribeHostStream(harness, 'request-frame-revoked') + harness.emitMultiplex({ type: 'subscribed', streamId: hostStreamId }) + harness.workspaceAuthority.synchronize([]) + + expect( + harness.emitFrame({ + opcode: TerminalStreamOpcode.Output, + streamId: hostStreamId, + seq: 0, + payload: encodeTerminalStreamText('hi') + }) + ).toBe(true) + + expect(harness.sentFrames.at(-1)).toMatchObject({ + opcode: TerminalStreamOpcode.Unsubscribe, + streamId: hostStreamId + }) + expect(harness.closures).toEqual([ + { subscriptionId: SUBSCRIPTION_ID, closure: { code: 'not_found', retryable: false } } + ]) + }) + + it('closes the page terminal when a multiplex event sweeps an unauthorized record', async () => { + const harness = createHarness() + const hostStreamId = await subscribeHostStream(harness, 'request-sweep-revoked') + harness.workspaceAuthority.synchronize([]) + + harness.emitMultiplex({ type: 'ready' }) + + expect(harness.sentFrames.at(-1)).toMatchObject({ + opcode: TerminalStreamOpcode.Unsubscribe, + streamId: hostStreamId + }) + expect(harness.closures).toEqual([ + { subscriptionId: SUBSCRIPTION_ID, closure: { code: 'not_found', retryable: false } } + ]) + }) + + it('closes the page terminal when its own request finds the binding revoked', async () => { + const harness = createHarness() + const hostStreamId = await subscribeHostStream(harness, 'request-revoked-closure') + harness.workspaceAuthority.synchronize([]) + + expect(() => + harness.streams.handle( + { operation: 'resize', streamId: SUBSCRIPTION_ID, viewport: { cols: 100, rows: 30 } }, + harness.client + ) + ).toThrow('not_found') + + expect(harness.sentFrames.at(-1)).toMatchObject({ + opcode: TerminalStreamOpcode.Unsubscribe, + streamId: hostStreamId + }) + expect(harness.closures).toEqual([ + { subscriptionId: SUBSCRIPTION_ID, closure: { code: 'not_found', retryable: false } } + ]) + }) + + it('closes the page terminal when bridge delivery fails', async () => { + const harness = createHarness({ postEvent: () => Promise.reject(new Error('bridge gone')) }) + const hostStreamId = await subscribeHostStream(harness, 'request-delivery-failure') + + harness.emitMultiplex({ type: 'subscribed', streamId: hostStreamId }) + await settle() + + expect(harness.sentFrames.at(-1)).toMatchObject({ + opcode: TerminalStreamOpcode.Unsubscribe, + streamId: hostStreamId + }) + expect(harness.closures).toEqual([ + { subscriptionId: SUBSCRIPTION_ID, closure: { code: 'unavailable', retryable: true } } + ]) + }) + + it('tells the page to re-subscribe when the transport is swapped underneath it', async () => { + const harness = createHarness() + await subscribeHostStream(harness, 'request-client-swap') + + // What MobileWebCapabilityBroker.replaceClient passes: the old transport can no longer carry an + // unsubscribe, so the page has to hear that its terminal is gone and ask again. + harness.streams.dispose(null, MOBILE_WEB_TERMINAL_CLIENT_CLOSURE) + + expect(harness.closures).toEqual([ + { subscriptionId: SUBSCRIPTION_ID, closure: MOBILE_WEB_TERMINAL_CLIENT_CLOSURE } + ]) + expect(harness.multiplexUnsubscribe).toHaveBeenCalledOnce() + expect(harness.streams.countForOperation('terminal.subscribe')).toBe(0) + }) + + it('closes a lease stream whose workspace binding disappears', async () => { + const harness = createHarness() + await harness.streams.start({ + requestId: 'request-lease-revoked', + subscriptionId: SUBSCRIPTION_ID, + payload: { ...subscribePayload(), visible: false, leaseOnly: true }, + client: harness.client, + isRequestActive: () => true + }) + harness.workspaceAuthority.synchronize([]) + + harness.emitLease({ type: 'subscribed' }) + + expect(harness.leaseUnsubscribe).toHaveBeenCalledOnce() + expect(harness.closures).toEqual([ + { subscriptionId: SUBSCRIPTION_ID, closure: { code: 'not_found', retryable: false } } + ]) + }) }) -function createHarness() { +async function subscribeHostStream( + harness: ReturnType, + requestId: string +): Promise { + await harness.streams.start({ + requestId, + subscriptionId: SUBSCRIPTION_ID, + payload: subscribePayload(), + client: harness.client, + isRequestActive: () => true + }) + harness.emitMultiplex({ type: 'ready' }) + return decodeTerminalStreamJson>(harness.sentFrames.at(-1)!.payload)! + .streamId as number +} + +function createHarness( + overrides: { isActive?: () => boolean; postEvent?: () => Promise } = {} +) { const sentFrames: TerminalStreamFrame[] = [] + const closures: { subscriptionId: string; closure: MobileWebSubscriptionClosure }[] = [] const events: { sequence: number; event: MobileWebTerminalEvent }[] = [] const resyncReasons: string[] = [] const flowMetrics: { ackLagMs: number | undefined; outstandingBytes: number }[] = [] @@ -548,6 +699,7 @@ function createHarness() { let emitFrame = (_frame: TerminalStreamFrame): boolean => false let emitLease = (_result: unknown): void => {} const leaseUnsubscribe = vi.fn() + const multiplexUnsubscribe = vi.fn() const workspaceAuthority = new MobileWebWorkspaceAuthority(() => new Uint8Array(16).fill(9)) workspaceAuthority.synchronize([{ workspaceId: HOST_WORKSPACE_ID, repoId: '/secret/repo' }]) const client = { @@ -572,7 +724,7 @@ function createHarness() { } emitMultiplex = listener emitFrame = options?.onTerminalBinaryFrame ?? emitFrame - return vi.fn() + return multiplexUnsubscribe }), sendTerminalBinaryFrame: vi.fn((frame) => { sentFrames.push(frame) @@ -580,7 +732,7 @@ function createHarness() { }) } as unknown as RpcClient const streams = new MobileWebTerminalStreams({ - isActive: () => true, + isActive: overrides.isActive ?? (() => true), clientId: 'device-secret', now: () => nowMs, onFlowMetrics: (metrics) => flowMetrics.push(metrics), @@ -588,7 +740,9 @@ function createHarness() { workspaceAuthority, postEvent: async (_subscriptionId, sequence, event) => { events.push({ sequence, event }) - } + await overrides.postEvent?.() + }, + postClosed: (subscriptionId, closure) => closures.push({ subscriptionId, closure }) }) return { streams, @@ -605,7 +759,9 @@ function createHarness() { emitMultiplex: (result: unknown) => emitMultiplex(result), emitFrame: (frame: TerminalStreamFrame) => emitFrame(frame), emitLease: (result: unknown) => emitLease(result), - leaseUnsubscribe + leaseUnsubscribe, + multiplexUnsubscribe, + closures } } diff --git a/mobile/src/mobile-web/mobile-web-terminal-streams.ts b/mobile/src/mobile-web/mobile-web-terminal-streams.ts index 589b54388ec..1ad7f2027b6 100644 --- a/mobile/src/mobile-web/mobile-web-terminal-streams.ts +++ b/mobile/src/mobile-web/mobile-web-terminal-streams.ts @@ -9,6 +9,10 @@ import { type TerminalStreamFrame } from '../transport/terminal-stream-protocol' import { MobileWebBrokerError } from './mobile-web-broker-error' +import type { + MobileWebPostSubscriptionClosed, + MobileWebSubscriptionClosure +} from './mobile-web-subscription-closure' import { runMobileWebTerminalAction } from './mobile-web-terminal-actions' import { acknowledgeMobileWebTerminalOutput, @@ -16,13 +20,17 @@ import { type MobileWebTerminalStreamRecord } from './mobile-web-terminal-flow-control' import { - safeUnsubscribeMobileWebTerminal, sendMobileWebTerminalFrame, sendMobileWebTerminalSubscribe } from './mobile-web-terminal-host-transport' import { handleMobileWebTerminalMultiplexEvent } from './mobile-web-terminal-multiplex-events' import { resolveMobileWebTerminal } from './mobile-web-terminal-resolution' import { MobileWebTerminalStreamRegistry } from './mobile-web-terminal-stream-registry' +import { + MOBILE_WEB_TERMINAL_AUTHORITY_CLOSURE, + MOBILE_WEB_TERMINAL_DELIVERY_CLOSURE, + MobileWebTerminalStreamRetirement +} from './mobile-web-terminal-stream-retirement' import type { MobileWebWorkspaceAuthority } from './mobile-web-workspace-authority' import { MobileWebTerminalLeaseStreams } from './mobile-web-terminal-lease-streams' import { sendMobileWebTerminalAckBytes } from './mobile-web-terminal-stream-control' @@ -35,6 +43,7 @@ import type { export class MobileWebTerminalStreams { private readonly registry: MobileWebTerminalStreamRegistry + private readonly retirement: MobileWebTerminalStreamRetirement private readonly leaseStreams: MobileWebTerminalLeaseStreams private disposeMultiplex: (() => void) | null = null private multiplexReady = false @@ -53,9 +62,14 @@ export class MobileWebTerminalStreams { sequence: number, event: MobileWebTerminalEvent ) => Promise + postClosed: MobileWebPostSubscriptionClosed } ) { this.registry = new MobileWebTerminalStreamRegistry(options.workspaceAuthority) + this.retirement = new MobileWebTerminalStreamRetirement({ + registry: this.registry, + postClosed: options.postClosed + }) this.leaseStreams = new MobileWebTerminalLeaseStreams(options) } @@ -124,8 +138,7 @@ export class MobileWebTerminalStreams { throw new MobileWebBrokerError('not_found') } if (!this.isAuthorized(record)) { - safeUnsubscribeMobileWebTerminal(client, record) - this.registry.retire(record) + this.retirement.retire(record, client, MOBILE_WEB_TERMINAL_AUTHORITY_CLOSURE) throw new MobileWebBrokerError('not_found') } if ( @@ -165,10 +178,7 @@ export class MobileWebTerminalStreams { if (!record) { return null } - if (client) { - safeUnsubscribeMobileWebTerminal(client, record) - } - this.registry.retire(record) + this.retirement.retire(record, client) return record.requestId } @@ -185,12 +195,10 @@ export class MobileWebTerminalStreams { return operationKey === 'terminal.subscribe' ? this.registry.size + this.leaseStreams.size : 0 } - dispose(client: RpcClient | null): void { - this.leaseStreams.dispose() + dispose(client: RpcClient | null, closure?: MobileWebSubscriptionClosure): void { + this.leaseStreams.dispose(closure) for (const record of this.registry.records()) { - if (client) { - safeUnsubscribeMobileWebTerminal(client, record) - } + this.retirement.retire(record, client, closure) } this.registry.clear() this.disposeMultiplex?.() @@ -229,8 +237,7 @@ export class MobileWebTerminalStreams { return false } if (!this.isAuthorized(record)) { - safeUnsubscribeMobileWebTerminal(client, record) - this.registry.retire(record) + this.retirement.retire(record, client, MOBILE_WEB_TERMINAL_AUTHORITY_CLOSURE) return true } handleMobileWebHostTerminalFrame(record, frame, this.flowContext(client)) @@ -266,26 +273,26 @@ export class MobileWebTerminalStreams { private post(record: MobileWebTerminalStreamRecord, event: MobileWebTerminalEvent): void { if (!this.options.isActive()) { - this.registry.retire(record) + // No closure: the shell drops every frame for an inactive page, so the host stream is all + // there is left to release. + this.retirement.retire(record, record.client) return } if (!this.isAuthorized(record)) { - safeUnsubscribeMobileWebTerminal(record.client, record) - this.registry.retire(record) + this.retirement.retire(record, record.client, MOBILE_WEB_TERMINAL_AUTHORITY_CLOSURE) return } const sequence = record.bridgeSequence++ record.delivery = record.delivery .then(() => this.options.postEvent(record.subscriptionId, sequence, event)) .catch(() => { - safeUnsubscribeMobileWebTerminal(record.client, record) - this.registry.retire(record) + this.retirement.retire(record, record.client, MOBILE_WEB_TERMINAL_DELIVERY_CLOSURE) }) } private retireUnauthorizedRecords(client: RpcClient): void { for (const record of this.registry.retireUnauthorized()) { - safeUnsubscribeMobileWebTerminal(client, record) + this.retirement.retire(record, client, MOBILE_WEB_TERMINAL_AUTHORITY_CLOSURE) } } 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-navigation-authority.ts b/mobile/src/mobile-web/use-mobile-web-navigation-authority.ts index 4489ed95160..ff0e72af275 100644 --- a/mobile/src/mobile-web/use-mobile-web-navigation-authority.ts +++ b/mobile/src/mobile-web/use-mobile-web-navigation-authority.ts @@ -51,6 +51,10 @@ export function useMobileWebNavigationAuthority({ return forceReconnectHost(hostId) }, removeHost() { + // The hosted document is served out of the host's package cache that the unpair deletes, + // so leave the route first instead of letting the hosts-list change unmount the view later. + clearColdResumeRoute() + leaveHostRoute(router) return removeHostAndCloseClient(hostId, hostPublicKeyB64, closeHostClient) } } diff --git a/mobile/src/mobile-web/use-mobile-web-package-refresh.test.ts b/mobile/src/mobile-web/use-mobile-web-package-refresh.test.ts index cacb777ddee..61cd55480f5 100644 --- a/mobile/src/mobile-web/use-mobile-web-package-refresh.test.ts +++ b/mobile/src/mobile-web/use-mobile-web-package-refresh.test.ts @@ -99,4 +99,57 @@ describe('useMobileWebPackageRefresh', () => { expect(await reuse?.(BUILD_ID)).toBe(true) expect(native.openSession).not.toHaveBeenCalled() }) + + // A flapping link aborts the download and drops every staged byte, so an unpaced restart + // re-downloads the whole bundle over cellular data as fast as the link comes back. + it('paces a restart that follows an attempt which never finished downloading', async () => { + vi.useFakeTimers() + try { + downloadPackage.mockImplementation(() => new Promise(() => {})) + const props = { + client: { sendRequest: vi.fn() } as unknown as RpcClient, + host: HOST, + state: 'connected', + packageCapability: { status: 'supported', gzip: false }, + cachedBuildProbeRef: ref({ + hostEpoch: 1, + promise: Promise.resolve(null), + resolve: vi.fn() + }), + hostEpochRef: ref(1), + ownedSessionRef: ref(null), + rejectedBuildIdsRef: ref(new Set()), + refreshingHostEpochRef: ref(null), + publishSession: vi.fn(async () => true), + refreshEpoch: 0, + setPackageLoading: vi.fn(), + setPackageWarning: vi.fn(), + setPackageProgress: vi.fn() + } as const + function Harness({ swap }: { swap: number }): null { + useMobileWebPackageRefresh({ + ...props, + client: { sendRequest: vi.fn(), swap } as unknown as RpcClient + }) + return null + } + + await act(async () => { + renderer = create(createElement(Harness, { swap: 0 })) + }) + expect(downloadPackage).toHaveBeenCalledTimes(1) + + await act(async () => { + renderer?.update(createElement(Harness, { swap: 1 })) + }) + expect(downloadPackage).toHaveBeenCalledTimes(1) + + await act(async () => { + await vi.advanceTimersByTimeAsync(1_200) + }) + expect(downloadPackage).toHaveBeenCalledTimes(2) + } finally { + vi.useRealTimers() + } + }) }) diff --git a/mobile/src/mobile-web/use-mobile-web-package-refresh.ts b/mobile/src/mobile-web/use-mobile-web-package-refresh.ts index 8c91dbe19e9..cc4097ab299 100644 --- a/mobile/src/mobile-web/use-mobile-web-package-refresh.ts +++ b/mobile/src/mobile-web/use-mobile-web-package-refresh.ts @@ -1,4 +1,4 @@ -import { useEffect, type MutableRefObject } from 'react' +import { useEffect, useRef, type MutableRefObject } from 'react' import ExpoMobileWebShell, { type MobileWebShellSession } from '@orca/expo-mobile-web-shell' import { MOBILE_WEB_BRIDGE_PROTOCOL_VERSION } from '../../../src/shared/mobile-web/bridge-contract' import { MOBILE_WEB_PACKAGE_MAX_RANGE_BYTES } from '../../../src/shared/mobile-web/package-rpc-contract' @@ -11,6 +11,10 @@ import { type MobileWebPackageDownloadProgress } from './mobile-web-package-downloader' import { mobileWebDiagnosticsStore } from './mobile-web-diagnostics-store' +import { + mobileWebPackageRefreshDelayMs, + waitBeforeMobileWebPackageRefresh +} from './mobile-web-package-refresh-backoff' import type { MobileWebCachedBuildProbe } from './mobile-web-cached-build-probe' import { mobileWebPackageRefreshWarning } from './mobile-web-package-refresh-warning' import type { MobileWebShellNotice } from './mobile-web-shell-notice' @@ -56,6 +60,7 @@ export function useMobileWebPackageRefresh(args: { setPackageWarning, setPackageProgress } = args + const retryRef = useRef({ hostId: '', refreshEpoch, attempts: 0 }) useEffect(() => { if (!host || !client || state !== 'connected' || packageCapability.status !== 'supported') { @@ -64,11 +69,20 @@ export function useMobileWebPackageRefresh(args: { const hostEpoch = hostEpochRef.current const cachedBuildProbe = cachedBuildProbeRef.current const controller = new AbortController() + // A user-driven retry and a host change both start the backoff ladder over. + if (retryRef.current.hostId !== host.id || retryRef.current.refreshEpoch !== refreshEpoch) { + retryRef.current = { hostId: host.id, refreshEpoch, attempts: 0 } + } + const retryDelayMs = mobileWebPackageRefreshDelayMs(retryRef.current.attempts) + retryRef.current.attempts += 1 refreshingHostEpochRef.current = hostEpoch setPackageLoading(true) setPackageWarning(undefined) setPackageProgress(undefined) void (async () => { + if (!(await waitBeforeMobileWebPackageRefresh(retryDelayMs, controller.signal))) { + return + } const refreshStartedAt = Date.now() try { const downloaded = await downloadMobileWebPackage( @@ -98,6 +112,8 @@ export function useMobileWebPackageRefresh(args: { } } ) + // The bundle is staged: whatever happens next costs no more download, so the ladder resets. + retryRef.current.attempts = 0 if (controller.signal.aborted || hostEpochRef.current !== hostEpoch) { return } 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 + } +} diff --git a/mobile/src/transport/host-removal-lifecycle.test.ts b/mobile/src/transport/host-removal-lifecycle.test.ts index 984b044ac03..45f923af076 100644 --- a/mobile/src/transport/host-removal-lifecycle.test.ts +++ b/mobile/src/transport/host-removal-lifecycle.test.ts @@ -146,6 +146,35 @@ describe('host removal lifecycle', () => { expect(closeHostClient).toHaveBeenCalledWith('host-1') }) + it('deletes the package cache only after the client that served it is closed', async () => { + // The hosted WebView reads its document out of that cache: deleting it first turned every + // later asset request into a 403 under a still-mounted document. + const order: string[] = [] + removeHostMock.mockImplementation(async () => { + order.push('remove-metadata') + }) + removeMobileWebHostCacheMock.mockImplementation(async () => { + order.push('delete-cache') + }) + clearMobileWebColdResumeRouteForHostMock.mockImplementation(async () => { + order.push('clear-cold-route') + }) + + await removeHostAndCloseClient('host-1', 'public-key-1', () => order.push('close-client')) + + expect(order).toEqual(['remove-metadata', 'close-client', 'delete-cache', 'clear-cold-route']) + }) + + it('keeps the package cache when the unpair itself never commits', async () => { + removeHostMock.mockRejectedValue(new Error('storage unavailable')) + + await expect(removeHostAndCloseClient('host-1', 'public-key-1', vi.fn())).rejects.toThrow( + 'storage unavailable' + ) + + expect(removeMobileWebHostCacheMock).not.toHaveBeenCalled() + }) + it('forgets removed-host logs even when client teardown throws', async () => { removeHostMock.mockResolvedValue(undefined) const closeHostClient = vi.fn(() => { diff --git a/mobile/src/transport/host-removal-lifecycle.ts b/mobile/src/transport/host-removal-lifecycle.ts index 8eb91e5b1a8..50778b50908 100644 --- a/mobile/src/transport/host-removal-lifecycle.ts +++ b/mobile/src/transport/host-removal-lifecycle.ts @@ -12,10 +12,6 @@ export async function removeHostAndCloseClient( hostPublicKey: string, forgetHostClient: (hostId: string) => void ): Promise { - // Why: cache deletion is recoverable by redownload, so a hybrid-only failure here must - // never block the unpair itself — on a native build the cache may not even exist. - await removeMobileWebHostCache(hostPublicKey).catch(() => null) - await clearMobileWebColdResumeRouteForHost(hostId).catch(() => null) // Why: closing before the metadata commit can strand a still-paired host on // storage failure; closing immediately after success prevents socket leaks. await removeHost(hostId) @@ -26,5 +22,11 @@ export async function removeHostAndCloseClient( forgetHostNotificationSession(hostId) void clearWatermark(hostId) connectionLogStore.delete(hostId) + // Why last: the hybrid WebView serves its document out of this cache, so deleting it while one + // is still mounted 403s every asset the page asks for next. Cache deletion is recoverable by + // redownload, so a hybrid-only failure here must never block the unpair either — on a native + // build the cache may not even exist. + await removeMobileWebHostCache(hostPublicKey).catch(() => null) + await clearMobileWebColdResumeRouteForHost(hostId).catch(() => null) } } diff --git a/mobile/src/transport/runtime-capability-probe.test.ts b/mobile/src/transport/runtime-capability-probe.test.ts index 269702d0a61..58aa3452ae7 100644 --- a/mobile/src/transport/runtime-capability-probe.test.ts +++ b/mobile/src/transport/runtime-capability-probe.test.ts @@ -172,6 +172,26 @@ describe('startRuntimeCapabilityProbe', () => { cancel() }) + it('stops polling at cutover speed when the link keeps cutting over', async () => { + const outcomes: ProbeOutcome[] = Array.from( + { length: 6 }, + () => new LogicalClientCutoverError() + ) + outcomes.push(ok(['a.v1'])) + const { client, calls } = makeClient(outcomes) + const seen: (readonly string[])[] = [] + const cancel = startRuntimeCapabilityProbe(client, (capabilities) => seen.push(capabilities)) + await flushMicrotasks() + + // Three prompt retries, then the backoff ladder: an endless 4 Hz poll would be 40 calls here. + await vi.advanceTimersByTimeAsync(250 * 4) + expect(calls()).toBe(4) + await vi.advanceTimersByTimeAsync(1_000 + 2_000 + 4_000) + expect(seen).toEqual([['a.v1']]) + expect(calls()).toBe(7) + cancel() + }) + it('stops retrying and dropping results once cancelled', async () => { const { client, calls } = makeClient([new Error('timeout'), ok(['a.v1'])]) const seen: (readonly string[])[] = [] diff --git a/mobile/src/transport/runtime-capability-probe.ts b/mobile/src/transport/runtime-capability-probe.ts index 35cb32eda20..409c6ecb30b 100644 --- a/mobile/src/transport/runtime-capability-probe.ts +++ b/mobile/src/transport/runtime-capability-probe.ts @@ -6,6 +6,9 @@ import { isLogicalClientCutoverError } from './stable-logical-rpc-client' // status.get without ever changing connState, so a one-shot probe would latch // capability-gated UI hidden until the screen remounts; retry until one lands. const CUTOVER_RETRY_DELAY_MS = 250 +// Why a cap: a link that keeps forcing cutovers would otherwise poll status.get four times a second +// for as long as the screen is mounted. After the prompt attempts, cutovers join the backoff ladder. +const CUTOVER_FAST_RETRY_LIMIT = 3 const FAILURE_RETRY_BASE_DELAY_MS = 1_000 const FAILURE_RETRY_MAX_DELAY_MS = 15_000 @@ -43,6 +46,7 @@ export function startRuntimeCapabilityRead( let cancelled = false let retryTimer: ReturnType | null = null let failureRetries = 0 + let cutoverRetries = 0 function attempt(): void { void readCapabilities().then( @@ -64,9 +68,10 @@ export function startRuntimeCapabilityRead( function scheduleRetry(cutover: boolean): void { // Why: cutover means the replacement transport is already authenticated — // re-ask promptly; other failures back off so a wedged host isn't hammered. - const delay = cutover - ? CUTOVER_RETRY_DELAY_MS - : Math.min(FAILURE_RETRY_BASE_DELAY_MS * 2 ** failureRetries++, FAILURE_RETRY_MAX_DELAY_MS) + const delay = + cutover && cutoverRetries++ < CUTOVER_FAST_RETRY_LIMIT + ? CUTOVER_RETRY_DELAY_MS + : Math.min(FAILURE_RETRY_BASE_DELAY_MS * 2 ** failureRetries++, FAILURE_RETRY_MAX_DELAY_MS) retryTimer = setTimeout(attempt, delay) }