diff --git a/src/vs/base/browser/ui/list/listView.ts b/src/vs/base/browser/ui/list/listView.ts index aee77ac1234f9..07d8fb9cf588d 100644 --- a/src/vs/base/browser/ui/list/listView.ts +++ b/src/vs/base/browser/ui/list/listView.ts @@ -1765,7 +1765,8 @@ export class ListView implements IListView { const diffs = new Array(range.end - range.start).fill(0); const measurements: IDynamicHeightMeasurement[] = []; - for (let index = range.start; index < range.end; index++) { + // Re-check `this.items.length` each iteration so a reentrant splice that shrinks the model mid-loop cannot read out of bounds. + for (let index = range.start; index < range.end && index < this.items.length; index++) { const item = this.items[index]; const delegateHeightDiff = this.probeDynamicHeightFromDelegate(item); if (delegateHeightDiff !== undefined) { diff --git a/src/vs/base/test/browser/ui/list/listView.test.ts b/src/vs/base/test/browser/ui/list/listView.test.ts index 98f603971a076..c496cc4075d85 100644 --- a/src/vs/base/test/browser/ui/list/listView.test.ts +++ b/src/vs/base/test/browser/ui/list/listView.test.ts @@ -465,6 +465,68 @@ suite('ListView', function () { } }); + test('handles a reentrant shrink that leaves later probed indices out of bounds', function () { + const element = document.createElement('div'); + element.style.height = '100px'; + element.style.width = '200px'; + document.body.appendChild(element); + + type TestElement = { id: string; height: number }; + const delegate: IListVirtualDelegate = { + getHeight() { return 100; }, + getTemplateId() { return 'template'; }, + hasDynamicHeight() { return true; } + }; + + const listViewRef: { value?: ListView } = {}; + let shrinkOnRender: TestElement | undefined; + const renderer: IListRenderer = { + templateId: 'template', + renderTemplate(container) { + Object.defineProperty(container, 'offsetHeight', { + configurable: true, + get: () => Number(container.dataset.testHeight) + }); + return container; + }, + renderElement(element, _index, templateData) { + templateData.dataset.testHeight = String(element.height); + if (shrinkOnRender === element) { + shrinkOnRender = undefined; + // Shrink mid-loop so the render range's end extends past the model while a later index is still pending. + listViewRef.value!.splice(1, listViewRef.value!.length - 1); + } + }, + disposeTemplate() { } + }; + + const elements: TestElement[] = range(4).map(index => ({ id: String(index), height: 100 })); + const listView = listViewRef.value = new ListView(element, delegate, [renderer], { supportDynamicHeights: true }); + try { + listView.layout(100, 200); + listView.splice(0, 0, elements); + // Shrink the first two rows so the render range includes indices 2, 3, leaving index 2 pending when the shrink fires at index 1. + elements[0].height = 20; + elements[1].height = 20; + listView.domElement(0)!.dataset.testHeight = String(elements[0].height); + listView.domElement(1)!.dataset.testHeight = String(elements[1].height); + shrinkOnRender = elements[1]; + + listView.layout(100, 201); + + assert.deepStrictEqual({ + length: listView.length, + rowsInDom: element.querySelectorAll('.monaco-list-row').length + }, { + length: 1, + rowsInDom: 1 + }); + } finally { + listView.dispose(); + element.remove(); + } + }); + test('publishes freshly measured dynamic heights', function () { const element = document.createElement('div'); element.style.height = '200px';