diff --git a/desktop/src-tauri/Cargo.lock b/desktop/src-tauri/Cargo.lock index fb60a351895..a3deed4f3e9 100644 --- a/desktop/src-tauri/Cargo.lock +++ b/desktop/src-tauri/Cargo.lock @@ -326,6 +326,18 @@ dependencies = [ "pin-project-lite", ] +[[package]] +name = "async-compression" +version = "0.4.43" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3976abdc8fe7d1133d43d304afd42abdf5bc3e1319d263d223bde07b5efc4be8" +dependencies = [ + "compression-codecs", + "compression-core", + "pin-project-lite", + "tokio", +] + [[package]] name = "async-executor" version = "1.14.0" @@ -1680,6 +1692,23 @@ dependencies = [ "static_assertions", ] +[[package]] +name = "compression-codecs" +version = "0.4.38" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ce2548391e9c1929c21bf6aa2680af86fe4c1b33e6cea9ac1cfeec0bd11218cf" +dependencies = [ + "compression-core", + "flate2", + "memchr", +] + +[[package]] +name = "compression-core" +version = "0.4.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cc14f565cf027a105f7a44ccf9e5b424348421a1d8952a8fc9d499d313107789" + [[package]] name = "concurrent-queue" version = "2.5.0" @@ -11481,12 +11510,17 @@ version = "0.6.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4cfcf7e2740e6fc6d4d688b4ef00650406bb94adf4731e43c096c3a19fe40840" dependencies = [ + "async-compression", "bitflags 2.13.0", "bytes", + "futures-core", "futures-util", "http", "http-body", + "http-body-util", "pin-project-lite", + "tokio", + "tokio-util", "tower", "tower-layer", "tower-service", diff --git a/desktop/src-tauri/Cargo.toml b/desktop/src-tauri/Cargo.toml index 3f7189deea1..e648e55156a 100644 --- a/desktop/src-tauri/Cargo.toml +++ b/desktop/src-tauri/Cargo.toml @@ -99,7 +99,7 @@ nostr = { version = "0.44", features = ["nip44", "nip49"] } getrandom = "0.2" zeroize = "1" percent-encoding = "2" -reqwest = { version = "0.13", features = ["json", "query", "stream", "blocking"] } +reqwest = { version = "0.13", features = ["json", "query", "stream", "blocking", "gzip"] } rustls = { version = "0.23", default-features = false, features = ["aws_lc_rs", "std"] } url = "2" buzz_core_pkg = { package = "buzz-core", path = "../../crates/buzz-core" } diff --git a/desktop/src-tauri/src/app_state.rs b/desktop/src-tauri/src/app_state.rs index 7c41f6bfe26..22786b96c3f 100644 --- a/desktop/src-tauri/src/app_state.rs +++ b/desktop/src-tauri/src/app_state.rs @@ -166,15 +166,6 @@ fn identity_from_env() -> Option { /// Returned as a `Result` so the fail-closed invariant is testable — callers /// must never substitute a redirect-following client on build failure. Shares /// the localhost `resolve`/pool config with the app-wide `http_client`. -pub fn build_media_fetch_client() -> reqwest::Result { - reqwest::Client::builder() - .resolve("localhost", std::net::SocketAddr::from(([127, 0, 0, 1], 0))) - .pool_idle_timeout(std::time::Duration::from_secs(10)) - .pool_max_idle_per_host(1) - .redirect(reqwest::redirect::Policy::none()) - .build() -} - pub fn build_app_state() -> AppState { // Env var takes precedence (dev/CI). If absent, resolve_persisted_identity() // in setup() will replace the ephemeral placeholder with a persisted key. @@ -192,13 +183,8 @@ pub fn build_app_state() -> AppState { AppState { keys: Mutex::new(keys), identity_storage: AtomicU8::new(identity_storage as u8), - http_client: reqwest::Client::builder() - .resolve("localhost", std::net::SocketAddr::from(([127, 0, 0, 1], 0))) - .pool_idle_timeout(std::time::Duration::from_secs(10)) - .pool_max_idle_per_host(1) - .build() - .unwrap_or_else(|_| reqwest::Client::new()), - media_fetch_client: build_media_fetch_client().expect( + http_client: crate::http_client::build().unwrap_or_else(|_| reqwest::Client::new()), + media_fetch_client: crate::http_client::build_media().expect( "media_fetch_client must build with redirect::Policy::none(); a \ redirect-following fallback would forward the minted media auth \ header across origins (redirect-hop SSRF)", diff --git a/desktop/src-tauri/src/commands/media_download.rs b/desktop/src-tauri/src/commands/media_download.rs index 7bc94da25d2..29f2925f54c 100644 --- a/desktop/src-tauri/src/commands/media_download.rs +++ b/desktop/src-tauri/src/commands/media_download.rs @@ -937,7 +937,7 @@ mod tests { }); // Drive the exact client the command path uses, not an ad-hoc one. - let client = crate::app_state::build_media_fetch_client() + let client = crate::http_client::build_media() .expect("media fetch client must build with no-redirect policy"); let resp = client .get(format!("http://{addr}/media/clip.mp4")) @@ -975,7 +975,7 @@ mod tests { // panics loudly (see `build_app_state`) rather than substituting an // insecure client. assert!( - crate::app_state::build_media_fetch_client().is_ok(), + crate::http_client::build_media().is_ok(), "media fetch client must build; a redirect-following fallback is forbidden", ); } diff --git a/desktop/src-tauri/src/http_client.rs b/desktop/src-tauri/src/http_client.rs new file mode 100644 index 00000000000..b22f9d60c09 --- /dev/null +++ b/desktop/src-tauri/src/http_client.rs @@ -0,0 +1,87 @@ +//! HTTP clients with separate decompression contracts for relay JSON and media. + +pub fn build() -> reqwest::Result { + base().build() +} + +/// Build the media client without redirects or transparent decoding. +/// +/// Media callers forward upstream Content-Length/Content-Range. Decoding would +/// change the body while stripping or invalidating those video-seeking headers. +pub fn build_media() -> reqwest::Result { + base() + .no_gzip() + .redirect(reqwest::redirect::Policy::none()) + .build() +} + +fn base() -> reqwest::ClientBuilder { + reqwest::Client::builder() + .resolve("localhost", std::net::SocketAddr::from(([127, 0, 0, 1], 0))) + .pool_idle_timeout(std::time::Duration::from_secs(10)) + .pool_max_idle_per_host(1) +} + +#[cfg(test)] +mod tests { + use axum::{http::HeaderMap, response::Response, routing::get, Router}; + + const GZIP_BODY: &[u8] = &[ + 31, 139, 8, 0, 0, 0, 0, 0, 2, 255, 43, 72, 76, 207, 204, 75, 44, 201, 204, 207, 83, 40, 72, + 172, 204, 201, 79, 76, 1, 0, 132, 249, 73, 160, 18, 0, 0, 0, + ]; + + async fn serve(app: Router) -> (std::net::SocketAddr, tokio::task::JoinHandle<()>) { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + let server = tokio::spawn(async move { axum::serve(listener, app).await.unwrap() }); + (address, server) + } + + #[tokio::test] + async fn app_client_transparently_decodes_gzip() { + let app = Router::new().route( + "/", + get(|headers: HeaderMap| async move { + assert!(headers + .get("accept-encoding") + .and_then(|value| value.to_str().ok()) + .is_some_and(|value| value.split(',').any(|coding| coding.trim() == "gzip"))); + Response::builder() + .header("content-encoding", "gzip") + .header("content-length", GZIP_BODY.len()) + .body(axum::body::Body::from(GZIP_BODY)) + .unwrap() + }), + ); + let (address, server) = serve(app).await; + let response = super::build() + .unwrap() + .get(format!("http://{address}/")) + .send() + .await + .unwrap(); + assert_eq!(response.text().await.unwrap(), "pagination payload"); + server.abort(); + } + + #[tokio::test] + async fn media_client_does_not_advertise_gzip() { + let app = Router::new().route( + "/", + get(|headers: HeaderMap| async move { + assert!(headers.get("accept-encoding").is_none()); + "range bytes" + }), + ); + let (address, server) = serve(app).await; + let response = super::build_media() + .unwrap() + .get(format!("http://{address}/")) + .send() + .await + .unwrap(); + assert_eq!(response.text().await.unwrap(), "range bytes"); + server.abort(); + } +} diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 71a5eb3806e..f8041ec38b7 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -8,6 +8,7 @@ mod deep_link; mod egress_guard; mod event_sync; mod events; +mod http_client; mod huddle; mod identity_storage; mod initial_window; @@ -354,10 +355,9 @@ pub fn run() { tauri::async_runtime::spawn(crate::mesh_llm::start_coordinator(app_handle.clone())); } - // Start the localhost media streaming proxy. Uses the shared HTTP - // client so VPN tunnelling applies. The port is stored in AppState - // and exposed to the frontend via the `get_media_proxy_port` command. - let proxy_client = state.http_client.clone(); + // Use the no-redirect, no-transparent-decompression media client: + // the proxy forwards range metadata verbatim for video seeking. + let proxy_client = state.media_fetch_client.clone(); let proxy_handle = app_handle.clone(); tauri::async_runtime::spawn(async move { let port = media_proxy::spawn_media_proxy(proxy_client, proxy_handle.clone()).await; diff --git a/desktop/src-tauri/src/media_proxy.rs b/desktop/src-tauri/src/media_proxy.rs index 21692ce5237..58357533fbc 100644 --- a/desktop/src-tauri/src/media_proxy.rs +++ b/desktop/src-tauri/src/media_proxy.rs @@ -181,7 +181,7 @@ pub async fn handle_buzz_media( // Forward Range header if present — enables video seeking through the proxy. let mut upstream = state - .http_client + .media_fetch_client .get(&upstream_url) .timeout(std::time::Duration::from_secs(60)); diff --git a/desktop/src/features/messages/lib/channelWindowStore.test.mjs b/desktop/src/features/messages/lib/channelWindowStore.test.mjs index b9ccae0db70..e9265ec2a35 100644 --- a/desktop/src/features/messages/lib/channelWindowStore.test.mjs +++ b/desktop/src/features/messages/lib/channelWindowStore.test.mjs @@ -11,6 +11,7 @@ import { mergeLiveChannelWindowEvent, mergeLiveThreadSummary, replaceNewestChannelWindow, + stageOlderChannelWindow, } from "./channelWindowStore.ts"; function event(id, createdAt, kind = 9) { @@ -113,23 +114,57 @@ test("rejects inconsistent exhaustion and cursor facts", () => { ); }); -test("newest refresh drops a stale tail when its boundary moves", () => { +test("newest refresh drops a stale tail and its staged successor", () => { const first = page(null, [event("a", 100)]); - const loaded = appendOlderChannelWindow( + const staged = page(first.nextCursor, [event("z", 90)], { hasMore: false }); + const loaded = stageOlderChannelWindow( replaceNewestChannelWindow(emptyChannelWindowStore(), first), - page(first.nextCursor, [event("z", 90)], { hasMore: false }), + staged, ); const refreshed = replaceNewestChannelWindow( loaded, page(null, [event("n", 110), event("a", 100)]), ); assert.equal(refreshed.pages.length, 1); + assert.equal(refreshed.stagedPage, null); assert.deepEqual( flattenChannelWindowEvents(refreshed).map((item) => item.content), ["a", "n"], ); }); +test("staging replaces at most one matching successor without rendering it", () => { + const first = page(null, [event("a", 100)]); + const store = replaceNewestChannelWindow(emptyChannelWindowStore(), first); + const staged = page(first.nextCursor, [event("b", 90)]); + const replacement = page(first.nextCursor, [event("c", 80)], { + hasMore: false, + }); + + const once = stageOlderChannelWindow(store, staged); + const twice = stageOlderChannelWindow(once, replacement); + + assert.equal(twice.stagedPage, replacement); + assert.equal(twice.pages.length, 1); + assert.deepEqual( + flattenChannelWindowEvents(twice).map((item) => item.content), + ["a"], + ); +}); + +test("staging discards a page outside the retained tail cursor", () => { + const first = page(null, [event("a", 100)]); + const store = replaceNewestChannelWindow(emptyChannelWindowStore(), first); + + assert.equal( + stageOlderChannelWindow( + store, + page(cursor(event("wrong", 70)), [event("older", 60)]), + ), + store, + ); +}); + test("live rows arriving before page zero enter the overlay", () => { const live = event("n", 110); const store = mergeLiveChannelWindowEvent(emptyChannelWindowStore(), live); diff --git a/desktop/src/features/messages/lib/channelWindowStore.ts b/desktop/src/features/messages/lib/channelWindowStore.ts index 1bd921aabf4..91f9fea346b 100644 --- a/desktop/src/features/messages/lib/channelWindowStore.ts +++ b/desktop/src/features/messages/lib/channelWindowStore.ts @@ -25,6 +25,8 @@ export type ChannelWindowPage = { }; export type ChannelWindowStore = { pages: ChannelWindowPage[]; + /** One fetched successor held outside the rendered page chain. */ + stagedPage: ChannelWindowPage | null; /** Top-level live events not represented in an authoritative relay page. */ liveOverlay: RelayEvent[]; /** Live structural events retained independently from frozen page closure. */ @@ -38,12 +40,13 @@ export type ChannelWindowStore = { export const emptyChannelWindowStore = (): ChannelWindowStore => ({ pages: [], + stagedPage: null, liveOverlay: [], liveAux: [], liveSummaries: {}, }); -function cursorsEqual( +export function cursorsEqual( left: ChannelWindowCursor | null, right: ChannelWindowCursor | null, ) { @@ -112,6 +115,7 @@ export function replaceNewestChannelWindow( const auxIds = new Set(page.aux.map((event) => event.id)); return { pages: [page], + stagedPage: null, liveOverlay: current.liveOverlay.filter((event) => !ids.has(event.id)), liveAux: current.liveAux.filter((event) => !auxIds.has(event.id)), // A head refetch is the authoritative resync moment (subscribe/reconnect @@ -151,10 +155,23 @@ export function appendOlderChannelWindow( return { ...current, pages: [...current.pages, page], + stagedPage: null, liveOverlay: current.liveOverlay.filter((event) => !pageIds.has(event.id)), }; } +/** Hold one cursor-matched successor outside the rendered page chain. */ +export function stageOlderChannelWindow( + current: ChannelWindowStore, + page: ChannelWindowPage, +): ChannelWindowStore { + assertValidPage(page); + const tail = current.pages[current.pages.length - 1]; + if (!tail?.hasMore || !tail.nextCursor) return current; + if (!cursorsEqual(page.startCursor, tail.nextCursor)) return current; + return { ...current, stagedPage: page }; +} + /** * Record a relay-pushed live `39005` summary. Newest `created_at` wins per * root: the relay pushes a full recount on every thread mutation, so the diff --git a/desktop/src/features/messages/lib/pageOlderMessages.test.mjs b/desktop/src/features/messages/lib/pageOlderMessages.test.mjs new file mode 100644 index 00000000000..467120915a5 --- /dev/null +++ b/desktop/src/features/messages/lib/pageOlderMessages.test.mjs @@ -0,0 +1,226 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { QueryClient } from "@tanstack/react-query"; + +import { + emptyChannelWindowStore, + replaceNewestChannelWindow, +} from "./channelWindowStore.ts"; +import { channelMessagesKey, channelWindowKey } from "./messageQueryKeys.ts"; +import { pageOlderMessagesUntilRowFloor } from "./pageOlderMessages.ts"; + +function event(id, createdAt) { + return { + id: id.padEnd(64, "0"), + pubkey: "a".repeat(64), + created_at: createdAt, + kind: 9, + tags: [["h", "channel"]], + content: id, + sig: "b".repeat(128), + }; +} +const cursor = (item) => ({ createdAt: item.created_at, eventId: item.id }); +function page(startCursor, rows, hasMore = true) { + return { + startCursor, + rows: rows.map((item) => ({ event: item, thread: null })), + aux: [], + nextCursor: hasMore ? cursor(rows.at(-1)) : null, + hasMore, + }; +} +function harness(channelId) { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + const head = page(null, [event(`${channelId}-head`, 300)]); + queryClient.setQueryData( + channelWindowKey(channelId), + replaceNewestChannelWindow(emptyChannelWindowStore(), head), + ); + queryClient.setQueryData(channelMessagesKey(channelId), [head.rows[0].event]); + return { queryClient, head }; +} +const flush = () => new Promise((resolve) => setImmediate(resolve)); + +test("commits one page and stages exactly one successor outside projection", async () => { + const channelId = "stage-successor"; + const { queryClient, head } = harness(channelId); + const older = page(head.nextCursor, [event("older", 200)]); + const successor = page(older.nextCursor, [event("successor", 100)], false); + const calls = []; + const fetchPage = async (_channelId, requestCursor) => { + calls.push(requestCursor); + return calls.length === 1 ? older : successor; + }; + + await pageOlderMessagesUntilRowFloor( + queryClient, + channelId, + () => true, + fetchPage, + ); + await flush(); + + const store = queryClient.getQueryData(channelWindowKey(channelId)); + assert.deepEqual(calls, [head.nextCursor, older.nextCursor]); + assert.equal(store.pages.length, 2); + assert.equal(store.stagedPage, successor); + assert.deepEqual( + queryClient + .getQueryData(channelMessagesKey(channelId)) + .map((item) => item.content), + ["older", `${channelId}-head`], + ); +}); + +test("trigger during successor staging awaits the same request", async () => { + const channelId = "await-in-flight-stage"; + const { queryClient, head } = harness(channelId); + const older = page(head.nextCursor, [event("older", 200)]); + const successor = page(older.nextCursor, [event("successor", 100)], false); + let releaseSuccessor; + const successorPending = new Promise((resolve) => { + releaseSuccessor = () => resolve(successor); + }); + const calls = []; + const fetchPage = async (_channelId, requestCursor) => { + calls.push(requestCursor); + return calls.length === 1 ? older : successorPending; + }; + + await pageOlderMessagesUntilRowFloor( + queryClient, + channelId, + () => true, + fetchPage, + ); + const secondPass = pageOlderMessagesUntilRowFloor( + queryClient, + channelId, + () => true, + fetchPage, + ); + await flush(); + + assert.deepEqual(calls, [head.nextCursor, older.nextCursor]); + releaseSuccessor(); + await secondPass; + assert.equal( + queryClient.getQueryData(channelWindowKey(channelId)).pages.length, + 3, + ); +}); + +test("replaced tail does not reuse its stale in-flight stage", async () => { + const channelId = "reject-stale-in-flight-stage"; + const { queryClient, head } = harness(channelId); + const older = page(head.nextCursor, [event("older", 200)]); + const staleSuccessor = page( + older.nextCursor, + [event("stale-successor", 100)], + false, + ); + const freshSuccessor = page( + older.nextCursor, + [event("fresh-successor", 90)], + false, + ); + let releaseStaleSuccessor; + const staleSuccessorPending = new Promise((resolve) => { + releaseStaleSuccessor = () => resolve(staleSuccessor); + }); + const calls = []; + const fetchPage = async (_channelId, requestCursor) => { + calls.push(requestCursor); + if (calls.length === 1) return older; + if (calls.length === 2) return staleSuccessorPending; + return freshSuccessor; + }; + + await pageOlderMessagesUntilRowFloor( + queryClient, + channelId, + () => true, + fetchPage, + ); + const replacementHead = page(null, [older.rows[0].event]); + queryClient.setQueryData( + channelWindowKey(channelId), + replaceNewestChannelWindow( + queryClient.getQueryData(channelWindowKey(channelId)), + replacementHead, + ), + ); + + const replacementPass = pageOlderMessagesUntilRowFloor( + queryClient, + channelId, + () => true, + fetchPage, + ); + try { + await flush(); + assert.deepEqual(calls, [ + head.nextCursor, + older.nextCursor, + replacementHead.nextCursor, + ]); + } finally { + releaseStaleSuccessor(); + } + await replacementPass; + assert.deepEqual( + queryClient + .getQueryData(channelMessagesKey(channelId)) + .map((item) => item.content), + ["fresh-successor", "older"], + ); + + await flush(); + assert.equal( + queryClient.getQueryData(channelWindowKey(channelId)).stagedPage, + null, + ); +}); + +test("next paging pass consumes the staged page without network wait", async () => { + const channelId = "consume-staged"; + const { queryClient, head } = harness(channelId); + const older = page(head.nextCursor, [event("older", 200)]); + const successor = page(older.nextCursor, [event("successor", 100)], false); + let calls = 0; + const fetchPage = async () => { + calls += 1; + return calls === 1 ? older : successor; + }; + + await pageOlderMessagesUntilRowFloor( + queryClient, + channelId, + () => true, + fetchPage, + ); + await flush(); + assert.equal(calls, 2); + + await pageOlderMessagesUntilRowFloor( + queryClient, + channelId, + () => true, + () => { + throw new Error("staged page should make this pass network-free"); + }, + ); + + const store = queryClient.getQueryData(channelWindowKey(channelId)); + assert.equal(store.pages.length, 3); + assert.equal(store.stagedPage, null); + assert.deepEqual( + queryClient + .getQueryData(channelMessagesKey(channelId)) + .map((item) => item.content), + ["successor", "older", `${channelId}-head`], + ); +}); diff --git a/desktop/src/features/messages/lib/pageOlderMessages.ts b/desktop/src/features/messages/lib/pageOlderMessages.ts index af51a19348c..8ea86ce9aa2 100644 --- a/desktop/src/features/messages/lib/pageOlderMessages.ts +++ b/desktop/src/features/messages/lib/pageOlderMessages.ts @@ -2,7 +2,11 @@ import type { QueryClient } from "@tanstack/react-query"; import { appendOlderChannelWindow, + type ChannelWindowCursor, + type ChannelWindowPage, type ChannelWindowStore, + cursorsEqual, + stageOlderChannelWindow, } from "@/features/messages/lib/channelWindowStore"; import { projectChannelWindowMessages } from "@/features/messages/lib/projectChannelWindow"; import { parseChannelWindowResponse } from "@/features/messages/lib/channelWindowResponse"; @@ -12,26 +16,66 @@ import { getChannelWindowEvents } from "@/shared/api/channelWindow"; const CHANNEL_WINDOW_PAGE_SIZE = 50; export type PageOlderResult = { hasOlderMessages: boolean }; const inFlightPasses = new Map>(); +type StagedFetch = { + cursor: ChannelWindowCursor; + fromPage: ChannelWindowPage; + page: Promise; +}; +const inFlightStagedPages = new Map(); +type FetchPage = ( + channelId: string, + requestCursor: ChannelWindowCursor, +) => Promise; -/** Fetch exactly one server-defined older window and append it atomically. */ +/** Append one older window, then stage its successor without projection. */ export function pageOlderMessagesUntilRowFloor( queryClient: QueryClient, channelId: string, shouldContinue: () => boolean, + fetchPageFn: FetchPage = fetchPage, ): Promise { const running = inFlightPasses.get(channelId); if (running) return running; - const pass = runPage(queryClient, channelId, shouldContinue).finally(() => { + const pass = runPage( + queryClient, + channelId, + shouldContinue, + fetchPageFn, + ).finally(() => { inFlightPasses.delete(channelId); }); inFlightPasses.set(channelId, pass); return pass; } +async function fetchPage( + channelId: string, + requestCursor: ChannelWindowCursor, +) { + const events = await getChannelWindowEvents( + channelId, + requestCursor, + CHANNEL_WINDOW_PAGE_SIZE, + ); + return parseChannelWindowResponse(events, channelId, requestCursor); +} + +function retainedTailIs( + queryClient: QueryClient, + channelId: string, + page: ChannelWindowPage, +) { + const store = queryClient.getQueryData( + channelWindowKey(channelId), + ); + return store?.pages[store.pages.length - 1] === page; +} + async function runPage( queryClient: QueryClient, channelId: string, shouldContinue: () => boolean, + fetchPageFn: FetchPage, ): Promise { const store = queryClient.getQueryData( channelWindowKey(channelId), @@ -42,13 +86,22 @@ async function runPage( } const requestCursor = tail.nextCursor; - const events = await getChannelWindowEvents( - channelId, - requestCursor, - CHANNEL_WINDOW_PAGE_SIZE, - ); + const matchingStagedPage = store.stagedPage; + const staging = inFlightStagedPages.get(channelId); + const matchingStaging = + staging && + retainedTailIs(queryClient, channelId, staging.fromPage) && + cursorsEqual(staging.cursor, requestCursor) + ? staging + : null; + const page = + matchingStagedPage?.startCursor && + cursorsEqual(matchingStagedPage.startCursor, requestCursor) + ? matchingStagedPage + : matchingStaging + ? await matchingStaging.page + : await fetchPageFn(channelId, requestCursor); if (!shouldContinue()) return { hasOlderMessages: true }; - const page = parseChannelWindowResponse(events, channelId, requestCursor); const retained = queryClient.getQueryData( channelWindowKey(channelId), ); @@ -56,5 +109,32 @@ async function runPage( const next = appendOlderChannelWindow(retained, page); queryClient.setQueryData(channelWindowKey(channelId), next); projectChannelWindowMessages(queryClient, channelId); - return { hasOlderMessages: page.hasMore }; + + if (!page.hasMore || !page.nextCursor || !shouldContinue()) { + return { hasOlderMessages: false }; + } + const stagedFetch = fetchPageFn(channelId, page.nextCursor); + inFlightStagedPages.set(channelId, { + cursor: page.nextCursor, + fromPage: page, + page: stagedFetch, + }); + void stagedFetch + .then((staged) => { + if (!shouldContinue()) return; + queryClient.setQueryData( + channelWindowKey(channelId), + (current) => + current ? stageOlderChannelWindow(current, staged) : current, + ); + }) + .catch((error) => { + console.error("Failed to stage older messages", channelId, error); + }) + .finally(() => { + if (inFlightStagedPages.get(channelId)?.page === stagedFetch) { + inFlightStagedPages.delete(channelId); + } + }); + return { hasOlderMessages: true }; } diff --git a/desktop/src/features/messages/ui/MessageTimeline.tsx b/desktop/src/features/messages/ui/MessageTimeline.tsx index df9af09e674..5e96b5e11b8 100644 --- a/desktop/src/features/messages/ui/MessageTimeline.tsx +++ b/desktop/src/features/messages/ui/MessageTimeline.tsx @@ -23,7 +23,6 @@ import { TimelineSkeleton, useTimelineSkeletonRows } from "./TimelineSkeleton"; import { TimelineMessageList } from "./TimelineMessageList"; import type { TimelineVirtualizerApi } from "./TimelineMessageList"; import { useAnchoredScroll } from "./useAnchoredScroll"; -import { useLoadOlderOnScroll } from "./useLoadOlderOnScroll"; import { useBufferedTimelineMessages } from "./useBufferedTimelineMessages"; import { DirectMessageIntroAvatarStack, @@ -215,7 +214,6 @@ const MessageTimelineBase = React.forwardRef< const internalScrollRef = React.useRef(null); const scrollContainerRef = externalScrollRef ?? internalScrollRef; const contentRef = React.useRef(null); - const topSentinelRef = React.useRef(null); const [virtualizerScrollParent, setVirtualizerScrollParent] = React.useState(null); const [virtualizerRenderVersion, bumpVirtualizerRenderVersion] = @@ -570,12 +568,17 @@ const MessageTimelineBase = React.forwardRef< // Indexed find navigation can legitimately land near the current history // boundary. Do not mistake that programmatic jump for scrollback intent and // prepend underneath the active match. - // A settle-gate hold means the reader is still parked at the OLD - // boundary — don't stack more page fetches behind the held commit. + // A landed page that has not painted yet — deferred snapshot still behind + // the live cache, or held by the settle gate — means the reader is still + // parked at the OLD boundary. The scroll events WebKit keeps emitting + // there would otherwise start the next page in the gap between + // `isFetchingOlder` clearing and the prepend committing, cascading one + // gesture into several pages. Same predicate the spinner uses below. if ( searchActiveMessageId || !fetchOlder || isFetchingOlder || + isRenderedTimelineBehindHistoryPrepend(deferredMessages, messages) || isHoldingPrepend || showTimelineSkeleton || !hasOlderMessages @@ -585,22 +588,16 @@ const MessageTimelineBase = React.forwardRef< void fetchOlder(); return true; }, [ + deferredMessages, fetchOlder, hasOlderMessages, isFetchingOlder, isHoldingPrepend, + messages, searchActiveMessageId, showTimelineSkeleton, ]); - useLoadOlderOnScroll({ - fetchOlder: useTimelineVirtualizer ? undefined : fetchOlder, - hasOlderMessages, - isLoading: showTimelineSkeleton, - scrollContainerRef: activeScrollContainerRef, - sentinelRef: topSentinelRef, - }); - const timelineSkeletonRows = useTimelineSkeletonRows({ channelId, isLoading: showTimelineSkeleton, @@ -770,9 +767,7 @@ const MessageTimelineBase = React.forwardRef< )} ref={contentRef} > - {omitHistoryLeadIn ? null : ( -
- )} + {omitHistoryLeadIn ? null :
} {/* Fixed-height history slot keeps the virtual spacer's offset stable across load-older fetches. The intro-only state has no diff --git a/desktop/src/features/messages/ui/TimelineMessageList.tsx b/desktop/src/features/messages/ui/TimelineMessageList.tsx index d7ef78ea04b..89f63dabf72 100644 --- a/desktop/src/features/messages/ui/TimelineMessageList.tsx +++ b/desktop/src/features/messages/ui/TimelineMessageList.tsx @@ -718,8 +718,10 @@ function VirtualizedTimelineRows({ // touch, and key listeners are the authoritative user-interaction gate. onAtBottomStateChange?.(distanceFromBottom <= 32); updatePinnedDayLabel(offset); - if (offset <= 200) { - // Layout scrolls near the top must not poison the reader's next input. + if (offset <= list.viewportSize * 1.5) { + // Begin loading before momentum reaches the retained history boundary. + // Arming here also prevents layout-only near-top scrolls from poisoning + // the reader's next upward wheel input. armUpwardMomentum(onStartReached?.() ?? false); } }, diff --git a/desktop/src/features/messages/ui/useSettleGatedPrependMessages.test.mjs b/desktop/src/features/messages/ui/useSettleGatedPrependMessages.test.mjs index b134b832afe..12bc9332b10 100644 --- a/desktop/src/features/messages/ui/useSettleGatedPrependMessages.test.mjs +++ b/desktop/src/features/messages/ui/useSettleGatedPrependMessages.test.mjs @@ -86,3 +86,122 @@ test("passes when the previous snapshot was empty (initial load)", () => { "pass", ); }); + +// Hook-level: the hold is released by the scroller's observed motion, never by +// an assumed one. Frames and the clock are both driven by hand. +async function mountHold({ motionBeforeHoldAt = null } = {}) { + const { JSDOM } = await import("jsdom"); + const React = await import("react"); + const { act } = React; + const { createRoot } = await import("react-dom/client"); + const { useSettleGatedPrependMessages } = await import( + "./useSettleGatedPrependMessages.ts" + ); + const dom = new JSDOM( + "
", + ); + const frames = []; + let now = 0; + const saved = { + document: globalThis.document, + window: globalThis.window, + IS_REACT_ACT_ENVIRONMENT: globalThis.IS_REACT_ACT_ENVIRONMENT, + requestAnimationFrame: globalThis.requestAnimationFrame, + cancelAnimationFrame: globalThis.cancelAnimationFrame, + performanceNow: performance.now, + }; + Object.assign(globalThis, { + document: dom.window.document, + window: dom.window, + IS_REACT_ACT_ENVIRONMENT: true, + requestAnimationFrame(callback) { + frames.push(callback); + return frames.length; + }, + cancelAnimationFrame() {}, + }); + performance.now = () => now; + + const scroller = dom.window.document.getElementById("scroller"); + scroller.scrollTop = 400; + if (motionBeforeHoldAt !== null) { + now = motionBeforeHoldAt; + } + let output; + function Harness({ messages }) { + output = useSettleGatedPrependMessages({ + channelId: "c", + messages, + meta: null, + scrollElementRef: { current: scroller }, + }); + return null; + } + const root = createRoot(dom.window.document.getElementById("root")); + await act(async () => + root.render(React.createElement(Harness, { messages: rows("a", "b") })), + ); + if (motionBeforeHoldAt !== null) { + scroller.dispatchEvent(new dom.window.Event("scroll")); + } + now = motionBeforeHoldAt === null ? 1_000 : motionBeforeHoldAt + 50; + await act(async () => + root.render( + React.createElement(Harness, { messages: rows("older", "a", "b") }), + ), + ); + const tick = async (advanceMs) => { + now += advanceMs; + const pending = frames.splice(0); + await act(async () => { + for (const frame of pending) frame(); + }); + }; + const cleanup = async () => { + await act(async () => root.unmount()); + dom.window.close(); + performance.now = saved.performanceNow; + for (const key of [ + "document", + "window", + "IS_REACT_ACT_ENVIRONMENT", + "requestAnimationFrame", + "cancelAnimationFrame", + ]) { + if (saved[key] === undefined) delete globalThis[key]; + else globalThis[key] = saved[key]; + } + }; + return { output: () => output, tick, cleanup }; +} + +test("a scroller at rest admits after the stable-frame count with no assumed quiet window", async () => { + const hold = await mountHold(); + assert.equal(hold.output().isHoldingPrepend, true); + // Three frames, clock barely moving: nothing has reported motion, so the + // only thing the gate may wait for is frame-over-frame stability. + await hold.tick(1); + await hold.tick(1); + assert.equal(hold.output().isHoldingPrepend, true); + await hold.tick(1); + assert.equal(hold.output().isHoldingPrepend, false); + assert.deepEqual( + hold.output().messages.map(({ id }) => id), + ["older", "a", "b"], + ); + await hold.cleanup(); +}); + +test("motion observed before the hold opened still charges the quiet window", async () => { + const hold = await mountHold({ motionBeforeHoldAt: 10_000 }); + // Scroll event at t=10000, hold opened at t=10050. Stable frames alone must + // not admit until 100ms have passed since that event. + await hold.tick(10); + await hold.tick(10); + await hold.tick(10); + await hold.tick(10); + assert.equal(hold.output().isHoldingPrepend, true); + await hold.tick(20); + assert.equal(hold.output().isHoldingPrepend, false); + await hold.cleanup(); +}); diff --git a/desktop/src/features/messages/ui/useSettleGatedPrependMessages.ts b/desktop/src/features/messages/ui/useSettleGatedPrependMessages.ts index bad8d0f04a7..0f42eae443d 100644 --- a/desktop/src/features/messages/ui/useSettleGatedPrependMessages.ts +++ b/desktop/src/features/messages/ui/useSettleGatedPrependMessages.ts @@ -27,13 +27,15 @@ import * as React from "react"; export const SETTLE_MOTION_WINDOW_MS = 100; export const SETTLE_FRAME_COUNT = 3; /** - * Upper bound on how long a fetched page may be withheld. Trackpad momentum - * decays in well under a second; a reader actively driving the scroller for - * this long has moved on, and admitting under continuous REAL input is safe — - * the dropped-write hazard is specific to the inertial momentum phase, which - * cannot outlive this deadline. + * Upper bound on how long a fetched page may be withheld. The dropped-write + * hazard is specific to the inertial momentum phase, and trackpad momentum + * decays in well under a second — so a scroller still reporting motion past + * this deadline is under continuous REAL input, where admitting is safe. + * Every millisecond above the momentum bound is pure wait for a reader who + * keeps two fingers on the pad while history loads (#1698 used 4s; a 450ms + * page then painted seconds after it arrived). */ -export const SETTLE_HOLD_DEADLINE_MS = 4_000; +export const SETTLE_HOLD_DEADLINE_MS = 1_500; export type SettleGateDecision = | { kind: "pass" } @@ -126,6 +128,28 @@ export function useSettleGatedPrependMessages({ const latestMetaRef = React.useRef(meta); latestMetaRef.current = meta; + // Track motion for the scroller's whole life, not just while holding, so a + // hold that begins on a scroller at rest knows it is at rest. #1698 assumed + // motion at hold start instead, which charged every page a full quiet + // window (~100ms) even when the reader had stopped long before it landed. + // A live fling keeps this fresh on its own: its scroll events precede the + // hold, so a hold opened mid-fling still waits out the quiet window even if + // WebKit starves the first events after it opens. + const lastMotionTsRef = React.useRef(Number.NEGATIVE_INFINITY); + React.useEffect(() => { + const scroller = scrollElementRef.current; + if (!scroller) return; + const markMotion = () => { + lastMotionTsRef.current = performance.now(); + }; + scroller.addEventListener("scroll", markMotion, { passive: true }); + scroller.addEventListener("wheel", markMotion, { passive: true }); + return () => { + scroller.removeEventListener("scroll", markMotion); + scroller.removeEventListener("wheel", markMotion); + }; + }, [scrollElementRef]); + React.useEffect(() => { if (!isHoldingPrepend) return; const scroller = scrollElementRef.current; @@ -138,24 +162,15 @@ export function useSettleGatedPrependMessages({ } let frame: number | null = null; const deadline = performance.now() + SETTLE_HOLD_DEADLINE_MS; - // Assume motion at hold start: worst case this costs one quiet window - // (~100ms) behind the fetching-older spinner when the reader was already - // at rest; the alternative admits mid-fling if WebKit starves the first - // scroll events. - let lastMotionTs = performance.now(); let previousScrollTop = scroller.scrollTop; let settledFrames = 0; - const markMotion = () => { - lastMotionTs = performance.now(); - }; - scroller.addEventListener("scroll", markMotion, { passive: true }); - scroller.addEventListener("wheel", markMotion, { passive: true }); const watch = () => { const scrollTop = scroller.scrollTop; settledFrames = Math.abs(scrollTop - previousScrollTop) < 0.5 ? settledFrames + 1 : 0; previousScrollTop = scrollTop; - const quiet = performance.now() - lastMotionTs >= SETTLE_MOTION_WINDOW_MS; + const quiet = + performance.now() - lastMotionTsRef.current >= SETTLE_MOTION_WINDOW_MS; if ( (quiet && settledFrames >= SETTLE_FRAME_COUNT) || performance.now() >= deadline @@ -170,8 +185,6 @@ export function useSettleGatedPrependMessages({ }; frame = requestAnimationFrame(watch); return () => { - scroller.removeEventListener("scroll", markMotion); - scroller.removeEventListener("wheel", markMotion); if (frame !== null) cancelAnimationFrame(frame); }; }, [isHoldingPrepend, scrollElementRef]); diff --git a/desktop/src/features/messages/ui/useTimelineRetention.test.mjs b/desktop/src/features/messages/ui/useTimelineRetention.test.mjs index d965e190107..dcd2c79db55 100644 --- a/desktop/src/features/messages/ui/useTimelineRetention.test.mjs +++ b/desktop/src/features/messages/ui/useTimelineRetention.test.mjs @@ -86,3 +86,52 @@ it("does not keep the full timeline mounted before the viewport is measured", as await act(async () => root.unmount()); dom.window.close(); }); + +it("keeps measured rows mounted while resting at the older-page boundary", async () => { + const dom = new JSDOM( + "
", + ); + let initialRefresh; + Object.assign(globalThis, { + cancelAnimationFrame() { + initialRefresh = undefined; + }, + document: dom.window.document, + IS_REACT_ACT_ENVIRONMENT: true, + requestAnimationFrame(callback) { + initialRefresh = callback; + return 1; + }, + window: dom.window, + }); + + const keys = Array.from({ length: 10_000 }, (_, index) => `message-${index}`); + const itemHeight = 100; + const list = { + findItemIndex(offset) { + return Math.min(keys.length - 1, Math.floor(offset / itemHeight)); + }, + scrollOffset: 500_000, + scrollSize: keys.length * itemHeight, + viewportSize: 1_000, + }; + let retention; + function Harness() { + retention = useTimelineRetention(keys, { current: list }, false); + return null; + } + + const root = createRoot(document.getElementById("root")); + await act(async () => root.render(React.createElement(Harness))); + await act(async () => initialRefresh()); + assert.ok(retention.retainedIndices.includes(5_000)); + + list.scrollOffset = 100; + await act(async () => retention.onScrollEnd()); + + assert.ok(retention.retainedIndices.includes(5_000)); + assert.ok(!retention.retainedIndices.includes(1_000)); + + await act(async () => root.unmount()); + dom.window.close(); +}); diff --git a/desktop/src/features/messages/ui/useTimelineRetention.ts b/desktop/src/features/messages/ui/useTimelineRetention.ts index 05336d4a139..bdcb822b0e4 100644 --- a/desktop/src/features/messages/ui/useTimelineRetention.ts +++ b/desktop/src/features/messages/ui/useTimelineRetention.ts @@ -70,12 +70,16 @@ export function useTimelineRetention( [keys, retainedKeys], ); const onScrollEnd = React.useCallback(() => { + const list = listRef.current; + if (list && list.scrollOffset <= list.viewportSize * 1.5) { + evictionNotBeforeRef.current = performance.now() + 3_000; + } if (refreshTimerRef.current !== null) { clearTimeout(refreshTimerRef.current); refreshTimerRef.current = null; } refreshRetainedKeys(); - }, [refreshRetainedKeys]); + }, [listRef, refreshRetainedKeys]); return { retainedIndices, onScrollEnd }; } diff --git a/desktop/tests/e2e/scroll-history.spec.ts b/desktop/tests/e2e/scroll-history.spec.ts index 0e1d4cfed6f..45e704308a3 100644 --- a/desktop/tests/e2e/scroll-history.spec.ts +++ b/desktop/tests/e2e/scroll-history.spec.ts @@ -1338,7 +1338,15 @@ test("fast middle-page scroll settles with continuous mounted coverage", async ( // Simulate a fast trackpad pass through several middle-page ranges, then // stop. The final evaluate emits the last scroll event; all coverage samples // after it are passive observations. + // + // A real trackpad pass starts with a wheel event, and that is what retires + // the virtualizer's bottom intent. Programmatic `scrollTop` writes are not + // reader input, so without it the prepend's extent resize can legitimately + // re-pin the floor mid-burst and the coverage read measures the wrong place. await timeline.evaluate((element) => { + element.dispatchEvent( + new WheelEvent("wheel", { deltaY: -1, bubbles: true }), + ); const maxOffset = element.scrollHeight - element.clientHeight; for (const fraction of [0.72, 0.28, 0.64, 0.36, 0.58, 0.44, 0.52]) { element.scrollTop = maxOffset * fraction; @@ -1976,18 +1984,51 @@ test("one scroll-up gesture pages older history once, not to the channel top", a // Let any cascade run unimpeded for a generous window. With the bug, the // observer re-arms after each page and digs through all ~1200 older roots in // this window; with the fix it pages once and waits for the next gesture. - await page.waitForTimeout(2_500); + // + // WKWebView keeps emitting `scroll` events while the reader rubber-bands at + // the boundary; each one re-enters the load-older trigger. Chromium emits + // none at rest, which hid the resolve→commit re-fire gap from this test. + // Emulate WebKit: nudge the offset 0↔1 every frame while parked in the + // trigger band, until the prepend commit carries the reader out of it. + await timeline.evaluate(async (element) => { + const deadline = performance.now() + 2_500; + while (performance.now() < deadline) { + if (element.scrollTop <= 200) { + element.scrollTop = element.scrollTop === 0 ? 1 : 0; + } + await new Promise((resolve) => requestAnimationFrame(resolve)); + } + }); const pagesFetched = await fetchCount(); const deepest = await oldestRenderedIndex(); - // One gesture should yield a small, bounded number of pages — not dozens. - // pageOlderMessagesUntilRowFloor may fetch up to MAX_BATCHES_PER_FETCH (3) - // relay pages to satisfy one visible row floor, so allow that ceiling plus a - // little slack; a cascade blows far past it. - expect(pagesFetched).toBeLessThanOrEqual(4); - // And it must NOT have reached the oldest seeded root on its own. + // One gesture commits one page and starts exactly one successor fetch for the + // staged slot. The successor remains outside projection, so a second request + // is expected but a third would still be a resolve→commit cascade. + expect(pagesFetched).toBeLessThanOrEqual(2); + // The staged successor must NOT have rendered on its own. expect(deepest ?? Number.POSITIVE_INFINITY).toBeGreaterThan(50); + + // Once staging is complete, the next gesture consumes that page locally. + // Make any accidental network request visibly too slow, then require the next + // page to paint within 500ms without incrementing the request counter. + await expect.poll(fetchCount).toBe(2); + await page.evaluate(() => { + window.__BUZZ_E2E__ = { + ...window.__BUZZ_E2E__, + mock: { ...window.__BUZZ_E2E__?.mock, channelWindowDelayMs: 2_000 }, + }; + }); + await page.mouse.wheel(0, 1_000); + await page.waitForTimeout(50); + await page.mouse.wheel(0, -4_000); + await expect + .poll(oldestRenderedIndex, { timeout: 500 }) + .toBeLessThan(deepest ?? Number.POSITIVE_INFINITY); + // The only new request is the background successor stage; rendering did not + // wait for its 2s delay. + expect(await fetchCount()).toBe(3); }); // Regression for Wes's "after a page loads I'm yanked to the oldest of the new diff --git a/desktop/tests/e2e/timeline-no-shift.spec.ts b/desktop/tests/e2e/timeline-no-shift.spec.ts index 7065d293785..544a7f4a348 100644 --- a/desktop/tests/e2e/timeline-no-shift.spec.ts +++ b/desktop/tests/e2e/timeline-no-shift.spec.ts @@ -209,7 +209,9 @@ test("timeline does not recompute row estimates during ordinary scroll", async ( await page.goto("/"); await waitForMockTimelineBridge(page); await page.evaluate(() => { - for (let index = 0; index < 120; index += 1) { + // Keep the seed within one channel-window page. This test isolates estimate + // stability during ordinary scrolling; pagination has separate coverage. + for (let index = 0; index < 40; index += 1) { window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ channelName: "general", content: `estimate memo row ${index}\nsecond line ${index}`, @@ -221,7 +223,7 @@ test("timeline does not recompute row estimates during ordinary scroll", async ( await page.getByTestId("channel-general").click(); await expect(page.getByTestId("chat-title")).toHaveText("general"); const timeline = page.getByTestId("message-timeline"); - await expect(timeline).toContainText("estimate memo row 119"); + await expect(timeline).toContainText("estimate memo row 39"); await page.waitForFunction(() => { const element = document.querySelector( '[data-testid="message-timeline"]', diff --git a/desktop/tests/e2e/virtualization.spec.ts b/desktop/tests/e2e/virtualization.spec.ts index f7a44f05c72..7d407948827 100644 --- a/desktop/tests/e2e/virtualization.spec.ts +++ b/desktop/tests/e2e/virtualization.spec.ts @@ -251,9 +251,9 @@ test.describe("list virtualization", () => { // reproduce Chromium/WebKit's native wheel → scroll callback ordering. The // old boundary rollback moved the viewport back down before the fetch // committed; keep that pre-prepend reversal below the same 5px frame bar. - // A 300ms relay delay leaves the input boundary and prepend commit as two - // distinct phases so this assertion cannot accidentally measure only the - // later anchor correction. + // Start outside the history lookahead and let native wheel input cross it. + // This preserves the pre-prepend rollback measurement whether the page is + // already staged locally or still incurs the configured relay delay. await installMockBridge(page, { deepHistoryMessageCount: 1_800, channelWindowDelayMs: 300, @@ -284,6 +284,7 @@ test.describe("list virtualization", () => { id: row.dataset.messageId ?? "", top: row.getBoundingClientRect().top - scrollerTop, scrollHeight: s.scrollHeight, + clientHeight: s.clientHeight, bottomDistance: s.scrollHeight - s.clientHeight - s.scrollTop, }; } @@ -306,7 +307,7 @@ test.describe("list virtualization", () => { }); await page.waitForTimeout(300); await timeline.evaluate((element) => { - element.scrollTop = 180; + element.scrollTop = element.clientHeight * 2; }); await page.waitForTimeout(150); const before = await sampleVisibleAnchor(); @@ -353,12 +354,14 @@ test.describe("list virtualization", () => { const box = await timeline.boundingBox(); if (!box) throw new Error("timeline has no bounding box"); await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2); - for (const deltaY of [-60, -30, -20, -15]) { + for (const deltaY of [-before.clientHeight * 0.75]) { await page.mouse.wheel(0, deltaY); await page.waitForTimeout(12); } const wheelTrace = await wheelTracePromise; - expect(wheelTrace.minScrollTop).toBeLessThanOrEqual(350); + expect(wheelTrace.minScrollTop).toBeLessThanOrEqual( + before.clientHeight * 1.5, + ); expect(wheelTrace.maxBoundaryRollback).toBeLessThan(5); // Linux Chromium delivers CDP wheel input with more latency than macOS, // so the burst's final delta can land AFTER the anchor baseline sample @@ -483,19 +486,19 @@ test.describe("list virtualization", () => { } }); - test("09 — older-page render commit waits for scroller rest under continued wheel input", async ({ + test("09 — older-page render commit is bounded under continued wheel input, and holds the anchor", async ({ page, }) => { test.setTimeout(60_000); - // Production shape for the WKWebView dropped-write hazard: heavy - // variable-height rows, a slow older-page fetch, and wheel input that - // KEEPS ARRIVING through fetch resolution. Every prepend-compensation - // mechanism is a scrollTop write, and macOS WebKit can drop those writes - // while trackpad momentum owns the offset — so the contract under test is - // that the fetched page's RENDER COMMIT (the scrollHeight jump) is - // deferred until input quiesces, and that the at-rest commit then holds - // the anchored row. Chromium cannot reproduce the dropped write itself; - // it CAN prove the commit-at-rest scheduling that makes it unreachable. + // Production shape: heavy variable-height rows, a slow older-page fetch, + // and wheel input that KEEPS ARRIVING through fetch resolution. #1698 held + // the landed page until input stopped entirely; under a sustained trackpad + // gesture that turned a ~450ms page into a multi-second wait behind the + // spinner. The contract now is a bounded hold: the page's RENDER COMMIT + // (the scrollHeight jump) lands within SETTLE_HOLD_DEADLINE_MS of the + // fetch resolving even though input never quiesces, and that commit still + // holds the reader's anchored row. Chromium cannot reproduce WebKit's + // dropped-write hazard; it CAN prove the scheduling bound and the anchor. await installMockBridge(page, { deepHistoryMessageCount: 1_800, channelWindowDelayMs: 300, @@ -505,119 +508,143 @@ test.describe("list virtualization", () => { await expect(timeline.locator("[data-message-id]").first()).toBeVisible(); await page.waitForTimeout(1_000); - // Mount mid-history rows clear of the load-older sentinel, then trip it. + // Mount mid-history rows clear of the load-older trigger, then trip it. await timeline.evaluate((element) => { element.scrollTop = 4000; }); await page.waitForTimeout(300); - // In-page observer: tracks the last wheel-input timestamp, captures the - // first at-rest anchor after input stops, and records when the prepend - // commit (scrollHeight jump) lands relative to the last input. - const tracePromise = timeline.evaluate(async (scroller) => { + // In-page observer: records when the spinner first appears (the fetch is + // in flight), when the prepend commit (scrollHeight jump) lands, how long + // input had been quiet at that moment, and the anchored row's drift + // across the commit. The anchor is re-captured on every frame before the + // commit so it reflects the reader's CURRENT position under live input. + const WHEEL_TICK_PX = 30; + const tracePromise = timeline.evaluate(async (scroller, WHEEL_TICK_PX) => { const s = scroller as HTMLElement; const baseHeight = s.scrollHeight; let lastInputTs = 0; - let sawInput = false; - let restAnchor: { id: string; top: number } | null = null; + let wheelEvents = 0; + // Wheel ticks since the last anchor sample. The commit frame is heavy + // (~100ms of prepend render), so ticks land inside it, and each tick's + // main-thread event and compositor scroll apply on independent + // schedules. The reader's own motion at the commit frame is therefore + // some whole number of ticks — up to one more than the events seen. + // Only displacement that is NOT a whole number of ticks is drift. + let wheelTicksSinceSample = 0; const onWheel = () => { lastInputTs = performance.now(); - sawInput = true; - // Input after a lull invalidates any anchor captured during it — - // the commit must be measured against the FINAL at-rest position. - restAnchor = null; + wheelEvents += 1; + wheelTicksSinceSample += 1; }; s.addEventListener("wheel", onWheel, { passive: true }); - let commit: { ts: number; gapSinceInput: number } | null = null; - let sawSpinnerDuringHold = false; - let anchorDriftAfterCommit: number | null = null; + const topRow = () => { + const scrollerTop = s.getBoundingClientRect().top; + const row = Array.from( + s.querySelectorAll("[data-message-id]"), + ).find( + (candidate) => + candidate.getBoundingClientRect().top - scrollerTop >= 0, + ); + return row?.dataset.messageId + ? { + id: row.dataset.messageId, + top: row.getBoundingClientRect().top - scrollerTop, + } + : null; + }; + let spinnerAt: number | null = null; + let anchor: { id: string; top: number } | null = null; + let commit: { + ts: number; + sinceSpinner: number; + gapSinceInput: number; + wheelEventsBefore: number; + } | null = null; + let anchorDriftAtCommit: number | null = null; const deadline = performance.now() + 8_000; while (performance.now() < deadline) { const now = performance.now(); - if (commit === null) { - if ( - document.querySelector( - '[data-testid="message-timeline-fetching-older"]', - ) !== null - ) { - sawSpinnerDuringHold = true; - } - // First frame at rest (input quiet for 60ms — shorter than the - // gate's own window, so this reading always precedes admission): - // capture the row the at-rest commit must hold. - if (restAnchor === null && sawInput && now - lastInputTs >= 60) { - const scrollerTop = s.getBoundingClientRect().top; + if ( + spinnerAt === null && + document.querySelector( + '[data-testid="message-timeline-fetching-older"]', + ) !== null + ) { + spinnerAt = now; + } + if (s.scrollHeight > baseHeight + 800) { + commit = { + ts: now, + sinceSpinner: spinnerAt === null ? -1 : now - spinnerAt, + gapSinceInput: now - lastInputTs, + wheelEventsBefore: wheelEvents, + }; + if (anchor !== null) { const row = Array.from( s.querySelectorAll("[data-message-id]"), - ).find( - (candidate) => - candidate.getBoundingClientRect().top - scrollerTop >= 0, - ); - if (row?.dataset.messageId) { - restAnchor = { - id: row.dataset.messageId, - top: row.getBoundingClientRect().top - scrollerTop, - }; + ).find((candidate) => candidate.dataset.messageId === anchor?.id); + if (row) { + const top = + row.getBoundingClientRect().top - s.getBoundingClientRect().top; + anchorDriftAtCommit = Number.POSITIVE_INFINITY; + for (let ticks = 0; ticks <= wheelTicksSinceSample + 1; ticks++) { + anchorDriftAtCommit = Math.min( + anchorDriftAtCommit, + Math.abs(top - (anchor.top - ticks * WHEEL_TICK_PX)), + ); + } + } else { + anchorDriftAtCommit = Number.POSITIVE_INFINITY; } } - if (s.scrollHeight > baseHeight + 800) { - commit = { ts: now, gapSinceInput: now - lastInputTs }; - } - } else if (restAnchor !== null) { - const anchor = restAnchor; - const scrollerTop = s.getBoundingClientRect().top; - const row = Array.from( - s.querySelectorAll("[data-message-id]"), - ).find((candidate) => candidate.dataset.messageId === anchor.id); - if (row) { - anchorDriftAfterCommit = Math.max( - anchorDriftAfterCommit ?? 0, - Math.abs( - row.getBoundingClientRect().top - scrollerTop - anchor.top, - ), - ); - } - // Watch a settle window after the commit, then finish. - if (now - commit.ts > 700) break; + break; + } + const sampled = topRow(); + if (sampled) { + anchor = sampled; + wheelTicksSinceSample = 0; } await new Promise((resolve) => requestAnimationFrame(resolve)); } s.removeEventListener("wheel", onWheel); - return { - commit, - capturedRestAnchor: restAnchor !== null, - sawSpinnerDuringHold, - anchorDriftAfterCommit, - }; - }); + return { commit, anchorDriftAtCommit, wheelEvents }; + }, WHEEL_TICK_PX); // Trip the boundary, then keep real wheel input flowing DOWN (away from // the boundary) through and well past the 300ms fetch resolution — the - // mid-gesture window in which the ungated build commits the page. + // sustained-gesture window in which #1698 withheld the page. await timeline.evaluate((element) => { element.scrollTop = 150; }); const box = await timeline.boundingBox(); if (!box) throw new Error("timeline has no bounding box"); await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2); - for (let burst = 0; burst < 30; burst += 1) { - await page.mouse.wheel(0, 30); + for (let burst = 0; burst < 60; burst += 1) { + await page.mouse.wheel(0, WHEEL_TICK_PX); await page.waitForTimeout(40); } const trace = await tracePromise; - // The page must eventually commit — the gate defers, never strands. + // The page committed — and while input was still flowing (the gesture + // above outlasts the fetch by ~2s), not after the reader gave up. expect(trace.commit).not.toBeNull(); - // The commit landed only after input quiesced. On the ungated build the - // deferred snapshot flushes as soon as the fetch resolves — between wheel - // bursts, a gap far below the quiet window — so this line is the red/green - // signal for the settle gate. - expect(trace.commit?.gapSinceInput ?? 0).toBeGreaterThanOrEqual(80); - // The reader saw the fetching affordance while the page was held. - expect(trace.sawSpinnerDuringHold).toBe(true); - // The at-rest commit held the anchored row (writes land at rest). - expect(trace.capturedRestAnchor).toBe(true); - expect(trace.anchorDriftAfterCommit ?? 0).toBeLessThan(5); + expect( + trace.commit?.gapSinceInput ?? Number.POSITIVE_INFINITY, + ).toBeLessThan(80); + // Bounded hold: the commit landed within the gate's deadline of the fetch + // going in flight (spinner) — 300ms network + SETTLE_HOLD_DEADLINE_MS, with + // slack for the rAF watcher. #1698's unbounded hold lands ~2.4s here. + expect(trace.commit?.sinceSpinner ?? Number.POSITIVE_INFINITY).toBeLessThan( + 300 + 1_500 + 250, + ); + // The commit held the reader's anchored row within the 5px contract even + // though it landed under live input (modulo the reader's own whole wheel + // ticks — see the trace). #2855's lost correction read 452px here. + expect(trace.anchorDriftAtCommit).not.toBeNull(); + expect(trace.anchorDriftAtCommit ?? Number.POSITIVE_INFINITY).toBeLessThan( + 5, + ); }); });