Skip to content

Commit ffa1600

Browse files
authored
fix(scroll): coalesce forced repaints across a new-content burst (WebKitGTK half-freeze) (#1012)
## Summary Fixes the residual WebKitGTK "half-freeze" that testers still hit on 0.17.x — even though #860 (pin-loop convergence) shipped in 0.17.1. **Root cause.** The pin-to-bottom `forceRepaint()` (an `overflowY` toggle → forced reflow) is a *full scroller re-layout + repaint* on WebKitGTK, ~50–150ms each (near-free on Chromium/WKWebView-macOS). #860 made each pin run *converge*, but never coalesced the **rate** of runs: the `new-message` trigger is ungated, so every arriving message supersedes the loop and fires a fresh synchronous repaint. A **burst** of new content — live group chatter, a reaction/media storm, or a reconnect flushing queued messages — therefore fires one forced WebKitGTK repaint per arrival, saturating the main thread for up to ~1.5s. This matches the field reports exactly: the freeze only ever reproduces while **new content arrives** (messages, reactions, pictures), never on scrolling or room-switching, and correlates with flaky/Tor connections (a reconnect delivers a burst). **Why it stayed unconfirmed.** `[PinLoopProbe]` is per-run with a 50ms threshold + 5s cooldown, so a burst of ten 60ms repaints logs *one* line and hides the other ~540ms — structurally blind to bursts. ## Fix Generalizes the existing MAM-catchup repaint suppression to **live bursts** (new pure module `pinRepaintBurst.ts`): while content-arrival pins keep firing within `PIN_BURST_WINDOW_MS` (200ms), the intermediate `forceRepaint`s are suppressed — the scroll position is still written via `scrollToIndex`, so the layout stays correct; only the paint is deferred — and the pin loop's convergence forces exactly **one** trailing repaint. A burst of N repaints collapses to ~1–2. The first arrival of a burst still paints immediately, so single sends stay snappy. Burst state is a hook-level ref so the debt survives loop supersede and is flushed by whichever run finally converges; reset on user-scroll takeover and conversation switch. Also adds a burst-aware `[PinBurstProbe] burst settled: … suppressedRepaints=N` log line so an on-device log confirms the mechanism was real and the coalescing engaged. ## Verification - TDD red/green: `MessageList.pinBottomRepaint.test.tsx` → a burst of 8 messages produces 8 forced repaints without the fix, <8 with it; plus 8 unit tests for the tracker. - All 50 `test:scroll` WebKit + Chromium scroll invariants pass (send-stick, at-bottom-stick, reaction-stick, media-drift all intact). - 653 conversation unit tests, full typecheck (SDK + app), and lint all green. WebKitGTK repaint cost is not reproducible on macOS/Chromium, so on-device confirmation from the reporter (via the new `[PinBurstProbe]` line) is the final check.
1 parent 5e5faeb commit ffa1600

4 files changed

Lines changed: 354 additions & 2 deletions

File tree

apps/fluux/src/components/conversation/MessageList.pinBottomRepaint.test.tsx

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,4 +132,51 @@ describe('MessageList — pin forces a repaint after a programmatic scroll (WebK
132132
expect(repaint.overflowSets).toContain('hidden')
133133
expect(repaint.overflowSets[repaint.overflowSets.length - 1]).toBe('')
134134
})
135+
136+
// Regression for the WebKitGTK "half-freeze": a BURST of new bottom rows (live chatter, a reconnect
137+
// flushing queued messages, a reaction/media storm) used to fire one forced overflow-toggle repaint
138+
// PER arrival. On WebKitGTK each is a ~50–150ms full scroller re-layout+repaint, so a burst
139+
// saturated the main thread. The burst coalescer suppresses the intermediate repaints (position is
140+
// still written) and forces ONE trailing repaint on settle — a burst of N collapses to ~1–2 toggles.
141+
it('coalesces a burst of new messages into far fewer forced repaints', () => {
142+
const isAtBottomRef = { current: true }
143+
const base = makeMessages(50)
144+
const { container, rerender } = render(
145+
<MessageList messages={base} conversationId="conv-burst" isAtBottomRef={isAtBottomRef} {...props} />,
146+
)
147+
const scroller = container.querySelector('[data-message-list]') as HTMLElement
148+
instrumentScroller(scroller)
149+
flush(70) // settle the entry pin
150+
repaint.overflowSets = []
151+
scrollToEndCalls.count = 0
152+
153+
// BURST — 8 incoming messages land in rapid succession, each growing the content and firing the
154+
// new-message pin, with only a couple of frames between them (the loop cannot fully settle).
155+
const burst: BaseMessage[] = []
156+
for (let i = 0; i < 8; i++) {
157+
burst.push({
158+
id: `burst-${i}`, from: `user${i}@example.com`, body: `burst ${i}`,
159+
timestamp: new Date(2024, 0, 1, 13, i), isOutgoing: false, type: 'chat',
160+
})
161+
geo.scrollHeight = 2000 + (i + 1) * 40
162+
rerender(
163+
<MessageList messages={[...base, ...burst]} conversationId="conv-burst" isAtBottomRef={isAtBottomRef} {...props} />,
164+
)
165+
flush(2)
166+
}
167+
// Arrival stops — let the loop converge and flush the single trailing repaint.
168+
flush(20)
169+
170+
// Position was still written for every arrival (layout stays correct — nothing is stranded).
171+
expect(scrollToEndCalls.count).toBeGreaterThanOrEqual(8)
172+
173+
// But the expensive forced repaints were coalesced: far fewer overflow 'hidden' toggles than the
174+
// 8 arrivals. Pre-fix this was ~8+ (one per arrival); post-fix it is the first arrival's immediate
175+
// paint plus the trailing settle paint.
176+
const hiddenToggles = repaint.overflowSets.filter((v) => v === 'hidden').length
177+
expect(hiddenToggles).toBeLessThan(8)
178+
expect(hiddenToggles).toBeGreaterThan(0) // the final position IS painted (not left stale)
179+
// Whatever the last toggle, overflow is restored so scrolling still works.
180+
expect(repaint.overflowSets[repaint.overflowSets.length - 1]).toBe('')
181+
})
135182
})
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
import { describe, it, expect } from 'vitest'
2+
import { createPinRepaintBurst, pinBurstProbeLine, PIN_BURST_WINDOW_MS } from './pinRepaintBurst'
3+
4+
describe('createPinRepaintBurst', () => {
5+
it('does NOT suppress an isolated single arrival (snappy common case)', () => {
6+
const b = createPinRepaintBurst()
7+
b.note(1000)
8+
expect(b.suppress(1000)).toBe(false) // first arrival always paints
9+
expect(b.owed()).toBe(false)
10+
})
11+
12+
it('suppresses once a second arrival lands within the window', () => {
13+
const b = createPinRepaintBurst({ windowMs: 200 })
14+
b.note(1000)
15+
expect(b.suppress(1000)).toBe(false)
16+
b.note(1100) // 100ms later → burst
17+
expect(b.suppress(1100)).toBe(true)
18+
expect(b.suppress(1200)).toBe(true) // still inside window (1200 - 1100 < 200)
19+
})
20+
21+
it('stops suppressing once the window elapses with no further arrival', () => {
22+
const b = createPinRepaintBurst({ windowMs: 200 })
23+
b.note(1000)
24+
b.note(1100)
25+
expect(b.suppress(1100)).toBe(true)
26+
expect(b.suppress(1301)).toBe(false) // 1301 - 1100 = 201 >= 200 → window expired
27+
})
28+
29+
it('collapses a burst of N repaints to one owed trailing repaint', () => {
30+
const b = createPinRepaintBurst({ windowMs: 200 })
31+
let suppressed = 0
32+
// Simulate 6 arrivals ~30ms apart, each of which would have painted.
33+
for (let i = 0; i < 6; i++) {
34+
const now = 1000 + i * 30
35+
b.note(now)
36+
const wouldPaint = true
37+
if (wouldPaint && b.suppress(now)) {
38+
b.markSuppressed()
39+
suppressed++
40+
}
41+
}
42+
// First arrival painted (not suppressed); the other five were coalesced.
43+
expect(suppressed).toBe(5)
44+
expect(b.owed()).toBe(true)
45+
const summary = b.settle()
46+
expect(summary.triggers).toBe(6)
47+
expect(summary.suppressedRepaints).toBe(5)
48+
expect(summary.spanMs).toBe(150)
49+
expect(b.owed()).toBe(false) // settle consumed the debt
50+
})
51+
52+
it('a fresh burst after a quiet gap starts a new count', () => {
53+
const b = createPinRepaintBurst({ windowMs: 200 })
54+
b.note(1000)
55+
b.note(1050)
56+
b.settle()
57+
b.note(5000) // long gap → new burst
58+
expect(b.suppress(5000)).toBe(false)
59+
b.note(5050)
60+
expect(b.suppress(5050)).toBe(true)
61+
})
62+
63+
it('reset drops all state', () => {
64+
const b = createPinRepaintBurst()
65+
b.note(1000)
66+
b.note(1050)
67+
b.markSuppressed()
68+
expect(b.owed()).toBe(true)
69+
b.reset()
70+
expect(b.owed()).toBe(false)
71+
expect(b.suppress(1050)).toBe(false)
72+
})
73+
74+
it('exports a sane default window', () => {
75+
expect(PIN_BURST_WINDOW_MS).toBeGreaterThanOrEqual(133) // above the 8-frame settle
76+
})
77+
78+
it('probe line reports the coalesced burst', () => {
79+
const line = pinBurstProbeLine('new-message', { triggers: 10, suppressedRepaints: 9, spanMs: 280 })
80+
expect(line).toContain('[PinBurstProbe]')
81+
expect(line).toContain('trigger=new-message')
82+
expect(line).toContain('arrivals=10')
83+
expect(line).toContain('suppressedRepaints=9')
84+
expect(line).toContain('spanMs=280')
85+
})
86+
})
Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
/**
2+
* Cross-run burst coalescing for `pinVirtualizedBottom`'s forced repaint.
3+
*
4+
* WHY THIS EXISTS
5+
* ---------------
6+
* The pin's `forceRepaint()` (an `overflowY` toggle → forced reflow) is the
7+
* dominant main-thread cost on WebKitGTK — a full scroller re-layout + repaint,
8+
* ~50–150ms each (near-free on Chromium / WKWebView-macOS). PR #860 made each
9+
* INDIVIDUAL pin run converge, but the `new-message` trigger is ungated: every
10+
* arriving message supersedes the running loop and starts a fresh run whose first
11+
* `writePin` forces a repaint. A BURST of new content — live group chatter, a
12+
* reaction storm, images decoding, or a reconnect flushing queued messages —
13+
* therefore fires one forced WebKitGTK repaint per message/per frame, saturating
14+
* the main thread for hundreds of ms: the "half-freeze" testers hit only while new
15+
* content is arriving (never while merely scrolling or switching rooms, which add
16+
* no bottom row).
17+
*
18+
* WHAT IT DOES
19+
* ------------
20+
* This generalizes the MAM-catch-up suppression (`shouldForceRepaint`'s
21+
* `suppressForBackgroundLoad`) to LIVE bursts: while content-arrival pins keep
22+
* firing within a short window, intermediate forced repaints are suppressed (the
23+
* scroll position is still written, so the layout stays correct — only the paint
24+
* is deferred, exactly as during a catch-up). Once arrival quiesces, the pin loop's
25+
* convergence forces exactly ONE trailing repaint. A burst of N repaints collapses
26+
* to ~1, turning a multi-hundred-ms freeze into a single paint.
27+
*
28+
* The state lives OUTSIDE any single pin run (a burst spans many superseded runs),
29+
* so it is owned by the hook and passed timestamps — pure and unit-testable, like
30+
* `pinBottomRun.ts`.
31+
*/
32+
33+
/**
34+
* Two content-arrival pins landing within this window count as a burst; while a
35+
* burst is live, forced repaints are suppressed. Sized a touch above the pin
36+
* loop's settle time (8 frames ≈ 133ms) so that "arrival stopped long enough for
37+
* the loop to converge" reliably implies "burst window expired", letting the
38+
* convergence own the single trailing repaint.
39+
*/
40+
export const PIN_BURST_WINDOW_MS = 200
41+
42+
/** Summary of a coalesced burst, emitted on the trailing repaint for fluux.log. */
43+
export interface PinBurstSummary {
44+
/** Content-arrival pins observed during the burst (the first one still painted). */
45+
triggers: number
46+
/** Forced repaints suppressed by the burst — each ~50–150ms of WebKitGTK freeze avoided. */
47+
suppressedRepaints: number
48+
/** Wall-clock span from the first to the last arrival in the burst. */
49+
spanMs: number
50+
}
51+
52+
export interface PinRepaintBurst {
53+
/**
54+
* Record a content-arrival pin (new-message / content-growth / media-load /
55+
* reaction / mam-catchup-complete). Call at the top of `pinVirtualizedBottom`
56+
* for those triggers — BEFORE it supersedes the running loop — so every arrival
57+
* is counted even though its run may be immediately replaced.
58+
*/
59+
note(now: number): void
60+
/**
61+
* Should this frame's forced repaint be suppressed because a burst is in
62+
* progress? True once ≥2 arrivals have landed within {@link PIN_BURST_WINDOW_MS}
63+
* and the most recent is still inside the window. The very first arrival of a
64+
* burst is NOT suppressed, so an isolated single message still paints promptly.
65+
*/
66+
suppress(now: number): boolean
67+
/** Note that a forced repaint was skipped due to the burst (a trailing paint is now owed). */
68+
markSuppressed(): void
69+
/** Whether a trailing repaint is owed (repaints were suppressed since the last settle). */
70+
owed(): boolean
71+
/**
72+
* Consume the owed trailing repaint: clears the owed flag and returns the burst
73+
* summary for the probe line. Call from the pin loop's convergence / frames-
74+
* exhausted path right before forcing the one final repaint.
75+
*/
76+
settle(): PinBurstSummary
77+
/** Drop all burst state (conversation switch, user-scroll takeover). */
78+
reset(): void
79+
}
80+
81+
export function createPinRepaintBurst(
82+
opts: { windowMs?: number } = {}
83+
): PinRepaintBurst {
84+
const windowMs = opts.windowMs ?? PIN_BURST_WINDOW_MS
85+
86+
let firstNoteAt: number | null = null
87+
let lastNoteAt: number | null = null
88+
let triggers = 0
89+
let suppressedRepaints = 0
90+
let owedRepaint = false
91+
92+
const active = (now: number): boolean =>
93+
lastNoteAt !== null && triggers >= 2 && now - lastNoteAt < windowMs
94+
95+
return {
96+
note(now: number): void {
97+
if (lastNoteAt !== null && now - lastNoteAt >= windowMs) {
98+
// The previous burst went quiet long enough to end; this arrival starts a
99+
// fresh one. (A trailing repaint owed by the old burst is left owed for the
100+
// converging loop to flush.)
101+
firstNoteAt = now
102+
triggers = 1
103+
} else {
104+
if (firstNoteAt === null) firstNoteAt = now
105+
triggers++
106+
}
107+
lastNoteAt = now
108+
},
109+
110+
suppress(now: number): boolean {
111+
return active(now)
112+
},
113+
114+
markSuppressed(): void {
115+
suppressedRepaints++
116+
owedRepaint = true
117+
},
118+
119+
owed(): boolean {
120+
return owedRepaint
121+
},
122+
123+
settle(): PinBurstSummary {
124+
const summary: PinBurstSummary = {
125+
triggers,
126+
suppressedRepaints,
127+
spanMs:
128+
firstNoteAt !== null && lastNoteAt !== null
129+
? Math.round(lastNoteAt - firstNoteAt)
130+
: 0,
131+
}
132+
firstNoteAt = null
133+
lastNoteAt = null
134+
triggers = 0
135+
suppressedRepaints = 0
136+
owedRepaint = false
137+
return summary
138+
},
139+
140+
reset(): void {
141+
firstNoteAt = null
142+
lastNoteAt = null
143+
triggers = 0
144+
suppressedRepaints = 0
145+
owedRepaint = false
146+
},
147+
}
148+
}
149+
150+
/** One fluux.log line attributing a coalesced burst (emitted on the trailing repaint). */
151+
export function pinBurstProbeLine(trigger: string, summary: PinBurstSummary): string {
152+
return (
153+
`[PinBurstProbe] burst settled: trigger=${trigger} arrivals=${summary.triggers} ` +
154+
`suppressedRepaints=${summary.suppressedRepaints} spanMs=${summary.spanMs} trailingRepaint=1`
155+
)
156+
}

0 commit comments

Comments
 (0)