From 77e79dfbd8f1096b1332c4d221b1bc8496f6fe50 Mon Sep 17 00:00:00 2001 From: Erwan Leboucher Date: Sat, 1 Aug 2026 12:27:01 +0200 Subject: [PATCH 1/2] fix(media): serve the newest media request first and cache picker images in session --- package.json | 1 + pnpm-lock.yaml | 3 + src-tauri/src/network/media_protocol.rs | 57 +++++---- src-tauri/src/network/media_protocol/lane.rs | 101 +++++++++++++++ src/app/components/emoji-board/EmojiBoard.tsx | 2 +- .../emoji-board/components/Item.tsx | 4 + src/app/components/media/Image.tauri.test.tsx | 65 ++++++++++ src/app/components/media/Image.tsx | 35 +++++- src/app/hooks/useTauriMediaObjectUrl.test.ts | 116 ++++++++++++++++++ src/app/hooks/useTauriMediaObjectUrl.ts | 45 +++++++ src/app/hooks/useUserProfile.test.tsx | 35 ++++++ src/app/hooks/useUserProfile.ts | 3 +- src/app/utils/mediaCache.ts | 3 + src/app/utils/mediaConcurrency.test.ts | 32 ++++- src/app/utils/mediaConcurrency.ts | 4 +- src/app/utils/mediaObjectUrlCache.test.ts | 58 +++++++++ src/app/utils/mediaObjectUrlCache.ts | 59 +++++++++ src/app/utils/mediaUrl.ts | 18 +++ src/test/objectUrlStub.ts | 16 +++ 19 files changed, 627 insertions(+), 30 deletions(-) create mode 100644 src-tauri/src/network/media_protocol/lane.rs create mode 100644 src/app/components/media/Image.tauri.test.tsx create mode 100644 src/app/hooks/useTauriMediaObjectUrl.test.ts create mode 100644 src/app/hooks/useTauriMediaObjectUrl.ts create mode 100644 src/app/utils/mediaObjectUrlCache.test.ts create mode 100644 src/app/utils/mediaObjectUrlCache.ts create mode 100644 src/test/objectUrlStub.ts diff --git a/package.json b/package.json index 1cc987ce9f..ac8a069494 100644 --- a/package.json +++ b/package.json @@ -47,6 +47,7 @@ "@fontsource-variable/nunito": "5.2.7", "@fontsource/space-mono": "5.2.9", "@lottiefiles/dotlottie-react": "^0.12.0", + "@lottiefiles/dotlottie-web": "0.40.1", "@noble/hashes": "^2.2.0", "@phosphor-icons/react": "^2.1.10", "@sableclient/twemoji-font": "^1.0.4", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3ca5c94739..6a1617d982 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -46,6 +46,9 @@ importers: '@lottiefiles/dotlottie-react': specifier: ^0.12.0 version: 0.12.3(react@18.3.1) + '@lottiefiles/dotlottie-web': + specifier: 0.40.1 + version: 0.40.1 '@noble/hashes': specifier: ^2.2.0 version: 2.2.0 diff --git a/src-tauri/src/network/media_protocol.rs b/src-tauri/src/network/media_protocol.rs index a64aef577b..3026bb018e 100644 --- a/src-tauri/src/network/media_protocol.rs +++ b/src-tauri/src/network/media_protocol.rs @@ -14,10 +14,12 @@ use tauri::{ }; mod crypto; +mod lane; mod response; mod session; use crypto::EncryptionStore; +use lane::{LanePermit, LifoLane}; use response::{ apply_cors_headers, error_response, ok_response, read_full, serve_range, serve_range_memory, session_unavailable_response, sniff_image_content_type, @@ -27,10 +29,7 @@ use tauri_plugin_http::reqwest::{ header::{AUTHORIZATION, CONTENT_TYPE}, Client, Url, }; -use tokio::{ - sync::{Mutex as AsyncMutex, Semaphore}, - time::Instant, -}; +use tokio::{sync::Mutex as AsyncMutex, time::Instant}; pub const MEDIA_URI_SCHEME: &str = "sable-media"; const MEDIA_SESSION_MARKER: &str = "__sable_media_session"; @@ -60,8 +59,8 @@ pub struct MediaSessionState { session_store: SessionStore, encryption: EncryptionStore, client: OnceLock, - thumbnail_semaphore: Semaphore, - download_semaphore: Semaphore, + thumbnail_lane: LifoLane, + download_lane: LifoLane, cache_miss_gates: Mutex>>>>, negative_cache: Mutex>, } @@ -72,8 +71,8 @@ impl Default for MediaSessionState { session_store: SessionStore::default(), encryption: EncryptionStore::default(), client: OnceLock::new(), - thumbnail_semaphore: Semaphore::new(MAX_CONCURRENT_THUMBNAIL_REQUESTS), - download_semaphore: Semaphore::new(MAX_CONCURRENT_DOWNLOAD_REQUESTS), + thumbnail_lane: LifoLane::new(MAX_CONCURRENT_THUMBNAIL_REQUESTS), + download_lane: LifoLane::new(MAX_CONCURRENT_DOWNLOAD_REQUESTS), cache_miss_gates: Mutex::new(HashMap::new()), negative_cache: Mutex::new(HashMap::new()), } @@ -488,23 +487,29 @@ async fn ensure_cached_with_limits( } // Thumbnails queue separately so a few large downloads cannot stall a painting timeline. -async fn acquire_lane<'a>( - state: &'a MediaSessionState, - media_url: &Url, -) -> Result, StatusCode> { - let semaphore = if is_thumbnail_request(media_url) { - &state.thumbnail_semaphore +async fn acquire_lane<'a>(state: &'a MediaSessionState, media_url: &Url) -> LanePermit<'a> { + let lane = if is_thumbnail_request(media_url) { + &state.thumbnail_lane } else { - &state.download_semaphore + &state.download_lane }; - semaphore - .acquire() - .await - .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR) + lane.acquire().await } fn is_thumbnail_request(media_url: &Url) -> bool { - media_url.path().contains("/thumbnail/") + let Some(segments) = media_url.path_segments() else { + return false; + }; + let mut after_media = segments.skip_while(|segment| *segment != "media").skip(1); + match after_media.next() { + // The legacy endpoints carry a version segment before the action. + Some(segment) if is_media_api_version(segment) => after_media.next() == Some("thumbnail"), + segment => segment == Some("thumbnail"), + } +} + +fn is_media_api_version(segment: &str) -> bool { + matches!(segment, "v1" | "v3" | "r0") } #[allow(clippy::too_many_arguments)] @@ -521,7 +526,7 @@ async fn fetch_and_cache( max_persistent_cache_bytes: u64, max_temp_cache_bytes: u64, ) -> Result<(String, Option>>, PathBuf), StatusCode> { - let permit = acquire_lane(state, &media_url).await?; + let permit = acquire_lane(state, &media_url).await; let mut upstream = state .client() @@ -1408,6 +1413,16 @@ mod tests { .unwrap(); assert!(super::is_thumbnail_request(&thumbnail)); assert!(!super::is_thumbnail_request(&download)); + + let legacy = + super::Url::parse("https://matrix.example.org/_matrix/media/v3/thumbnail/x/y").unwrap(); + assert!(super::is_thumbnail_request(&legacy)); + + // A media id spelled "thumbnail" is still a download. + let lookalike = + super::Url::parse("https://matrix.example.org/_matrix/media/v3/download/x/thumbnail") + .unwrap(); + assert!(!super::is_thumbnail_request(&lookalike)); } #[test] diff --git a/src-tauri/src/network/media_protocol/lane.rs b/src-tauri/src/network/media_protocol/lane.rs new file mode 100644 index 0000000000..7ace82b445 --- /dev/null +++ b/src-tauri/src/network/media_protocol/lane.rs @@ -0,0 +1,101 @@ +use std::sync::{Mutex, MutexGuard, PoisonError}; + +use tokio::sync::oneshot; + +/// Hands a freed slot to the newest waiter. Media is requested for what is on screen, so +/// the latest request is the one being waited on; `Semaphore` is fair, which buries a +/// just-opened picker behind the avatar backlog of a member list that started first. +pub(super) struct LifoLane { + inner: Mutex, +} + +struct LaneInner { + available: usize, + waiters: Vec>, +} + +impl LifoLane { + pub(super) fn new(permits: usize) -> Self { + Self { + inner: Mutex::new(LaneInner { + available: permits, + waiters: Vec::new(), + }), + } + } + + fn lock(&self) -> MutexGuard<'_, LaneInner> { + self.inner.lock().unwrap_or_else(PoisonError::into_inner) + } + + pub(super) async fn acquire(&self) -> LanePermit<'_> { + let receiver = { + let mut inner = self.lock(); + if inner.available > 0 { + inner.available -= 1; + return LanePermit { lane: self }; + } + let (sender, receiver) = oneshot::channel(); + inner.waiters.push(sender); + receiver + }; + + let _ = receiver.await; + LanePermit { lane: self } + } + + fn release(&self) { + let mut inner = self.lock(); + while let Some(waiter) = inner.waiters.pop() { + // A waiter that went away passes its slot to the next one down. + if waiter.send(()).is_ok() { + return; + } + } + inner.available += 1; + } +} + +pub(super) struct LanePermit<'a> { + lane: &'a LifoLane, +} + +impl Drop for LanePermit<'_> { + fn drop(&mut self) { + self.lane.release(); + } +} + +#[cfg(test)] +mod tests { + use std::sync::{Arc, Mutex}; + + use super::LifoLane; + + #[tokio::test] + async fn lane_serves_the_newest_waiter_first() { + let lane = Arc::new(LifoLane::new(1)); + let started = Arc::new(Mutex::new(Vec::new())); + + let held = lane.acquire().await; + + let mut queued = Vec::new(); + for id in 0..3 { + let lane = lane.clone(); + let started = started.clone(); + queued.push(tokio::spawn(async move { + let _permit = lane.acquire().await; + started.lock().unwrap().push(id); + })); + // Queue in a known order. + tokio::task::yield_now().await; + } + + drop(held); + for task in queued { + task.await.unwrap(); + } + + assert_eq!(*started.lock().unwrap(), vec![2, 1, 0]); + } +} diff --git a/src/app/components/emoji-board/EmojiBoard.tsx b/src/app/components/emoji-board/EmojiBoard.tsx index b273bd1a29..760c3e6c39 100644 --- a/src/app/components/emoji-board/EmojiBoard.tsx +++ b/src/app/components/emoji-board/EmojiBoard.tsx @@ -443,7 +443,7 @@ const SEARCH_OPTIONS: UseAsyncSearchOptions = { }, }; -const VIRTUAL_OVER_SCAN = 2; +const VIRTUAL_OVER_SCAN = 10; type EmojiBoardProps = { tab?: EmojiBoardTab; diff --git a/src/app/components/emoji-board/components/Item.tsx b/src/app/components/emoji-board/components/Item.tsx index ab69db393f..4c08a6179a 100644 --- a/src/app/components/emoji-board/components/Item.tsx +++ b/src/app/components/emoji-board/components/Item.tsx @@ -106,8 +106,10 @@ export function CustomEmojiItem({ > @@ -143,8 +145,10 @@ export function StickerItem({ > diff --git a/src/app/components/media/Image.tauri.test.tsx b/src/app/components/media/Image.tauri.test.tsx new file mode 100644 index 0000000000..cef78fc706 --- /dev/null +++ b/src/app/components/media/Image.tauri.test.tsx @@ -0,0 +1,65 @@ +// @vitest-environment jsdom +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import { stubObjectUrls } from '../../../test/objectUrlStub'; + +vi.mock('@tauri-apps/api/core', () => ({ + isTauri: () => true, +})); + +import { clearMediaObjectUrls } from '$utils/mediaObjectUrlCache'; +import { Image } from './Image'; + +const wrapMediaUrl = (target: string) => + `http://sable-media.localhost/${encodeURIComponent(target)}?__sable_media_cache=3`; + +const THUMBNAIL_URL = wrapMediaUrl('https://example.org/_matrix/media/v3/thumbnail/a/b'); +const DOWNLOAD_URL = wrapMediaUrl('https://example.org/_matrix/media/v3/download/a/b'); +const LOOKALIKE_URL = wrapMediaUrl('https://example.org/_matrix/media/v3/download/a/thumbnail.png'); + +beforeEach(() => { + stubObjectUrls(); + clearMediaObjectUrls(); + vi.spyOn(globalThis, 'fetch').mockImplementation(() => + Promise.resolve(new Response('image-bytes', { status: 200 })) + ); +}); + +afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllGlobals(); +}); + +describe('Image on Tauri', () => { + it('serves thumbnails from the session blob cache, fetching once across remounts', async () => { + const first = render(thumb); + await waitFor(() => expect(screen.getByAltText('thumb')).toHaveAttribute('src', 'blob:mock-1')); + first.unmount(); + + render(thumb-again); + expect(screen.getByAltText('thumb-again')).toHaveAttribute('src', 'blob:mock-1'); + expect(fetch).toHaveBeenCalledTimes(1); + }); + + it('keeps full-size downloads on the native scheme path', () => { + render(full); + + expect(screen.getByAltText('full')).toHaveAttribute('src', DOWNLOAD_URL); + expect(fetch).not.toHaveBeenCalled(); + }); + + it('does not treat a download named "thumbnail" as a thumbnail', () => { + render(lookalike); + + expect(screen.getByAltText('lookalike')).toHaveAttribute('src', LOOKALIKE_URL); + expect(fetch).not.toHaveBeenCalled(); + }); + + it('session-caches small picker downloads and skips files over the size gate', async () => { + render(emote); + await waitFor(() => expect(screen.getByAltText('emote')).toHaveAttribute('src', 'blob:mock-1')); + + render(big); + expect(screen.getByAltText('big')).toHaveAttribute('src', DOWNLOAD_URL); + }); +}); diff --git a/src/app/components/media/Image.tsx b/src/app/components/media/Image.tsx index e4a2ea68c2..5fdd2e94bb 100644 --- a/src/app/components/media/Image.tsx +++ b/src/app/components/media/Image.tsx @@ -3,6 +3,8 @@ import { forwardRef, lazy, Suspense, useCallback, useEffect, useRef, useState } import classNames from 'classnames'; import type { DotLottieReact as DotLottieReactComponent } from '@lottiefiles/dotlottie-react'; import { useSetting } from '$state/hooks/settings'; +import { useTauriMediaObjectUrl } from '$hooks/useTauriMediaObjectUrl'; +import { isThumbnailMediaUrl } from '$utils/mediaUrl'; import { isPixelatedRendering, settingsAtom } from '$state/settings'; import * as css from './media.css'; import type { IImageInfo } from '$types/matrix/common'; @@ -13,6 +15,7 @@ type ImageProps = Omit, 'onPointerDown'> & { disableDefaultSizing?: boolean; disablePixelation?: boolean; pixelated?: boolean; + sessionCache?: boolean; onLottieLoad?: (canvas?: HTMLCanvasElement) => void; onLottieError?: () => void; onPointerDown?: PointerEventHandler; @@ -27,12 +30,17 @@ type DotLottieInstance = Parameters< >[0]; const DotLottieReact = lazy(() => - import('@lottiefiles/dotlottie-react').then((module) => ({ - default: module.DotLottieReact, - })) + Promise.all([ + import('@lottiefiles/dotlottie-react'), + import('@lottiefiles/dotlottie-web/dist/dotlottie-player.wasm?url'), + ]).then(([module, wasm]) => { + module.setWasmUrl(wasm.default); + return { default: module.DotLottieReact }; + }) ) as typeof DotLottieReactComponent; const GZIPPED_LOTTIE_MIME = /^application\/(?:(?:x-)?gzip|x-tgsticker)(?:;|$)/i; +const MAX_SESSION_CACHE_BYTES = 512 * 1024; const MAX_COMPRESSED_LOTTIE_BYTES = 1024 * 1024; const MAX_DECOMPRESSED_LOTTIE_BYTES = 8 * 1024 * 1024; const MAX_LOTTIE_DIMENSION = 4096; @@ -391,6 +399,7 @@ export const Image = forwardRef boolean>(() => true); +vi.mock('@tauri-apps/api/core', () => ({ + isTauri: () => isTauriMock(), +})); + +import { clearMediaObjectUrls, getMediaObjectUrl } from '$utils/mediaObjectUrlCache'; +import { useTauriMediaObjectUrl } from './useTauriMediaObjectUrl'; + +const SRC = + 'http://sable-media.localhost/https%3A%2F%2fexample.org%2F_matrix%2Fmedia%2Fv3%2Fdownload%2Fa%2Fb?__sable_media_cache=3'; + +const mockFetchResolve = (body = 'image-bytes') => + vi + .spyOn(globalThis, 'fetch') + .mockImplementation(() => Promise.resolve(new Response(body, { status: 200 }))); + +beforeEach(() => { + isTauriMock.mockReturnValue(true); + stubObjectUrls(); + clearMediaObjectUrls(); +}); + +afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllGlobals(); +}); + +describe('useTauriMediaObjectUrl', () => { + it('passes through untouched outside Tauri', async () => { + isTauriMock.mockReturnValue(false); + const fetchSpy = mockFetchResolve(); + + const { result } = renderHook(() => useTauriMediaObjectUrl(SRC)); + + expect(result.current).toBe(SRC); + await waitFor(() => expect(fetchSpy).not.toHaveBeenCalled()); + }); + + it('resolves to a cached object URL and fetches only once per URL', async () => { + const fetchSpy = mockFetchResolve(); + + const first = renderHook(() => useTauriMediaObjectUrl(SRC)); + expect(first.result.current).toBeUndefined(); + await waitFor(() => expect(first.result.current).toBe('blob:mock-1')); + + const second = renderHook(() => useTauriMediaObjectUrl(SRC)); + expect(second.result.current).toBe('blob:mock-1'); + expect(fetchSpy).toHaveBeenCalledTimes(1); + }); + + it('falls back to the raw URL when the fetch fails, and retries on remount', async () => { + const fetchSpy = vi + .spyOn(globalThis, 'fetch') + .mockRejectedValueOnce(new Error('network down')) + .mockResolvedValue(new Response('image-bytes', { status: 200 })); + + const first = renderHook(() => useTauriMediaObjectUrl(SRC)); + await waitFor(() => expect(first.result.current).toBe(SRC)); + first.unmount(); + + const second = renderHook(() => useTauriMediaObjectUrl(SRC)); + await waitFor(() => expect(second.result.current).toBe('blob:mock-1')); + expect(fetchSpy).toHaveBeenCalledTimes(2); + }); + + it('returns undefined until the fetch resolves', async () => { + let release!: (response: Response) => void; + vi.spyOn(globalThis, 'fetch').mockImplementation( + () => + new Promise((resolve) => { + release = resolve; + }) + ); + + const { result } = renderHook(() => useTauriMediaObjectUrl(SRC)); + expect(result.current).toBeUndefined(); + + release(new Response('image-bytes', { status: 200 })); + await waitFor(() => expect(result.current).toBe('blob:mock-1')); + }); + + it('dedupes concurrent mounts of the same URL', async () => { + const fetchSpy = mockFetchResolve(); + + const a = renderHook(() => useTauriMediaObjectUrl(SRC)); + const b = renderHook(() => useTauriMediaObjectUrl(SRC)); + + await waitFor(() => { + expect(a.result.current).toBe('blob:mock-1'); + expect(b.result.current).toBe('blob:mock-1'); + }); + expect(fetchSpy).toHaveBeenCalledTimes(1); + }); + + it('keeps changes of src independent', async () => { + mockFetchResolve(); + const OTHER = `${SRC}&other=1`; + + const { result, rerender } = renderHook(({ src }) => useTauriMediaObjectUrl(src), { + initialProps: { src: SRC }, + }); + await waitFor(() => expect(result.current).toBe('blob:mock-1')); + + rerender({ src: OTHER }); + expect(result.current).toBeUndefined(); + await waitFor(() => expect(result.current).toBe('blob:mock-2')); + + // SRC is still cached and resolves synchronously. + expect(getMediaObjectUrl(SRC)).toBe('blob:mock-1'); + }); +}); diff --git a/src/app/hooks/useTauriMediaObjectUrl.ts b/src/app/hooks/useTauriMediaObjectUrl.ts new file mode 100644 index 0000000000..f4a1ddf29d --- /dev/null +++ b/src/app/hooks/useTauriMediaObjectUrl.ts @@ -0,0 +1,45 @@ +import { isTauri } from '@tauri-apps/api/core'; +import { useEffect, useState } from 'react'; +import { ensureMediaObjectUrl, getMediaObjectUrl } from '$utils/mediaObjectUrlCache'; + +/** + * Resolves `src` to a session-cached blob object URL on Tauri, where the media scheme + * bypasses the webview's HTTP cache. Falls back to the raw URL on fetch failure. + * Pass-through outside Tauri, where the service worker caches media. + */ +export const useTauriMediaObjectUrl = (src: string | undefined): string | undefined => { + const active = isTauri() && src !== undefined; + const [resolved, setResolved] = useState(() => + active ? (getMediaObjectUrl(src) ?? undefined) : src + ); + + useEffect(() => { + if (!active) { + setResolved(src); + return undefined; + } + + let cancelled = false; + const cached = getMediaObjectUrl(src); + if (cached !== undefined) { + setResolved(cached); + return undefined; + } + + setResolved(undefined); + ensureMediaObjectUrl(src).then( + (objectUrl) => { + if (!cancelled) setResolved(objectUrl); + }, + () => { + if (!cancelled) setResolved(src); + } + ); + + return () => { + cancelled = true; + }; + }, [active, src]); + + return active ? resolved : src; +}; diff --git a/src/app/hooks/useUserProfile.test.tsx b/src/app/hooks/useUserProfile.test.tsx index 7c9a462ad8..dc9b3db49e 100644 --- a/src/app/hooks/useUserProfile.test.tsx +++ b/src/app/hooks/useUserProfile.test.tsx @@ -80,4 +80,39 @@ describe('useUserProfile', () => { expect(mx.getProfileInfo).toHaveBeenCalledOnce(); expect(mx.getProfileInfo).toHaveBeenCalledWith('@alice:example.org'); }); + + it('serves the newest queued profile first so a backlog cannot bury it', async () => { + vi.useFakeTimers(); + const userIds = Array.from({ length: 6 }, (_unused, index) => `@user${index}:example.org`); + const requested: string[] = []; + const release: (() => void)[] = []; + const mx = { + getProfileInfo: vi.fn((userId: string) => { + requested.push(userId); + return new Promise((resolve) => { + release.push(() => resolve({ displayname: userId })); + }); + }), + getUser: vi.fn<() => void>(), + getUserId: vi.fn<() => string>().mockReturnValue('@me:example.org'), + } as unknown as MatrixClient; + + renderHook(() => userIds.map((userId) => useUserProfile(userId)), { + wrapper: makeWrapper(mx, false), + }); + + await act(async () => { + await vi.advanceTimersByTimeAsync(150); + }); + + // The cap is four, so the last two queue. + expect(requested).toEqual(userIds.slice(0, 4)); + + await act(async () => { + release.shift()?.(); + await vi.advanceTimersByTimeAsync(0); + }); + + expect(requested[4]).toBe(userIds[5]); + }); }); diff --git a/src/app/hooks/useUserProfile.ts b/src/app/hooks/useUserProfile.ts index 64c91977f1..4a1005d3bd 100644 --- a/src/app/hooks/useUserProfile.ts +++ b/src/app/hooks/useUserProfile.ts @@ -42,7 +42,8 @@ const scheduleProfileRequest = ( .finally(() => { const active = Math.max(0, (activeProfileRequests.get(mx) ?? 1) - 1); activeProfileRequests.set(mx, active); - if (active < MAX_CONCURRENT_PROFILE_REQUESTS) profileRequestQueues.get(mx)?.shift()?.(); + // Newest first: a profile just scrolled to would otherwise wait behind the backlog. + if (active < MAX_CONCURRENT_PROFILE_REQUESTS) profileRequestQueues.get(mx)?.pop()?.(); }); }; diff --git a/src/app/utils/mediaCache.ts b/src/app/utils/mediaCache.ts index ff702d612b..5ed010b241 100644 --- a/src/app/utils/mediaCache.ts +++ b/src/app/utils/mediaCache.ts @@ -1,3 +1,5 @@ +import { clearMediaObjectUrls } from './mediaObjectUrlCache'; + const CACHE_NAME = 'sable-media-v2'; const LEGACY_CACHE_NAMES = ['sable-media-v1']; const MAX_ENTRIES = 500; @@ -11,6 +13,7 @@ let legacyCachesCleaned = false; export async function clearMediaCache(): Promise { pendingBlobs.clear(); + clearMediaObjectUrls(); writesSinceEviction = 0; if (typeof caches === 'undefined') return; try { diff --git a/src/app/utils/mediaConcurrency.test.ts b/src/app/utils/mediaConcurrency.test.ts index 2d22f5bf82..0571f39abb 100644 --- a/src/app/utils/mediaConcurrency.test.ts +++ b/src/app/utils/mediaConcurrency.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { withMediaFetchSlot } from './mediaConcurrency'; describe('withMediaFetchSlot', () => { @@ -31,6 +31,36 @@ describe('withMediaFetchSlot', () => { expect(peak).toBe(3); }); + it('serves the newest waiter first so late requests are not stuck behind a backlog', async () => { + const started: number[] = []; + const release: (() => void)[] = []; + const enqueue = (id: number) => + withMediaFetchSlot(async () => { + started.push(id); + await new Promise((resolve) => { + release.push(resolve); + }); + }); + + // 0-2 take the three slots; 3-5 are the backlog; 6 is the request the user just made. + const tasks = [0, 1, 2, 3, 4, 5, 6].map(enqueue); + await Promise.resolve(); + expect(started).toEqual([0, 1, 2]); + + release.shift()?.(); + await vi.waitFor(() => expect(started).toHaveLength(4)); + expect(started[3]).toBe(6); + + while (release.length > 0) { + release.shift()?.(); + // eslint-disable-next-line no-await-in-loop + await new Promise((resolve) => { + setTimeout(resolve, 0); + }); + } + await Promise.all(tasks); + }); + it('releases the slot when a task rejects', async () => { await expect(withMediaFetchSlot(() => Promise.reject(new Error('boom')))).rejects.toThrow( 'boom' diff --git a/src/app/utils/mediaConcurrency.ts b/src/app/utils/mediaConcurrency.ts index 1429801269..816d77079d 100644 --- a/src/app/utils/mediaConcurrency.ts +++ b/src/app/utils/mediaConcurrency.ts @@ -18,7 +18,9 @@ export async function withMediaFetchSlot(task: () => Promise): Promise return await task(); } finally { // Hand the slot straight to the next waiter instead of releasing and re-acquiring it. - const next = waiters.shift(); + // Newest first: a picker opened over a loading member list would otherwise wait + // behind every one of its avatars. + const next = waiters.pop(); if (next) next(); else active -= 1; } diff --git a/src/app/utils/mediaObjectUrlCache.test.ts b/src/app/utils/mediaObjectUrlCache.test.ts new file mode 100644 index 0000000000..3a3486d66c --- /dev/null +++ b/src/app/utils/mediaObjectUrlCache.test.ts @@ -0,0 +1,58 @@ +// @vitest-environment jsdom +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { stubObjectUrls } from '../../test/objectUrlStub'; + +vi.mock('@tauri-apps/api/core', () => ({ + isTauri: () => false, +})); + +import { clearMediaObjectUrls, ensureMediaObjectUrl, getMediaObjectUrl } from './mediaObjectUrlCache'; + +beforeEach(() => { + stubObjectUrls(); + clearMediaObjectUrls(); +}); + +afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllGlobals(); +}); + +describe('mediaObjectUrlCache', () => { + it('holds picker loads to the shared media fetch budget', async () => { + let inFlight = 0; + let peak = 0; + + vi.spyOn(globalThis, 'fetch').mockImplementation(() => { + inFlight += 1; + peak = Math.max(peak, inFlight); + return new Promise((resolve) => { + setTimeout(() => { + inFlight -= 1; + resolve(new Response('image-bytes', { status: 200 })); + }, 5); + }); + }); + + const resolved = await Promise.all( + Array.from({ length: 10 }, (_unused, index) => + ensureMediaObjectUrl(`https://example.org/media/${index}`) + ) + ); + + expect(resolved).toHaveLength(10); + expect(peak).toBe(3); + }); + + it('revokes and forgets every entry on clear', async () => { + vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response('image-bytes', { status: 200 })); + + await ensureMediaObjectUrl('https://example.org/media/a'); + expect(getMediaObjectUrl('https://example.org/media/a')).toBe('blob:mock-1'); + + clearMediaObjectUrls(); + + expect(URL.revokeObjectURL).toHaveBeenCalledWith('blob:mock-1'); + expect(getMediaObjectUrl('https://example.org/media/a')).toBeUndefined(); + }); +}); diff --git a/src/app/utils/mediaObjectUrlCache.ts b/src/app/utils/mediaObjectUrlCache.ts new file mode 100644 index 0000000000..1f7dc78e3d --- /dev/null +++ b/src/app/utils/mediaObjectUrlCache.ts @@ -0,0 +1,59 @@ +import { fetch } from '$utils/fetch'; +import { withMediaFetchSlot } from './mediaConcurrency'; + +// Tauri webviews don't HTTP-cache responses from the sable-media scheme handler, so +// every remount round-trips into the native layer. Session object URLs avoid that. +const MAX_ENTRIES = 256; + +type CacheEntry = string | Promise; + +const objectUrls = new Map(); + +function remember(url: string, objectUrl: string): void { + objectUrls.delete(url); + objectUrls.set(url, objectUrl); + while (objectUrls.size > MAX_ENTRIES) { + const oldestKey = objectUrls.keys().next().value; + if (oldestKey === undefined) break; + const oldest = objectUrls.get(oldestKey); + objectUrls.delete(oldestKey); + if (typeof oldest === 'string') URL.revokeObjectURL(oldest); + } +} + +export function getMediaObjectUrl(url: string): string | undefined { + const entry = objectUrls.get(url); + if (typeof entry !== 'string') return undefined; + remember(url, entry); + return entry; +} + +export function ensureMediaObjectUrl(url: string): Promise { + const existing = objectUrls.get(url); + if (existing !== undefined) return Promise.resolve(existing); + + const request = withMediaFetchSlot(async () => { + const response = await fetch(url); + if (!response.ok) throw new Error(`media fetch failed: ${response.status}`); + return response.blob(); + }) + .then((blob) => { + const objectUrl = URL.createObjectURL(blob); + remember(url, objectUrl); + return objectUrl; + }) + .catch((err: unknown) => { + if (objectUrls.get(url) === request) objectUrls.delete(url); + throw err; + }); + + objectUrls.set(url, request); + return request; +} + +export function clearMediaObjectUrls(): void { + objectUrls.forEach((entry) => { + if (typeof entry === 'string') URL.revokeObjectURL(entry); + }); + objectUrls.clear(); +} diff --git a/src/app/utils/mediaUrl.ts b/src/app/utils/mediaUrl.ts index d1808f76c0..dc925ba968 100644 --- a/src/app/utils/mediaUrl.ts +++ b/src/app/utils/mediaUrl.ts @@ -78,6 +78,24 @@ const getTauriMediaInnerTarget = (mediaUrl: string): string | undefined => { return mediaUrl; }; +const MEDIA_THUMBNAIL_PATH_PREFIXES = [ + '/_matrix/client/v1/media/thumbnail/', + '/_matrix/media/v3/thumbnail/', + '/_matrix/media/r0/thumbnail/', +]; + +// Matches on the inner http(s) target so tauri-wrapped and plain URLs classify alike. +export const isThumbnailMediaUrl = (mediaUrl: string): boolean => { + const innerTarget = getTauriMediaInnerTarget(mediaUrl); + if (!innerTarget) return false; + try { + const { pathname } = new URL(innerTarget); + return MEDIA_THUMBNAIL_PATH_PREFIXES.some((prefix) => pathname.startsWith(prefix)); + } catch { + return false; + } +}; + // Embeds a retry revision as a fragment on the inner http(s) target (stripping the // outer cache/session markers, which the rewrite re-adds). The fragment makes Rust's // cache_key(scope, target) distinct but is stripped natively before the upstream diff --git a/src/test/objectUrlStub.ts b/src/test/objectUrlStub.ts new file mode 100644 index 0000000000..1186a88516 --- /dev/null +++ b/src/test/objectUrlStub.ts @@ -0,0 +1,16 @@ +import { vi } from 'vitest'; + +// jsdom has no URL.createObjectURL. Subclassing keeps `new URL()` working for code under test. +export const stubObjectUrls = (): void => { + let counter = 0; + class StubbedURL extends globalThis.URL { + static createObjectURL = vi.fn<() => string>(() => { + counter += 1; + return `blob:mock-${counter}`; + }); + + static revokeObjectURL = vi.fn<() => void>(); + } + + vi.stubGlobal('URL', StubbedURL); +}; From 8b938483bf780157ed2d1d106fc84ce8f2930796 Mon Sep 17 00:00:00 2001 From: 7w1 Date: Sat, 8 Aug 2026 12:38:28 -0500 Subject: [PATCH 2/2] formatting --- src/app/hooks/useUserProfile.test.tsx | 14 ++++++++------ src/app/utils/mediaObjectUrlCache.test.ts | 6 +++++- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/app/hooks/useUserProfile.test.tsx b/src/app/hooks/useUserProfile.test.tsx index dc9b3db49e..914be09daf 100644 --- a/src/app/hooks/useUserProfile.test.tsx +++ b/src/app/hooks/useUserProfile.test.tsx @@ -87,12 +87,14 @@ describe('useUserProfile', () => { const requested: string[] = []; const release: (() => void)[] = []; const mx = { - getProfileInfo: vi.fn((userId: string) => { - requested.push(userId); - return new Promise((resolve) => { - release.push(() => resolve({ displayname: userId })); - }); - }), + getProfileInfo: vi.fn<(userId: string) => Promise<{ displayname: string }>>( + (userId: string) => { + requested.push(userId); + return new Promise((resolve) => { + release.push(() => resolve({ displayname: userId })); + }); + } + ), getUser: vi.fn<() => void>(), getUserId: vi.fn<() => string>().mockReturnValue('@me:example.org'), } as unknown as MatrixClient; diff --git a/src/app/utils/mediaObjectUrlCache.test.ts b/src/app/utils/mediaObjectUrlCache.test.ts index 3a3486d66c..aef87c29c4 100644 --- a/src/app/utils/mediaObjectUrlCache.test.ts +++ b/src/app/utils/mediaObjectUrlCache.test.ts @@ -6,7 +6,11 @@ vi.mock('@tauri-apps/api/core', () => ({ isTauri: () => false, })); -import { clearMediaObjectUrls, ensureMediaObjectUrl, getMediaObjectUrl } from './mediaObjectUrlCache'; +import { + clearMediaObjectUrls, + ensureMediaObjectUrl, + getMediaObjectUrl, +} from './mediaObjectUrlCache'; beforeEach(() => { stubObjectUrls();