Buffer untitled/text file model changed events and make the event type consistent (fixes #14186)

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

@@ -17,8 +17,12 @@ import { IModeService } from 'vs/editor/common/services/modeService';
 import { IModelService } from 'vs/editor/common/services/modelService';
 import { IMode } from 'vs/editor/common/modes';
 import Event, { Emitter } from 'vs/base/common/event';
+import { RunOnceScheduler } from 'vs/base/common/async';
 
 export class UntitledEditorModel extends StringEditorModel implements IEncodingSupport {
+
+	public static DEFAULT_CONTENT_CHANGE_BUFFER_DELAY = 1000;
+
 	private textModelChangeListener: IDisposable;
 	private configurationChangeListener: IDisposable;
 
@@ -27,6 +31,8 @@ export class UntitledEditorModel extends StringEditorModel implements IEncodingS
 	private _onDidChangeDirty: Emitter<void>;
 	private _onDidChangeEncoding: Emitter<void>;
 
+	private contentChangeEventScheduler: RunOnceScheduler;
+
 	private configuredEncoding: string;
 	private preferredEncoding: string;
 
@@ -50,6 +56,8 @@ export class UntitledEditorModel extends StringEditorModel implements IEncodingS
 		this._onDidChangeDirty = new Emitter<void>();
 		this._onDidChangeEncoding = new Emitter<void>();
 
+		this.contentChangeEventScheduler = new RunOnceScheduler(() => this._onDidChangeContent.fire(), UntitledEditorModel.DEFAULT_CONTENT_CHANGE_BUFFER_DELAY);
+
 		this.registerListeners();
 	}
 
@@ -124,8 +132,11 @@ export class UntitledEditorModel extends StringEditorModel implements IEncodingS
 	public revert(): void {
 		this.dirty = false;
 
-		this._onDidChangeContent.fire();
+		// Events
 		this._onDidChangeDirty.fire();
+
+		// Handle content change event buffered
+		this.contentChangeEventScheduler.schedule();
 	}
 
 	public load(): TPromise<EditorModel> {
@@ -157,10 +168,10 @@ export class UntitledEditorModel extends StringEditorModel implements IEncodingS
 		else if (!this.dirty) {
 			this.dirty = true;
 			this._onDidChangeDirty.fire();
-
 		}
 
-		this._onDidChangeContent.fire();
+		// Handle content change event buffered
+		this.contentChangeEventScheduler.schedule();
 	}
 
 	public dispose(): void {
@@ -176,6 +187,8 @@ export class UntitledEditorModel extends StringEditorModel implements IEncodingS
 			this.configurationChangeListener = null;
 		}
 
+		this.contentChangeEventScheduler.dispose();
+
 		this._onDidChangeContent.dispose();
 		this._onDidChangeDirty.dispose();
 		this._onDidChangeEncoding.dispose();

src/vs/workbench/parts/backup/common/backupModelTracker.ts

@@ -8,7 +8,7 @@
 import Uri from 'vs/base/common/uri';
 import { IBackupService, IBackupFileService } from 'vs/workbench/services/backup/common/backup';
 import { IDisposable, dispose } from 'vs/base/common/lifecycle';
-import { ITextFileService, TextFileModelChangeEvent } from 'vs/workbench/services/textfile/common/textfiles';
+import { ITextFileService, TextFileModelChangeEvent, StateChange } from 'vs/workbench/services/textfile/common/textfiles';
 import { IFileService } from 'vs/platform/files/common/files';
 import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace';
 import { IUntitledEditorService } from 'vs/workbench/services/untitled/common/untitledEditorService';
@@ -43,7 +43,6 @@ export class BackupModelTracker implements IWorkbenchContribution {
 		// Listen for text file model changes
 		this.toDispose.push(this.textFileService.models.onModelContentChanged((e) => this.onTextFileModelChanged(e)));
 		this.toDispose.push(this.textFileService.models.onModelSaved((e) => this.discardBackup(e.resource)));
-		this.toDispose.push(this.textFileService.models.onModelReverted((e) => this.discardBackup(e.resource)));
 		this.toDispose.push(this.textFileService.models.onModelDisposed((e) => this.discardBackup(e)));
 
 		// Listen for untitled model changes
@@ -52,9 +51,13 @@ export class BackupModelTracker implements IWorkbenchContribution {
 	}
 
 	private onTextFileModelChanged(event: TextFileModelChangeEvent): void {
-		if (this.backupService.isHotExitEnabled) {
-			const model = this.textFileService.models.get(event.resource);
-			this.backupService.doBackup(model.getResource(), model.getValue());
+		if (event.kind === StateChange.REVERTED) {
+			this.discardBackup(event.resource);
+		} else if (event.kind === StateChange.CONTENT_CHANGE) {
+			if (this.backupService.isHotExitEnabled) {
+				const model = this.textFileService.models.get(event.resource);
+				this.backupFileService.backupResource(model.getResource(), model.getValue());
+			}
 		}
 	}
 
@@ -62,7 +65,7 @@ export class BackupModelTracker implements IWorkbenchContribution {
 		if (this.backupService.isHotExitEnabled) {
 			const input = this.untitledEditorService.get(resource);
 			if (input.isDirty()) {
-				this.backupService.doBackup(resource, input.getValue());
+				this.backupFileService.backupResource(resource, input.getValue());
 			} else {
 				this.backupFileService.discardResourceBackup(resource);
 			}

src/vs/workbench/services/backup/common/backup.ts

@@ -27,8 +27,6 @@ export interface IBackupService {
 	isHotExitEnabled: boolean;
 	backupBeforeShutdown(dirtyToBackup: Uri[], textFileEditorModelManager: ITextFileEditorModelManager, quitRequested: boolean): TPromise<IBackupResult>;
 	cleanupBackupsBeforeShutdown(): TPromise<void>;
-
-	doBackup(resource: Uri, content: string, immediate?: boolean): TPromise<void>;
 }
 
 /**

src/vs/workbench/services/backup/node/backupService.ts

@@ -26,8 +26,6 @@ export class BackupService implements IBackupService {
 
 	private toDispose: IDisposable[];
 
-	private backupPromises: TPromise<void>[];
-
 	private configuredHotExit: boolean;
 
 	constructor(
@@ -39,7 +37,6 @@ export class BackupService implements IBackupService {
 		@IEnvironmentService private environmentService: IEnvironmentService
 	) {
 		this.toDispose = [];
-		this.backupPromises = [];
 
 		this.registerListeners();
 	}
@@ -53,38 +50,6 @@ export class BackupService implements IBackupService {
 		this.configuredHotExit = this.contextService.getWorkspace() && configuration && configuration.files && configuration.files.hotExit;
 	}
 
-	private backupImmediately(resource: Uri, content: string): TPromise<void> {
-		if (!resource) {
-			return TPromise.as(void 0);
-		}
-
-		return this.doBackup(resource, content, true);
-	}
-
-	public doBackup(resource: Uri, content: string, immediate?: boolean): TPromise<void> {
-		// Cancel any currently running backups to make this the one that succeeds
-		this.cancelBackupPromises();
-
-		if (immediate) {
-			return this.backupFileService.backupResource(resource, content);
-		}
-
-		// Create new backup promise and keep it
-		const promise = TPromise.timeout(1000).then(() => {
-			this.backupFileService.backupResource(resource, content); // Very important here to not return the promise because if the timeout promise is canceled it will bubble up the error otherwise - do not change
-		});
-
-		this.backupPromises.push(promise);
-
-		return promise;
-	}
-
-	private cancelBackupPromises(): void {
-		while (this.backupPromises.length) {
-			this.backupPromises.pop().cancel();
-		}
-	}
-
 	/**
 	 * Performs an immedate backup of all dirty file and untitled models.
 	 */
@@ -106,7 +71,7 @@ export class BackupService implements IBackupService {
 	private doBackupAll(dirtyFileModels: ITextFileEditorModel[], untitledResources: Uri[]): TPromise<void> {
 		// Handle file resources first
 		return TPromise.join(dirtyFileModels.map(model => {
-			return this.backupImmediately(model.getResource(), model.getValue()).then(() => void 0);
+			return this.backupFileService.backupResource(model.getResource(), model.getValue());
 		})).then(results => {
 			// Handle untitled resources
 			const untitledModelPromises = untitledResources.map(untitledResource => this.untitledEditorService.get(untitledResource))
@@ -115,7 +80,7 @@ export class BackupService implements IBackupService {
 
 			return TPromise.join(untitledModelPromises).then(untitledModels => {
 				const untitledBackupPromises = untitledModels.map(model => {
-					return this.backupImmediately(model.getResource(), model.getValue());
+					return this.backupFileService.backupResource(model.getResource(), model.getValue());
 				});
 				return TPromise.join(untitledBackupPromises).then(() => void 0);
 			});
@@ -162,7 +127,5 @@ export class BackupService implements IBackupService {
 
 	public dispose(): void {
 		this.toDispose = dispose(this.toDispose);
-
-		this.cancelBackupPromises();
 	}
 }
\ No newline at end of file

src/vs/workbench/services/textfile/common/textFileEditorModel.ts

@@ -29,6 +29,7 @@ import { IMessageService, Severity } from 'vs/platform/message/common/message';
 import { IModeService } from 'vs/editor/common/services/modeService';
 import { IModelService } from 'vs/editor/common/services/modelService';
 import { ITelemetryService, anonymize } from 'vs/platform/telemetry/common/telemetry';
+import { RunOnceScheduler } from 'vs/base/common/async';
 
 /**
  * The text file editor model listens to changes to its underlying code editor model and saves these changes through the file service back to the disk.
@@ -37,6 +38,8 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
 
 	public static ID = 'workbench.editors.files.textFileEditorModel';
 
+	public static DEFAULT_CONTENT_CHANGE_BUFFER_DELAY = 1000;
+
 	private static saveErrorHandler: ISaveErrorHandler;
 	private static saveParticipant: ISaveParticipant;
 
@@ -52,13 +55,14 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
 	private autoSaveAfterMillies: number;
 	private autoSaveAfterMilliesEnabled: boolean;
 	private autoSavePromises: TPromise<void>[];
+	private contentChangeEventScheduler: RunOnceScheduler;
 	private mapPendingSaveToVersionId: { [versionId: string]: TPromise<void> };
 	private disposed: boolean;
 	private inConflictResolutionMode: boolean;
 	private inErrorMode: boolean;
 	private lastSaveAttemptTime: number;
 	private createTextEditorModelPromise: TPromise<TextFileEditorModel>;
-	private _onDidContentChange: Emitter<void>;
+	private _onDidContentChange: Emitter<StateChange>;
 	private _onDidStateChange: Emitter<StateChange>;
 
 	constructor(
@@ -80,7 +84,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
 
 		this.resource = resource;
 		this.toDispose = [];
-		this._onDidContentChange = new Emitter<void>();
+		this._onDidContentChange = new Emitter<StateChange>();
 		this._onDidStateChange = new Emitter<StateChange>();
 		this.toDispose.push(this._onDidContentChange);
 		this.toDispose.push(this._onDidStateChange);
@@ -90,6 +94,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
 		this.versionId = 0;
 		this.lastSaveAttemptTime = 0;
 		this.mapPendingSaveToVersionId = {};
+		this.contentChangeEventScheduler = new RunOnceScheduler(() => this._onDidContentChange.fire(StateChange.CONTENT_CHANGE), TextFileEditorModel.DEFAULT_CONTENT_CHANGE_BUFFER_DELAY);
 
 		this.updateAutoSaveConfiguration(textFileService.getAutoSaveConfiguration());
 
@@ -99,6 +104,14 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
 	private registerListeners(): void {
 		this.toDispose.push(this.textFileService.onAutoSaveConfigurationChange(config => this.updateAutoSaveConfiguration(config)));
 		this.toDispose.push(this.textFileService.onFilesAssociationChange(e => this.onFilesAssociationChange()));
+		this.toDispose.push(this.onDidStateChange(e => {
+			if (e === StateChange.REVERTED) {
+				// Refire reverted events as content change events, cancelling any content change
+				// promises that are in flight.
+				this.contentChangeEventScheduler.cancel();
+				this._onDidContentChange.fire(StateChange.REVERTED);
+			}
+		}));
 	}
 
 	private updateAutoSaveConfiguration(config: IAutoSaveConfiguration): void {
@@ -115,7 +128,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
 		this.updateTextEditorModelMode();
 	}
 
-	public get onDidContentChange(): Event<void> {
+	public get onDidContentChange(): Event<StateChange> {
 		return this._onDidContentChange.event;
 	}
 
@@ -360,7 +373,8 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
 			}
 		}
 
-		this._onDidContentChange.fire();
+		// Handle content change events
+		this.contentChangeEventScheduler.schedule();
 	}
 
 	private makeDirty(): void {

src/vs/workbench/services/textfile/common/textFileEditorModelManager.ts

@@ -235,9 +235,8 @@ export class TextFileEditorModelManager implements ITextFileEditorModelManager {
 			});
 
 			// Install model content change listener
-			this.mapResourceToModelContentChangeListener[resource.toString()] = model.onDidContentChange(() => {
-				const newEvent = new TextFileModelChangeEvent(model, StateChange.CONTENT_CHANGE);
-				this._onModelContentChanged.fire(newEvent);
+			this.mapResourceToModelContentChangeListener[resource.toString()] = model.onDidContentChange(e => {
+				this._onModelContentChanged.fire(new TextFileModelChangeEvent(model, e));
 			});
 		}
 

src/vs/workbench/services/textfile/common/textfiles.ts

@@ -206,7 +206,7 @@ export interface IModelSaveOptions {
 
 export interface ITextFileEditorModel extends ITextEditorModel, IEncodingSupport {
 
-	onDidContentChange: Event<void>;
+	onDidContentChange: Event<StateChange>;
 	onDidStateChange: Event<StateChange>;
 
 	getResource(): URI;

src/vs/workbench/services/textfile/test/textFileEditorModelManager.test.ts

@@ -7,6 +7,7 @@
 
 import * as assert from 'assert';
 import URI from 'vs/base/common/uri';
+import { TPromise } from 'vs/base/common/winjs.base';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { TextFileEditorModelManager } from 'vs/workbench/services/textfile/common/textFileEditorModelManager';
 import { EditorModel } from 'vs/workbench/common/editor';
@@ -285,6 +286,8 @@ suite('Files - TextFileEditorModelManager', () => {
 		let disposeCounter = 0;
 		let contentCounter = 0;
 
+		TextFileEditorModel.DEFAULT_CONTENT_CHANGE_BUFFER_DELAY = 0;
+
 		manager.onModelDirty(e => {
 			dirtyCounter++;
 			assert.equal(e.resource.toString(), resource1.toString());
@@ -325,23 +328,26 @@ suite('Files - TextFileEditorModelManager', () => {
 					return model1.save().then(() => {
 						model1.dispose();
 						model2.dispose();
-
-						//assert.equal(disposeCounter, 2);
+						assert.equal(disposeCounter, 2);
 
 						return model1.revert().then(() => { // should not trigger another event if disposed
 							assert.equal(dirtyCounter, 2);
 							assert.equal(revertedCounter, 1);
 							assert.equal(savedCounter, 1);
 							assert.equal(encodingCounter, 2);
-							//assert.equal(contentCounter, 2);
 
-							model1.dispose();
-							model2.dispose();
+							// content change event if done async
+							TPromise.timeout(0).then(() => {
+								assert.equal(contentCounter, 2);
+
+								model1.dispose();
+								model2.dispose();
 
-							assert.ok(!accessor.modelService.getModel(resource1));
-							assert.ok(!accessor.modelService.getModel(resource2));
+								assert.ok(!accessor.modelService.getModel(resource1));
+								assert.ok(!accessor.modelService.getModel(resource2));
 
-							done();
+								done();
+							});
 						});
 					});
 				});

src/vs/workbench/test/common/editor/untitledEditor.test.ts

@@ -6,6 +6,7 @@
 
 import URI from 'vs/base/common/uri';
 import * as assert from 'assert';
+import { TPromise } from 'vs/base/common/winjs.base';
 import { join } from 'vs/base/common/paths';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { IUntitledEditorService, UntitledEditorService } from 'vs/workbench/services/untitled/common/untitledEditorService';
@@ -151,6 +152,8 @@ suite('Workbench - Untitled Editor', () => {
 		const service = accessor.untitledEditorService;
 		const input = service.createOrGet();
 
+		UntitledEditorModel.DEFAULT_CONTENT_CHANGE_BUFFER_DELAY = 0;
+
 		let counter = 0;
 
 		service.onDidChangeContent(r => {
@@ -160,23 +163,35 @@ suite('Workbench - Untitled Editor', () => {
 
 		input.resolve().then((model: UntitledEditorModel) => {
 			model.append('foo');
-			assert.equal(counter, 1, 'Dirty model should trigger event');
+			assert.equal(counter, 0, 'Dirty model should not trigger event immediately');
 
-			model.append('bar');
-			assert.equal(counter, 2, 'Content change when dirty should trigger event');
+			TPromise.timeout(3).then(() => {
+				assert.equal(counter, 1, 'Dirty model should trigger event');
 
-			model.clearValue();
-			assert.equal(counter, 3, 'Manual revert should trigger event');
+				model.append('bar');
+				TPromise.timeout(3).then(() => {
+					assert.equal(counter, 2, 'Content change when dirty should trigger event');
 
-			model.append('foo');
-			assert.equal(counter, 4, 'Dirty model should trigger event');
+					model.clearValue();
+					TPromise.timeout(3).then(() => {
+						assert.equal(counter, 3, 'Manual revert should trigger event');
 
-			model.revert();
-			assert.equal(counter, 5, 'Revert should trigger event');
+						model.append('foo');
+						TPromise.timeout(3).then(() => {
+							assert.equal(counter, 4, 'Dirty model should trigger event');
 
-			input.dispose();
+							model.revert();
+							TPromise.timeout(3).then(() => {
+								assert.equal(counter, 5, 'Revert should trigger event');
 
-			done();
+								input.dispose();
+
+								done();
+							});
+						});
+					});
+				});
+			});
 		});
 	});