dirty diff: use IEditorGroupsService instead of IEditorService (microsoft#166634)

* dirty diff: use IEditorGroupsService instead of IEditorService

also, do not dispose DirtyDiffItem of non-active tabs
fixes microsoft#163499

* address feedback

Co-authored-by: Benjamin Pasero <[email protected]>

Author: joaomoreno

src/vs/workbench/contrib/scm/browser/dirtydiffDecorator.ts

@@ -10,18 +10,16 @@ import { ThrottledDelayer, first } from 'vs/base/common/async';
 import { IDisposable, dispose, toDisposable, Disposable, DisposableStore } from 'vs/base/common/lifecycle';
 import { Event, Emitter } from 'vs/base/common/event';
 import * as ext from 'vs/workbench/common/contributions';
-import { CodeEditorWidget } from 'vs/editor/browser/widget/codeEditorWidget';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { IResolvedTextEditorModel, ITextModelService } from 'vs/editor/common/services/resolverService';
-import { IEditorService } from 'vs/workbench/services/editor/common/editorService';
 import { IEditorWorkerService } from 'vs/editor/common/services/editorWorker';
 import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
 import { URI } from 'vs/base/common/uri';
 import { ISCMService, ISCMRepository, ISCMProvider } from 'vs/workbench/contrib/scm/common/scm';
 import { ModelDecorationOptions } from 'vs/editor/common/model/textModel';
 import { IColorTheme, themeColorFromId, IThemeService, ThemeIcon } from 'vs/platform/theme/common/themeService';
 import { editorErrorForeground, registerColor, transparent } from 'vs/platform/theme/common/colorRegistry';
-import { ICodeEditor, IEditorMouseEvent, MouseTargetType } from 'vs/editor/browser/editorBrowser';
+import { ICodeEditor, IEditorMouseEvent, isCodeEditor, MouseTargetType } from 'vs/editor/browser/editorBrowser';
 import { registerEditorAction, registerEditorContribution, ServicesAccessor, EditorAction } from 'vs/editor/browser/editorExtensions';
 import { PeekViewWidget, getOuterEditor, peekViewBorder, peekViewTitleBackground, peekViewTitleForeground, peekViewTitleInfoForeground } from 'vs/editor/contrib/peekView/browser/peekView';
 import { IContextKeyService, IContextKey, ContextKeyExpr, RawContextKey } from 'vs/platform/contextkey/common/contextkey';
@@ -53,6 +51,10 @@ import { IProgressService, ProgressLocation } from 'vs/platform/progress/common/
 import { IChange } from 'vs/editor/common/diff/smartLinesDiffComputer';
 import { Color } from 'vs/base/common/color';
 import { Iterable } from 'vs/base/common/iterator';
+import { ResourceMap } from 'vs/base/common/map';
+import { IEditorService } from 'vs/workbench/services/editor/common/editorService';
+import { DEFAULT_EDITOR_ASSOCIATION } from 'vs/workbench/common/editor';
+import { FILE_EDITOR_INPUT_ID } from 'vs/workbench/contrib/files/common/files';
 
 class DiffActionRunner extends ActionRunner {
 
@@ -66,7 +68,7 @@ class DiffActionRunner extends ActionRunner {
 }
 
 export interface IModelRegistry {
-	getModel(editorModel: IEditorModel): DirtyDiffModel | null;
+	getModel(editorModel: IEditorModel): DirtyDiffModel | undefined;
 }
 
 export const isDirtyDiffVisible = new RawContextKey<boolean>('dirtyDiffVisible', false);
@@ -1328,7 +1330,10 @@ export class DirtyDiffModel extends Disposable {
 
 class DirtyDiffItem {
 
-	constructor(readonly model: DirtyDiffModel, readonly decorator: DirtyDiffDecorator) { }
+	constructor(
+		readonly model: DirtyDiffModel,
+		readonly decorator: DirtyDiffDecorator
+	) { }
 
 	dispose(): void {
 		this.decorator.dispose();
@@ -1345,7 +1350,7 @@ export class DirtyDiffWorkbenchController extends Disposable implements ext.IWor
 
 	private enabled = false;
 	private viewState: IViewState = { width: 3, visibility: 'always' };
-	private items = new Map<IResolvedTextFileEditorModel, DirtyDiffItem>();
+	private items = new ResourceMap<DirtyDiffItem>();
 	private readonly transientDisposables = this._register(new DisposableStore());
 	private stylesheet: HTMLStyleElement;
 
@@ -1425,7 +1430,7 @@ export class DirtyDiffWorkbenchController extends Disposable implements ext.IWor
 			this.disable();
 		}
 
-		this.transientDisposables.add(this.editorService.onDidVisibleEditorsChange(() => this.onEditorsChanged()));
+		this.transientDisposables.add(Event.any(this.editorService.onDidCloseEditor, this.editorService.onDidVisibleEditorsChange)(() => this.onEditorsChanged()));
 		this.onEditorsChanged();
 		this.enabled = true;
 	}
@@ -1445,59 +1450,38 @@ export class DirtyDiffWorkbenchController extends Disposable implements ext.IWor
 		this.enabled = false;
 	}
 
-	// HACK: This is the best current way of figuring out whether to draw these decorations
-	// or not. Needs context from the editor, to know whether it is a diff editor, in place editor
-	// etc.
 	private onEditorsChanged(): void {
-		const models = this.editorService.visibleTextEditorControls
-
-			// only interested in code editor widgets
-			.filter(c => c instanceof CodeEditorWidget)
+		for (const editor of this.editorService.visibleTextEditorControls) {
+			if (isCodeEditor(editor)) {
+				const textModel = editor.getModel();
+				const controller = DirtyDiffController.get(editor);
 
-			// set model registry and map to models
-			.map(editor => {
-				const codeEditor = editor as CodeEditorWidget;
-				const controller = DirtyDiffController.get(codeEditor);
 				if (controller) {
 					controller.modelRegistry = this;
 				}
-				return codeEditor.getModel();
-			})
-
-			// remove nulls and duplicates
-			.filter((m, i, a) => !!m && !!m.uri && a.indexOf(m, i + 1) === -1)
-
-			// only want resolved text file service models
-			.map(m => this.textFileService.files.get(m!.uri))
-			.filter(m => m?.isResolved()) as IResolvedTextFileEditorModel[];
-
-		const set = new Set(models);
-		const newModels = models.filter(o => !this.items.has(o));
-		const oldModels = [...this.items.keys()].filter(m => !set.has(m));
-
-		oldModels.forEach(m => this.onModelInvisible(m));
-		newModels.forEach(m => this.onModelVisible(m));
-	}
 
-	private onModelVisible(textFileModel: IResolvedTextFileEditorModel): void {
-		const model = this.instantiationService.createInstance(DirtyDiffModel, textFileModel);
-		const decorator = new DirtyDiffDecorator(textFileModel.textEditorModel, model, this.configurationService);
-		this.items.set(textFileModel, new DirtyDiffItem(model, decorator));
-	}
+				if (textModel && !this.items.has(textModel.uri)) {
+					const textFileModel = this.textFileService.files.get(textModel.uri);
 
-	private onModelInvisible(textFileModel: IResolvedTextFileEditorModel): void {
-		this.items.get(textFileModel)!.dispose();
-		this.items.delete(textFileModel);
-	}
+					if (textFileModel?.isResolved()) {
+						const dirtyDiffModel = this.instantiationService.createInstance(DirtyDiffModel, textFileModel);
+						const decorator = new DirtyDiffDecorator(textFileModel.textEditorModel, dirtyDiffModel, this.configurationService);
+						this.items.set(textModel.uri, new DirtyDiffItem(dirtyDiffModel, decorator));
+					}
+				}
+			}
+		}
 
-	getModel(editorModel: ITextModel): DirtyDiffModel | null {
-		for (const [model, diff] of this.items) {
-			if (model.textEditorModel.id === editorModel.id) {
-				return diff.model;
+		for (const [uri, item] of this.items) {
+			if (!this.editorService.isOpened({ resource: uri, typeId: FILE_EDITOR_INPUT_ID, editorId: DEFAULT_EDITOR_ASSOCIATION.id })) {
+				item.dispose();
+				this.items.delete(uri);
 			}
 		}
+	}
 
-		return null;
+	getModel(editorModel: ITextModel): DirtyDiffModel | undefined {
+		return this.items.get(editorModel.uri)?.model;
 	}
 
 	override dispose(): void {