From 33cc09da7a314ea0ee5677d1927739af60d16734 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Mon, 28 Oct 2019 12:08:07 +0100 Subject: [PATCH] Revert "Removing EXT_HOST_CREATION_DELAY hacks" (#83422) This reverts commit 2b61a8817b47cc5a30a7667c01db251dd00eee1a. --- .../api/browser/mainThreadTerminalService.ts | 11 ++++++++--- .../api/common/extHostTerminalService.ts | 18 +++++++++++++++++- .../contrib/terminal/browser/terminal.ts | 10 ---------- .../terminal/browser/terminalInstance.ts | 13 ++----------- .../terminal/browser/terminalProcessManager.ts | 17 +++++------------ .../terminal/browser/terminalService.ts | 5 +---- .../contrib/terminal/common/terminal.ts | 6 +++++- 7 files changed, 38 insertions(+), 42 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadTerminalService.ts b/src/vs/workbench/api/browser/mainThreadTerminalService.ts index 5ac85577086..8344b323c7e 100644 --- a/src/vs/workbench/api/browser/mainThreadTerminalService.ts +++ b/src/vs/workbench/api/browser/mainThreadTerminalService.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { DisposableStore, Disposable } from 'vs/base/common/lifecycle'; -import { IShellLaunchConfig, ITerminalProcessExtHostProxy, ISpawnExtHostProcessRequest, ITerminalDimensions, IAvailableShellsRequest, IDefaultShellAndArgsRequest, IStartExtensionTerminalRequest } from 'vs/workbench/contrib/terminal/common/terminal'; +import { IShellLaunchConfig, ITerminalProcessExtHostProxy, ISpawnExtHostProcessRequest, ITerminalDimensions, EXT_HOST_CREATION_DELAY, IAvailableShellsRequest, IDefaultShellAndArgsRequest, IStartExtensionTerminalRequest } from 'vs/workbench/contrib/terminal/common/terminal'; import { ExtHostContext, ExtHostTerminalServiceShape, MainThreadTerminalServiceShape, MainContext, IExtHostContext, IShellLaunchConfigDto, TerminalLaunchConfig, ITerminalDimensionsDto } from 'vs/workbench/api/common/extHost.protocol'; import { extHostNamedCustomer } from 'vs/workbench/api/common/extHostCustomers'; import { URI } from 'vs/base/common/uri'; @@ -36,8 +36,13 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape // ITerminalService listeners this._toDispose.add(_terminalService.onInstanceCreated((instance) => { - this._onTerminalOpened(instance); - this._onInstanceDimensionsChanged(instance); + // Delay this message so the TerminalInstance constructor has a chance to finish and + // return the ID normally to the extension host. The ID that is passed here will be + // used to register non-extension API terminals in the extension host. + setTimeout(() => { + this._onTerminalOpened(instance); + this._onInstanceDimensionsChanged(instance); + }, EXT_HOST_CREATION_DELAY); })); this._toDispose.add(_terminalService.onInstanceDisposed(instance => this._onTerminalDisposed(instance))); diff --git a/src/vs/workbench/api/common/extHostTerminalService.ts b/src/vs/workbench/api/common/extHostTerminalService.ts index bf24a7408c2..16a1cafd977 100644 --- a/src/vs/workbench/api/common/extHostTerminalService.ts +++ b/src/vs/workbench/api/common/extHostTerminalService.ts @@ -9,7 +9,7 @@ import { ExtHostTerminalServiceShape, MainContext, MainThreadTerminalServiceShap import { ExtHostConfigProvider } from 'vs/workbench/api/common/extHostConfiguration'; import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; import { URI, UriComponents } from 'vs/base/common/uri'; -import { ITerminalChildProcess, ITerminalDimensions } from 'vs/workbench/contrib/terminal/common/terminal'; +import { EXT_HOST_CREATION_DELAY, ITerminalChildProcess, ITerminalDimensions } from 'vs/workbench/contrib/terminal/common/terminal'; import { timeout } from 'vs/base/common/async'; import { IExtHostRpcService } from 'vs/workbench/api/common/extHostRpcService'; import { TerminalDataBufferer } from 'vs/workbench/contrib/terminal/common/terminalDataBuffering'; @@ -412,6 +412,22 @@ export abstract class BaseExtHostTerminalService implements IExtHostTerminalServ } } + public performTerminalIdAction(id: number, callback: (terminal: ExtHostTerminal) => void): void { + // TODO: Use await this._getTerminalByIdEventually(id); + let terminal = this._getTerminalById(id); + if (terminal) { + callback(terminal); + } else { + // Retry one more time in case the terminal has not yet been initialized. + setTimeout(() => { + terminal = this._getTerminalById(id); + if (terminal) { + callback(terminal); + } + }, EXT_HOST_CREATION_DELAY * 2); + } + } + public async $startExtensionTerminal(id: number, initialDimensions: ITerminalDimensionsDto | undefined): Promise { // Make sure the ExtHostTerminal exists so onDidOpenTerminal has fired before we call // Pseudoterminal.start diff --git a/src/vs/workbench/contrib/terminal/browser/terminal.ts b/src/vs/workbench/contrib/terminal/browser/terminal.ts index 7aeb65f7e10..c3ede8986ad 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminal.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminal.ts @@ -196,16 +196,6 @@ export interface ITerminalInstance { onDisposed: Event; onFocused: Event; - - /** - * An event that fires when the terminal's process has been _created_, this will happen sometime - * after the `ITerminalInstance` has been constructed in a different event loop frame. - */ - onProcessCreated: Event; - - /** - * An event that fires when the process ID running inside the terminal has been fetched. - */ onProcessIdReady: Event; onRequestExtHostProcess: Event; onDimensionsChanged: Event; diff --git a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts index f81da49a3bc..21f719aa6b2 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts @@ -239,8 +239,6 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { public get onDisposed(): Event { return this._onDisposed.event; } private readonly _onFocused = new Emitter(); public get onFocused(): Event { return this._onFocused.event; } - private readonly _onProcessCreated = new Emitter(); - public get onProcessCreated(): Event { return this._onProcessCreated.event; } private readonly _onProcessIdReady = new Emitter(); public get onProcessIdReady(): Event { return this._onProcessIdReady.event; } private readonly _onTitleChanged = new Emitter(); @@ -947,12 +945,6 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { protected _createProcess(): void { this._processManager = this._instantiationService.createInstance(TerminalProcessManager, this._id, this._configHelper); - const processCreatedListener = this._processManager.onProcessStateChange(state => { - if (state === ProcessState.LAUNCHING) { - this._onProcessCreated.fire(); - processCreatedListener.dispose(); - } - }); this._processManager.onProcessReady(() => this._onProcessIdReady.fire(this)); this._processManager.onProcessExit(exitCode => this._onProcessExit(exitCode)); this._processManager.onProcessData(data => this._onData.fire(data)); @@ -991,9 +983,8 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { }); } - // Create the process asynchronously to allow the terminal's container to be created so - // dimensions are accurate. This also ensures that process manager listeners are ready for - // `onProcessStateChange` events. + // Create the process asynchronously to allow the terminal's container + // to be created so dimensions are accurate setTimeout(() => { this._processManager.createProcess(this._shellLaunchConfig, this._cols, this._rows, this._isScreenReaderOptimized()); }, 0); diff --git a/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts b/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts index f5e97f7d320..d221c1bf8c8 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts @@ -59,8 +59,6 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce private _latencyLastMeasured: number = 0; private _initialCwd: string | undefined; - private readonly _onProcessStateChange = this._register(new Emitter()); - public get onProcessStateChange(): Event { return this._onProcessStateChange.event; } private readonly _onProcessReady = this._register(new Emitter()); public get onProcessReady(): Event { return this._onProcessReady.event; } private readonly _onBeforeProcessData = this._register(new Emitter()); @@ -105,18 +103,13 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce // If the process was still connected this dispose came from // within VS Code, not the process, so mark the process as // killed by the user. - this._setProcessState(ProcessState.KILLED_BY_USER); + this.processState = ProcessState.KILLED_BY_USER; this._process.shutdown(immediate); this._process = null; } super.dispose(); } - private _setProcessState(newState: ProcessState): void { - this.processState = newState; - this._onProcessStateChange.fire(this.processState); - } - public async createProcess( shellLaunchConfig: IShellLaunchConfig, cols: number, @@ -155,7 +148,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce this._process = await this._launchProcess(shellLaunchConfig, cols, rows, isScreenReaderModeEnabled); } } - this._setProcessState(ProcessState.LAUNCHING); + this.processState = ProcessState.LAUNCHING; this._process.onProcessData(data => { const beforeProcessDataEvent: IBeforeProcessDataEvent = { data }; @@ -188,7 +181,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce setTimeout(() => { if (this.processState === ProcessState.LAUNCHING) { - this._setProcessState(ProcessState.RUNNING); + this.processState = ProcessState.RUNNING; } }, LAUNCHING_DURATION); } @@ -299,13 +292,13 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce // during launch. This typically means that there is a problem with the // shell and args. if (this.processState === ProcessState.LAUNCHING) { - this._setProcessState(ProcessState.KILLED_DURING_LAUNCH); + this.processState = ProcessState.KILLED_DURING_LAUNCH; } // If TerminalInstance did not know about the process exit then it was // triggered by the process, not on VS Code's side. if (this.processState === ProcessState.RUNNING) { - this._setProcessState(ProcessState.KILLED_BY_PROCESS); + this.processState = ProcessState.KILLED_BY_PROCESS; } this._onProcessExit.fire(exitCode); diff --git a/src/vs/workbench/contrib/terminal/browser/terminalService.ts b/src/vs/workbench/contrib/terminal/browser/terminalService.ts index ef7a324469e..173957233b9 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalService.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalService.ts @@ -576,10 +576,7 @@ export class TerminalService implements ITerminalService { public createInstance(container: HTMLElement | undefined, shellLaunchConfig: IShellLaunchConfig): ITerminalInstance { const instance = this._instantiationService.createInstance(TerminalInstance, this._terminalFocusContextKey, this._configHelper, container, shellLaunchConfig); - // This event must fire after the process has been created, otherwise terminals created via - // the extension API do not have a change to have their ID returned to them which will - // result in 2 terminal objects for a single terminal on the extension host. - instance.onProcessCreated(() => this._onInstanceCreated.fire(instance)); + this._onInstanceCreated.fire(instance); return instance; } diff --git a/src/vs/workbench/contrib/terminal/common/terminal.ts b/src/vs/workbench/contrib/terminal/common/terminal.ts index c3e106a6da7..fdbc74996a0 100644 --- a/src/vs/workbench/contrib/terminal/common/terminal.ts +++ b/src/vs/workbench/contrib/terminal/common/terminal.ts @@ -42,6 +42,11 @@ export const KEYBINDING_CONTEXT_TERMINAL_FIND_WIDGET_INPUT_NOT_FOCUSED: ContextK export const IS_WORKSPACE_SHELL_ALLOWED_STORAGE_KEY = 'terminal.integrated.isWorkspaceShellAllowed'; export const NEVER_MEASURE_RENDER_TIME_STORAGE_KEY = 'terminal.integrated.neverMeasureRenderTime'; +// The creation of extension host terminals is delayed by this value (milliseconds). The purpose of +// this delay is to allow the terminal instance to initialize correctly and have its ID set before +// trying to create the corressponding object on the ext host. +export const EXT_HOST_CREATION_DELAY = 100; + export const ITerminalNativeService = createDecorator('terminalNativeService'); export const TerminalCursorStyle = { @@ -276,7 +281,6 @@ export interface ITerminalProcessManager extends IDisposable { readonly os: OperatingSystem | undefined; readonly userHome: string | undefined; - readonly onProcessStateChange: Event; readonly onProcessReady: Event; readonly onBeforeProcessData: Event; readonly onProcessData: Event;