Rework opener api proposal

For #109277

- Add more explicit two phase structure to api
- Make opener pass along label when registered

extensions/simple-browser/src/extension.ts

@@ -12,7 +12,6 @@ const localize = nls.loadMessageBundle();
 
 const openApiCommand = 'simpleBrowser.api.open';
 const showCommand = 'simpleBrowser.show';
-const internalOpenCommand = '_simpleBrowser.open';
 
 export function activate(context: vscode.ExtensionContext) {
 
@@ -36,15 +35,11 @@ export function activate(context: vscode.ExtensionContext) {
 		manager.show(url.toString(), showOptions);
 	}));
 
-	context.subscriptions.push(vscode.commands.registerCommand(internalOpenCommand, (resolvedUrl: vscode.Uri, _originalUri: vscode.Uri) => {
-		manager.show(resolvedUrl.toString());
-	}));
-
 	context.subscriptions.push(vscode.window.registerExternalUriOpener(['http', 'https'], {
-		openExternalUri(uri: vscode.Uri): vscode.Command | undefined {
+		canOpenExternalUri(uri: vscode.Uri) {
 			const configuration = vscode.workspace.getConfiguration('simpleBrowser');
 			if (!configuration.get('opener.enabled', false)) {
-				return undefined;
+				return false;
 			}
 
 			const enabledHosts = configuration.get<string[]>('opener.enabledHosts', [
@@ -54,17 +49,18 @@ export function activate(context: vscode.ExtensionContext) {
 			try {
 				const originalUri = new URL(uri.toString());
 				if (!enabledHosts.includes(originalUri.hostname)) {
-					return;
+					return false;
 				}
 			} catch {
-				return undefined;
+				return false;
 			}
 
-			return {
-				title: localize('openTitle', "Open in simple browser"),
-				command: internalOpenCommand,
-				arguments: [uri]
-			};
+			return true;
+		},
+		openExternalUri(_opener, resolveUri) {
+			return manager.show(resolveUri.toString());
 		}
+	}, {
+		label: localize('openTitle', "Open in simple browser"),
 	}));
 }

src/vs/vscode.proposed.d.ts

@@ -2303,32 +2303,60 @@ declare module 'vscode' {
 	//#region Opener service (https://github.com/microsoft/vscode/issues/109277)
 
 	/**
-	 * Handles opening external uris.
+	 * Handles opening uris to external resources, such as http(s) links.
 	 *
-	 * An extension can use this to open a `http` link to a webserver inside of VS Code instead of
-	 * having the link be opened by the webbrowser.
+	 * Extensions can implement an `ExternalUriOpener` to open `http` links to a webserver
+	 * inside of VS Code instead of having the link be opened by the webbrowser.
 	 *
 	 * Currently openers may only be registered for `http` and `https` uris.
 	 */
 	export interface ExternalUriOpener {
 
 		/**
-		 * Try to open a given uri.
+		 * Check if the opener can handle a given uri.
 		 *
-		 * @param uri The uri to open. This uri may have been transformed by port forwarding. To access
-		 * the original uri that triggered the open, use `ctx.original`.
-		 * @param ctx Additional metadata about the triggered open.
-		 * @param token Cancellation token.
+		 * @param uri The uri being opened. This is the uri that the user clicked on. It has
+		 * not yet gone through port forwarding.
+		 * @param token Cancellation token indicating that the result is no longer needed.
 		 *
-		 * @return Optional command that opens the uri. If no command is returned, VS Code will
-		 * continue checking to see if any other openers are available.
+		 * @return True if the opener can open the external uri.
+		 */
+		canOpenExternalUri(uri: Uri, token: CancellationToken): ProviderResult<boolean>;
+
+		/**
+		 * Open the given uri.
+		 *
+		 * @param resolvedUri The uri to open. This uri may have been transformed by port forwarding, so it
+		 * may not match the original uri passed to `canOpenExternalUri`. Use `ctx.originalUri` to check the
+		 * original uri.
+		 * @param ctx Additional information about the uri being opened.
+		 * @param token Cancellation token indicating that opening has been canceled.
+		 *
+		 * @return Thenable indicating that the opening has completed
+		 */
+		openExternalUri(resolvedUri: Uri, ctx: OpenExternalUriContext, token: CancellationToken): Thenable<void> | void;
+	}
+
+	/**
+	 * Additional information about the uri being opened.
+	 */
+	interface OpenExternalUriContext {
+		/**
+		 * The uri that triggered the open.
 		 *
-		 * This command is given the resolved uri to open. This may be different from the original `uri` due
-		 * to port forwarding.
+		 * Due to port forwarding, this may not match the `resolvedUri` passed to `openExternalUri`
+		 */
+		readonly sourceUri: Uri;
+	}
+
+
+	interface ExternalUriOpenerMetadata {
+		/**
+		 * Text displayed to the user that explains what the opener does.
 		 *
-		 * If multiple openers are available for a given uri, then the `Command.title` is shown in the UI.
+		 * For example, 'Open in browser preview'
 		 */
-		openExternalUri(uri: Uri, ctx: OpenExternalUriContext, token: CancellationToken): ProviderResult<Command>;
+		readonly label: string;
 	}
 
 	namespace window {
@@ -2343,7 +2371,7 @@ declare module 'vscode' {
 		 *
 		* @returns Disposable that unregisters the opener.
 		 */
-		export function registerExternalUriOpener(schemes: readonly string[], opener: ExternalUriOpener,): Disposable;
+		export function registerExternalUriOpener(schemes: readonly string[], opener: ExternalUriOpener, metadata: ExternalUriOpenerMetadata): Disposable;
 	}
 
 	//#endregion

src/vs/workbench/api/browser/mainThreadUriOpeners.ts

@@ -7,13 +7,16 @@ import { CancellationToken } from 'vs/base/common/cancellation';
 import { Disposable } from 'vs/base/common/lifecycle';
 import { Schemas } from 'vs/base/common/network';
 import { URI } from 'vs/base/common/uri';
-import { ExtHostContext, ExtHostUriOpener, ExtHostUriOpenersShape, IExtHostContext, MainContext, MainThreadUriOpenersShape } from 'vs/workbench/api/common/extHost.protocol';
-import { ExternalOpenerEntry, ExternalOpenerSet, IExternalOpenerProvider, IExternalUriOpenerService } from 'vs/workbench/contrib/externalUriOpener/common/externalUriOpenerService';
+import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions';
+import { ExtHostContext, ExtHostUriOpenersShape, IExtHostContext, MainContext, MainThreadUriOpenersShape } from 'vs/workbench/api/common/extHost.protocol';
+import { ExternalOpenerEntry, IExternalOpenerProvider, IExternalUriOpenerService } from 'vs/workbench/contrib/externalUriOpener/common/externalUriOpenerService';
 import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions';
 import { extHostNamedCustomer } from '../common/extHostCustomers';
 
 interface RegisteredOpenerMetadata {
 	readonly schemes: ReadonlySet<string>;
+	readonly extensionId: ExtensionIdentifier;
+	readonly label: string;
 }
 
 @extHostNamedCustomer(MainContext.MainThreadUriOpeners)
@@ -33,12 +36,12 @@ export class MainThreadUriOpeners extends Disposable implements MainThreadUriOpe
 		this._register(this.externalUriOpenerService.registerExternalOpenerProvider(this));
 	}
 
-	public async provideExternalOpeners(href: string | URI): Promise<ExternalOpenerSet | undefined> {
+	public async provideExternalOpeners(href: string | URI): Promise<readonly ExternalOpenerEntry[]> {
 		const targetUri = typeof href === 'string' ? URI.parse(href) : href;
 
 		// Currently we only allow openers for http and https urls
 		if (targetUri.scheme !== Schemas.http && targetUri.scheme !== Schemas.https) {
-			return undefined;
+			return [];
 		}
 
 		await this.extensionService.activateByEvent(`onUriOpen:${targetUri.scheme}`);
@@ -46,41 +49,38 @@ export class MainThreadUriOpeners extends Disposable implements MainThreadUriOpe
 		// If there are no handlers there is no point in making a round trip
 		const hasHandler = Array.from(this.registeredOpeners.values()).some(x => x.schemes.has(targetUri.scheme));
 		if (!hasHandler) {
-			return undefined;
+			return [];
 		}
 
-		const { openers, cacheId } = await this.proxy.$getOpenersForUri(targetUri, CancellationToken.None);
-
-		if (openers.length === 0) {
-			this.proxy.$releaseOpener(cacheId);
-			return undefined;
-		} else {
-			return {
-				openers: openers.map(opener => this.openerForCommand(cacheId, opener)),
-				dispose: () => {
-					this.proxy.$releaseOpener(cacheId);
-				}
-			};
-		}
+		const openerHandles = await this.proxy.$getOpenersForUri(targetUri, CancellationToken.None);
+
+		return openerHandles.map(handle => this.openerForCommand(handle, targetUri));
 	}
 
-	private openerForCommand(
-		cacheId: number,
-		opener: ExtHostUriOpener
-	): ExternalOpenerEntry {
+	private openerForCommand(openerHandle: number, sourceUri: URI): ExternalOpenerEntry {
+		const metadata = this.registeredOpeners.get(openerHandle)!;
 		return {
-			id: opener.extensionId.value,
-			label: opener.title,
+			id: metadata.extensionId.value,
+			label: metadata.label,
 			openExternal: async (href) => {
-				const targetUri = URI.parse(href);
-				await this.proxy.$openUri([cacheId, opener.commandId], targetUri);
+				const resolveUri = URI.parse(href);
+				await this.proxy.$openUri(openerHandle, { resolveUri, sourceUri }, CancellationToken.None);
 				return true;
 			},
 		};
 	}
 
-	async $registerUriOpener(handle: number, schemes: readonly string[]): Promise<void> {
-		this.registeredOpeners.set(handle, { schemes: new Set(schemes) });
+	async $registerUriOpener(
+		handle: number,
+		schemes: readonly string[],
+		extensionId: ExtensionIdentifier,
+		label: string,
+	): Promise<void> {
+		this.registeredOpeners.set(handle, {
+			schemes: new Set(schemes),
+			label,
+			extensionId,
+		});
 	}
 
 	async $unregisterUriOpener(handle: number): Promise<void> {

src/vs/workbench/api/common/extHost.api.impl.ts

@@ -158,7 +158,7 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I
 	const extHostCustomEditors = rpcProtocol.set(ExtHostContext.ExtHostCustomEditors, new ExtHostCustomEditors(rpcProtocol, extHostDocuments, extensionStoragePaths, extHostWebviews, extHostWebviewPanels));
 	const extHostWebviewViews = rpcProtocol.set(ExtHostContext.ExtHostWebviewViews, new ExtHostWebviewViews(rpcProtocol, extHostWebviews));
 	const extHostTesting = rpcProtocol.set(ExtHostContext.ExtHostTesting, new ExtHostTesting(rpcProtocol, extHostDocumentsAndEditors, extHostWorkspace));
-	const extHostUriOpeners = rpcProtocol.set(ExtHostContext.ExtHostUriOpeners, new ExtHostUriOpeners(rpcProtocol, extHostCommands));
+	const extHostUriOpeners = rpcProtocol.set(ExtHostContext.ExtHostUriOpeners, new ExtHostUriOpeners(rpcProtocol));
 
 	// Check that no named customers are missing
 	const expected: ProxyIdentifier<any>[] = values(ExtHostContext);
@@ -672,9 +672,9 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I
 				checkProposedApiEnabled(extension);
 				return extHostNotebook.showNotebookDocument(document, options);
 			},
-			registerExternalUriOpener(schemes: readonly string[], opener: vscode.ExternalUriOpener) {
+			registerExternalUriOpener(schemes: readonly string[], opener: vscode.ExternalUriOpener, metadata: vscode.ExternalUriOpenerMetadata) {
 				checkProposedApiEnabled(extension);
-				return extHostUriOpeners.registerUriOpener(extension.identifier, schemes, opener);
+				return extHostUriOpeners.registerUriOpener(extension.identifier, schemes, opener, metadata);
 			},
 		};
 

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

@@ -801,20 +801,13 @@ export interface ExtHostUrlsShape {
 }
 
 export interface MainThreadUriOpenersShape extends IDisposable {
-	$registerUriOpener(handle: number, schemes: readonly string[], extensionId: ExtensionIdentifier): Promise<void>;
+	$registerUriOpener(handle: number, schemes: readonly string[], extensionId: ExtensionIdentifier, label: string): Promise<void>;
 	$unregisterUriOpener(handle: number): Promise<void>;
 }
 
-export interface ExtHostUriOpener {
-	readonly extensionId: ExtensionIdentifier;
-	readonly commandId: number;
-	readonly title: string;
-}
-
 export interface ExtHostUriOpenersShape {
-	$getOpenersForUri(uri: UriComponents, token: CancellationToken): Promise<{ cacheId: number, openers: ReadonlyArray<ExtHostUriOpener> }>;
-	$openUri(id: ChainedCacheId, uri: UriComponents): Promise<void>;
-	$releaseOpener(cacheId: number): void;
+	$getOpenersForUri(uri: UriComponents, token: CancellationToken): Promise<readonly number[]>;
+	$openUri(handle: number, context: { resolveUri: UriComponents, sourceUri: UriComponents }, token: CancellationToken): Promise<void>;
 }
 
 export interface ITextSearchComplete {

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

@@ -8,15 +8,14 @@ import { CancellationToken } from 'vs/base/common/cancellation';
 import { toDisposable } from 'vs/base/common/lifecycle';
 import { URI, UriComponents } from 'vs/base/common/uri';
 import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions';
-import { ExtHostCommands } from 'vs/workbench/api/common/extHostCommands';
 import type * as vscode from 'vscode';
-import { Cache } from './cache';
-import { ChainedCacheId, ExtHostUriOpener, ExtHostUriOpenersShape, IMainContext, MainContext, MainThreadUriOpenersShape } from './extHost.protocol';
+import { ExtHostUriOpenersShape, IMainContext, MainContext, MainThreadUriOpenersShape } from './extHost.protocol';
 
 interface OpenerEntry {
 	readonly extension: ExtensionIdentifier;
 	readonly schemes: ReadonlySet<string>;
 	readonly opener: vscode.ExternalUriOpener;
+	readonly metadata: vscode.ExternalUriOpenerMetadata;
 }
 
 export class ExtHostUriOpeners implements ExtHostUriOpenersShape {
@@ -24,23 +23,20 @@ export class ExtHostUriOpeners implements ExtHostUriOpenersShape {
 	private static HandlePool = 0;
 
 	private readonly _proxy: MainThreadUriOpenersShape;
-	private readonly _commands: ExtHostCommands;
 
-	private readonly _cache = new Cache<{ command: vscode.Command }>('CodeAction');
 	private readonly _openers = new Map<number, OpenerEntry>();
 
 	constructor(
 		mainContext: IMainContext,
-		commands: ExtHostCommands,
 	) {
 		this._proxy = mainContext.getProxy(MainContext.MainThreadUriOpeners);
-		this._commands = commands;
 	}
 
 	registerUriOpener(
 		extensionId: ExtensionIdentifier,
 		schemes: readonly string[],
 		opener: vscode.ExternalUriOpener,
+		metadata: vscode.ExternalUriOpenerMetadata,
 	): vscode.Disposable {
 		const handle = ExtHostUriOpeners.HandlePool++;
 
@@ -48,8 +44,9 @@ export class ExtHostUriOpeners implements ExtHostUriOpenersShape {
 			opener,
 			extension: extensionId,
 			schemes: new Set(schemes),
+			metadata
 		});
-		this._proxy.$registerUriOpener(handle, schemes, extensionId);
+		this._proxy.$registerUriOpener(handle, schemes, extensionId, metadata.label);
 
 		return toDisposable(() => {
 			this._openers.delete(handle);
@@ -57,47 +54,35 @@ export class ExtHostUriOpeners implements ExtHostUriOpenersShape {
 		});
 	}
 
-	async $getOpenersForUri(uriComponents: UriComponents, token: CancellationToken): Promise<{ cacheId: number, openers: Array<ExtHostUriOpener> }> {
+	async $getOpenersForUri(uriComponents: UriComponents, token: CancellationToken): Promise<readonly number[]> {
 		const uri = URI.revive(uriComponents);
 
-		const promises = Array.from(this._openers.values()).map(async ({ schemes, opener, extension }): Promise<{ command: vscode.Command, extension: ExtensionIdentifier } | undefined> => {
+		const promises = Array.from(this._openers.entries()).map(async ([handle, { schemes, opener, }]): Promise<number | undefined> => {
 			if (!schemes.has(uri.scheme)) {
 				return undefined;
 			}
 
 			try {
-				const result = await opener.openExternalUri(uri, {}, token);
-
-				if (result) {
-					return {
-						command: result,
-						extension
-					};
+				if (await opener.canOpenExternalUri(uri, token)) {
+					return handle;
 				}
 			} catch (e) {
+				console.log(e);
 				// noop
 			}
 			return undefined;
 		});
 
-		const commands = coalesce(await Promise.all(promises));
-		const cacheId = this._cache.add(commands);
-		return {
-			cacheId,
-			openers: commands.map((entry, i) => ({ title: entry.command.title, commandId: i, extensionId: entry.extension })),
-		};
+		return (await Promise.all(promises)).filter(handle => typeof handle === 'number') as number[];
 	}
 
-	async $openUri(id: ChainedCacheId, uri: UriComponents): Promise<void> {
-		const entry = this._cache.get(id[0], id[1]);
+	async $openUri(handle: number, context: { resolveUri: UriComponents, sourceUri: UriComponents }, token: CancellationToken): Promise<void> {
+		const entry = this._openers.get(handle);
 		if (!entry) {
-			return;
+			throw new Error(`Unknown opener handle: '${handle}'`);
 		}
-
-		return this._commands.executeCommand(entry.command.command, URI.revive(uri), ...(entry.command.arguments || []));
-	}
-
-	$releaseOpener(cacheId: number): void {
-		this._cache.delete(cacheId);
+		return entry.opener.openExternalUri(URI.revive(context.resolveUri), {
+			sourceUri: URI.revive(context.sourceUri)
+		}, token);
 	}
 }

src/vs/workbench/contrib/externalUriOpener/common/externalUriOpenerService.ts

@@ -3,16 +3,16 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { Disposable, DisposableStore, IDisposable } from 'vs/base/common/lifecycle';
+import { Disposable, IDisposable } from 'vs/base/common/lifecycle';
 import { LinkedList } from 'vs/base/common/linkedList';
 import { URI } from 'vs/base/common/uri';
+import * as nls from 'vs/nls';
+import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
 import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
 import { IExternalOpener, IOpenerService } from 'vs/platform/opener/common/opener';
 import { IQuickInputService, IQuickPickItem, IQuickPickSeparator } from 'vs/platform/quickinput/common/quickInput';
-import * as nls from 'vs/nls';
-import { IPreferencesService } from 'vs/workbench/services/preferences/common/preferences';
-import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
 import { ExternalUriOpenerConfiguration, externalUriOpenersSettingId } from 'vs/workbench/contrib/externalUriOpener/common/configuration';
+import { IPreferencesService } from 'vs/workbench/services/preferences/common/preferences';
 
 export const IExternalUriOpenerService = createDecorator<IExternalUriOpenerService>('externalUriOpenerService');
 
@@ -21,13 +21,10 @@ export interface ExternalOpenerEntry extends IExternalOpener {
 	readonly label: string;
 }
 
-export interface ExternalOpenerSet {
-	readonly openers: readonly ExternalOpenerEntry[];
-	dispose(): void;
-}
+
 
 export interface IExternalOpenerProvider {
-	provideExternalOpeners(resource: URI | string): Promise<ExternalOpenerSet | undefined>;
+	provideExternalOpeners(resource: URI | string): Promise<readonly ExternalOpenerEntry[]>;
 }
 
 export interface IExternalUriOpenerService {
@@ -64,71 +61,62 @@ export class ExternalUriOpenerService extends Disposable implements IExternalUri
 
 		const targetUri = typeof href === 'string' ? URI.parse(href) : href;
 
-		const toDispose = new DisposableStore();
 		const openers: ExternalOpenerEntry[] = [];
 		for (const provider of this._externalOpenerProviders) {
-			const set = await provider.provideExternalOpeners(targetUri);
-			if (set) {
-				toDispose.add(set);
-				openers.push(...set.openers);
-			}
+			openers.push(...(await provider.provideExternalOpeners(targetUri)));
 		}
 
-		try {
-			if (openers.length === 0) {
-				return false;
-			}
+		if (openers.length === 0) {
+			return false;
+		}
 
-			const url = new URL(targetUri.toString());
-			const config = this.configurationService.getValue<readonly ExternalUriOpenerConfiguration[]>(externalUriOpenersSettingId) || [];
-			for (const entry of config) {
-				if (entry.hostname === url.hostname) {
-					const opener = openers.find(opener => opener.id === entry.id);
-					if (opener) {
-						return opener.openExternal(href);
-					}
+		const authority = targetUri.authority;
+		const config = this.configurationService.getValue<readonly ExternalUriOpenerConfiguration[]>(externalUriOpenersSettingId) || [];
+		for (const entry of config) {
+			if (entry.hostname === authority) {
+				const opener = openers.find(opener => opener.id === entry.id);
+				if (opener) {
+					return opener.openExternal(href);
 				}
 			}
+		}
 
-			type PickItem = IQuickPickItem & { opener?: IExternalOpener | 'configureDefault' };
-			const items: Array<PickItem | IQuickPickSeparator> = openers.map((opener, i): PickItem => {
-				return {
-					label: opener.label,
-					opener: opener
-				};
-			});
-			items.push(
-				{
-					label: 'Default',
-					opener: undefined
-				},
-				{ type: 'separator' },
-				{
-					label: nls.localize('selectOpenerConfigureTitle', "Configure default opener..."),
-					opener: 'configureDefault'
-				});
-
-			const picked = await this.quickInputService.pick(items, {
-				placeHolder: nls.localize('selectOpenerPlaceHolder', "Select opener for {0}", targetUri.toString())
+		type PickItem = IQuickPickItem & { opener?: IExternalOpener | 'configureDefault' };
+		const items: Array<PickItem | IQuickPickSeparator> = openers.map((opener, i): PickItem => {
+			return {
+				label: opener.label,
+				opener: opener
+			};
+		});
+		items.push(
+			{
+				label: 'Default',
+				opener: undefined
+			},
+			{ type: 'separator' },
+			{
+				label: nls.localize('selectOpenerConfigureTitle', "Configure default opener..."),
+				opener: 'configureDefault'
 			});
 
-			if (!picked) {
-				// Still cancel the default opener here since we prompted the user
-				return true;
-			}
+		const picked = await this.quickInputService.pick(items, {
+			placeHolder: nls.localize('selectOpenerPlaceHolder', "Select opener for {0}", targetUri.toString())
+		});
 
-			if (typeof picked.opener === 'undefined') {
-				return true;
-			} else if (picked.opener === 'configureDefault') {
-				await this.preferencesService.openGlobalSettings(true, {
-					revealSetting: { key: externalUriOpenersSettingId, edit: true }
-				});
-				return true;
-			} else {
-				return picked.opener.openExternal(href);
-			}
-		} finally {
-			toDispose.dispose();
+		if (!picked) {
+			// Still cancel the default opener here since we prompted the user
+			return true;
+		}
+
+		if (typeof picked.opener === 'undefined') {
+			return true;
+		} else if (picked.opener === 'configureDefault') {
+			await this.preferencesService.openGlobalSettings(true, {
+				revealSetting: { key: externalUriOpenersSettingId, edit: true }
+			});
+			return true;
+		} else {
+			return picked.opener.openExternal(href);
 		}
 	}
 }