From d4b310ea0127625d52a275efe186e0f351f55da1 Mon Sep 17 00:00:00 2001 From: Joao Moreno Date: Wed, 11 Jul 2018 17:24:10 +0200 Subject: [PATCH] fix UI calls from shared process fix #54070 --- src/vs/base/parts/ipc/common/ipc.ts | 28 +++++++++---------- .../sharedProcess/sharedProcessMain.ts | 17 ++++------- src/vs/code/electron-main/app.ts | 2 +- .../platform/driver/electron-main/driver.ts | 8 +++--- src/vs/platform/windows/common/windows.ts | 19 +++++++++++-- src/vs/platform/windows/common/windowsIpc.ts | 6 ++++ .../windows/electron-main/windowsService.ts | 12 +++++++- .../dialogs/electron-browser/dialogService.ts | 8 +++++- .../workbench/test/workbenchTestServices.ts | 4 +++ 9 files changed, 68 insertions(+), 36 deletions(-) diff --git a/src/vs/base/parts/ipc/common/ipc.ts b/src/vs/base/parts/ipc/common/ipc.ts index ed35efadee0..37555c26b07 100644 --- a/src/vs/base/parts/ipc/common/ipc.ts +++ b/src/vs/base/parts/ipc/common/ipc.ts @@ -94,8 +94,8 @@ export interface IChannelClient { * channels (each from a separate client) to pick from. */ export interface IClientRouter { - routeCall(command: string, arg: any): string; - routeEvent(event: string, arg: any): string; + routeCall(command: string, arg: any): TPromise; + routeEvent(event: string, arg: any): TPromise; } /** @@ -433,24 +433,20 @@ export class IPCServer implements IChannelServer, IRoutingChannelClient, IDispos getChannel(channelName: string, router: IClientRouter): T { const call = (command: string, arg: any) => { - const id = router.routeCall(command, arg); + const channelPromise = router.routeCall(command, arg) + .then(id => this.getClient(id)) + .then(client => client.getChannel(channelName)); - if (!id) { - return TPromise.wrapError(new Error('Client id should be provided')); - } - - return getDelayedChannel(this.getClient(id).then(client => client.getChannel(channelName))) + return getDelayedChannel(channelPromise) .call(command, arg); }; const listen = (event: string, arg: any) => { - const id = router.routeEvent(event, arg); - - if (!id) { - return TPromise.wrapError(new Error('Client id should be provided')); - } + const channelPromise = router.routeEvent(event, arg) + .then(id => this.getClient(id)) + .then(client => client.getChannel(channelName)); - return getDelayedChannel(this.getClient(id).then(client => client.getChannel(channelName))) + return getDelayedChannel(channelPromise) .listen(event, arg); }; @@ -462,6 +458,10 @@ export class IPCServer implements IChannelServer, IRoutingChannelClient, IDispos } private getClient(clientId: string): TPromise { + if (!clientId) { + return TPromise.wrapError(new Error('Client id should be provided')); + } + const client = this.channelClients[clientId]; if (client) { diff --git a/src/vs/code/electron-browser/sharedProcess/sharedProcessMain.ts b/src/vs/code/electron-browser/sharedProcess/sharedProcessMain.ts index deaea3955f3..d8f64bec42b 100644 --- a/src/vs/code/electron-browser/sharedProcess/sharedProcessMain.ts +++ b/src/vs/code/electron-browser/sharedProcess/sharedProcessMain.ts @@ -67,7 +67,8 @@ function main(server: Server, initData: ISharedProcessInitData, configuration: I process.once('exit', () => dispose(disposables)); const environmentService = new EnvironmentService(initData.args, process.execPath); - const logLevelClient = new LogLevelSetterChannelClient(server.getChannel('loglevel', { routeCall: () => 'main', routeEvent: () => 'main' })); + const mainRoute = () => TPromise.as('main'); + const logLevelClient = new LogLevelSetterChannelClient(server.getChannel('loglevel', { routeCall: mainRoute, routeEvent: mainRoute })); const logService = new FollowerLogService(logLevelClient, createSpdLogService('sharedprocess', initData.logLevel, environmentService.logsPath)); disposables.push(logService); @@ -78,21 +79,13 @@ function main(server: Server, initData: ISharedProcessInitData, configuration: I services.set(IConfigurationService, new SyncDescriptor(ConfigurationService)); services.set(IRequestService, new SyncDescriptor(RequestService)); - const windowsChannel = server.getChannel('windows', { routeCall: () => 'main', routeEvent: () => 'main' }); + const windowsChannel = server.getChannel('windows', { routeCall: mainRoute, routeEvent: mainRoute }); const windowsService = new WindowsChannelClient(windowsChannel); services.set(IWindowsService, windowsService); const activeWindowManager = new ActiveWindowManager(windowsService); - const dialogChannel = server.getChannel('dialog', { - routeCall: () => { - logService.info('Routing dialog call request to the client', activeWindowManager.activeClientId); - return activeWindowManager.activeClientId; - }, - routeEvent: () => { - logService.info('Routing dialog listen request to the client', activeWindowManager.activeClientId); - return activeWindowManager.activeClientId; - } - }); + const route = () => activeWindowManager.getActiveClientId(); + const dialogChannel = server.getChannel('dialog', { routeCall: route, routeEvent: route }); services.set(IDialogService, new DialogChannelClient(dialogChannel)); const instantiationService = new InstantiationService(services); diff --git a/src/vs/code/electron-main/app.ts b/src/vs/code/electron-main/app.ts index f2a7d0cae04..72479ad26f5 100644 --- a/src/vs/code/electron-main/app.ts +++ b/src/vs/code/electron-main/app.ts @@ -426,7 +426,7 @@ export class CodeApplication { // Create a URL handler which forwards to the last active window const activeWindowManager = new ActiveWindowManager(windowsService); - const route = () => activeWindowManager.activeClientId; + const route = () => activeWindowManager.getActiveClientId(); const urlHandlerChannel = this.electronIpcServer.getChannel('urlHandler', { routeCall: route, routeEvent: route }); const multiplexURLHandler = new URLHandlerChannelClient(urlHandlerChannel); diff --git a/src/vs/platform/driver/electron-main/driver.ts b/src/vs/platform/driver/electron-main/driver.ts index ba2b3c673a2..f9667d4d695 100644 --- a/src/vs/platform/driver/electron-main/driver.ts +++ b/src/vs/platform/driver/electron-main/driver.ts @@ -27,12 +27,12 @@ class WindowRouter implements IClientRouter { constructor(private windowId: number) { } - routeCall(): string { - return `window:${this.windowId}`; + routeCall(): TPromise { + return TPromise.as(`window:${this.windowId}`); } - routeEvent(): string { - return `window:${this.windowId}`; + routeEvent(): TPromise { + return TPromise.as(`window:${this.windowId}`); } } diff --git a/src/vs/platform/windows/common/windows.ts b/src/vs/platform/windows/common/windows.ts index 9c94e242e06..65855f4b03e 100644 --- a/src/vs/platform/windows/common/windows.ts +++ b/src/vs/platform/windows/common/windows.ts @@ -162,6 +162,7 @@ export interface IWindowsService { getWindowCount(): TPromise; log(severity: string, ...messages: string[]): TPromise; showItemInFolder(path: string): TPromise; + getActiveWindowId(): TPromise; // This needs to be handled from browser process to prevent // foreground ordering issues on Windows @@ -357,19 +358,31 @@ export interface IRunActionInWindowRequest { export class ActiveWindowManager implements IDisposable { private disposables: IDisposable[] = []; - private _activeWindowId: number; + private firstActiveWindowIdPromise: TPromise | null; + private _activeWindowId: number | undefined; constructor(@IWindowsService windowsService: IWindowsService) { const onActiveWindowChange = latch(anyEvent(windowsService.onWindowOpen, windowsService.onWindowFocus)); onActiveWindowChange(this.setActiveWindow, this, this.disposables); + + this.firstActiveWindowIdPromise = windowsService.getActiveWindowId() + .then(id => (typeof this._activeWindowId === 'undefined') && this.setActiveWindow(id)); } private setActiveWindow(windowId: number) { + if (this.firstActiveWindowIdPromise) { + this.firstActiveWindowIdPromise = null; + } + this._activeWindowId = windowId; } - get activeClientId(): string { - return `window:${this._activeWindowId}`; + getActiveClientId(): TPromise { + if (this.firstActiveWindowIdPromise) { + return this.firstActiveWindowIdPromise; + } + + return TPromise.as(`window:${this._activeWindowId}`); } dispose() { diff --git a/src/vs/platform/windows/common/windowsIpc.ts b/src/vs/platform/windows/common/windowsIpc.ts index 0adca2f73b4..fb9cd086488 100644 --- a/src/vs/platform/windows/common/windowsIpc.ts +++ b/src/vs/platform/windows/common/windowsIpc.ts @@ -68,6 +68,7 @@ export interface IWindowsChannel extends IChannel { call(command: 'toggleSharedProcess'): TPromise; call(command: 'log', arg: [string, string[]]): TPromise; call(command: 'showItemInFolder', arg: string): TPromise; + call(command: 'getActiveWindowId'): TPromise; call(command: 'openExternal', arg: string): TPromise; call(command: 'startCrashReporter', arg: CrashReporterStartOptions): TPromise; call(command: 'openAccessibilityOptions'): TPromise; @@ -166,6 +167,7 @@ export class WindowsChannel implements IWindowsChannel { case 'quit': return this.service.quit(); case 'log': return this.service.log(arg[0], arg[1]); case 'showItemInFolder': return this.service.showItemInFolder(arg); + case 'getActiveWindowId': return this.service.getActiveWindowId(); case 'openExternal': return this.service.openExternal(arg); case 'startCrashReporter': return this.service.startCrashReporter(arg); case 'openAccessibilityOptions': return this.service.openAccessibilityOptions(); @@ -364,6 +366,10 @@ export class WindowsChannelClient implements IWindowsService { return this.channel.call('showItemInFolder', path); } + getActiveWindowId(): TPromise { + return this.channel.call('getActiveWindowId'); + } + openExternal(url: string): TPromise { return this.channel.call('openExternal', url); } diff --git a/src/vs/platform/windows/electron-main/windowsService.ts b/src/vs/platform/windows/electron-main/windowsService.ts index 2987443c204..dcc72f14b81 100644 --- a/src/vs/platform/windows/electron-main/windowsService.ts +++ b/src/vs/platform/windows/electron-main/windowsService.ts @@ -14,7 +14,7 @@ import product from 'vs/platform/node/product'; import { IWindowsService, OpenContext, INativeOpenDialogOptions, IEnterWorkspaceResult, IMessageBoxResult, IDevToolsOptions } from 'vs/platform/windows/common/windows'; import { IEnvironmentService, ParsedArgs } from 'vs/platform/environment/common/environment'; import { shell, crashReporter, app, Menu, clipboard, BrowserWindow } from 'electron'; -import { Event, fromNodeEventEmitter, mapEvent, filterEvent, anyEvent } from 'vs/base/common/event'; +import { Event, fromNodeEventEmitter, mapEvent, filterEvent, anyEvent, latch } from 'vs/base/common/event'; import { IURLService, IURLHandler } from 'vs/platform/url/common/url'; import { ILifecycleService } from 'vs/platform/lifecycle/electron-main/lifecycleMain'; import { IWindowsMainService, ISharedProcess } from 'vs/platform/windows/electron-main/windows'; @@ -32,6 +32,8 @@ export class WindowsService implements IWindowsService, IURLHandler, IDisposable private disposables: IDisposable[] = []; + private _activeWindowId: number | undefined; + readonly onWindowOpen: Event = filterEvent(fromNodeEventEmitter(app, 'browser-window-created', (_, w: Electron.BrowserWindow) => w.id), id => !!this.windowsMainService.getWindowById(id)); readonly onWindowFocus: Event = anyEvent( mapEvent(filterEvent(mapEvent(this.windowsMainService.onWindowsCountChanged, () => this.windowsMainService.getLastActiveWindow()), w => !!w), w => w.id), @@ -54,6 +56,10 @@ export class WindowsService implements IWindowsService, IURLHandler, IDisposable @ILogService private logService: ILogService ) { urlService.registerHandler(this); + + // remember last active window id + latch(anyEvent(this.onWindowOpen, this.onWindowFocus)) + (id => this._activeWindowId = id, null, this.disposables); } pickFileFolderAndOpen(options: INativeOpenDialogOptions): TPromise { @@ -435,6 +441,10 @@ export class WindowsService implements IWindowsService, IURLHandler, IDisposable return TPromise.as(null); } + getActiveWindowId(): TPromise { + return TPromise.as(this._activeWindowId); + } + openExternal(url: string): TPromise { this.logService.trace('windowsService#openExternal'); return TPromise.as(shell.openExternal(url)); diff --git a/src/vs/workbench/services/dialogs/electron-browser/dialogService.ts b/src/vs/workbench/services/dialogs/electron-browser/dialogService.ts index 610c5ef1946..c2c1ee52f58 100644 --- a/src/vs/workbench/services/dialogs/electron-browser/dialogService.ts +++ b/src/vs/workbench/services/dialogs/electron-browser/dialogService.ts @@ -13,6 +13,7 @@ import { isLinux, isWindows } from 'vs/base/common/platform'; import { IWindowService } from 'vs/platform/windows/common/windows'; import { mnemonicButtonLabel } from 'vs/base/common/labels'; import { IDialogService, IConfirmation, IConfirmationResult, IDialogOptions } from 'vs/platform/dialogs/common/dialogs'; +import { ILogService } from 'vs/platform/log/common/log'; interface IMassagedMessageBoxOptions { @@ -34,10 +35,13 @@ export class DialogService implements IDialogService { _serviceBrand: any; constructor( - @IWindowService private windowService: IWindowService + @IWindowService private windowService: IWindowService, + @ILogService private logService: ILogService ) { } confirm(confirmation: IConfirmation): TPromise { + this.logService.trace('DialogService#confirm', confirmation.message); + const { options, buttonIndexMap } = this.massageMessageBoxOptions(this.getConfirmOptions(confirmation)); return this.windowService.showMessageBox(options).then(result => { @@ -86,6 +90,8 @@ export class DialogService implements IDialogService { } show(severity: Severity, message: string, buttons: string[], dialogOptions?: IDialogOptions): TPromise { + this.logService.trace('DialogService#show', message); + const { options, buttonIndexMap } = this.massageMessageBoxOptions({ message, buttons, diff --git a/src/vs/workbench/test/workbenchTestServices.ts b/src/vs/workbench/test/workbenchTestServices.ts index d9a1cb607c7..46060e5bc2d 100644 --- a/src/vs/workbench/test/workbenchTestServices.ts +++ b/src/vs/workbench/test/workbenchTestServices.ts @@ -1295,6 +1295,10 @@ export class TestWindowsService implements IWindowsService { return TPromise.as(void 0); } + getActiveWindowId(): TPromise { + return TPromise.as(undefined); + } + // This needs to be handled from browser process to prevent // foreground ordering issues on Windows openExternal(url: string): TPromise { -- GitLab