chat: defer chat row resize dispatch to break ResizeObserver loop#314882
chat: defer chat row resize dispatch to break ResizeObserver loop#314882bryanchen-d wants to merge 2 commits intomainfrom
Conversation
The `ChatListItemRenderer` row ResizeObserver dispatched `fireItemHeightChange` synchronously inside the observer callback. That re-enters layout in the same animation frame, causing the observer to fire again, which Chromium reports as `ResizeObserver loop completed with undelivered notifications`. Telemetry attribution from #314411 (DisposableResizeObserver named slots) shows this row observer is the dominant source of the warning across our 7-day error stream. Defer the dispatch onto the next task with `setTimeout(0)` so the observer callback returns cleanly before the layout mutation runs. Coalesce repeated observations into a single dispatch using the latest blockSize, and clear the pending handle on template disposal.
There was a problem hiding this comment.
Pull request overview
This PR mitigates Chromium’s synthetic ResizeObserver loop completed with undelivered notifications warning in the chat list by preventing synchronous layout re-entry from the chat row ResizeObserver callback in ChatListItemRenderer.
Changes:
- Defers
fireItemHeightChangedispatch to the next macrotask viasetTimeout(0)from the rowResizeObservercallback. - Coalesces multiple resize notifications into a single pending dispatch using the latest observed
blockSize. - Clears any pending timeout when the template is disposed to avoid late callbacks.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts | Defers/coalesces row height-change dispatches from ResizeObserver to break synchronous resize/layout feedback loops. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 1
| const blockSize = entry.borderBoxSize.at(0)?.blockSize; | ||
| if (typeof blockSize !== 'number') { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Good catch — fixed in 01a64dc. The dispatch is now always scheduled; only the cached pendingBlockSize is gated on typeof blockSize === 'number', so when borderBoxSize is empty/missing we still call fireItemHeightChange(template, undefined) and fall back to getBoundingClientRect() as before.
| if (pendingDispatchHandle !== undefined) { | ||
| return; | ||
| } | ||
| pendingDispatchHandle = setTimeout(() => { |
There was a problem hiding this comment.
Deferring this is going to add the kinds of issues I was talking about. It's not expected that fireItemHeightChange would cause the size of this element to change, we should only change the parent list row to accomodate it, do you know what causes its height to change?
There was a problem hiding this comment.
And since this is mine, I can investigate, do you know of repro steps?
There was a problem hiding this comment.
It's interesting because I don't remember seeing that warning, but this observer fires really often
There was a problem hiding this comment.
Sure, for repro: Start a new chat session with some prompt like "research the repo"
The chat row
ResizeObservercallback inChatListItemRendererdispatchedfireItemHeightChangesynchronously, which re-enters layout in the same animation frame and causes Chromium to fire the syntheticResizeObserver loop completed with undelivered notificationserror.Telemetry attribution shipped in #314411 confirms this row observer (named
ChatListItemRenderer.itemHeight) is the dominant source of the loop warning across our error stream.Fix
Defer the dispatch onto the next task with
setTimeout(0)so the observer callback returns cleanly before the layout mutation runs. Coalesce repeated observations into a single dispatch using the latestblockSize, and clear the pending handle on template disposal.Validation
ResizeObserver loop completed with undelivered notificationsfires repeatedly during streaming and on row hover.Notes
setTimeout(0)(notqueueMicrotask) is required: V8 microtask checkpoints run after each ResizeObserver callback returns, but Chromium dispatches the loop error after the entire observation phase finishes — so deferring to the next task is what breaks the cycle. Same reasoning as dom: attribute ResizeObserver loop telemetry to DisposableResizeObserver name #314411.setTimeout(0)per observed resize per row, coalesced. Negligible relative to the layout work that was already running.