From a62b7dabe7399bd30580a5b3f68612a3047df0fb Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Mon, 23 Sep 2019 10:40:58 -0700 Subject: [PATCH] Make resolveExternalUri return just a plain uri instead of a disposable result For #81131 We had trouble coming up with cases where extensions could actually reliably dispose of the resolved uri --- .../editor/browser/services/openerService.ts | 11 +++++----- src/vs/vscode.proposed.d.ts | 8 +++----- .../workbench/api/browser/mainThreadWindow.ts | 20 ++----------------- .../workbench/api/common/extHost.protocol.ts | 3 +-- src/vs/workbench/api/common/extHostWindow.ts | 12 +++-------- 5 files changed, 15 insertions(+), 39 deletions(-) diff --git a/src/vs/editor/browser/services/openerService.ts b/src/vs/editor/browser/services/openerService.ts index aa9901845bb3b..759811e6091a2 100644 --- a/src/vs/editor/browser/services/openerService.ts +++ b/src/vs/editor/browser/services/openerService.ts @@ -58,15 +58,18 @@ export class OpenerService extends Disposable implements IOpenerService { } } + const { resolved } = await this.resolveExternalUri(resource, options); + // check with contributed openers for (const opener of this._openers.toArray()) { - const handled = await opener.open(resource, options); + const handled = await opener.open(resolved, options); if (handled) { return true; } } + // use default openers - return this._doOpen(resource, options); + return this._doOpen(resolved, options); } public async resolveExternalUri(resource: URI, options?: { readonly allowTunneling?: boolean }): Promise<{ resolved: URI, dispose(): void }> { @@ -135,9 +138,7 @@ export class OpenerService extends Disposable implements IOpenerService { } private async _doOpenExternal(resource: URI, options: OpenOptions | undefined): Promise { - const { resolved } = await this.resolveExternalUri(resource, options); - dom.windowOpenNoOpener(encodeURI(resolved.toString(true))); - + dom.windowOpenNoOpener(encodeURI(resource.toString(true))); return Promise.resolve(true); } diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 1101392631bb0..98ec3a9da550e 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -1147,18 +1147,16 @@ declare module 'vscode' { * Resolves an *external* uri, such as a `http:` or `https:` link, from where the extension is running to a * uri to the same resource on the client machine. * - * This is a no-oop if the extension is running locally. Currently only supports `https:` and `http:`. + * This is a no-op if the extension is running locally. Currently only supports `https:` and `http:`. * * If the extension is running remotely, this function automatically establishes port forwarding from * the local machine to `target` on the remote and returns a local uri that can be used to for this connection. * * Note that uris passed through `openExternal` are automatically resolved. * - * @return A uri that can be used on the client machine. Extensions should dispose of the returned value when - * both the extension and the user are no longer using the value. For port forwarded uris, `dispose` will - * close the connection. + * @return A uri that can be used on the client machine. */ - export function resolveExternalUri(target: Uri): Thenable<{ resolved: Uri, dispose(): void }>; + export function resolveExternalUri(target: Uri): Thenable; } //#endregion diff --git a/src/vs/workbench/api/browser/mainThreadWindow.ts b/src/vs/workbench/api/browser/mainThreadWindow.ts index eef446e0123bf..2fb07578e85cf 100644 --- a/src/vs/workbench/api/browser/mainThreadWindow.ts +++ b/src/vs/workbench/api/browser/mainThreadWindow.ts @@ -14,8 +14,6 @@ import { ExtHostContext, ExtHostWindowShape, IExtHostContext, IOpenUriOptions, M @extHostNamedCustomer(MainContext.MainThreadWindow) export class MainThreadWindow implements MainThreadWindowShape { - private handlePool = 1; - private readonly proxy: ExtHostWindowShape; private readonly disposables = new DisposableStore(); private readonly resolved = new Map(); @@ -49,23 +47,9 @@ export class MainThreadWindow implements MainThreadWindowShape { return this.openerService.open(uri, { openExternal: true, allowTunneling: options.allowTunneling }); } - async $resolveExternalUri(uriComponents: UriComponents, options: IOpenUriOptions): Promise<{ handle: number, result: UriComponents }> { + async $resolveExternalUri(uriComponents: UriComponents, options: IOpenUriOptions): Promise { const uri = URI.revive(uriComponents); - const handle = ++this.handlePool; - const result = await this.openerService.resolveExternalUri(uri, options); - this.resolved.set(handle, result); - - return { handle, result: result.resolved }; - } - - async $releaseResolvedExternalUri(handle: number): Promise { - const entry = this.resolved.get(handle); - if (entry) { - entry.dispose(); - this.resolved.delete(handle); - return true; - } - return false; + return result.resolved; } } diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 1641e4fac4325..f2bc8f0e7179b 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -744,8 +744,7 @@ export interface IOpenUriOptions { export interface MainThreadWindowShape extends IDisposable { $getWindowVisibility(): Promise; $openUri(uri: UriComponents, options: IOpenUriOptions): Promise; - $resolveExternalUri(uri: UriComponents, options: IOpenUriOptions): Promise<{ readonly handle: number, readonly result: UriComponents }>; - $releaseResolvedExternalUri(handle: number): Promise; + $resolveExternalUri(uri: UriComponents, options: IOpenUriOptions): Promise; } // -- extension host diff --git a/src/vs/workbench/api/common/extHostWindow.ts b/src/vs/workbench/api/common/extHostWindow.ts index c2c68a4bd0610..fa36961f1523e 100644 --- a/src/vs/workbench/api/common/extHostWindow.ts +++ b/src/vs/workbench/api/common/extHostWindow.ts @@ -9,7 +9,6 @@ import { WindowState } from 'vscode'; import { URI } from 'vs/base/common/uri'; import { Schemas } from 'vs/base/common/network'; import { isFalsyOrWhitespace } from 'vs/base/common/strings'; -import { once } from 'vs/base/common/functional'; export class ExtHostWindow implements ExtHostWindowShape { @@ -55,19 +54,14 @@ export class ExtHostWindow implements ExtHostWindowShape { return this._proxy.$openUri(stringOrUri, options); } - async resolveExternalUri(uri: URI, options: IOpenUriOptions): Promise<{ resolved: URI, dispose(): void }> { + async resolveExternalUri(uri: URI, options: IOpenUriOptions): Promise { if (isFalsyOrWhitespace(uri.scheme)) { return Promise.reject('Invalid scheme - cannot be empty'); } else if (!new Set([Schemas.http, Schemas.https]).has(uri.scheme)) { return Promise.reject(`Invalid scheme '${uri.scheme}'`); } - const { result, handle } = await this._proxy.$resolveExternalUri(uri, options); - return { - resolved: URI.from(result), - dispose: once(() => { - this._proxy.$releaseResolvedExternalUri(handle); - }), - }; + const result = await this._proxy.$resolveExternalUri(uri, options); + return URI.from(result); } }