From 95054677eb16f470c6bc4e3597d0a276aefc55c0 Mon Sep 17 00:00:00 2001 From: Erwan Leboucher Date: Sun, 2 Aug 2026 11:38:24 +0200 Subject: [PATCH] fix(timeline): refresh rows whose event decrypted after they were cached --- .../timeline/useProcessedTimeline.test.tsx | 91 +++++++++++++++++++ .../hooks/timeline/useProcessedTimeline.ts | 26 +++++- .../hooks/timeline/useTimelineSync.test.tsx | 90 +++++++++++++++++- src/app/hooks/timeline/useTimelineSync.ts | 21 ++++- 4 files changed, 221 insertions(+), 7 deletions(-) diff --git a/src/app/hooks/timeline/useProcessedTimeline.test.tsx b/src/app/hooks/timeline/useProcessedTimeline.test.tsx index aa636988a..4143ba2b9 100644 --- a/src/app/hooks/timeline/useProcessedTimeline.test.tsx +++ b/src/app/hooks/timeline/useProcessedTimeline.test.tsx @@ -834,3 +834,94 @@ describe('append-only fast path vs full reprocess (fuzz)', () => { } }); }); + +function createEncryptedEvent(id: string, ts: number) { + const state = { + type: EventType.RoomMessageEncrypted as string, + content: { algorithm: 'm.megolm.v1.aes-sha2', ciphertext: 'AwgAEnB' } as Record< + string, + unknown + >, + }; + const mEvent = { + getId: () => id, + getType: () => state.type, + getSender: () => OTHER_USER, + getContent: () => state.content, + getPrevContent: () => ({}), + getWireContent: () => state.content, + getTs: () => ts, + isRedacted: () => false, + isRedaction: () => false, + isEncrypted: () => true, + getRelation: () => null, + threadRootId: undefined, + } as unknown as MatrixEvent; + const decrypt = () => { + state.type = EventType.RoomMessage as string; + state.content = { msgtype: 'm.text', body: 'the secret' }; + }; + return { mEvent, decrypt }; +} + +const bodyOf = (processed: ProcessedEvent[], id: string) => + (processed.find((e) => e.id === id)?.content as Record | undefined)?.body; + +describe('useProcessedTimeline decryption', () => { + // Stable so the append-only fast path is reachable. + const ignoredUsersSet = new Set(); + const renderTimeline = (getEvents: () => MatrixEvent[]) => + renderHook(() => + useProcessedTimeline({ + items: getEvents().map((_, i) => i), + linkedTimelines: [createTimeline(getEvents())], + ignoredUsersSet, + hiddenEvents, + mxUserId: MY_USER, + readUptoEventId: undefined, + hideMembershipEvents: true, + hideNickAvatarEvents: true, + isReadOnly: false, + hideMemberInReadOnly: false, + }) + ); + + it('refreshes a row whose event decrypted since it was cached', () => { + const { mEvent: encrypted, decrypt } = createEncryptedEvent('$enc', 1_000_000); + let events: MatrixEvent[] = [createEvent({ id: '$a', ts: 999_000 }), encrypted]; + + const { result, rerender } = renderTimeline(() => events); + expect(renderedIds(result.current)).toEqual(['$a', '$enc']); + + decrypt(); + rerender(); + + expect(bodyOf(result.current, '$enc')).toBe('the secret'); + }); + + it('does not carry a stale encrypted row through the append-only fast path', () => { + const { mEvent: encrypted, decrypt } = createEncryptedEvent('$enc', 1_000_000); + let events: MatrixEvent[] = [createEvent({ id: '$a', ts: 999_000 }), encrypted]; + + const { result, rerender } = renderTimeline(() => events); + + decrypt(); + events = [...events, createEvent({ id: '$live', ts: 1_001_000 })]; + rerender(); + + expect(renderedIds(result.current)).toEqual(['$a', '$enc', '$live']); + expect(bodyOf(result.current, '$enc')).toBe('the secret'); + }); + + it('keeps the append-only fast path for unencrypted events', () => { + let events: MatrixEvent[] = [createEvent({ id: '$a', ts: 999_000 })]; + const { result, rerender } = renderTimeline(() => events); + const firstRow = result.current[0]; + + events = [...events, createEvent({ id: '$b', ts: 1_000_000 })]; + rerender(); + + expect(renderedIds(result.current)).toEqual(['$a', '$b']); + expect(result.current[0]).toBe(firstRow); + }); +}); diff --git a/src/app/hooks/timeline/useProcessedTimeline.ts b/src/app/hooks/timeline/useProcessedTimeline.ts index 8b8689e47..ef3ab9a5c 100644 --- a/src/app/hooks/timeline/useProcessedTimeline.ts +++ b/src/app/hooks/timeline/useProcessedTimeline.ts @@ -110,17 +110,37 @@ type ProcessedEventDraft = Omit< type TimelineEventEntry = { mEvent: MatrixEvent; timelineSet: EventTimelineSet; + // Decryption rewrites a MatrixEvent in place, so identity alone does not prove a cached + // row still matches it. Undefined for unencrypted events. + clearType: string | undefined; + clearContent: unknown; }; const flattenTimelineEvents = (linkedTimelines: EventTimeline[]): TimelineEventEntry[] => { const entries: TimelineEventEntry[] = []; linkedTimelines.forEach((timeline) => { const timelineSet = timeline.getTimelineSet(); - timeline.getEvents().forEach((mEvent) => entries.push({ mEvent, timelineSet })); + timeline.getEvents().forEach((mEvent) => { + const encrypted = mEvent.isEncrypted(); + entries.push({ + mEvent, + timelineSet, + clearType: encrypted ? mEvent.getType() : undefined, + clearContent: encrypted ? mEvent.getContent() : undefined, + }); + }); }); return entries; }; +const isCachedEntryCurrent = ( + cached: TimelineEventEntry, + current: TimelineEventEntry | undefined +): boolean => + cached.mEvent === current?.mEvent && + cached.clearType === current.clearType && + cached.clearContent === current.clearContent; + const computeCollapseAndDividers = ( drafts: ProcessedEventDraft[], mxUserId: string | null, @@ -648,8 +668,8 @@ export function useProcessedTimeline({ items.every((item, index) => item === index) && // Cached rows are reused verbatim, so anchoring on the first and last event // alone would accept a run that both inserted and removed within the prefix. - previous.timelineEvents.every( - (entry, index) => entry.mEvent === timelineEvents[index]?.mEvent + previous.timelineEvents.every((entry, index) => + isCachedEntryCurrent(entry, timelineEvents[index]) ) && (appendedEntries.length === 0 || (previous.timelineEvents.at(-1)?.mEvent.getTs() ?? 0) <= diff --git a/src/app/hooks/timeline/useTimelineSync.test.tsx b/src/app/hooks/timeline/useTimelineSync.test.tsx index 664aabbad..fadc47677 100644 --- a/src/app/hooks/timeline/useTimelineSync.test.tsx +++ b/src/app/hooks/timeline/useTimelineSync.test.tsx @@ -987,7 +987,9 @@ describe('live-arrive edge cases', () => { await act(async () => { mxEmitter.emit(MatrixEventEvent.Decrypted, { getRoomId: () => room.roomId }); - await Promise.resolve(); + await new Promise((resolve) => { + requestAnimationFrame(() => resolve(undefined)); + }); }); expect(result.current.timeline).not.toBe(before); @@ -1000,7 +1002,9 @@ describe('live-arrive edge cases', () => { await act(async () => { mxEmitter.emit(MatrixEventEvent.Decrypted, { getRoomId: () => '!other:test' }); - await Promise.resolve(); + await new Promise((resolve) => { + requestAnimationFrame(() => resolve(undefined)); + }); }); expect(result.current.timeline).toBe(before); @@ -1244,3 +1248,85 @@ describe('sync transport fuzz', () => { } ); }); + +const flushFrame = async () => { + await act(async () => { + await new Promise((resolve) => { + requestAnimationFrame(() => resolve(undefined)); + }); + }); +}; + +describe('decryption refresh coalescing', () => { + // Counts distinct timeline objects, not renders: unrelated re-renders reuse the object. + const renderTrackingHook = (room: FakeRoom) => { + const seen: unknown[] = []; + renderHook(() => { + const sync = useTimelineSync({ + room: room as Room, + mx: makeMx(), + isAtBottom: true, + isAtBottomRef: { current: true }, + scrollToBottom: vi.fn<() => void>(), + unreadInfo: undefined, + setUnreadInfo: vi.fn<() => void>(), + hideReadsRef: { current: false }, + readUptoEventIdRef: { current: undefined }, + }); + if (!seen.includes(sync.timeline)) seen.push(sync.timeline); + return sync; + }); + return seen; + }; + + // One act() per event mirrors decryptions landing in separate tasks. + const emitDecryptedAcrossTasks = async (room: FakeRoom, eventCount: number) => { + for (let i = 0; i < eventCount; i += 1) { + // eslint-disable-next-line no-await-in-loop + await act(async () => { + mxEmitter.emit(MatrixEventEvent.Decrypted, { getRoomId: () => room.roomId }); + }); + } + }; + + it('collapses a backlog decryption burst into a single timeline update', async () => { + const { room } = createRoom(); + const seen = renderTrackingHook(room); + const initial = seen.length; + + await emitDecryptedAcrossTasks(room, 25); + await flushFrame(); + + // Uncoalesced this is 25. A frame boundary may split the burst, so allow a small range. + expect(seen.length - initial).toBeGreaterThan(0); + expect(seen.length - initial).toBeLessThan(5); + }); + + it('re-arms for the next burst', async () => { + const { room } = createRoom(); + const seen = renderTrackingHook(room); + const initial = seen.length; + + await emitDecryptedAcrossTasks(room, 5); + await flushFrame(); + const afterFirstBurst = seen.length; + await emitDecryptedAcrossTasks(room, 5); + await flushFrame(); + + expect(afterFirstBurst).toBeGreaterThan(initial); + expect(seen.length).toBeGreaterThan(afterFirstBurst); + }); + + it('ignores decryption bursts from another room', async () => { + const { room } = createRoom(); + const seen = renderTrackingHook(room); + const initial = seen.length; + + await act(async () => { + mxEmitter.emit(MatrixEventEvent.Decrypted, { getRoomId: () => '!other:test' }); + }); + await flushFrame(); + + expect(seen.length).toBe(initial); + }); +}); diff --git a/src/app/hooks/timeline/useTimelineSync.ts b/src/app/hooks/timeline/useTimelineSync.ts index b7f1475a9..85aa4a66f 100644 --- a/src/app/hooks/timeline/useTimelineSync.ts +++ b/src/app/hooks/timeline/useTimelineSync.ts @@ -580,12 +580,29 @@ export function useTimelineSync({ useMatrixEvent(room, RoomEvent.LocalEchoUpdated, handleLocalEchoUpdated); + // A cached room decrypts its whole backlog, one event per task, so batching does not + // collapse it. One reprocess a frame instead of one per event. + const decryptedFrameRef = useRef(); const handleDecrypted = useCallback( (mEvent: MatrixEvent) => { if (mEvent.getRoomId() !== room.roomId) return; - setTimeline((ct) => ({ ...ct })); + if (decryptedFrameRef.current !== undefined) return; + decryptedFrameRef.current = requestAnimationFrame(() => { + decryptedFrameRef.current = undefined; + if (!alive()) return; + setTimeline((ct) => ({ ...ct })); + }); }, - [room, setTimeline] + [alive, room, setTimeline] + ); + + useEffect( + () => () => { + if (decryptedFrameRef.current !== undefined) { + cancelAnimationFrame(decryptedFrameRef.current); + } + }, + [] ); useMatrixEvent(mx, MatrixEventEvent.Decrypted, handleDecrypted);