Cleanup open-url handling (microsoft#91234)

Benjamin Pasero · web-flow · commit 646aaf67aa89 · 2020-02-23T16:54:53.000+01:00
* cleanup open-url handling

* open url - allow to open a remote workpsace without opening a window first

* help build

* undo build change

* support files too
diff --git a/src/main.js b/src/main.js
@@ -136,7 +136,7 @@ function configureCommandlineSwitchesSync(cliArgs) {
 		// override for the color profile to use
 		'force-color-profile'
 	];
-	
+
 	if (process.platform === 'linux') {
 		SUPPORTED_ELECTRON_SWITCHES.push('force-renderer-accessibility');
 	}
@@ -344,7 +344,7 @@ function setCurrentWorkingDirectory() {
 function registerListeners() {
 
 	/**
-	 * Mac: when someone drops a file to the not-yet running VSCode, the open-file event fires even before
+	 * macOS: when someone drops a file to the not-yet running VSCode, the open-file event fires even before
 	 * the app-ready event. We listen very early for open-file and remember this upon startup as path to open.
 	 *
 	 * @type {string[]}
@@ -356,7 +356,7 @@ function registerListeners() {
 	});
 
 	/**
-	 * React to open-url requests.
+	 * macOS: react to open-url requests.
 	 *
 	 * @type {string[]}
 	 */
diff --git a/src/vs/code/electron-main/app.ts b/src/vs/code/electron-main/app.ts
@@ -24,7 +24,7 @@ import { ILogService } from 'vs/platform/log/common/log';
 import { IStateService } from 'vs/platform/state/node/state';
 import { IEnvironmentService } from 'vs/platform/environment/common/environment';
 import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
-import { IURLService, IOpenURLOptions } from 'vs/platform/url/common/url';
+import { IURLService } from 'vs/platform/url/common/url';
 import { URLHandlerChannelClient, URLHandlerRouter } from 'vs/platform/url/common/urlIpc';
 import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
 import { NullTelemetryService, combinedAppender, LogAppender } from 'vs/platform/telemetry/common/telemetryUtils';
@@ -73,10 +73,10 @@ import { IDiagnosticsService } from 'vs/platform/diagnostics/node/diagnosticsSer
 import { ExtensionHostDebugBroadcastChannel } from 'vs/platform/debug/common/extensionHostDebugIpc';
 import { IElectronMainService, ElectronMainService } from 'vs/platform/electron/electron-main/electronMainService';
 import { ISharedProcessMainService, SharedProcessMainService } from 'vs/platform/ipc/electron-main/sharedProcessMainService';
-import { assign } from 'vs/base/common/objects';
 import { IDialogMainService, DialogMainService } from 'vs/platform/dialogs/electron-main/dialogs';
 import { withNullAsUndefined } from 'vs/base/common/types';
 import { parseArgs, OPTIONS } from 'vs/platform/environment/node/argv';
+import { coalesce } from 'vs/base/common/arrays';
 
 export class CodeApplication extends Disposable {
 
@@ -395,7 +395,7 @@ export class CodeApplication extends Disposable {
 		const windows = appInstantiationService.invokeFunction(accessor => this.openFirstWindow(accessor, electronIpcServer, sharedProcessClient));
 
 		// Post Open Windows Tasks
-		appInstantiationService.invokeFunction(this.afterWindowOpen.bind(this));
+		appInstantiationService.invokeFunction(accessor => this.afterWindowOpen(accessor));
 
 		// Tracing: Stop tracing after windows are ready if enabled
 		if (this.environmentService.args.trace) {
@@ -575,9 +575,8 @@ export class CodeApplication extends Disposable {
 		electronIpcServer.registerChannel('logger', loggerChannel);
 		sharedProcessClient.then(client => client.registerChannel('logger', loggerChannel));
 
-		const windowsMainService = this.windowsMainService = accessor.get(IWindowsMainService);
-
 		// ExtensionHost Debug broadcast service
+		const windowsMainService = this.windowsMainService = accessor.get(IWindowsMainService);
 		electronIpcServer.registerChannel(ExtensionHostDebugBroadcastChannel.ChannelName, new ElectronExtensionHostDebugBroadcastChannel(windowsMainService));
 
 		// Signal phase: ready (services set)
@@ -586,47 +585,67 @@ export class CodeApplication extends Disposable {
 		// Propagate to clients
 		this.dialogMainService = accessor.get(IDialogMainService);
 
-		// Create a URL handler to open file URIs in the active window
+		// Check for initial URLs to handle from protocol link invocations
 		const environmentService = accessor.get(IEnvironmentService);
+		const pendingWindowOpenablesFromProtocolLinks: IWindowOpenable[] = [];
+		const pendingProtocolLinksToHandle = coalesce([
+
+			// Windows/Linux: protocol handler invokes CLI with --open-url
+			...environmentService.args['open-url'] ? environmentService.args._urls || [] : [],
+
+			// macOS: open-url events
+			...((<any>global).getOpenUrls() || []) as string[]
+		].map(pendingUrlToHandle => {
+			try {
+				return URI.parse(pendingUrlToHandle);
+			} catch (error) {
+				return undefined;
+			}
+		})).filter(pendingUriToHandle => {
+			// filter out any protocol link that wants to open as window so that
+			// we open the right set of windows on startup and not restore the
+			// previous workspace too.
+			const windowOpenable = this.getWindowOpenableFromProtocolLink(pendingUriToHandle);
+			if (windowOpenable) {
+				pendingWindowOpenablesFromProtocolLinks.push(windowOpenable);
+
+				return false;
+			}
+
+			return true;
+		});
+
+		// Create a URL handler to open file URIs in the active window
+		const app = this;
 		urlService.registerHandler({
-			async handleURL(uri: URI, options?: IOpenURLOptions): Promise<boolean> {
-
-				// Catch file/remote URLs
-				if ((uri.authority === Schemas.file || uri.authority === Schemas.vscodeRemote) && !!uri.path) {
-					const cli = assign(Object.create(null), environmentService.args);
-					const urisToOpen: IWindowOpenable[] = [];
-
-					// File path
-					if (uri.authority === Schemas.file) {
-						// we configure as fileUri, but later validation will
-						// make sure to open as folder or workspace if possible
-						urisToOpen.push({ fileUri: URI.file(uri.fsPath) });
-					}
+			async handleURL(uri: URI): Promise<boolean> {
+
+				// Check for URIs to open in window
+				const windowOpenableFromProtocolLink = app.getWindowOpenableFromProtocolLink(uri);
+				if (windowOpenableFromProtocolLink) {
+					windowsMainService.open({
+						context: OpenContext.API,
+						cli: { ...environmentService.args },
+						urisToOpen: [windowOpenableFromProtocolLink],
+						gotoLineMode: true
+					});
 
-					// Remote path
-					else {
-						// Example conversion:
-						// From: vscode://vscode-remote/wsl+ubuntu/mnt/c/GitDevelopment/monaco
-						//   To: vscode-remote://wsl+ubuntu/mnt/c/GitDevelopment/monaco
-						const secondSlash = uri.path.indexOf(posix.sep, 1 /* skip over the leading slash */);
-						if (secondSlash !== -1) {
-							const authority = uri.path.substring(1, secondSlash);
-							const path = uri.path.substring(secondSlash);
-							const remoteUri = URI.from({ scheme: Schemas.vscodeRemote, authority, path, query: uri.query, fragment: uri.fragment });
-
-							if (hasWorkspaceFileExtension(path)) {
-								urisToOpen.push({ workspaceUri: remoteUri });
-							} else {
-								urisToOpen.push({ folderUri: remoteUri });
-							}
-						}
-					}
+					return true;
+				}
 
-					if (urisToOpen.length > 0) {
-						windowsMainService.open({ context: OpenContext.API, cli, urisToOpen, gotoLineMode: true });
+				// If we have not yet handled the URI and we have no window opened (macOS only)
+				// we first open a window and then try to open that URI within that window
+				if (isMacintosh && windowsMainService.getWindowCount() === 0) {
+					const [window] = windowsMainService.open({
+						context: OpenContext.API,
+						cli: { ...environmentService.args },
+						forceEmpty: true,
+						gotoLineMode: true
+					});
 
-						return true;
-					}
+					await window.ready();
+
+					return urlService.open(uri);
 				}
 
 				return false;
@@ -638,37 +657,13 @@ export class CodeApplication extends Disposable {
 		const activeWindowRouter = new StaticRouter(ctx => activeWindowManager.getActiveClientId().then(id => ctx === id));
 		const urlHandlerRouter = new URLHandlerRouter(activeWindowRouter);
 		const urlHandlerChannel = electronIpcServer.getChannel('urlHandler', urlHandlerRouter);
-		const multiplexURLHandler = new URLHandlerChannelClient(urlHandlerChannel);
-
-		// On Mac, Code can be running without any open windows, so we must create a window to handle urls,
-		// if there is none
-		if (isMacintosh) {
-			urlService.registerHandler({
-				async handleURL(uri: URI, options?: IOpenURLOptions): Promise<boolean> {
-					if (windowsMainService.getWindowCount() === 0) {
-						const cli = { ...environmentService.args };
-						const [window] = windowsMainService.open({ context: OpenContext.API, cli, forceEmpty: true, gotoLineMode: true });
-
-						await window.ready();
-
-						return urlService.open(uri);
-					}
-
-					return false;
-				}
-			});
-		}
-
-		// Register the multiple URL handler
-		urlService.registerHandler(multiplexURLHandler);
+		urlService.registerHandler(new URLHandlerChannelClient(urlHandlerChannel));
 
 		// Watch Electron URLs and forward them to the UrlService
-		const args = this.environmentService.args;
-		const urls = args['open-url'] ? args._urls : [];
-		const urlListener = new ElectronURLListener(urls || [], urlService, windowsMainService, this.environmentService);
-		this._register(urlListener);
+		this._register(new ElectronURLListener(pendingProtocolLinksToHandle, urlService, windowsMainService, this.environmentService));
 
 		// Open our first window
+		const args = this.environmentService.args;
 		const macOpenFiles: string[] = (<any>global).macOpenFiles;
 		const context = !!process.env['VSCODE_CLI'] ? OpenContext.CLI : OpenContext.DESKTOP;
 		const hasCliArgs = args._.length;
@@ -677,6 +672,19 @@ export class CodeApplication extends Disposable {
 		const noRecentEntry = args['skip-add-to-recently-opened'] === true;
 		const waitMarkerFileURI = args.wait && args.waitMarkerFilePath ? URI.file(args.waitMarkerFilePath) : undefined;
 
+		// check for a pending window to open from URI
+		// e.g. when running code with --open-uri from
+		// a protocol handler
+		if (pendingWindowOpenablesFromProtocolLinks.length > 0) {
+			return windowsMainService.open({
+				context,
+				cli: args,
+				urisToOpen: pendingWindowOpenablesFromProtocolLinks,
+				gotoLineMode: true,
+				initialStartup: true
+			});
+		}
+
 		// new window if "-n" or "--remote" was used without paths
 		if ((args['new-window'] || args.remote) && !hasCliArgs && !hasFolderURIs && !hasFileURIs) {
 			return windowsMainService.open({
@@ -698,7 +706,6 @@ export class CodeApplication extends Disposable {
 				urisToOpen: macOpenFiles.map(file => this.getWindowOpenableFromPathSync(file)),
 				noRecentEntry,
 				waitMarkerFileURI,
-				gotoLineMode: false,
 				initialStartup: true
 			});
 		}
@@ -716,6 +723,40 @@ export class CodeApplication extends Disposable {
 		});
 	}
 
+	private getWindowOpenableFromProtocolLink(uri: URI): IWindowOpenable | undefined {
+		if (!uri.path) {
+			return undefined;
+		}
+
+		// File path
+		if (uri.authority === Schemas.file) {
+			// we configure as fileUri, but later validation will
+			// make sure to open as folder or workspace if possible
+			return { fileUri: URI.file(uri.fsPath) };
+		}
+
+		// Remote path
+		else if (uri.authority === Schemas.vscodeRemote) {
+			// Example conversion:
+			// From: vscode://vscode-remote/wsl+ubuntu/mnt/c/GitDevelopment/monaco
+			//   To: vscode-remote://wsl+ubuntu/mnt/c/GitDevelopment/monaco
+			const secondSlash = uri.path.indexOf(posix.sep, 1 /* skip over the leading slash */);
+			if (secondSlash !== -1) {
+				const authority = uri.path.substring(1, secondSlash);
+				const path = uri.path.substring(secondSlash);
+				const remoteUri = URI.from({ scheme: Schemas.vscodeRemote, authority, path, query: uri.query, fragment: uri.fragment });
+
+				if (hasWorkspaceFileExtension(path)) {
+					return { workspaceUri: remoteUri };
+				} else {
+					return { folderUri: remoteUri };
+				}
+			}
+		}
+
+		return undefined;
+	}
+
 	private getWindowOpenableFromPathSync(path: string): IWindowOpenable {
 		try {
 			const fileStat = statSync(path);
@@ -734,6 +775,7 @@ export class CodeApplication extends Disposable {
 	}
 
 	private afterWindowOpen(accessor: ServicesAccessor): void {
+
 		// Signal phase: after window open
 		this.lifecycleMainService.phase = LifecycleMainPhase.AfterWindowOpen;
 
@@ -763,7 +805,7 @@ class ElectronExtensionHostDebugBroadcastChannel<TContext> extends ExtensionHost
 		super();
 	}
 
-	call(ctx: TContext, command: string, arg?: any): Promise<any> {
+	async call(ctx: TContext, command: string, arg?: any): Promise<any> {
 		if (command === 'openExtensionDevelopmentHostWindow') {
 			const env = arg[1];
 			const pargs = parseArgs(arg[0], OPTIONS);
@@ -775,7 +817,6 @@ class ElectronExtensionHostDebugBroadcastChannel<TContext> extends ExtensionHost
 					userEnv: Object.keys(env).length > 0 ? env : undefined
 				});
 			}
-			return Promise.resolve();
 		} else {
 			return super.call(ctx, command, arg);
 		}
diff --git a/src/vs/code/node/paths.ts b/src/vs/code/node/paths.ts
@@ -19,8 +19,8 @@ export function validatePaths(args: ParsedArgs): ParsedArgs {
 		args._ = [];
 	}
 
+	// Normalize paths and watch out for goto line mode
 	if (!args['remote']) {
-		// Normalize paths and watch out for goto line mode
 		const paths = doValidatePaths(args._, args.goto);
 		args._ = paths;
 	}
diff --git a/src/vs/platform/url/electron-main/electronUrlListener.ts b/src/vs/platform/url/electron-main/electronUrlListener.ts
@@ -12,7 +12,6 @@ import { URI } from 'vs/base/common/uri';
 import { IDisposable, DisposableStore, Disposable } from 'vs/base/common/lifecycle';
 import { IWindowsMainService } from 'vs/platform/windows/electron-main/windows';
 import { isWindows } from 'vs/base/common/platform';
-import { coalesce } from 'vs/base/common/arrays';
 import { disposableTimeout } from 'vs/base/common/async';
 
 function uriFromRawUrl(url: string): URI | null {
@@ -23,6 +22,16 @@ function uriFromRawUrl(url: string): URI | null {
 	}
 }
 
+/**
+ * A listener for URLs that are opened from the OS and handled by VSCode.
+ * Depending on the platform, this works differently:
+ * - Windows: we use `app.setAsDefaultProtocolClient()` to register VSCode with the OS
+ *            and additionally add the `open-url` command line argument to identify.
+ * - macOS:   we rely on `app.on('open-url')` to be called by the OS
+ * - Linux:   we have a special shortcut installed (`resources/linux/code-url-handler.desktop`)
+ *            that calls VSCode with the `open-url` command line argument
+ *            (https://github.com/microsoft/vscode/pull/56727)
+ */
 export class ElectronURLListener {
 
 	private uris: URI[] = [];
@@ -31,36 +40,34 @@ export class ElectronURLListener {
 	private disposables = new DisposableStore();
 
 	constructor(
-		initial: string | string[],
-		@IURLService private readonly urlService: IURLService,
-		@IWindowsMainService windowsMainService: IWindowsMainService,
-		@IEnvironmentService environmentService: IEnvironmentService
+		initialUrisToHandle: URI[],
+		private readonly urlService: IURLService,
+		windowsMainService: IWindowsMainService,
+		environmentService: IEnvironmentService
 	) {
-		const globalBuffer = ((<any>global).getOpenUrls() || []) as string[];
-		const rawBuffer = [
-			...(typeof initial === 'string' ? [initial] : initial),
-			...globalBuffer
-		];
 
-		this.uris = coalesce(rawBuffer.map(uriFromRawUrl));
+		// the initial set of URIs we need to handle once the window is ready
+		this.uris = initialUrisToHandle;
 
+		// Windows: install as protocol handler
 		if (isWindows) {
 			const windowsParameters = environmentService.isBuilt ? [] : [`"${environmentService.appRoot}"`];
 			windowsParameters.push('--open-url', '--');
 			app.setAsDefaultProtocolClient(product.urlProtocol, process.execPath, windowsParameters);
 		}
 
+		// macOS: listen to `open-url` events from here on to handle
 		const onOpenElectronUrl = Event.map(
 			Event.fromNodeEventEmitter(app, 'open-url', (event: ElectronEvent, url: string) => ({ event, url })),
 			({ event, url }) => {
-				// always prevent default and return the url as string
-				event.preventDefault();
+				event.preventDefault(); // always prevent default and return the url as string
 				return url;
 			});
 
 		const onOpenUrl = Event.filter<URI | null, URI>(Event.map(onOpenElectronUrl, uriFromRawUrl), (uri): uri is URI => !!uri);
 		onOpenUrl(this.urlService.open, this.urlService, this.disposables);
 
+		// Send initial links to the window once it has loaded
 		const isWindowReady = windowsMainService.getWindows()
 			.filter(w => w.isReady)
 			.length > 0;

Original file line number	Diff line number	Diff line change
`@@ -19,8 +19,8 @@ export function validatePaths(args: ParsedArgs): ParsedArgs {`
`19`	`19`	`args._ = [];`
`20`	`20`	`}`
`21`	`21`
	`22`	`+ // Normalize paths and watch out for goto line mode`
`22`	`23`	`if (!args['remote']) {`
`23`		`- // Normalize paths and watch out for goto line mode`
`24`	`24`	`const paths = doValidatePaths(args._, args.goto);`
`25`	`25`	`args._ = paths;`
`26`	`26`	`}`