wire-up dispose calls, #9384

src/vs/workbench/api/node/extHost.protocol.ts

@@ -78,6 +78,7 @@ function ni() { return new Error('Not implemented'); }
 
 export abstract class MainThreadCommandsShape {
 	$registerCommand(id: string): TPromise<any> { throw ni(); }
+	$unregisterCommand(id: string): TPromise<any> { throw ni(); }
 	$executeCommand<T>(id: string, args: any[]): Thenable<T> { throw ni(); }
 	$getCommands(): Thenable<string[]> { throw ni(); }
 }

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

@@ -48,7 +48,11 @@ export class ExtHostCommands extends ExtHostCommandsShape {
 		this._commands[id] = { callback, thisArg, description };
 		this._proxy.$registerCommand(id);
 
-		return new extHostTypes.Disposable(() => delete this._commands[id]);
+		return new extHostTypes.Disposable(() => {
+			if (delete this._commands[id]) {
+				this._proxy.$unregisterCommand(id);
+			}
+		});
 	}
 
 	executeCommand<T>(id: string, ...args: any[]): Thenable<T> {

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

@@ -6,11 +6,13 @@
 
 import {IThreadService} from 'vs/workbench/services/thread/common/threadService';
 import {ICommandService, CommandsRegistry, ICommandHandlerDescription} from 'vs/platform/commands/common/commands';
+import {IDisposable} from 'vs/base/common/lifecycle';
 import {TPromise} from 'vs/base/common/winjs.base';
 import {ExtHostContext, MainThreadCommandsShape, ExtHostCommandsShape} from './extHost.protocol';
 
 export class MainThreadCommands extends MainThreadCommandsShape {
 
+	private _disposables: { [id: string]: IDisposable } = Object.create(null);
 	private _proxy: ExtHostCommandsShape;
 
 	constructor(
@@ -21,11 +23,25 @@ export class MainThreadCommands extends MainThreadCommandsShape {
 		this._proxy = this._threadService.get(ExtHostContext.ExtHostCommands);
 	}
 
+	dispose() {
+		for (let id in this._disposables) {
+			this._disposables[id].dispose();
+		}
+	}
+
 	$registerCommand(id: string): TPromise<any> {
-		CommandsRegistry.registerCommand(id, (accessor, ...args) => this._proxy.$executeContributedCommand(id, ...args));
+		this._disposables[id] = CommandsRegistry.registerCommand(id, (accessor, ...args) => this._proxy.$executeContributedCommand(id, ...args));
 		return undefined;
 	}
 
+	$unregisterCommand(id: string): TPromise<any> {
+		if (this._disposables[id]) {
+			this._disposables[id].dispose();
+			delete this._disposables[id];
+		}
+		return;
+	}
+
 	$executeCommand<T>(id: string, args: any[]): Thenable<T> {
 		return this._commandService.executeCommand(id, ...args);
 	}

src/vs/workbench/test/node/api/extHostCommands.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 {ExtHostCommands} from 'vs/workbench/api/node/extHostCommands';
+import {MainThreadCommandsShape} from 'vs/workbench/api/node/extHost.protocol';
+import {TPromise} from 'vs/base/common/winjs.base';
+import {CommandsRegistry} from 'vs/platform/commands/common/commands';
+import {OneGetThreadService} from './testThreadService';
+
+suite('ExtHostCommands', function () {
+
+	test('dispose calls unregister', function () {
+
+		let lastUnregister: string;
+
+		const shape = new class extends MainThreadCommandsShape {
+			$registerCommand(id: string): TPromise<any> {
+				return;
+			}
+			$unregisterCommand(id: string): TPromise<any> {
+				lastUnregister = id;
+				return;
+			}
+		};
+
+		const commands = new ExtHostCommands(OneGetThreadService(shape), undefined);
+		commands.registerCommand('foo', () => { }).dispose();
+		assert.equal(lastUnregister, 'foo');
+		assert.equal(CommandsRegistry.getCommand('foo'), undefined);
+
+	});
+
+	test('dispose bubbles only once', function () {
+
+		let unregisterCounter = 0;
+
+		const shape = new class extends MainThreadCommandsShape {
+			$registerCommand(id: string): TPromise<any> {
+				return;
+			}
+			$unregisterCommand(id: string): TPromise<any> {
+				unregisterCounter += 1;
+				return;
+			}
+		};
+
+		const commands = new ExtHostCommands(OneGetThreadService(shape), undefined);
+		const reg = commands.registerCommand('foo', () => { });
+		reg.dispose();
+		reg.dispose();
+		reg.dispose();
+		assert.equal(unregisterCounter, 1);
+	});
+});
\ No newline at end of file

src/vs/workbench/test/node/api/mainThreadCommands.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 {MainThreadCommands} from 'vs/workbench/api/node/mainThreadCommands';
+import {CommandsRegistry} from 'vs/platform/commands/common/commands';
+import {OneGetThreadService} from './testThreadService';
+
+suite('MainThreadCommands', function () {
+
+	test('dispose on unregister', function () {
+
+		const commands = new MainThreadCommands(OneGetThreadService(null), undefined);
+		assert.equal(CommandsRegistry.getCommand('foo'), undefined);
+
+		// register
+		commands.$registerCommand('foo');
+		assert.ok(CommandsRegistry.getCommand('foo'));
+
+		// unregister
+		commands.$unregisterCommand('foo');
+		assert.equal(CommandsRegistry.getCommand('foo'), undefined);
+	});
+});
\ No newline at end of file

src/vs/workbench/test/node/api/testThreadService.ts

@@ -9,6 +9,18 @@ import {TPromise} from 'vs/base/common/winjs.base';
 import {AbstractThreadService} from 'vs/workbench/services/thread/common/abstractThreadService';
 import {IThreadService, ProxyIdentifier} from 'vs/workbench/services/thread/common/threadService';
 
+export function OneGetThreadService(thing: any): IThreadService {
+	return {
+		_serviceBrand: undefined,
+		get<T>(): T {
+			return thing;
+		},
+		set<T>(): void {
+			throw new Error();
+		}
+	};
+}
+
 export class TestThreadService extends AbstractThreadService implements IThreadService {
 	public _serviceBrand: any;