editors - fix listener leak (#110336)

src/vs/workbench/browser/parts/editor/editorPane.ts

@@ -220,18 +220,7 @@ export class EditorMemento<T> implements IEditorMemento<T> {
 
 		// Automatically clear when editor input gets disposed if any
 		if (resourceOrEditor instanceof EditorInput) {
-			const editor = resourceOrEditor;
-
-			if (!this.editorDisposables) {
-				this.editorDisposables = new Map<EditorInput, IDisposable>();
-			}
-
-			if (!this.editorDisposables.has(editor)) {
-				this.editorDisposables.set(editor, Event.once(resourceOrEditor.onDispose)(() => {
-					this.clearEditorState(resource);
-					this.editorDisposables?.delete(editor);
-				}));
-			}
+			this.clearEditorStateOnDispose(resource, resourceOrEditor);
 		}
 	}
 
@@ -240,7 +229,7 @@ export class EditorMemento<T> implements IEditorMemento<T> {
 	loadEditorState(group: IEditorGroup, resourceOrEditor: URI | EditorInput, fallbackToOtherGroupState?: boolean): T | undefined {
 		const resource = this.doGetResource(resourceOrEditor);
 		if (!resource || !group) {
-			return undefined; // we are not in a good state to load any state for a resource
+			return; // we are not in a good state to load any state for a resource
 		}
 
 		const cache = this.doLoad();
@@ -261,12 +250,16 @@ export class EditorMemento<T> implements IEditorMemento<T> {
 			}
 		}
 
-		return undefined;
+		return;
 	}
 
 	clearEditorState(resource: URI, group?: IEditorGroup): void;
 	clearEditorState(editor: EditorInput, group?: IEditorGroup): void;
 	clearEditorState(resourceOrEditor: URI | EditorInput, group?: IEditorGroup): void {
+		if (resourceOrEditor instanceof EditorInput) {
+			this.editorDisposables?.delete(resourceOrEditor);
+		}
+
 		const resource = this.doGetResource(resourceOrEditor);
 		if (resource) {
 			const cache = this.doLoad();
@@ -286,6 +279,19 @@ export class EditorMemento<T> implements IEditorMemento<T> {
 		}
 	}
 
+	clearEditorStateOnDispose(resource: URI, editor: EditorInput): void {
+		if (!this.editorDisposables) {
+			this.editorDisposables = new Map<EditorInput, IDisposable>();
+		}
+
+		if (!this.editorDisposables.has(editor)) {
+			this.editorDisposables.set(editor, Event.once(editor.onDispose)(() => {
+				this.clearEditorState(resource);
+				this.editorDisposables?.delete(editor);
+			}));
+		}
+	}
+
 	moveEditorState(source: URI, target: URI, comparer: IExtUri): void {
 		const cache = this.doLoad();
 
@@ -342,7 +348,7 @@ export class EditorMemento<T> implements IEditorMemento<T> {
 	saveState(): void {
 		const cache = this.doLoad();
 
-		// Cleanup once during shutdown
+		// Cleanup once during session
 		if (!this.cleanedUp) {
 			this.cleanUp();
 			this.cleanedUp = true;

src/vs/workbench/browser/parts/editor/textDiffEditor.ts

@@ -24,7 +24,6 @@ import { ScrollType, IDiffEditorViewState, IDiffEditorModel } from 'vs/editor/co
 import { DisposableStore } from 'vs/base/common/lifecycle';
 import { Registry } from 'vs/platform/registry/common/platform';
 import { URI } from 'vs/base/common/uri';
-import { Event } from 'vs/base/common/event';
 import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editorGroupsService';
 import { IEditorService, ACTIVE_GROUP } from 'vs/workbench/services/editor/common/editorService';
 import { CancellationToken } from 'vs/base/common/cancellation';
@@ -309,12 +308,7 @@ export class TextDiffEditor extends BaseTextEditor implements ITextDiffEditorPan
 
 		// Otherwise save it
 		else {
-			super.saveTextEditorViewState(resource);
-
-			// Make sure to clean up when the input gets disposed
-			Event.once(input.onDispose)(() => {
-				super.clearTextEditorViewState([resource], this.group);
-			});
+			super.saveTextEditorViewState(resource, input);
 		}
 	}
 

src/vs/workbench/browser/parts/editor/textEditor.ts

@@ -223,13 +223,17 @@ export abstract class BaseTextEditor extends EditorPane implements ITextEditorPa
 		return this.editorControl;
 	}
 
-	protected saveTextEditorViewState(resource: URI): void {
+	protected saveTextEditorViewState(resource: URI, editor?: IEditorInput): void {
 		const editorViewState = this.retrieveTextEditorViewState(resource);
 		if (!editorViewState || !this.group) {
 			return;
 		}
 
 		this.editorMemento.saveEditorState(this.group, resource, editorViewState);
+
+		if (editor) {
+			this.editorMemento.clearEditorStateOnDispose(resource, editor);
+		}
 	}
 
 	protected shouldRestoreTextEditorViewState(editor: IEditorInput, context?: IEditorOpenContext): boolean {

src/vs/workbench/browser/parts/editor/textResourceEditor.ts

@@ -16,7 +16,6 @@ import { IStorageService } from 'vs/platform/storage/common/storage';
 import { ITextResourceConfigurationService } from 'vs/editor/common/services/textResourceConfigurationService';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { IThemeService } from 'vs/platform/theme/common/themeService';
-import { Event } from 'vs/base/common/event';
 import { ScrollType, IEditor } from 'vs/editor/common/editorCommon';
 import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editorGroupsService';
 import { CancellationToken } from 'vs/base/common/cancellation';
@@ -158,12 +157,7 @@ export class AbstractTextResourceEditor extends BaseTextEditor {
 
 		// Otherwise save it
 		else {
-			super.saveTextEditorViewState(resource);
-
-			// Make sure to clean up when the input gets disposed
-			Event.once(input.onDispose)(() => {
-				super.clearTextEditorViewState([resource]);
-			});
+			super.saveTextEditorViewState(resource, input);
 		}
 	}
 }

src/vs/workbench/common/editor.ts

@@ -1422,13 +1422,15 @@ export const enum CloseDirection {
 export interface IEditorMemento<T> {
 
 	saveEditorState(group: IEditorGroup, resource: URI, state: T): void;
-	saveEditorState(group: IEditorGroup, editor: EditorInput, state: T): void;
+	saveEditorState(group: IEditorGroup, editor: IEditorInput, state: T): void;
 
 	loadEditorState(group: IEditorGroup, resource: URI): T | undefined;
-	loadEditorState(group: IEditorGroup, editor: EditorInput): T | undefined;
+	loadEditorState(group: IEditorGroup, editor: IEditorInput): T | undefined;
 
 	clearEditorState(resource: URI, group?: IEditorGroup): void;
-	clearEditorState(editor: EditorInput, group?: IEditorGroup): void;
+	clearEditorState(editor: IEditorInput, group?: IEditorGroup): void;
+
+	clearEditorStateOnDispose(resource: URI, editor: IEditorInput): void;
 
 	moveEditorState(source: URI, target: URI, comparer: IExtUri): void;
 }

src/vs/workbench/contrib/files/browser/editors/textFileEditor.ts

@@ -65,7 +65,7 @@ export class TextFileEditor extends BaseTextEditor {
 	private onDidFilesChange(e: FileChangesEvent): void {
 		const deleted = e.getDeleted();
 		if (deleted?.length) {
-			this.clearTextEditorViewState(deleted.map(d => d.resource));
+			this.clearTextEditorViewState(deleted.map(({ resource }) => resource));
 		}
 	}
 

src/vs/workbench/test/browser/parts/editor/editorPane.test.ts

@@ -325,6 +325,62 @@ suite('Workbench EditorPane', () => {
 		assert.ok(!res);
 	});
 
+	test('EditoMemento - clear on editor dispose', function () {
+		const testGroup0 = new TestEditorGroupView(0);
+
+		interface TestViewState {
+			line: number;
+		}
+
+		class TestEditorInput extends EditorInput {
+			constructor(public resource: URI, private id = 'testEditorInputForMementoTest') {
+				super();
+			}
+			getTypeId() { return 'testEditorInputForMementoTest'; }
+			resolve(): Promise<IEditorModel> { return Promise.resolve(null!); }
+
+			matches(other: TestEditorInput): boolean {
+				return other && this.id === other.id && other instanceof TestEditorInput;
+			}
+		}
+
+		const rawMemento = Object.create(null);
+		let memento = new EditorMemento<TestViewState>('id', 'key', rawMemento, 3, new TestEditorGroupsService());
+
+		const testInputA = new TestEditorInput(URI.file('/A'));
+
+		let res = memento.loadEditorState(testGroup0, testInputA);
+		assert.ok(!res);
+
+		memento.saveEditorState(testGroup0, testInputA.resource, { line: 3 });
+		res = memento.loadEditorState(testGroup0, testInputA);
+		assert.ok(res);
+		assert.equal(res!.line, 3);
+
+		// State not yet removed when input gets disposed
+		// because we used resource
+		testInputA.dispose();
+		res = memento.loadEditorState(testGroup0, testInputA);
+		assert.ok(res);
+
+		const testInputB = new TestEditorInput(URI.file('/B'));
+
+		res = memento.loadEditorState(testGroup0, testInputB);
+		assert.ok(!res);
+
+		memento.saveEditorState(testGroup0, testInputB.resource, { line: 3 });
+		res = memento.loadEditorState(testGroup0, testInputB);
+		assert.ok(res);
+		assert.equal(res!.line, 3);
+
+		memento.clearEditorStateOnDispose(testInputB.resource, testInputB);
+
+		// State removed when input gets disposed
+		testInputB.dispose();
+		res = memento.loadEditorState(testGroup0, testInputB);
+		assert.ok(!res);
+	});
+
 	return {
 		MyEditor: MyEditor,
 		MyOtherEditor: MyOtherEditor