list view: make sure all templates are disposed when list is disposed

joaomoreno · joaomoreno · commit f3ac15e5be9b · 2018-06-22T10:56:05.000+02:00
diff --git a/src/vs/base/browser/ui/list/listView.ts b/src/vs/base/browser/ui/list/listView.ts
@@ -484,7 +484,17 @@ export class ListView<T> implements ISpliceable<T>, IDisposable {
 	// Dispose
 
 	dispose() {
-		this.items = null;
+		if (this.items) {
+			for (const item of this.items) {
+				if (item.row) {
+					const renderer = this.renderers.get(item.row.templateId);
+					renderer.disposeTemplate(item.row.templateData);
+					item.row = null;
+				}
+			}
+
+			this.items = null;
+		}
 
 		if (this._domNode && this._domNode.parentElement) {
 			this._domNode.parentNode.removeChild(this._domNode);
diff --git a/src/vs/base/test/browser/ui/list/listView.test.ts b/src/vs/base/test/browser/ui/list/listView.test.ts
@@ -0,0 +1,40 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import * as assert from 'assert';
+import { ListView } from 'vs/base/browser/ui/list/listView';
+import { IDelegate, IRenderer } from 'vs/base/browser/ui/list/list';
+import { range } from 'vs/base/common/arrays';
+
+suite('ListView', function () {
+	test('all rows get disposed', function () {
+		const element = document.createElement('div');
+		element.style.height = '200px';
+		element.style.width = '200px';
+
+		const delegate: IDelegate<number> = {
+			getHeight() { return 20; },
+			getTemplateId() { return 'template'; }
+		};
+
+		let templatesCount = 0;
+
+		const renderer: IRenderer<number, void> = {
+			templateId: 'template',
+			renderTemplate() { templatesCount++; },
+			renderElement() { },
+			disposeTemplate() { templatesCount--; }
+		};
+
+		const listView = new ListView<number>(element, delegate, [renderer]);
+		listView.layout(200);
+
+		assert.equal(templatesCount, 0, 'no templates have been allocated');
+		listView.splice(0, 0, range(100));
+		assert.equal(templatesCount, 10, 'some templates have been allocated');
+		listView.dispose();
+		assert.equal(templatesCount, 0, 'all templates have been disposed');
+	});
+});
