Skip to content
Open
3 changes: 2 additions & 1 deletion src/vs/base/browser/ui/list/listView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1765,7 +1765,8 @@ export class ListView<T> implements IListView<T> {
const diffs = new Array<number>(range.end - range.start).fill(0);
const measurements: IDynamicHeightMeasurement<T>[] = [];

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++) {
Comment thread
vs-code-engineering[bot] marked this conversation as resolved.
const item = this.items[index];
const delegateHeightDiff = this.probeDynamicHeightFromDelegate(item);
if (delegateHeightDiff !== undefined) {
Expand Down
62 changes: 62 additions & 0 deletions src/vs/base/test/browser/ui/list/listView.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<TestElement> = {
getHeight() { return 100; },
getTemplateId() { return 'template'; },
hasDynamicHeight() { return true; }
};

const listViewRef: { value?: ListView<TestElement> } = {};
let shrinkOnRender: TestElement | undefined;
const renderer: IListRenderer<TestElement, HTMLElement> = {
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<TestElement>(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';
Expand Down