Merge pull request #333 from Microsoft/fix42

Make sure not each provider registers a support

src/vs/workbench/api/common/languageFeatures.ts

@@ -58,8 +58,8 @@ export abstract class AbstractMainThreadFeature<T> {
 
 	private _id: string;
 	protected _commands: PluginHostCommands;
-	protected _handlePool = 0;
-	protected _disposable: { [handle: number]: IDisposable } = Object.create(null);
+	protected _refCount = 0;
+	protected _disposable: IDisposable;
 	protected _registry: LanguageFeatureRegistry<T>;
 
 	constructor(id: string, registry: LanguageFeatureRegistry<T>, @IThreadService threadService: IThreadService) {
@@ -72,16 +72,16 @@ export abstract class AbstractMainThreadFeature<T> {
 		return TPromise.as(this._id);
 	}
 
-	_register(selector: vscode.DocumentSelector): TPromise<number> {
-		const handle = this._handlePool++;
-		this._disposable[handle] = this._registry.register(selector, <any> this);
-		return TPromise.as(handle);
+	_register(selector: vscode.DocumentSelector): TPromise<any> {
+		if (this._refCount++ === 0) {
+			this._disposable = this._registry.register(selector, <any> this);
+		}
+		return undefined;
 	}
 
-	_unregister(handle: number): TPromise<any> {
-		if (this._disposable[handle]) {
-			this._disposable[handle].dispose();
-			delete this._disposable[handle];
+	_unregister(): TPromise<any> {
+		if (--this._refCount === 0) {
+			this._disposable.dispose();
 		}
 		return undefined;
 	}
@@ -112,11 +112,11 @@ export abstract class AbstractExtensionHostFeature<T, P extends AbstractMainThre
 	register(selector: vscode.DocumentSelector, provider: T): vscode.Disposable {
 
 		let disposable = this._registry.register(selector, provider);
-		let handle = this._proxy._register(selector);
+		let registered = this._proxy._register(selector);
 
 		return new Disposable(() => {
 			disposable.dispose(); // remove locally
-			handle.then(value => this._proxy._unregister(value));
+			registered.then(() => this._proxy._unregister());
 		});
 	}
 
@@ -721,11 +721,11 @@ export class ExtHostFormatOnType extends AbstractExtensionHostFeature<FormatOnTy
 	register(selector: vscode.DocumentSelector, provider: FormatOnTypeEntry): vscode.Disposable {
 
 		let disposable = this._registry.register(selector, provider);
-		let handle = this._proxy._register(selector, provider.triggerCharacters);
+		let registered = this._proxy._register(selector, provider.triggerCharacters);
 
 		return new Disposable(() => {
 			disposable.dispose();
-			handle.then(value => this._proxy._unregister(value));
+			registered.then(() => this._proxy._unregister());
 		});
 	}
 
@@ -792,11 +792,11 @@ export class ExtHostSignatureHelp extends AbstractExtensionHostFeature<Signature
 	register(selector: vscode.DocumentSelector, entry: SignatureHelpEntry): vscode.Disposable {
 
 		let disposable = this._registry.register(selector, entry);
-		let handle = this._proxy._register(selector, entry.triggerCharacters);
+		let registered = this._proxy._register(selector, entry.triggerCharacters);
 
 		return new Disposable(() => {
 			disposable.dispose();
-			handle.then(value => this._proxy._unregister(value));
+			registered.then(() => this._proxy._unregister());
 		});
 	}
 
@@ -908,10 +908,10 @@ export class ExtHostCompletions extends AbstractExtensionHostFeature<CompletionI
 
 	register(selector: vscode.DocumentSelector, entry: CompletionItemEnty): vscode.Disposable {
 		let disposable = this._registry.register(selector, entry);
-		let handle = this._proxy._register(selector, entry.triggerCharacters);
+		let registered = this._proxy._register(selector, entry.triggerCharacters);
 		return new Disposable(() => {
 			disposable.dispose();
-			handle.then(value => this._proxy._unregister(value));
+			registered.then(() => this._proxy._unregister());
 		});
 	}
 

src/vs/workbench/test/common/api/languageFeatures.test.ts

+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+'use strict';
+
+import * as assert from 'assert';
+import {create} from 'vs/base/common/types';
+import URI from 'vs/base/common/uri';
+import {TPromise} from 'vs/base/common/winjs.base';
+import {PluginHostDocument} from 'vs/workbench/api/common/pluginHostDocuments';
+import * as phTypes from 'vs/workbench/api/common/pluginHostTypes';
+import {Range as CodeEditorRange} from 'vs/editor/common/core/range';
+import * as EditorCommon from 'vs/editor/common/editorCommon';
+import {NullThreadService} from 'vs/platform/test/common/nullThreadService'
+import * as LF from 'vs/workbench/api/common/languageFeatures';
+import {PluginHostCommands, MainThreadCommands} from 'vs/workbench/api/common/pluginHostCommands';
+import {PluginHostModelService} from 'vs/workbench/api/common/pluginHostDocuments';
+import {SyncDescriptor0} from 'vs/platform/instantiation/common/descriptors';
+import QuickOutlineRegistry from 'vs/editor/contrib/quickOpen/common/quickOpen';
+import {LanguageSelector, ModelLike} from 'vs/editor/common/modes/languageSelector';
+
+class ThreadService extends NullThreadService {
+
+	protected _registerAndInstantiateMainProcessActor<T>(id: string, descriptor: SyncDescriptor0<T>): T {
+
+		let instance: any;
+
+		return this._getOrCreateProxyInstance({
+			callOnRemote: (proxyId: string, path: string, args: any[]): TPromise<any> => {
+				if (!instance) {
+					instance = create(descriptor.ctor, this);
+				}
+				try {
+					let result = (<Function>instance[path]).apply(instance, args);
+					return TPromise.is(result) ? result : TPromise.as(result);
+				} catch (err) {
+					return TPromise.wrapError(err);
+				}
+			}
+		}, id, descriptor)
+	}
+
+	protected _registerAndInstantiatePluginHostActor<T>(id: string, descriptor: SyncDescriptor0<T>): T {
+		return this._getOrCreateLocalInstance(id, descriptor);
+	}
+}
+
+let threadService: ThreadService;
+let model: ModelLike = { language: 'far', uri: URI.parse('far://testing/file.a') };
+
+let extHost: LF.ExtensionHostDocumentSymbols;
+let mainHost: LF.MainThreadDocumentSymbols;
+
+suite('ExtHostLanguageFeatures', function() {
+
+	suiteSetup(() => {
+		threadService = new ThreadService();
+		let documents = threadService.getRemotable(PluginHostModelService);
+		documents._acceptModelAdd({
+			isDirty: false,
+			versionId: 1,
+			modeId: 'far',
+			url: model.uri,
+			value: {
+				EOL: '\n',
+				lines: [
+					'This is the first line',
+					'This is the second line',
+					'This is the third line',
+				],
+				BOM: '',
+				length: -1
+			},
+		})
+		threadService.getRemotable(PluginHostCommands);
+		threadService.getRemotable(MainThreadCommands);
+		threadService.getRemotable(LF.MainThreadDocumentSymbols);
+		extHost = new LF.ExtensionHostDocumentSymbols(threadService);
+		mainHost = threadService.getRemotable(LF.MainThreadDocumentSymbols);
+	});
+
+	test('DocumentSymbols, register/deregister', function() {
+
+
+		// register
+		assert.equal(QuickOutlineRegistry.all(model).length, 0);
+		let disposable = extHost.register('far', {
+			provideDocumentSymbols() {
+				return [];
+			}
+		});
+		assert.equal(QuickOutlineRegistry.all(model).length, 1);
+
+		// deregister
+		disposable.dispose();
+		assert.equal(QuickOutlineRegistry.all(model).length, 0);
+
+		// all extension host provider appear as one
+		disposable = extHost.register('far', {
+			provideDocumentSymbols() {
+				return [];
+			}
+		});
+		let disposable2 = extHost.register('far', {
+			provideDocumentSymbols() {
+				return [];
+			}
+		});
+		assert.equal(QuickOutlineRegistry.all(model).length, 1);
+
+		disposable.dispose();
+		assert.equal(QuickOutlineRegistry.all(model).length, 1);
+		disposable2.dispose();
+		assert.equal(QuickOutlineRegistry.all(model).length, 0);
+	});
+
+	test('DocumentSymbols, evil provider', function(done) {
+
+
+		let disposable = extHost.register('far', {
+			provideDocumentSymbols():any {
+				throw new Error('ddd');
+			}
+		});
+		let disposable2 = extHost.register('far', {
+			provideDocumentSymbols():any {
+				return [
+					new phTypes.SymbolInformation('boo', phTypes.SymbolKind.Field, new phTypes.Range(0, 0, 0, 0))
+				];
+			}
+		});
+
+		mainHost.getOutline(model.uri).then(result => {
+			assert.equal(result.length, 1);
+			done();
+
+			disposable.dispose();
+			disposable2.dispose();
+
+		}, err => {
+			done(err);
+		});
+	});
+
+	test('DocumentSymbols, data conversion', function(done) {
+
+		let d = extHost.register('far', {
+			provideDocumentSymbols():any {
+				return [
+					new phTypes.SymbolInformation('boo',
+						phTypes.SymbolKind.Field,
+						new phTypes.Range(0, 0, 0, 0),
+						model.uri,
+						'far')
+				];
+			}
+		});
+
+		mainHost.getOutline(model.uri).then(result => {
+			assert.equal(result.length, 1);
+			let entry = result[0];
+
+			assert.equal(entry.label, 'boo');
+			assert.equal(entry.containerLabel, 'far');
+			assert.equal(entry.children, undefined);
+			assert.deepEqual(entry.range, { startLineNumber: 1, startColumn: 1, endLineNumber: 1, endColumn: 1 });
+			d.dispose();
+			done();
+
+		}, err => {
+			done(err);
+		});
+
+	});
+});
\ No newline at end of file