Fix multi-diff browser test teardown

Wait for the responsive-layout diff computation before disposing the test widget, and revert the unrelated virtualized item manager experiment.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 628b1c91-5489-4cfc-baf3-ca556c6e50bc

Author: hediet

src/vs/editor/browser/widget/multiDiffEditor/virtualizedItemManager.ts

@@ -6,7 +6,7 @@
 import { BugIndicatingError } from '../../../../base/common/errors.js';
 import { Emitter, Event } from '../../../../base/common/event.js';
 import { Disposable, DisposableStore, IDisposable, IReference, toDisposable } from '../../../../base/common/lifecycle.js';
-import { autorun, derived, IObservable, ITransaction, observableValue, transaction } from '../../../../base/common/observable.js';
+import { autorun, derived, IObservable, ITransaction, mapObservableArrayCached, observableValue, transaction } from '../../../../base/common/observable.js';
 import { OffsetRange } from '../../../common/core/ranges/offsetRange.js';
 import { ICompressedVirtualizedScrollItem, ICompressedVirtualizedScrollItemContext, ICompressedVirtualizedScrollViewContext } from './compressedVirtualizedScrollView.js';
 
@@ -93,40 +93,21 @@ export interface IVirtualizedItemDelegate<TItem, TBinding extends IVirtualizedIt
 
 export class VirtualizedItemManager<TItem, TBinding extends IVirtualizedItemBinding<TItem>, TTemplate extends IVirtualizedItemTemplate<TItem, TBinding>> extends Disposable {
 	private readonly _pools = new Map<string, VirtualizedTemplatePool<TItem, TBinding, TTemplate>>();
-	private readonly _managedItems = new Map<unknown, ManagedVirtualizedItem<TItem, TBinding, TTemplate>>();
-	private readonly _virtualizedItems = observableValue<readonly ManagedVirtualizedItem<TItem, TBinding, TTemplate>[]>(this, []);
-	readonly virtualizedItems: IObservable<readonly ManagedVirtualizedItem<TItem, TBinding, TTemplate>[]> = this._virtualizedItems;
+	readonly virtualizedItems: IObservable<readonly ManagedVirtualizedItem<TItem, TBinding, TTemplate>[]>;
 
 	constructor(
 		items: IObservable<readonly TItem[]>,
 		private readonly _context: ICompressedVirtualizedScrollViewContext,
 		private readonly _delegate: IVirtualizedItemDelegate<TItem, TBinding, TTemplate>,
 	) {
 		super();
-		this._register(autorun(reader => {
-			const nextItems = items.read(reader);
-			const itemsToRemove = new Set(this._managedItems.keys());
-			const nextManagedItems = nextItems.map(item => {
-				const key = _delegate.getId(item);
-				itemsToRemove.delete(key);
-				let managedItem = this._managedItems.get(key);
-				if (!managedItem) {
-					managedItem = new ManagedVirtualizedItem(item, this, _delegate);
-					this._managedItems.set(key, managedItem);
-				}
-				return managedItem;
-			});
-			for (const key of itemsToRemove) {
-				this._managedItems.get(key)!.dispose();
-				this._managedItems.delete(key);
-			}
-			transaction(tx => this._virtualizedItems.set(nextManagedItems, tx));
-		}));
+		this.virtualizedItems = mapObservableArrayCached(
+			this,
+			items,
+			(item, store) => store.add(new ManagedVirtualizedItem(item, this, _delegate)),
+			item => _delegate.getId(item),
+		).recomputeInitiallyAndOnChange(this._store);
 		this._register(toDisposable(() => {
-			for (const item of this._managedItems.values()) {
-				item.dispose();
-			}
-			this._managedItems.clear();
 			for (const pool of this._pools.values()) {
 				pool.dispose();
 			}

src/vs/editor/test/browser/widget/multiDiffEditorWidget.test.ts

@@ -90,6 +90,7 @@ suite('MultiDiffEditorWidget', () => {
 			const activeControl = widget.getActiveControl();
 			const renderSideBySideWhenNarrow = activeControl?.renderSideBySide;
 			widget.layout(new Dimension(1000, 600));
+			await activeControl?.waitForDiff();
 			assert.deepStrictEqual({
 				configuredAccessibilitySupport: updateOptionsSpy.firstCall.args[0].accessibilitySupport,
 				configuredRenderSideBySide: updateOptionsSpy.firstCall.args[0].renderSideBySide,

src/vs/editor/test/browser/widget/virtualizedItemManager.test.ts

@@ -4,7 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import assert from 'assert';
-import { autorun, constObservable, derived, IObservable, observableValue } from '../../../../base/common/observable.js';
+import { constObservable, IObservable, observableValue } from '../../../../base/common/observable.js';
 import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js';
 import { OffsetRange } from '../../../common/core/ranges/offsetRange.js';
 import { ICompressedVirtualizedScrollViewContext } from '../../../browser/widget/multiDiffEditor/compressedVirtualizedScrollView.js';
@@ -80,37 +80,6 @@ suite('VirtualizedItemManager', () => {
 
 		assert.deepStrictEqual(createdTemplateIds, ['text', 'image']);
 	});
-
-	test('disconnects from source items when disposed while its output is observed', () => {
-		const item = new TestItem('a', 100);
-		const items = observableValue<readonly TestItem[]>('items', [item]);
-		let sourceReadCount = 0;
-		const sourceItems = derived(reader => {
-			sourceReadCount++;
-			return items.read(reader);
-		});
-		const manager = disposables.add(new VirtualizedItemManager<TestItem, TestBinding, TestTemplate>(sourceItems, createContext(), {
-			getId: item => item.id,
-			getTemplateId: () => 'test',
-			getUnboundSize: item => item.size,
-			createTemplate: () => new TestTemplate(),
-		}));
-		const observer = disposables.add(autorun(reader => manager.virtualizedItems.read(reader)));
-
-		manager.dispose();
-		const readCountAfterDispose = sourceReadCount;
-		items.set([], undefined);
-
-		assert.deepStrictEqual({
-			readCountAfterDispose,
-			readCountAfterSourceChange: sourceReadCount,
-		}, {
-			readCountAfterDispose: 1,
-			readCountAfterSourceChange: 1,
-		});
-
-		observer.dispose();
-	});
 });
 
 class TestItem {