editor - document dispose better

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

@@ -525,21 +525,23 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 		const editor = event.editor;
 		const editorsToClose = [editor];
 
-		// Include both sides of side by side editors when being closed and not opened multiple times
-		if (editor instanceof SideBySideEditorInput && !this.accessor.groups.some(groupView => groupView.group.contains(editor, { strictEquals: true }))) {
+		// Include both sides of side by side editors when being closed
+		if (editor instanceof SideBySideEditorInput) {
 			editorsToClose.push(editor.master, editor.details);
 		}
 
-		// Dispose the editor if it is the last opened one
-		// including diff editors
-		editorsToClose.forEach(editorToClose => {
-			if (!this.accessor.groups.some(groupView => groupView.group.contains(editorToClose, {
-				strictEquals: true,				// only if this input is not shared across editor groups
-				searchInSideBySideEditors: true // include side by side editor master & details
+		// For each editor to close, we call dispose() to free up any resources.
+		// However, certain editors might be shared across multiple editor groups
+		// (including being visible in side by side / diff editors) and as such we
+		// only dispose when they are not opened elsewhere.
+		for (const editor of editorsToClose) {
+			if (!this.accessor.groups.some(groupView => groupView.group.contains(editor, {
+				strictEquals: true,		// only if this input is not shared across editor groups
+				supportSideBySide: true // include side by side editor master & details
 			}))) {
-				editorToClose.dispose();
+				editor.dispose();
 			}
-		});
+		}
 
 		/* __GDPR__
 			"editorClosed" : {

src/vs/workbench/common/editor.ts

@@ -697,6 +697,28 @@ export class SideBySideEditorInput extends EditorInput {
 		this.registerListeners();
 	}
 
+	private registerListeners(): void {
+
+		// When the details or master input gets disposed, dispose this diff editor input
+		const onceDetailsDisposed = Event.once(this.details.onDispose);
+		this._register(onceDetailsDisposed(() => {
+			if (!this.isDisposed()) {
+				this.dispose();
+			}
+		}));
+
+		const onceMasterDisposed = Event.once(this.master.onDispose);
+		this._register(onceMasterDisposed(() => {
+			if (!this.isDisposed()) {
+				this.dispose();
+			}
+		}));
+
+		// Reemit some events from the master side to the outside
+		this._register(this.master.onDidChangeDirty(() => this._onDidChangeDirty.fire()));
+		this._register(this.master.onDidChangeLabel(() => this._onDidChangeLabel.fire()));
+	}
+
 	get resource(): URI | undefined {
 		return undefined;
 	}
@@ -759,28 +781,6 @@ export class SideBySideEditorInput extends EditorInput {
 		return Object.assign(descriptor, super.getTelemetryDescriptor());
 	}
 
-	private registerListeners(): void {
-
-		// When the details or master input gets disposed, dispose this diff editor input
-		const onceDetailsDisposed = Event.once(this.details.onDispose);
-		this._register(onceDetailsDisposed(() => {
-			if (!this.isDisposed()) {
-				this.dispose();
-			}
-		}));
-
-		const onceMasterDisposed = Event.once(this.master.onDispose);
-		this._register(onceMasterDisposed(() => {
-			if (!this.isDisposed()) {
-				this.dispose();
-			}
-		}));
-
-		// Reemit some events from the master side to the outside
-		this._register(this.master.onDidChangeDirty(() => this._onDidChangeDirty.fire()));
-		this._register(this.master.onDidChangeLabel(() => this._onDidChangeLabel.fire()));
-	}
-
 	matches(otherInput: unknown): boolean {
 		if (super.matches(otherInput) === true) {
 			return true;

src/vs/workbench/common/editor/editorGroup.ts

@@ -689,13 +689,13 @@ export class EditorGroup extends Disposable {
 		return [this.editors[index], index];
 	}
 
-	contains(candidate: EditorInput, options?: { searchInSideBySideEditors?: boolean, strictEquals?: boolean }): boolean {
+	contains(candidate: EditorInput, options?: { supportSideBySide?: boolean, strictEquals?: boolean }): boolean {
 		for (const editor of this.editors) {
 			if (this.matches(editor, candidate, options?.strictEquals)) {
 				return true;
 			}
 
-			if (options?.searchInSideBySideEditors && editor instanceof SideBySideEditorInput) {
+			if (options?.supportSideBySide && editor instanceof SideBySideEditorInput) {
 				if (this.matches(editor.master, candidate, options?.strictEquals) || this.matches(editor.details, candidate, options?.strictEquals)) {
 					return true;
 				}

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

@@ -278,10 +278,10 @@ suite('Workbench editor groups', () => {
 
 		assert.equal(group.contains(input1), true);
 		assert.equal(group.contains(input1, { strictEquals: true }), true);
-		assert.equal(group.contains(input1, { searchInSideBySideEditors: true }), true);
+		assert.equal(group.contains(input1, { supportSideBySide: true }), true);
 		assert.equal(group.contains(input2), false);
 		assert.equal(group.contains(input2, { strictEquals: true }), false);
-		assert.equal(group.contains(input2, { searchInSideBySideEditors: true }), false);
+		assert.equal(group.contains(input2, { supportSideBySide: true }), false);
 		assert.equal(group.contains(diffInput1), false);
 		assert.equal(group.contains(diffInput2), false);
 
@@ -309,7 +309,7 @@ suite('Workbench editor groups', () => {
 		group.closeEditor(input1);
 
 		assert.equal(group.contains(input1), false);
-		assert.equal(group.contains(input1, { searchInSideBySideEditors: true }), true);
+		assert.equal(group.contains(input1, { supportSideBySide: true }), true);
 		assert.equal(group.contains(input2), true);
 		assert.equal(group.contains(diffInput1), true);
 		assert.equal(group.contains(diffInput2), true);
@@ -317,27 +317,27 @@ suite('Workbench editor groups', () => {
 		group.closeEditor(input2);
 
 		assert.equal(group.contains(input1), false);
-		assert.equal(group.contains(input1, { searchInSideBySideEditors: true }), true);
+		assert.equal(group.contains(input1, { supportSideBySide: true }), true);
 		assert.equal(group.contains(input2), false);
-		assert.equal(group.contains(input2, { searchInSideBySideEditors: true }), true);
+		assert.equal(group.contains(input2, { supportSideBySide: true }), true);
 		assert.equal(group.contains(diffInput1), true);
 		assert.equal(group.contains(diffInput2), true);
 
 		group.closeEditor(diffInput1);
 
 		assert.equal(group.contains(input1), false);
-		assert.equal(group.contains(input1, { searchInSideBySideEditors: true }), true);
+		assert.equal(group.contains(input1, { supportSideBySide: true }), true);
 		assert.equal(group.contains(input2), false);
-		assert.equal(group.contains(input2, { searchInSideBySideEditors: true }), true);
+		assert.equal(group.contains(input2, { supportSideBySide: true }), true);
 		assert.equal(group.contains(diffInput1), false);
 		assert.equal(group.contains(diffInput2), true);
 
 		group.closeEditor(diffInput2);
 
 		assert.equal(group.contains(input1), false);
-		assert.equal(group.contains(input1, { searchInSideBySideEditors: true }), false);
+		assert.equal(group.contains(input1, { supportSideBySide: true }), false);
 		assert.equal(group.contains(input2), false);
-		assert.equal(group.contains(input2, { searchInSideBySideEditors: true }), false);
+		assert.equal(group.contains(input2, { supportSideBySide: true }), false);
 		assert.equal(group.contains(diffInput1), false);
 		assert.equal(group.contains(diffInput2), false);