Make sure we dispose of every custom editors input individually

Matt Bierner · Matt Bierner · commit 0621245e3235 · 2020-01-24T15:43:42.000-08:00
Fixes microsoft#88229 There should a 1:1 mapping between VS Code's custom editors and custom editors (specifically the webviews) that extensions know about. This was previously not true as a single `CustomEditorInput` could end up being shared between multiple editor groups. With this change, we always create a copy of the custom editor input during split, and then add a listener for `onDidCloseEditor` so that we can make sure we get rid of the unique inputs (and notify extensions of this)
diff --git a/src/vs/workbench/common/editor.ts b/src/vs/workbench/common/editor.ts
@@ -552,8 +552,10 @@ export abstract class EditorInput extends Disposable implements IEditorInput {
 	}
 
 	dispose(): void {
-		this.disposed = true;
-		this._onDispose.fire();
+		if (!this.disposed) {
+			this.disposed = true;
+			this._onDispose.fire();
+		}
 
 		super.dispose();
 	}
diff --git a/src/vs/workbench/contrib/customEditor/browser/customEditors.ts b/src/vs/workbench/contrib/customEditor/browser/customEditors.ts
@@ -4,6 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { coalesce } from 'vs/base/common/arrays';
+import { Emitter } from 'vs/base/common/event';
 import { Lazy } from 'vs/base/common/lazy';
 import { Disposable } from 'vs/base/common/lifecycle';
 import { basename, isEqual } from 'vs/base/common/resources';
@@ -15,9 +16,11 @@ import { IContextKey, IContextKeyService } from 'vs/platform/contextkey/common/c
 import { IEditorOptions, ITextEditorOptions } from 'vs/platform/editor/common/editor';
 import { FileOperation, IFileService } from 'vs/platform/files/common/files';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
+import { ILabelService } from 'vs/platform/label/common/label';
 import { IQuickInputService, IQuickPickItem } from 'vs/platform/quickinput/common/quickInput';
 import * as colorRegistry from 'vs/platform/theme/common/colorRegistry';
 import { registerThemingParticipant } from 'vs/platform/theme/common/themeService';
+import { EditorServiceImpl } from 'vs/workbench/browser/parts/editor/editor';
 import { IWorkbenchContribution } from 'vs/workbench/common/contributions';
 import { EditorInput, EditorOptions, IEditor, IEditorInput } from 'vs/workbench/common/editor';
 import { DiffEditorInput } from 'vs/workbench/common/editor/diffEditorInput';
@@ -30,8 +33,6 @@ import { IEditorGroup, IEditorGroupsService } from 'vs/workbench/services/editor
 import { IEditorService, IOpenEditorOverride } from 'vs/workbench/services/editor/common/editorService';
 import { IWorkingCopyService } from 'vs/workbench/services/workingCopy/common/workingCopyService';
 import { CustomFileEditorInput } from './customEditorInput';
-import { Emitter } from 'vs/base/common/event';
-import { ILabelService } from 'vs/platform/label/common/label';
 
 export const defaultEditorId = 'default';
 
@@ -314,12 +315,26 @@ export const customEditorsAssociationsKey = 'workbench.experimental.editorAssoci
 
 export type CustomEditorsAssociations = readonly (CustomEditorSelector & { readonly viewType: string; })[];
 
-export class CustomEditorContribution implements IWorkbenchContribution {
+export class CustomEditorContribution extends Disposable implements IWorkbenchContribution {
 	constructor(
-		@IEditorService private readonly editorService: IEditorService,
+		@IEditorService private readonly editorService: EditorServiceImpl,
 		@ICustomEditorService private readonly customEditorService: ICustomEditorService,
 	) {
-		this.editorService.overrideOpenEditor((editor, options, group) => this.onEditorOpening(editor, options, group));
+		super();
+
+		this._register(this.editorService.overrideOpenEditor((editor, options, group) => {
+			return this.onEditorOpening(editor, options, group);
+		}));
+
+		this._register(this.editorService.onDidCloseEditor(({ editor }) => {
+			if (!(editor instanceof CustomFileEditorInput)) {
+				return;
+			}
+
+			if (!this.editorService.editors.some(other => other === editor)) {
+				editor.dispose();
+			}
+		}));
 	}
 
 	private onEditorOpening(
@@ -329,7 +344,15 @@ export class CustomEditorContribution implements IWorkbenchContribution {
 	): IOpenEditorOverride | undefined {
 		if (editor instanceof CustomFileEditorInput) {
 			if (editor.group === group.id) {
+				// No need to do anything
 				return undefined;
+			} else {
+				// Create a copy of the input.
+				// Unlike normal editor inputs, we do not want to share custom editor inputs
+				// between multiple editors / groups.
+				return {
+					override: this.customEditorService.openWith(editor.getResource(), editor.viewType, options, group)
+				};
 			}
 		}
 
diff --git a/src/vs/workbench/contrib/webview/browser/webviewEditorInput.ts b/src/vs/workbench/contrib/webview/browser/webviewEditorInput.ts
@@ -39,9 +39,11 @@ export class WebviewInput extends EditorInput {
 	}
 
 	dispose() {
-		if (!this._didSomeoneTakeMyWebview) {
-			this._webview?.rawValue?.dispose();
-			this._onDisposeWebview.fire();
+		if (!this.isDisposed()) {
+			if (!this._didSomeoneTakeMyWebview) {
+				this._webview?.rawValue?.dispose();
+				this._onDisposeWebview.fire();
+			}
 		}
 		super.dispose();
 	}

Original file line number	Diff line number	Diff line change
`@@ -552,8 +552,10 @@ export abstract class EditorInput extends Disposable implements IEditorInput {`
`552`	`552`	`}`
`553`	`553`
`554`	`554`	`dispose(): void {`
`555`		`- this.disposed = true;`
`556`		`- this._onDispose.fire();`
	`555`	`+ if (!this.disposed) {`
	`556`	`+ this.disposed = true;`
	`557`	`+ this._onDispose.fire();`
	`558`	`+ }`
`557`	`559`
`558`	`560`	`super.dispose();`
`559`	`561`	`}`
Original file line number	Diff line number	Diff line change
`@@ -39,9 +39,11 @@ export class WebviewInput extends EditorInput {`
`39`	`39`	`}`
`40`	`40`
`41`	`41`	`dispose() {`
`42`		`- if (!this._didSomeoneTakeMyWebview) {`
`43`		`- this._webview?.rawValue?.dispose();`
`44`		`- this._onDisposeWebview.fire();`
	`42`	`+ if (!this.isDisposed()) {`
	`43`	`+ if (!this._didSomeoneTakeMyWebview) {`
	`44`	`+ this._webview?.rawValue?.dispose();`
	`45`	`+ this._onDisposeWebview.fire();`
	`46`	`+ }`
`45`	`47`	`}`
`46`	`48`	`super.dispose();`
`47`	`49`	`}`