allow to register for the same scheme twice. lifo behaviour

extensions/vscode-api-tests/src/workspace.test.ts

@@ -147,21 +147,6 @@ suite('workspace-namespace', () => {
 			workspace.registerTextDocumentContentProvider('file', { provideTextDocumentContent() { return null; } });
 		});
 
-		// duplicate registration
-		let registration = workspace.registerTextDocumentContentProvider('foo', {
-			provideTextDocumentContent(uri) {
-				return uri.toString();
-			}
-		});
-		assert.throws(function() {
-			workspace.registerTextDocumentContentProvider('foo', { provideTextDocumentContent() { return null; } });
-		});
-
-		// unregister & register
-		registration.dispose();
-		registration = workspace.registerTextDocumentContentProvider('foo', { provideTextDocumentContent() { return null; } });
-		registration.dispose();
-
 		// missing scheme
 		return workspace.openTextDocument(Uri.parse('notThere://foo/far/boo/bar')).then(() => {
 			assert.ok(false, 'expected failure')
@@ -170,6 +155,69 @@ suite('workspace-namespace', () => {
 		})
 	});
 
+	test('registerTextDocumentContentProvider, multiple', function() {
+
+		// duplicate registration
+		let registration1 = workspace.registerTextDocumentContentProvider('foo', {
+			provideTextDocumentContent(uri) {
+				if (uri.authority === 'foo') {
+					return '1'
+				}
+			}
+		});
+		let registration2 = workspace.registerTextDocumentContentProvider('foo', {
+			provideTextDocumentContent(uri) {
+				if (uri.authority === 'bar') {
+					return '2'
+				}
+			}
+		});
+
+		return Promise.all([
+			workspace.openTextDocument(Uri.parse('foo://foo/bla')).then(doc => { assert.equal(doc.getText(), '1') }),
+			workspace.openTextDocument(Uri.parse('foo://bar/bla')).then(doc => { assert.equal(doc.getText(), '2') })
+		]).then(() => {
+			registration1.dispose();
+			registration2.dispose();
+		});
+	});
+
+	test('registerTextDocumentContentProvider, evil provider', function() {
+
+		// duplicate registration
+		let registration1 = workspace.registerTextDocumentContentProvider('foo', {
+			provideTextDocumentContent(uri) {
+				return '1';
+			}
+		});
+		let registration2 = workspace.registerTextDocumentContentProvider('foo', {
+			provideTextDocumentContent(uri): string {
+				throw new Error('fail')
+			}
+		});
+
+		return workspace.openTextDocument(Uri.parse('foo://foo/bla')).then(doc => {
+			assert.equal(doc.getText(), '1');
+			registration1.dispose();
+			registration2.dispose();
+		});
+	});
+
+	test('registerTextDocumentContentProvider, invalid text', function() {
+
+		let registration = workspace.registerTextDocumentContentProvider('foo', {
+			provideTextDocumentContent(uri) {
+				return <any> 123
+			}
+		});
+		return workspace.openTextDocument(Uri.parse('foo://auth/path')).then(() => {
+			assert.ok(false, 'expected failure')
+		}, err => {
+			// expected
+			registration.dispose();
+		});
+	});
+
 	test('registerTextDocumentContentProvider, show virtual document', function() {
 
 		let registration = workspace.registerTextDocumentContentProvider('foo', {

src/vs/workbench/api/node/extHostDocuments.ts

@@ -52,6 +52,8 @@ export function getWordDefinitionFor(modeId: string): RegExp {
 @Remotable.PluginHostContext('ExtHostModelService')
 export class ExtHostModelService {
 
+	private static _handlePool: number = 0;
+
 	private _onDidAddDocumentEventEmitter: Emitter<vscode.TextDocument>;
 	public onDidAddDocument: Event<vscode.TextDocument>;
 
@@ -66,7 +68,7 @@ export class ExtHostModelService {
 
 	private _documentData: { [modelUri: string]: ExtHostDocumentData; };
 	private _documentLoader: { [modelUri: string]: TPromise<ExtHostDocumentData> };
-	private _documentContentProviders: { [scheme: string]: vscode.TextDocumentContentProvider };
+	private _documentContentProviders: { [handle: number]: vscode.TextDocumentContentProvider; };
 
 	private _proxy: MainThreadDocuments;
 
@@ -131,53 +133,50 @@ export class ExtHostModelService {
 	}
 
 	public registerTextDocumentContentProvider(scheme: string, provider: vscode.TextDocumentContentProvider): vscode.Disposable {
-		if (scheme === 'file' || scheme === 'untitled' || this._documentContentProviders[scheme]) {
+		if (scheme === 'file' || scheme === 'untitled') {
 			throw new Error(`scheme '${scheme}' already registered`);
 		}
-		this._documentContentProviders[scheme] = provider;
-		this._proxy.$registerTextContentProvider(scheme);
+
+		const handle = ExtHostModelService._handlePool++;
+
+		this._documentContentProviders[handle] = provider;
+		this._proxy.$registerTextContentProvider(handle, scheme);
 
 		let subscription: IDisposable;
 		if (typeof provider.onDidChange === 'function') {
 			subscription = provider.onDidChange(uri => {
 				if (this._documentData[uri.toString()]) {
-					this.$provideTextDocumentContent(<URI>uri).then(value => {
+					this.$provideTextDocumentContent(handle, <URI>uri).then(value => {
 						return this._proxy.$onVirtualDocumentChange(<URI>uri, value);
 					}, onUnexpectedError);
 				}
 			});
 		}
 		return new Disposable(() => {
-			this._proxy.$unregisterTextContentProvider(scheme);
-			this._documentContentProviders[scheme] = undefined; // keep the knowledge of that scheme
+			if (delete this._documentContentProviders[handle]) {
+				this._proxy.$unregisterTextContentProvider(handle);
+			}
 			if (subscription) {
 				subscription.dispose();
+				subscription = undefined;
 			}
 		});
 	}
 
-	$provideTextDocumentContent(uri: URI): TPromise<string> {
-		const provider = this._documentContentProviders[uri.scheme];
+	$provideTextDocumentContent(handle: number, uri: URI): TPromise<string> {
+		const provider = this._documentContentProviders[handle];
 		if (!provider) {
 			return TPromise.wrapError<string>(`unsupported uri-scheme: ${uri.scheme}`);
 		}
-		return asWinJsPromise(token => provider.provideTextDocumentContent(uri, token)).then(value => {
-			if (typeof value !== 'string') {
-				return TPromise.wrapError('received illegal value from text document provider');
-			}
-			return value;
-		});
+		return asWinJsPromise(token => provider.provideTextDocumentContent(uri, token));
 	}
 
-	$getUnreferencedDocuments(): TPromise<URI[]> {
-		const result: URI[] = [];
-		for (let key in this._documentData) {
-			let uri = URI.parse(key);
-			if (this._documentContentProviders[uri.scheme] && !this._documentData[key].isDocumentReferenced) {
-				result.push(uri);
-			}
+	$isDocumentReferenced(uri: URI): TPromise<boolean> {
+		const key = uri.toString();
+		const document = this._documentData[key];
+		if (document) {
+			return TPromise.as(document.isDocumentReferenced);
 		}
-		return TPromise.as(result);
 	}
 
 	public _acceptModelAdd(initData: IModelAddedData): void {
@@ -468,7 +467,8 @@ export class MainThreadDocuments {
 	private _modelToDisposeMap: { [modelUrl: string]: IDisposable; };
 	private _proxy: ExtHostModelService;
 	private _modelIsSynced: { [modelId: string]: boolean; };
-	private _resourceContentProvider: { [scheme: string]: IDisposable };
+	private _resourceContentProvider: { [handle: number]: IDisposable };
+	private _virtualDocumentSet: { [resource: string]: boolean };
 
 	constructor(
 		@IThreadService threadService: IThreadService,
@@ -509,6 +509,7 @@ export class MainThreadDocuments {
 
 		this._modelToDisposeMap = Object.create(null);
 		this._resourceContentProvider = Object.create(null);
+		this._virtualDocumentSet = Object.create(null);
 	}
 
 	public dispose(): void {
@@ -630,22 +631,26 @@ export class MainThreadDocuments {
 
 	// --- virtual document logic
 
-	$registerTextContentProvider(scheme: string): void {
-		this._resourceContentProvider[scheme] = ResourceEditorInput.registerResourceContentProvider(scheme, {
+	$registerTextContentProvider(handle:number, scheme: string): void {
+		this._resourceContentProvider[handle] = ResourceEditorInput.registerResourceContentProvider(scheme, {
 			provideTextContent: (uri: URI): TPromise<EditorCommon.IModel> => {
-				return this._proxy.$provideTextDocumentContent(uri).then(value => {
-					const firstLineText = value.substr(0, 1 + value.search(/\r?\n/));
-					const mode = this._modeService.getOrCreateModeByFilenameOrFirstLine(uri.fsPath, firstLineText);
-					return this._modelService.createModel(value, mode, uri);
+				return this._proxy.$provideTextDocumentContent(handle, uri).then(value => {
+					if (value) {
+						this._virtualDocumentSet[uri.toString()] = true;
+						const firstLineText = value.substr(0, 1 + value.search(/\r?\n/));
+						const mode = this._modeService.getOrCreateModeByFilenameOrFirstLine(uri.fsPath, firstLineText);
+						return this._modelService.createModel(value, mode, uri);
+					}
 				});
 			}
 		});
 	}
 
-	$unregisterTextContentProvider(scheme: string): void {
-		const registration = this._resourceContentProvider[scheme];
+	$unregisterTextContentProvider(handle: number): void {
+		const registration = this._resourceContentProvider[handle];
 		if (registration) {
 			registration.dispose();
+			delete this._resourceContentProvider[handle];
 		}
 	}
 
@@ -657,23 +662,25 @@ export class MainThreadDocuments {
 	}
 
 	private _runDocumentCleanup(): void {
-		this._proxy.$getUnreferencedDocuments().then(resources => {
 
-			const toBeDisposed: URI[] = [];
-			const promises = resources.map(resource => {
-				return this._editorService.inputToType({ resource }).then(input => {
-					if (!this._editorService.isVisible(input, true)) {
-						toBeDisposed.push(resource);
-					}
-				});
-			});
+		const toBeDisposed: URI[] = [];
 
-			return TPromise.join(promises).then(() => {
-				for (let resource of toBeDisposed) {
-					this._modelService.destroyModel(resource);
+		TPromise.join(Object.keys(this._virtualDocumentSet).map(key => {
+			let resource = URI.parse(key);
+			return this._proxy.$isDocumentReferenced(resource).then(referenced => {
+				if (!referenced) {
+					return this._editorService.inputToType({ resource }).then(input => {
+						if (!this._editorService.isVisible(input, true)) {
+							toBeDisposed.push(resource);
+						}
+					});
 				}
 			});
-
+		})).then(() => {
+			for (let resource of toBeDisposed) {
+				this._modelService.destroyModel(resource);
+				delete this._virtualDocumentSet[resource.toString()];
+			}
 		}, onUnexpectedError);
 	}
 }

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

@@ -5,6 +5,7 @@
 'use strict';
 
 import {TPromise} from 'vs/base/common/winjs.base';
+import {sequence} from 'vs/base/common/async';
 import {EditorModel, EditorInput} from 'vs/workbench/common/editor';
 import {ResourceEditorModel} from 'vs/workbench/common/editor/resourceEditorModel';
 import {IModel} from 'vs/editor/common/editorCommon';
@@ -31,11 +32,28 @@ export class ResourceEditorInput extends EditorInput {
 	// todo@joh,ben this should maybe be a service that is in charge of loading/resolving a uri from a scheme
 
 	private static loadingModels: { [uri: string]: TPromise<IModel> } = Object.create(null);
-	private static registry: { [scheme: string]: IResourceEditorContentProvider } = Object.create(null);
+	private static registry: { [scheme: string]: IResourceEditorContentProvider[] } = Object.create(null);
 
 	public static registerResourceContentProvider(scheme: string, provider: IResourceEditorContentProvider): IDisposable {
-		ResourceEditorInput.registry[scheme] = provider;
-		return { dispose() { delete ResourceEditorInput.registry[scheme] } };
+		let array = ResourceEditorInput.registry[scheme];
+		if (!array) {
+			array = [provider];
+			ResourceEditorInput.registry[scheme] = array;
+		} else {
+			array.unshift(provider);
+		}
+		return {
+			dispose() {
+				let array = ResourceEditorInput.registry[scheme];
+				let idx = array.indexOf(provider);
+				if (idx >= 0) {
+					array.splice(idx, 1);
+					if (array.length === 0) {
+						delete ResourceEditorInput.registry[scheme];
+					}
+				}
+			}
+		};
 	}
 
 	private static getOrCreateModel(modelService: IModelService, resource: URI): TPromise<IModel> {
@@ -49,8 +67,8 @@ export class ResourceEditorInput extends EditorInput {
 
 			// make sure we have a provider this scheme
 			// the resource uses
-			const provider = ResourceEditorInput.registry[resource.scheme];
-			if (!provider) {
+			const array = ResourceEditorInput.registry[resource.scheme];
+			if (!array) {
 				return TPromise.wrapError(`No model with uri '${resource}' nor a resolver for the scheme '${resource.scheme}'.`);
 			}
 
@@ -59,7 +77,26 @@ export class ResourceEditorInput extends EditorInput {
 			// twice
 			ResourceEditorInput.loadingModels[resource.toString()] = loadingModel = new TPromise<IModel>((resolve, reject) => {
 
-				provider.provideTextContent(resource).then(resolve, reject);
+				let result: IModel;
+				let lastError: any;
+
+				sequence(array.map(provider => {
+					return () => {
+						if (!result) {
+							return provider.provideTextContent(resource).then(value => {
+								result = value;
+							}, err => {
+								lastError = err;
+							});
+						}
+					}
+				})).then(() => {
+					if (!result && lastError) {
+						reject(lastError);
+					} else {
+						resolve(result);
+					}
+				}, reject);
 
 			}, function() {
 				// no cancellation when caching promises