diff --git a/.changeset/fix-background-sync-resume.md b/.changeset/fix-background-sync-resume.md new file mode 100644 index 0000000000..592de0cd32 --- /dev/null +++ b/.changeset/fix-background-sync-resume.md @@ -0,0 +1,5 @@ +--- +default: patch +--- + +Fix new messages not arriving on mobile until the app was restarted, after it had been in the background. diff --git a/src/app/features/settings/notifications/UnifiedPushNotifications.ts b/src/app/features/settings/notifications/UnifiedPushNotifications.ts index a426cf1bd1..529f97fe11 100644 --- a/src/app/features/settings/notifications/UnifiedPushNotifications.ts +++ b/src/app/features/settings/notifications/UnifiedPushNotifications.ts @@ -1193,7 +1193,7 @@ export async function listenForUnifiedPushMessages(getSettings: () => Notificati const listener = await addPluginListener('notifications', 'push-message', (data: unknown) => { const notification = parseUnifiedPushMessage(data); if (!notification) return; - getSlidingSyncManager(getSettings().mx)?.resumeForPush(); + getSlidingSyncManager(getSettings().mx)?.requestPushDrain(); dispatch(notification); }); diff --git a/src/app/hooks/useBackgroundSyncPause.test.tsx b/src/app/hooks/useBackgroundSyncPause.test.tsx deleted file mode 100644 index 7e6d0c664d..0000000000 --- a/src/app/hooks/useBackgroundSyncPause.test.tsx +++ /dev/null @@ -1,151 +0,0 @@ -import { describe, expect, it, vi, beforeEach } from 'vitest'; -import { renderHook } from '@testing-library/react'; - -const { - pause, - resume, - getSlidingSyncManager, - listen, - listenOff, - mockIsTauri, - mockIsMobileTauri, - mockCallEmbed, -} = vi.hoisted(() => ({ - pause: vi.fn<() => void>(), - resume: vi.fn<() => void>(), - getSlidingSyncManager: vi.fn<() => unknown>(), - listen: vi.fn<(_event: string, _cb: () => void) => Promise<() => void>>(), - listenOff: vi.fn<() => void>(), - mockIsTauri: { value: false }, - mockIsMobileTauri: { value: true }, - mockCallEmbed: { value: undefined as unknown }, -})); - -vi.mock('$client/initMatrix', () => ({ getSlidingSyncManager })); - -vi.mock('jotai', () => ({ - useAtomValue: () => mockCallEmbed.value, - atom: vi.fn<() => unknown>(), -})); - -vi.mock('../state/callEmbed', () => ({ callEmbedAtom: {} })); - -vi.mock('@tauri-apps/api/event', () => ({ - listen, - TauriEvent: { WINDOW_RESUMED: 'tauri://resumed' }, -})); - -vi.mock('@tauri-apps/api/core', () => ({ isTauri: () => mockIsTauri.value })); - -vi.mock('$utils/platform', () => ({ isMobileTauri: () => mockIsMobileTauri.value })); - -import { useBackgroundSyncPause } from './useBackgroundSyncPause'; - -const setVisibility = (state: 'visible' | 'hidden') => - vi.spyOn(document, 'visibilityState', 'get').mockReturnValue(state); - -describe('useBackgroundSyncPause', () => { - beforeEach(() => { - vi.restoreAllMocks(); - pause.mockReset(); - resume.mockReset(); - listenOff.mockReset(); - listen.mockReset().mockResolvedValue(listenOff); - getSlidingSyncManager.mockReset().mockReturnValue({ pause, resume }); - mockIsTauri.value = false; - mockIsMobileTauri.value = true; - mockCallEmbed.value = undefined; - }); - - it('pauses sync when the app is backgrounded', () => { - setVisibility('visible'); - renderHook(() => useBackgroundSyncPause({ clientRunning: true } as never)); - - setVisibility('hidden'); - document.dispatchEvent(new Event('visibilitychange')); - - expect(pause).toHaveBeenCalled(); - }); - - it('resumes sync when the app returns to the foreground', () => { - setVisibility('hidden'); - renderHook(() => useBackgroundSyncPause({ clientRunning: true } as never)); - pause.mockClear(); - - setVisibility('visible'); - document.dispatchEvent(new Event('visibilitychange')); - - expect(resume).toHaveBeenCalled(); - }); - - it('keeps polling in the background during a call', () => { - mockCallEmbed.value = { dispose: vi.fn<() => void>() }; - setVisibility('hidden'); - - renderHook(() => useBackgroundSyncPause({ clientRunning: true } as never)); - - expect(pause).not.toHaveBeenCalled(); - expect(resume).toHaveBeenCalled(); - }); - - it('resumes on tauri://resumed without waiting for visibilitychange', () => { - mockIsTauri.value = true; - setVisibility('hidden'); - renderHook(() => useBackgroundSyncPause({ clientRunning: true } as never)); - resume.mockClear(); - - const [, cb] = listen.mock.calls[0] as [string, () => void]; - cb(); - - expect(resume).toHaveBeenCalled(); - }); - - it('resumes on window focus when visibilitychange was missed', () => { - setVisibility('hidden'); - renderHook(() => useBackgroundSyncPause({ clientRunning: true } as never)); - resume.mockClear(); - - setVisibility('visible'); - window.dispatchEvent(new Event('focus')); - - expect(resume).toHaveBeenCalled(); - }); - - it('does not resume on a focus event while still hidden', () => { - setVisibility('hidden'); - renderHook(() => useBackgroundSyncPause({ clientRunning: true } as never)); - resume.mockClear(); - - window.dispatchEvent(new Event('focus')); - - expect(resume).not.toHaveBeenCalled(); - }); - - it('keeps polling when a browser tab is hidden', () => { - mockIsMobileTauri.value = false; - setVisibility('visible'); - renderHook(() => useBackgroundSyncPause({ clientRunning: true } as never)); - - setVisibility('hidden'); - document.dispatchEvent(new Event('visibilitychange')); - - expect(pause).not.toHaveBeenCalled(); - }); - - it('does nothing without a client', () => { - renderHook(() => useBackgroundSyncPause(undefined)); - - expect(pause).not.toHaveBeenCalled(); - expect(resume).not.toHaveBeenCalled(); - }); - - it('resumes on unmount so a paused transport is never left parked', () => { - setVisibility('hidden'); - const { unmount } = renderHook(() => useBackgroundSyncPause({ clientRunning: true } as never)); - resume.mockClear(); - - unmount(); - - expect(resume).toHaveBeenCalled(); - }); -}); diff --git a/src/app/hooks/useBackgroundSyncPause.ts b/src/app/hooks/useBackgroundSyncPause.ts deleted file mode 100644 index e0da083416..0000000000 --- a/src/app/hooks/useBackgroundSyncPause.ts +++ /dev/null @@ -1,50 +0,0 @@ -import { useEffect } from 'react'; -import { useAtomValue } from 'jotai'; -import { TauriEvent, listen } from '@tauri-apps/api/event'; -import { isTauri } from '@tauri-apps/api/core'; -import type { MatrixClient } from '$types/matrix-sdk'; -import { getSlidingSyncManager } from '$client/initMatrix'; -import { isMobileTauri } from '$utils/platform'; -import { callEmbedAtom } from '../state/callEmbed'; - -/** - * Stop polling while the app is backgrounded. wry only calls Android `WebView.onPause()`, - * which does not pause JavaScript, so the long-poll otherwise runs until the OS freezes - * the process. Driven by `visibilitychange` rather than `tauri://suspended`, which on iOS - * maps to applicationWillResignActive. - */ -export const useBackgroundSyncPause = (mx: MatrixClient | undefined): void => { - const callEmbed = useAtomValue(callEmbedAtom); - const callActive = callEmbed !== undefined; - - useEffect(() => { - if (!mx || !isMobileTauri()) return undefined; - - const resume = () => getSlidingSyncManager(mx)?.resume(); - const applyVisibility = () => { - const manager = getSlidingSyncManager(mx); - if (!manager) return; - if (document.visibilityState === 'hidden' && !callActive) manager.pause(); - else manager.resume(); - }; - - // Nothing re-arms a paused transport, so a missed `visible` event strands it until a - // restart. `focus` also fires in the Android webview. - const resumeIfVisible = () => { - if (document.visibilityState !== 'hidden') resume(); - }; - - document.addEventListener('visibilitychange', applyVisibility); - window.addEventListener('focus', resumeIfVisible); - const unlisten = isTauri() ? listen(TauriEvent.WINDOW_RESUMED, resume) : undefined; - - applyVisibility(); - - return () => { - document.removeEventListener('visibilitychange', applyVisibility); - window.removeEventListener('focus', resumeIfVisible); - unlisten?.then((off) => off()); - resume(); - }; - }, [mx, callActive]); -}; diff --git a/src/app/hooks/useSyncOrchestrator.test.tsx b/src/app/hooks/useSyncOrchestrator.test.tsx new file mode 100644 index 0000000000..1e7e5303f4 --- /dev/null +++ b/src/app/hooks/useSyncOrchestrator.test.tsx @@ -0,0 +1,231 @@ +import { describe, expect, it, vi, beforeEach } from 'vitest'; +import { act, renderHook } from '@testing-library/react'; + +const { + pause, + resume, + getSlidingSyncManager, + listen, + listenOff, + mockIsTauri, + mockIsMobileTauri, + mockCallEmbed, + transport, +} = vi.hoisted(() => ({ + pause: vi.fn<() => void>(), + resume: vi.fn<() => void>(), + getSlidingSyncManager: vi.fn<() => unknown>(), + listen: vi.fn<(_event: string, _cb: () => void) => Promise<() => void>>(), + listenOff: vi.fn<() => void>(), + mockIsTauri: { value: false }, + mockIsMobileTauri: { value: true }, + mockCallEmbed: { value: undefined as unknown }, + transport: { paused: false, draining: false, listeners: new Set<() => void>() }, +})); + +vi.mock('$client/initMatrix', () => ({ getSlidingSyncManager })); + +vi.mock('jotai', () => ({ + useAtomValue: () => mockCallEmbed.value, + atom: vi.fn<() => unknown>(), +})); + +vi.mock('../state/callEmbed', () => ({ callEmbedAtom: {} })); + +vi.mock('@tauri-apps/api/event', () => ({ + listen, + TauriEvent: { WINDOW_RESUMED: 'tauri://resumed' }, +})); + +vi.mock('@tauri-apps/api/core', () => ({ isTauri: () => mockIsTauri.value })); + +vi.mock('$utils/platform', () => ({ isMobileTauri: () => mockIsMobileTauri.value })); + +import { useSyncOrchestrator } from './useSyncOrchestrator'; + +const STOP_DELAY_MS = 3000; + +const setVisibility = (state: 'visible' | 'hidden') => + vi.spyOn(document, 'visibilityState', 'get').mockReturnValue(state); + +const setTransport = (next: { paused?: boolean; draining?: boolean }) => { + Object.assign(transport, next); + act(() => { + transport.listeners.forEach((listener) => listener()); + }); +}; + +const client = { clientRunning: true } as never; + +describe('useSyncOrchestrator', () => { + beforeEach(() => { + vi.restoreAllMocks(); + vi.useFakeTimers(); + pause.mockReset().mockImplementation(() => setTransport({ paused: true })); + resume.mockReset().mockImplementation(() => setTransport({ paused: false })); + listenOff.mockReset(); + listen.mockReset().mockResolvedValue(listenOff); + transport.paused = false; + transport.draining = false; + transport.listeners.clear(); + getSlidingSyncManager.mockReset().mockReturnValue({ + pause, + resume, + isPaused: () => transport.paused, + isDrainingPush: () => transport.draining, + onTransportStateChange: (listener: () => void) => { + transport.listeners.add(listener); + return () => transport.listeners.delete(listener); + }, + }); + mockIsTauri.value = false; + mockIsMobileTauri.value = true; + mockCallEmbed.value = undefined; + }); + + it('stops the transport once the app has been hidden for the grace period', () => { + setVisibility('visible'); + renderHook(() => useSyncOrchestrator(client)); + + setVisibility('hidden'); + act(() => { + document.dispatchEvent(new Event('visibilitychange')); + }); + expect(pause).not.toHaveBeenCalled(); + + act(() => vi.advanceTimersByTime(STOP_DELAY_MS)); + expect(pause).toHaveBeenCalled(); + }); + + it('does not stop for a glance at another app', () => { + setVisibility('visible'); + renderHook(() => useSyncOrchestrator(client)); + + setVisibility('hidden'); + act(() => { + document.dispatchEvent(new Event('visibilitychange')); + }); + act(() => vi.advanceTimersByTime(STOP_DELAY_MS - 500)); + setVisibility('visible'); + act(() => { + document.dispatchEvent(new Event('visibilitychange')); + }); + act(() => vi.advanceTimersByTime(STOP_DELAY_MS)); + + expect(pause).not.toHaveBeenCalled(); + }); + + it('starts immediately when the app becomes visible', () => { + setVisibility('hidden'); + transport.paused = true; + renderHook(() => useSyncOrchestrator(client)); + resume.mockClear(); + + setVisibility('visible'); + act(() => { + document.dispatchEvent(new Event('visibilitychange')); + }); + + expect(resume).toHaveBeenCalled(); + }); + + it('starts a parked transport when every visibility event is missed', () => { + setVisibility('hidden'); + transport.paused = true; + renderHook(() => useSyncOrchestrator(client)); + resume.mockClear(); + + setVisibility('visible'); + act(() => vi.advanceTimersByTime(2000)); + + expect(resume).toHaveBeenCalled(); + }); + + it('starts a parked transport for a push drain while still hidden', () => { + setVisibility('hidden'); + transport.paused = true; + renderHook(() => useSyncOrchestrator(client)); + resume.mockClear(); + + setTransport({ draining: true }); + + expect(resume).toHaveBeenCalled(); + }); + + it('parks again once the push drain settles', () => { + setVisibility('hidden'); + transport.draining = true; + renderHook(() => useSyncOrchestrator(client)); + + setTransport({ draining: false }); + act(() => vi.advanceTimersByTime(STOP_DELAY_MS)); + + expect(pause).toHaveBeenCalled(); + }); + + it('keeps polling in the background during a call', () => { + mockCallEmbed.value = { dispose: vi.fn<() => void>() }; + setVisibility('hidden'); + + renderHook(() => useSyncOrchestrator(client)); + act(() => vi.advanceTimersByTime(STOP_DELAY_MS)); + + expect(pause).not.toHaveBeenCalled(); + }); + + it('starts a parked transport even when the browser reports offline', () => { + vi.spyOn(navigator, 'onLine', 'get').mockReturnValue(false); + setVisibility('visible'); + transport.paused = true; + + renderHook(() => useSyncOrchestrator(client)); + + expect(resume).toHaveBeenCalled(); + }); + + it('starts on tauri://resumed without waiting for visibilitychange', async () => { + mockIsTauri.value = true; + setVisibility('hidden'); + transport.paused = true; + renderHook(() => useSyncOrchestrator(client)); + await act(async () => { + await Promise.resolve(); + }); + resume.mockClear(); + + const resumedCallback = listen.mock.calls.find(([event]) => event === 'tauri://resumed')?.[1]; + setVisibility('visible'); + act(() => resumedCallback?.()); + + expect(resume).toHaveBeenCalled(); + }); + + it('keeps polling when a browser tab is hidden', () => { + mockIsMobileTauri.value = false; + setVisibility('hidden'); + + renderHook(() => useSyncOrchestrator(client)); + act(() => vi.advanceTimersByTime(STOP_DELAY_MS)); + + expect(pause).not.toHaveBeenCalled(); + }); + + it('does nothing without a client', () => { + renderHook(() => useSyncOrchestrator(undefined)); + act(() => vi.advanceTimersByTime(STOP_DELAY_MS)); + + expect(pause).not.toHaveBeenCalled(); + expect(resume).not.toHaveBeenCalled(); + }); + + it('starts the transport on unmount so it is never left parked', () => { + setVisibility('hidden'); + transport.paused = true; + const { unmount } = renderHook(() => useSyncOrchestrator(client)); + resume.mockClear(); + + unmount(); + + expect(resume).toHaveBeenCalled(); + }); +}); diff --git a/src/app/hooks/useSyncOrchestrator.ts b/src/app/hooks/useSyncOrchestrator.ts new file mode 100644 index 0000000000..ac23a04d57 --- /dev/null +++ b/src/app/hooks/useSyncOrchestrator.ts @@ -0,0 +1,87 @@ +import { useCallback, useEffect, useSyncExternalStore } from 'react'; +import { useAtomValue } from 'jotai'; +import { TauriEvent, listen } from '@tauri-apps/api/event'; +import { isTauri } from '@tauri-apps/api/core'; +import type { MatrixClient } from '$types/matrix-sdk'; +import { getSlidingSyncManager } from '$client/initMatrix'; +import { nextSyncAction } from '$client/syncActivity'; +import { isMobileTauri } from '$utils/platform'; +import { callEmbedAtom } from '../state/callEmbed'; + +const VISIBILITY_RECHECK_MS = 2000; + +const STOP_DELAY_MS = 3000; + +const noopUnsubscribe = () => {}; + +const subscribeToVisibility = (onChange: () => void): (() => void) => { + document.addEventListener('visibilitychange', onChange); + window.addEventListener('focus', onChange); + window.addEventListener('pageshow', onChange); + const recheck = window.setInterval(onChange, VISIBILITY_RECHECK_MS); + + let cancelled = false; + let unlistenResumed: (() => void) | undefined; + if (isTauri()) { + void listen(TauriEvent.WINDOW_RESUMED, onChange).then((off) => { + if (cancelled) off(); + else unlistenResumed = off; + }); + } + + return () => { + cancelled = true; + document.removeEventListener('visibilitychange', onChange); + window.removeEventListener('focus', onChange); + window.removeEventListener('pageshow', onChange); + window.clearInterval(recheck); + unlistenResumed?.(); + }; +}; + +const getVisible = () => document.visibilityState !== 'hidden'; + +export const useSyncOrchestrator = (mx: MatrixClient | undefined): void => { + const callActive = useAtomValue(callEmbedAtom) !== undefined; + + const visible = useSyncExternalStore(subscribeToVisibility, getVisible); + + const subscribeToTransport = useCallback( + (onChange: () => void) => + mx + ? (getSlidingSyncManager(mx)?.onTransportStateChange(onChange) ?? noopUnsubscribe) + : noopUnsubscribe, + [mx] + ); + const paused = useSyncExternalStore( + subscribeToTransport, + useCallback(() => (mx ? (getSlidingSyncManager(mx)?.isPaused() ?? false) : false), [mx]) + ); + const drainingPush = useSyncExternalStore( + subscribeToTransport, + useCallback(() => (mx ? (getSlidingSyncManager(mx)?.isDrainingPush() ?? false) : false), [mx]) + ); + + useEffect(() => { + if (!mx || !isMobileTauri()) return undefined; + const manager = getSlidingSyncManager(mx); + if (!manager) return undefined; + + const action = nextSyncAction(paused, { visible, callActive, drainingPush }); + if (action === 'none') return undefined; + if (action === 'start') { + manager.resume(); + return undefined; + } + + const timer = window.setTimeout(() => manager.pause(), STOP_DELAY_MS); + return () => window.clearTimeout(timer); + }, [mx, paused, visible, callActive, drainingPush]); + + useEffect( + () => () => { + if (mx) getSlidingSyncManager(mx)?.resume(); + }, + [mx] + ); +}; diff --git a/src/app/pages/client/ClientRoot.tsx b/src/app/pages/client/ClientRoot.tsx index 5ffa201a52..1d9506ab59 100644 --- a/src/app/pages/client/ClientRoot.tsx +++ b/src/app/pages/client/ClientRoot.tsx @@ -44,7 +44,7 @@ import { useSyncNicknames } from '$hooks/useNickname'; import { useAppVisibility } from '$hooks/useAppVisibility'; import { useNetworkRecovery } from '$hooks/useNetworkRecovery'; import { useLoopbackMediaRecovery } from '$hooks/useLoopbackMediaRecovery'; -import { useBackgroundSyncPause } from '$hooks/useBackgroundSyncPause'; +import { useSyncOrchestrator } from '$hooks/useSyncOrchestrator'; import { composerIcon, DotsThreeOutlineVerticalIcon } from '$components/icons/phosphor'; import { getHomePath } from '$pages/pathUtils'; import { DIRECT_ROOM_PATH, HOME_ROOM_PATH, SPACE_ROOM_PATH } from '$pages/paths'; @@ -373,7 +373,7 @@ export function ClientRoot({ children }: ClientRootProps) { useLogoutListener(mx, activeSession); useAppVisibility(mx); useNetworkRecovery(mx); - useBackgroundSyncPause(mx); + useSyncOrchestrator(startState.status === AsyncStatus.Success ? mx : undefined); useLoopbackMediaRecovery(); useCrossSigningResetDetect(mx); diff --git a/src/client/reconnect.test.ts b/src/client/reconnect.test.ts index ecbd04f986..5aff647f87 100644 --- a/src/client/reconnect.test.ts +++ b/src/client/reconnect.test.ts @@ -1,9 +1,15 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; -const { slidingSyncResend, getSlidingSyncManager } = vi.hoisted(() => { +const { slidingSyncResend, getSlidingSyncManager, isPaused } = vi.hoisted(() => { const resend = vi.fn<() => void>(); - const manager = vi.fn<(mx: unknown) => { slidingSync: { resend: typeof resend } } | undefined>(); - return { slidingSyncResend: resend, getSlidingSyncManager: manager }; + const paused = vi.fn<() => boolean>(); + const manager = + vi.fn< + ( + mx: unknown + ) => { slidingSync: { resend: typeof resend }; isPaused: typeof paused } | undefined + >(); + return { slidingSyncResend: resend, getSlidingSyncManager: manager, isPaused: paused }; }); vi.mock('./initMatrix', () => ({ @@ -35,10 +41,42 @@ describe('nudgeReconnect', () => { vi.setSystemTime(0); slidingSyncResend.mockReset(); getSlidingSyncManager.mockReset(); + isPaused.mockReset().mockReturnValue(false); + }); + + it('does not nudge a transport the orchestrator has parked', () => { + isPaused.mockReturnValue(true); + getSlidingSyncManager.mockReturnValue({ + slidingSync: { resend: slidingSyncResend }, + isPaused, + } as never); + const mx = stubMx(); + + expect(nudgeReconnect(mx as never, 'stalled')).toBe(false); + expect(slidingSyncResend).not.toHaveBeenCalled(); + expect(mx.retryImmediately).not.toHaveBeenCalled(); + }); + + it('does not spend the throttle window on a parked transport', () => { + getSlidingSyncManager.mockReturnValue({ + slidingSync: { resend: slidingSyncResend }, + isPaused, + } as never); + const mx = stubMx(); + + isPaused.mockReturnValue(true); + nudgeReconnect(mx as never, 'stalled'); + isPaused.mockReturnValue(false); + + expect(nudgeReconnect(mx as never, 'visible')).toBe(true); + expect(slidingSyncResend).toHaveBeenCalledOnce(); }); it('calls slidingSync.resend() when a sliding-sync manager exists', () => { - getSlidingSyncManager.mockReturnValue({ slidingSync: { resend: slidingSyncResend } } as never); + getSlidingSyncManager.mockReturnValue({ + slidingSync: { resend: slidingSyncResend }, + isPaused, + } as never); const mx = stubMx(); const result = nudgeReconnect(mx as never, 'online'); @@ -68,7 +106,10 @@ describe('nudgeReconnect', () => { }); it('returns false and does nothing when client is not running', () => { - getSlidingSyncManager.mockReturnValue({ slidingSync: { resend: slidingSyncResend } } as never); + getSlidingSyncManager.mockReturnValue({ + slidingSync: { resend: slidingSyncResend }, + isPaused, + } as never); const mx = stubMx({ clientRunning: false }); const result = nudgeReconnect(mx as never, 'visible'); @@ -79,7 +120,10 @@ describe('nudgeReconnect', () => { }); it('throttles back-to-back calls within 3s', () => { - getSlidingSyncManager.mockReturnValue({ slidingSync: { resend: slidingSyncResend } } as never); + getSlidingSyncManager.mockReturnValue({ + slidingSync: { resend: slidingSyncResend }, + isPaused, + } as never); const mx = stubMx(); const first = nudgeReconnect(mx as never, 'online'); @@ -91,7 +135,10 @@ describe('nudgeReconnect', () => { }); it('allows a nudge after throttle window expires', () => { - getSlidingSyncManager.mockReturnValue({ slidingSync: { resend: slidingSyncResend } } as never); + getSlidingSyncManager.mockReturnValue({ + slidingSync: { resend: slidingSyncResend }, + isPaused, + } as never); const mx = stubMx(); nudgeReconnect(mx as never, 'online'); @@ -104,7 +151,10 @@ describe('nudgeReconnect', () => { }); it('throttles per-client (two distinct clients each get their nudge)', () => { - getSlidingSyncManager.mockReturnValue({ slidingSync: { resend: slidingSyncResend } } as never); + getSlidingSyncManager.mockReturnValue({ + slidingSync: { resend: slidingSyncResend }, + isPaused, + } as never); const mx1 = stubMx({ retryImmediately: vi.fn<() => boolean>() }); const mx2 = stubMx({ retryImmediately: vi.fn<() => boolean>() }); diff --git a/src/client/reconnect.ts b/src/client/reconnect.ts index e6e3dbf52a..639bacd937 100644 --- a/src/client/reconnect.ts +++ b/src/client/reconnect.ts @@ -21,12 +21,15 @@ export const nudgeReconnect = ( ): boolean => { if (!mx.clientRunning) return false; + const manager = getSlidingSyncManager(mx); + if (manager?.isPaused()) return false; + const now = Date.now(); const last = lastNudgeAt.get(mx); if (!opts?.force && last !== undefined && now - last < NUDGE_THROTTLE_MS) return false; lastNudgeAt.set(mx, now); - const slidingSync = getSlidingSyncManager(mx)?.slidingSync; + const slidingSync = manager?.slidingSync; let nudged: boolean; if (slidingSync) { slidingSync.resend(); diff --git a/src/client/slidingSync.test.ts b/src/client/slidingSync.test.ts index bfcee2a9c3..78dce3efb6 100644 --- a/src/client/slidingSync.test.ts +++ b/src/client/slidingSync.test.ts @@ -1815,70 +1815,71 @@ describe('SlidingSyncManager pause/resume', () => { await expect(manager.waitForResume()).resolves.toBeUndefined(); }); - it('resumeForPush() lifts the pause and parks once to-device comes back empty', () => { + it('requestPushDrain() flags a drain that clears once to-device comes back empty', () => { const manager = makeManager(makeMockMx()); manager.attach(); manager.pause(); - expect(manager.resumeForPush()).toBe(true); - expect(manager.isPaused()).toBe(false); + manager.requestPushDrain(); + expect(manager.isDrainingPush()).toBe(true); fireLifecycle(SlidingSyncState.Complete, { extensions: { to_device: { events: [] } } }); - expect(manager.isPaused()).toBe(true); + expect(manager.isDrainingPush()).toBe(false); }); it('keeps draining while to-device still carries events', () => { const manager = makeManager(makeMockMx()); manager.attach(); - manager.pause(); - manager.resumeForPush(); + manager.requestPushDrain(); const withKeys = { extensions: { to_device: { events: [{ type: 'm.room.key' }] } } }; fireLifecycle(SlidingSyncState.Complete, withKeys); - expect(manager.isPaused()).toBe(false); + expect(manager.isDrainingPush()).toBe(true); fireLifecycle(SlidingSyncState.Complete, { extensions: { to_device: { events: [] } } }); - expect(manager.isPaused()).toBe(true); + expect(manager.isDrainingPush()).toBe(false); }); - it('parks a never-draining queue once the poll budget runs out', () => { + it('gives up on a never-draining queue once the poll budget runs out', () => { const MAX_PUSH_DRAIN_POLLS = 5; const manager = makeManager(makeMockMx()); manager.attach(); - manager.pause(); - manager.resumeForPush(); + manager.requestPushDrain(); const withKeys = { extensions: { to_device: { events: [{ type: 'm.room.key' }] } } }; for (let i = 0; i < MAX_PUSH_DRAIN_POLLS; i += 1) { - expect(manager.isPaused()).toBe(false); + expect(manager.isDrainingPush()).toBe(true); fireLifecycle(SlidingSyncState.Complete, withKeys); } - expect(manager.isPaused()).toBe(true); + expect(manager.isDrainingPush()).toBe(false); }); - it('lets a real resume outrank an in-flight drain', () => { + it('does not park the transport when a drain settles', () => { const manager = makeManager(makeMockMx()); manager.attach(); - manager.pause(); - manager.resumeForPush(); + manager.requestPushDrain(); - manager.resume(); fireLifecycle(SlidingSyncState.Complete, { extensions: { to_device: { events: [] } } }); expect(manager.isPaused()).toBe(false); }); - it('resumeForPush() is a no-op when sync is already running', () => { + it('reports pause and drain transitions to transport-state listeners', () => { const manager = makeManager(makeMockMx()); manager.attach(); + const listener = vi.fn<() => void>(); + const unsubscribe = manager.onTransportStateChange(listener); - expect(manager.resumeForPush()).toBe(false); - - fireLifecycle(SlidingSyncState.Complete, { extensions: { to_device: { events: [] } } }); + manager.pause(); + manager.resume(); + manager.requestPushDrain(); + expect(listener).toHaveBeenCalledTimes(3); - expect(manager.isPaused()).toBe(false); + unsubscribe(); + manager.pause(); + expect(listener).toHaveBeenCalledTimes(3); }); it('silences the poll watchdog while paused', () => { diff --git a/src/client/slidingSync.ts b/src/client/slidingSync.ts index 58ebef3a34..0eb9282259 100644 --- a/src/client/slidingSync.ts +++ b/src/client/slidingSync.ts @@ -718,6 +718,8 @@ export class SlidingSyncManager { private readonly resumeWaiters = new Set<() => void>(); + private readonly transportStateListeners = new Set<() => void>(); + /** Span covering the period from attach() to the first successful complete cycle. */ private initialSyncSpan: ReturnType | null = null; @@ -1019,13 +1021,13 @@ export class SlidingSyncManager { * instead. */ public pause(): void { - this.pushDrainPollsLeft = 0; if (this.paused || this.disposed) return; this.paused = true; globalThis.clearTimeout(this.pollWatchdogTimer); this.pollWatchdogTimer = undefined; this.slidingSync.resend(); debugLog.info('sync', 'Sliding sync paused'); + this.notifyTransportState(); } private liftPause(): void { @@ -1035,19 +1037,17 @@ export class SlidingSyncManager { } public resume(): void { - this.pushDrainPollsLeft = 0; if (!this.paused) return; this.liftPause(); debugLog.info('sync', 'Sliding sync resumed'); + this.notifyTransportState(); } - /** Poll on while backgrounded, then park: to-device only arrives over a live `/sync`. */ - public resumeForPush(): boolean { - if (this.disposed || !this.paused) return false; + public requestPushDrain(): void { + if (this.disposed || this.pushDrainPollsLeft === MAX_PUSH_DRAIN_POLLS) return; this.pushDrainPollsLeft = MAX_PUSH_DRAIN_POLLS; - this.liftPause(); - debugLog.info('sync', 'Sliding sync resumed to drain to-device after a push'); - return true; + debugLog.info('sync', 'Sliding sync asked to drain to-device after a push'); + this.notifyTransportState(); } private settlePushDrain(resp: MSC3575SlidingSyncResponse): void { @@ -1056,13 +1056,29 @@ export class SlidingSyncManager { const drained = (toDevice?.events?.length ?? 0) === 0; this.pushDrainPollsLeft -= 1; if (!drained && this.pushDrainPollsLeft > 0) return; - this.pause(); + this.pushDrainPollsLeft = 0; + this.notifyTransportState(); } public isPaused(): boolean { return this.paused; } + public isDrainingPush(): boolean { + return this.pushDrainPollsLeft > 0; + } + + public onTransportStateChange(listener: () => void): () => void { + this.transportStateListeners.add(listener); + return () => { + this.transportStateListeners.delete(listener); + }; + } + + private notifyTransportState(): void { + this.transportStateListeners.forEach((listener) => listener()); + } + /** Resolves on the next resume(), or immediately when not paused. */ public waitForResume(): Promise { if (!this.paused) return Promise.resolve(); @@ -1107,6 +1123,7 @@ export class SlidingSyncManager { this.disposed = true; this.paused = false; this.pushDrainPollsLeft = 0; + this.transportStateListeners.clear(); this.releaseResumeWaiters(); globalThis.clearTimeout(this.pollWatchdogTimer); this.pollWatchdogTimer = undefined; diff --git a/src/client/syncActivity.test.ts b/src/client/syncActivity.test.ts new file mode 100644 index 0000000000..f8ea23398a --- /dev/null +++ b/src/client/syncActivity.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from 'vitest'; +import { type SyncActivity, nextSyncAction } from './syncActivity'; + +const activity = (overrides: Partial = {}): SyncActivity => ({ + visible: false, + callActive: false, + drainingPush: false, + ...overrides, +}); + +describe('nextSyncAction', () => { + it('stops a running transport once nothing needs it', () => { + expect(nextSyncAction(false, activity())).toBe('stop'); + }); + + it.each([ + ['visible', { visible: true }], + ['a call is active', { callActive: true }], + ['a push is draining', { drainingPush: true }], + ])('starts a parked transport when %s', (_label, overrides) => { + expect(nextSyncAction(true, activity(overrides))).toBe('start'); + }); + + it.each([ + ['visible', { visible: true }], + ['a call is active', { callActive: true }], + ['a push is draining', { drainingPush: true }], + ])('keeps a running transport while %s', (_label, overrides) => { + expect(nextSyncAction(false, activity(overrides))).toBe('none'); + }); + + it('starts a parked transport without consulting connectivity', () => { + expect(nextSyncAction(true, activity({ visible: true }))).toBe('start'); + }); + + it('reaches the same answer whichever input changed', () => { + expect(nextSyncAction(true, activity({ visible: true }))).toBe( + nextSyncAction(true, activity({ drainingPush: true })) + ); + }); +}); diff --git a/src/client/syncActivity.ts b/src/client/syncActivity.ts new file mode 100644 index 0000000000..a791ba873c --- /dev/null +++ b/src/client/syncActivity.ts @@ -0,0 +1,14 @@ +export type SyncActivity = { + visible: boolean; + callActive: boolean; + drainingPush: boolean; +}; + +export type SyncAction = 'start' | 'stop' | 'none'; + +export const nextSyncAction = (paused: boolean, activity: SyncActivity): SyncAction => { + const wanted = activity.visible || activity.callActive || activity.drainingPush; + if (!paused && !wanted) return 'stop'; + if (paused && wanted) return 'start'; + return 'none'; +};