File models get disposed when contents change on disk (fixes #20901)

src/vs/base/test/node/processes/processes.test.ts

@@ -25,7 +25,7 @@ function fork(id: string): cp.ChildProcess {
 }
 
 suite('Processes', () => {
-	test('pasero buffered sending - simple data', function (done: () => void) {
+	test('buffered sending - simple data', function (done: () => void) {
 		if (process.env['VSCODE_PID']) {
 			return done(); // TODO@Ben find out why test fails when run from within VS Code
 		}

src/vs/workbench/parts/files/common/editors/fileEditorTracker.ts

@@ -12,13 +12,14 @@ import { IEditor, IEditorViewState, isCommonCodeEditor } from 'vs/editor/common/
 import { IEditor as IBaseEditor } from 'vs/platform/editor/common/editor';
 import { toResource, EditorInput, IEditorStacksModel, SideBySideEditorInput, IEditorGroup } from 'vs/workbench/common/editor';
 import { BINARY_FILE_EDITOR_ID } from 'vs/workbench/parts/files/common/files';
-import { ITextFileService, ModelState } from 'vs/workbench/services/textfile/common/textfiles';
+import { ITextFileService, ModelState, ITextFileEditorModel } from 'vs/workbench/services/textfile/common/textfiles';
 import { FileOperationEvent, FileOperation, IFileService, FileChangeType, FileChangesEvent } from 'vs/platform/files/common/files';
 import { FileEditorInput } from 'vs/workbench/parts/files/common/editors/fileEditorInput';
 import { IEditorGroupService } from 'vs/workbench/services/group/common/groupService';
 import { ILifecycleService } from 'vs/platform/lifecycle/common/lifecycle';
 import { IWorkbenchEditorService } from 'vs/workbench/services/editor/common/editorService';
 import { IDisposable, dispose } from 'vs/base/common/lifecycle';
+import { distinct } from 'vs/base/common/arrays';
 
 export class FileEditorTracker implements IWorkbenchContribution {
 
@@ -77,8 +78,8 @@ export class FileEditorTracker implements IWorkbenchContribution {
 
 	private onFileChanges(e: FileChangesEvent): void {
 
-		// Handle updates to visible editors
-		this.handleUpdatesToVisibleEditors(e);
+		// Handle updates
+		this.handleUpdates(e);
 
 		// Handle deletes
 		if (e.gotDeleted()) {
@@ -193,7 +194,33 @@ export class FileEditorTracker implements IWorkbenchContribution {
 		return void 0;
 	}
 
-	private handleUpdatesToVisibleEditors(e: FileChangesEvent) {
+	private handleUpdates(e: FileChangesEvent): void {
+
+		// Collect distinct (saved) models to update.
+		//
+		// Note: we also consider the added event because it could be that a file was added
+		// and updated right after.
+		const modelsToUpdate = distinct([...e.getUpdated(), ...e.getAdded()]
+			.map(u => this.textFileService.models.get(u.resource))
+			.filter(model => model && model.getState() === ModelState.SAVED), m => m.getResource().toString());
+
+		// Handle updates to visible editors specially to preserve view state
+		const visibleModels = this.handleUpdatesToVisibleEditors(e);
+
+		// Handle updates to remaining models that are not visible
+		modelsToUpdate.forEach(model => {
+			if (visibleModels.indexOf(model) >= 0) {
+				return; // already updated
+			}
+
+			// Load model to update
+			model.load().done(null, errors.onUnexpectedError);
+		});
+	}
+
+	private handleUpdatesToVisibleEditors(e: FileChangesEvent): ITextFileEditorModel[] {
+		const models: ITextFileEditorModel[] = [];
+
 		const editors = this.editorService.getVisibleEditors();
 		editors.forEach(editor => {
 			let input = editor.input;
@@ -213,6 +240,7 @@ export class FileEditorTracker implements IWorkbenchContribution {
 
 					// Text file: check for last save time
 					if (textModel) {
+						models.push(textModel);
 
 						// We only ever update models that are in good saved state
 						if (textModel.getState() === ModelState.SAVED) {
@@ -239,6 +267,8 @@ export class FileEditorTracker implements IWorkbenchContribution {
 				}
 			}
 		});
+
+		return models;
 	}
 
 	private isEditorShowingPath(editor: IBaseEditor, resource: URI): boolean {

src/vs/workbench/parts/files/test/browser/fileEditorTracker.test.ts

@@ -17,6 +17,7 @@ import { IEditorGroupService } from 'vs/workbench/services/group/common/groupSer
 import { EditorStacksModel } from 'vs/workbench/common/editor/editorStacksModel';
 import { ITextFileService } from 'vs/workbench/services/textfile/common/textfiles';
 import { FileOperation, FileOperationEvent, FileChangesEvent, FileChangeType, IFileService } from 'vs/platform/files/common/files';
+import { TextFileEditorModel } from 'vs/workbench/services/textfile/common/textFileEditorModel';
 
 function toResource(path) {
 	return URI.file(join('C:\\', new Buffer(this.test.fullTitle()).toString('base64'), path));
@@ -75,6 +76,8 @@ suite('Files - FileEditorTracker', () => {
 		const to = toResource.call(this, '/foo/barfoo/change.js');
 		accessor.fileService.fireAfterOperation(new FileOperationEvent(resource, FileOperation.MOVE, to));
 		assert.ok(input.isDisposed());
+
+		tracker.dispose();
 	});
 
 	test('disposes when resource gets deleted - remote file changes', function () {
@@ -105,5 +108,30 @@ suite('Files - FileEditorTracker', () => {
 
 		accessor.fileService.fireFileChanges(new FileChangesEvent([{ resource: parent, type: FileChangeType.DELETED }]));
 		assert.ok(input.isDisposed());
+
+		tracker.dispose();
+	});
+
+	test('file change event updates model', function (done) {
+		const tracker = instantiationService.createInstance(FileEditorTracker);
+
+		const resource = toResource.call(this, '/path/index.txt');
+
+		accessor.textFileService.models.loadOrCreate(resource).then((model: TextFileEditorModel) => {
+			model.textEditorModel.setValue('Super Good');
+			assert.equal(model.getValue(), 'Super Good');
+
+			model.save().then(() => {
+
+				// change event (watcher)
+				accessor.fileService.fireFileChanges(new FileChangesEvent([{ resource, type: FileChangeType.UPDATED }]));
+
+				assert.equal(model.getValue(), 'Hello Html');
+
+				tracker.dispose();
+
+				done();
+			});
+		});
 	});
 });

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

@@ -17,10 +17,6 @@ import { FileOperation, FileOperationEvent, FileChangesEvent, IFileService } fro
 
 export class TextFileEditorModelManager implements ITextFileEditorModelManager {
 
-	// Delay in ms that we wait at minimum before we update a model from a file change event.
-	// This reduces the chance that a save from the client triggers an update of the editor.
-	private static FILE_CHANGE_UPDATE_DELAY = 2000;
-
 	private toUnbind: IDisposable[];
 
 	private _onModelDisposed: Emitter<URI>;
@@ -116,23 +112,6 @@ export class TextFileEditorModelManager implements ITextFileEditorModelManager {
 		e.getDeleted().forEach(deleted => {
 			this.disposeModelIfPossible(deleted.resource);
 		});
-
-		// Dispose models that got changed and are not visible. We do this because otherwise
-		// cached file models will be stale from the contents on disk.
-		e.getUpdated()
-			.map(u => this.get(u.resource))
-			.filter(model => {
-				if (!model) {
-					return false;
-				}
-
-				if (Date.now() - model.getLastSaveAttemptTime() < TextFileEditorModelManager.FILE_CHANGE_UPDATE_DELAY) {
-					return false; // this is a weak check to see if the change came from outside the editor or not
-				}
-
-				return true; // ok boss
-			})
-			.forEach(model => this.disposeModelIfPossible(model.getResource()));
 	}
 
 	private canDispose(textModel: ITextFileEditorModel): boolean {

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

@@ -238,25 +238,6 @@ suite('Files - TextFileEditorModelManager', () => {
 		manager.dispose();
 	});
 
-	test('file change event dispose model if happening > 2 second after last save', function () {
-		const manager: TestTextFileEditorModelManager = instantiationService.createInstance(TestTextFileEditorModelManager);
-
-		const resource = toResource('/path/index.txt');
-
-		const model: TextFileEditorModel = instantiationService.createInstance(TextFileEditorModel, resource, 'utf8');
-		manager.add(resource, model);
-
-		assert.ok(!model.isDisposed());
-
-		// change event (watcher)
-		accessor.fileService.fireFileChanges(new FileChangesEvent([{ resource, type: FileChangeType.UPDATED }]));
-
-		assert.ok(model.isDisposed());
-		assert.ok(!accessor.modelService.getModel(model.getResource()));
-
-		manager.dispose();
-	});
-
 	test('file change event does NOT dispose model if happening < 2 second after last save', function (done) {
 		const manager: TestTextFileEditorModelManager = instantiationService.createInstance(TestTextFileEditorModelManager);