Skip to content

Commit 9aacac6

Browse files
authored
Aux window: Main window editor hangs or is slow on aux window minimize (fix #197420) (#197433)
* Aux window: Main window editor hangs or is slow on aux window minimize (fix #197420) * aux window - ensure a window id in all cases * aux window - 💄 * aux window - 💄 * aux window - 💄 * aux window - 💄 * aux window - 💄
1 parent 22354c5 commit 9aacac6

File tree

12 files changed

+120
-116
lines changed

12 files changed

+120
-116
lines changed

src/bootstrap-window.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
* },
4040
* canModifyDOM?: (config: ISandboxConfiguration) => void,
4141
* beforeLoaderConfig?: (loaderConfig: object) => void,
42-
* beforeRequire?: () => void
42+
* beforeRequire?: (config: ISandboxConfiguration) => void
4343
* }} [options]
4444
*/
4545
async function load(modulePaths, resultCallback, options) {
@@ -143,7 +143,7 @@
143143

144144
// Signal before require()
145145
if (typeof options?.beforeRequire === 'function') {
146-
options.beforeRequire();
146+
options.beforeRequire(configuration);
147147
}
148148

149149
// Actually require the main module as specified

src/vs/base/browser/dom.ts

Lines changed: 67 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -18,27 +18,36 @@ import * as platform from 'vs/base/common/platform';
1818
import { URI } from 'vs/base/common/uri';
1919
import { hash } from 'vs/base/common/hash';
2020

21-
type WindowGlobal = Window & typeof globalThis;
21+
export type CodeWindow = Window & typeof globalThis & {
22+
readonly vscodeWindowId: number;
23+
};
2224

23-
interface IWindow {
24-
readonly window: WindowGlobal;
25+
interface IRegisteredCodeWindow {
26+
readonly window: CodeWindow;
2527
readonly disposables: DisposableStore;
2628
}
2729

28-
export const { registerWindow, getWindows, getWindowsCount, onDidRegisterWindow, onWillUnregisterWindow, onDidUnregisterWindow } = (function () {
29-
const windows = new Map<WindowGlobal, IWindow>();
30-
windows.set(window, { window, disposables: new DisposableStore() });
30+
export const { registerWindow, getWindows, getWindowsCount, getWindowId, onDidRegisterWindow, onWillUnregisterWindow, onDidUnregisterWindow } = (function () {
31+
const windows = new Map<number, IRegisteredCodeWindow>();
32+
33+
const mainWindow = window as CodeWindow;
34+
if (typeof mainWindow.vscodeWindowId !== 'number') {
35+
Object.defineProperty(window, 'vscodeWindowId', {
36+
get: () => -1
37+
});
38+
}
39+
windows.set(mainWindow.vscodeWindowId, { window: mainWindow, disposables: new DisposableStore() });
3140

32-
const onDidRegisterWindow = new event.Emitter<IWindow>();
33-
const onDidUnregisterWindow = new event.Emitter<WindowGlobal>();
34-
const onWillUnregisterWindow = new event.Emitter<WindowGlobal>();
41+
const onDidRegisterWindow = new event.Emitter<IRegisteredCodeWindow>();
42+
const onDidUnregisterWindow = new event.Emitter<CodeWindow>();
43+
const onWillUnregisterWindow = new event.Emitter<CodeWindow>();
3544

3645
return {
3746
onDidRegisterWindow: onDidRegisterWindow.event,
3847
onWillUnregisterWindow: onWillUnregisterWindow.event,
3948
onDidUnregisterWindow: onDidUnregisterWindow.event,
40-
registerWindow(window: WindowGlobal): IDisposable {
41-
if (windows.has(window)) {
49+
registerWindow(window: CodeWindow): IDisposable {
50+
if (windows.has(window.vscodeWindowId)) {
4251
return Disposable.None;
4352
}
4453

@@ -48,10 +57,10 @@ export const { registerWindow, getWindows, getWindowsCount, onDidRegisterWindow,
4857
window,
4958
disposables: disposables.add(new DisposableStore())
5059
};
51-
windows.set(window, registeredWindow);
60+
windows.set(window.vscodeWindowId, registeredWindow);
5261

5362
disposables.add(toDisposable(() => {
54-
windows.delete(window);
63+
windows.delete(window.vscodeWindowId);
5564
onDidUnregisterWindow.fire(window);
5665
}));
5766

@@ -63,11 +72,14 @@ export const { registerWindow, getWindows, getWindowsCount, onDidRegisterWindow,
6372

6473
return disposables;
6574
},
66-
getWindows(): Iterable<IWindow> {
75+
getWindows(): Iterable<IRegisteredCodeWindow> {
6776
return windows.values();
6877
},
6978
getWindowsCount(): number {
7079
return windows.size;
80+
},
81+
getWindowId(targetWindow: Window): number {
82+
return (targetWindow as CodeWindow).vscodeWindowId;
7183
}
7284
};
7385
})();
@@ -223,51 +235,65 @@ class AnimationFrameQueueItem implements IDisposable {
223235
/**
224236
* The runners scheduled at the next animation frame
225237
*/
226-
let NEXT_QUEUE: AnimationFrameQueueItem[] = [];
238+
const NEXT_QUEUE = new Map<number /* window ID */, AnimationFrameQueueItem[]>();
227239
/**
228240
* The runners scheduled at the current animation frame
229241
*/
230-
let CURRENT_QUEUE: AnimationFrameQueueItem[] | null = null;
242+
const CURRENT_QUEUE = new Map<number /* window ID */, AnimationFrameQueueItem[]>();
231243
/**
232244
* A flag to keep track if the native requestAnimationFrame was already called
233245
*/
234-
let animFrameRequested = false;
246+
const animFrameRequested = new Map<number /* window ID */, boolean>();
235247
/**
236248
* A flag to indicate if currently handling a native requestAnimationFrame callback
237249
*/
238-
let inAnimationFrameRunner = false;
250+
const inAnimationFrameRunner = new Map<number /* window ID */, boolean>();
239251

240-
const animationFrameRunner = () => {
241-
animFrameRequested = false;
252+
const animationFrameRunner = (targetWindowId: number) => {
253+
animFrameRequested.set(targetWindowId, false);
242254

243-
CURRENT_QUEUE = NEXT_QUEUE;
244-
NEXT_QUEUE = [];
255+
const currentQueue = NEXT_QUEUE.get(targetWindowId) ?? [];
256+
CURRENT_QUEUE.set(targetWindowId, currentQueue);
257+
NEXT_QUEUE.set(targetWindowId, []);
245258

246-
inAnimationFrameRunner = true;
247-
while (CURRENT_QUEUE.length > 0) {
248-
CURRENT_QUEUE.sort(AnimationFrameQueueItem.sort);
249-
const top = CURRENT_QUEUE.shift()!;
259+
inAnimationFrameRunner.set(targetWindowId, true);
260+
while (currentQueue.length > 0) {
261+
currentQueue.sort(AnimationFrameQueueItem.sort);
262+
const top = currentQueue.shift()!;
250263
top.execute();
251264
}
252-
inAnimationFrameRunner = false;
265+
inAnimationFrameRunner.set(targetWindowId, false);
253266
};
254267

255268
scheduleAtNextAnimationFrame = (runner: () => void, targetWindow: Window, priority: number = 0) => {
269+
const targetWindowId = getWindowId(targetWindow);
256270
const item = new AnimationFrameQueueItem(runner, priority);
257-
NEXT_QUEUE.push(item);
258271

259-
if (!animFrameRequested) {
260-
animFrameRequested = true;
261-
targetWindow.requestAnimationFrame(animationFrameRunner);
272+
let nextQueue = NEXT_QUEUE.get(targetWindowId);
273+
if (!nextQueue) {
274+
nextQueue = [];
275+
NEXT_QUEUE.set(targetWindowId, nextQueue);
276+
}
277+
nextQueue.push(item);
278+
279+
if (!animFrameRequested.get(targetWindowId)) {
280+
animFrameRequested.set(targetWindowId, true);
281+
targetWindow.requestAnimationFrame(() => animationFrameRunner(targetWindowId));
262282
}
263283

264284
return item;
265285
};
266286

267287
runAtThisOrScheduleAtNextAnimationFrame = (runner: () => void, targetWindow: Window, priority?: number) => {
268-
if (inAnimationFrameRunner) {
288+
const targetWindowId = getWindowId(targetWindow);
289+
if (inAnimationFrameRunner.get(targetWindowId)) {
269290
const item = new AnimationFrameQueueItem(runner, priority);
270-
CURRENT_QUEUE!.push(item);
291+
let currentQueue = CURRENT_QUEUE.get(targetWindowId);
292+
if (!currentQueue) {
293+
currentQueue = [];
294+
CURRENT_QUEUE.set(targetWindowId, currentQueue);
295+
}
296+
currentQueue.push(item);
271297
return item;
272298
} else {
273299
return scheduleAtNextAnimationFrame(runner, targetWindow, priority);
@@ -775,25 +801,25 @@ export function getActiveDocument(): Document {
775801
return documents.find(document => document.hasFocus()) ?? document;
776802
}
777803

778-
export function getActiveWindow(): WindowGlobal {
804+
export function getActiveWindow(): CodeWindow {
779805
const document = getActiveDocument();
780-
return document.defaultView?.window ?? window;
806+
return (document.defaultView?.window ?? window) as CodeWindow;
781807
}
782808

783-
export function getWindow(element: Node | undefined | null): WindowGlobal;
784-
export function getWindow(event: UIEvent | undefined | null): WindowGlobal;
785-
export function getWindow(e: unknown): WindowGlobal {
809+
export function getWindow(element: Node | undefined | null): CodeWindow;
810+
export function getWindow(event: UIEvent | undefined | null): CodeWindow;
811+
export function getWindow(e: unknown): CodeWindow {
786812
const candidateNode = e as Node | undefined | null;
787813
if (candidateNode?.ownerDocument?.defaultView) {
788-
return candidateNode.ownerDocument.defaultView.window;
814+
return candidateNode.ownerDocument.defaultView.window as CodeWindow;
789815
}
790816

791817
const candidateEvent = e as UIEvent | undefined | null;
792818
if (candidateEvent?.view) {
793-
return candidateEvent.view.window;
819+
return candidateEvent.view.window as CodeWindow;
794820
}
795821

796-
return window;
822+
return window as CodeWindow;
797823
}
798824

799825
export function focusWindow(element: Node): void {

src/vs/code/electron-sandbox/workbench/workbench.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,14 @@
4747
beforeLoaderConfig: function (loaderConfig) {
4848
loaderConfig.recordStats = true;
4949
},
50-
beforeRequire: function () {
50+
beforeRequire: function (windowConfig) {
5151
performance.mark('code/willLoadWorkbenchMain');
5252

53+
// Code windows have a `vscodeWindowId` property to identify them
54+
Object.defineProperty(window, 'vscodeWindowId', {
55+
get: () => windowConfig.windowId
56+
});
57+
5358
// It looks like browsers only lazily enable
5459
// the <canvas> element when needed. Since we
5560
// leverage canvas elements in our code in many
@@ -72,6 +77,7 @@
7277
/**
7378
* @typedef {import('../../../platform/window/common/window').INativeWindowConfiguration} INativeWindowConfiguration
7479
* @typedef {import('../../../platform/environment/common/argv').NativeParsedArgs} NativeParsedArgs
80+
* @typedef {import('../../../base/parts/sandbox/common/sandboxTypes').ISandboxConfiguration} ISandboxConfiguration
7581
*
7682
* @returns {{
7783
* load: (
@@ -86,7 +92,7 @@
8692
* },
8793
* canModifyDOM?: (config: INativeWindowConfiguration & NativeParsedArgs) => void,
8894
* beforeLoaderConfig?: (loaderConfig: object) => void,
89-
* beforeRequire?: () => void
95+
* beforeRequire?: (config: ISandboxConfiguration) => void
9096
* }
9197
* ) => Promise<unknown>
9298
* }}

src/vs/editor/browser/widget/diffEditor/utils.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ export function animatedObservable(targetWindow: Window, base: IObservable<numbe
144144
}, (reader, s) => {
145145
/** @description update value */
146146
if (animationFrame !== undefined) {
147-
cancelAnimationFrame(animationFrame);
147+
targetWindow.cancelAnimationFrame(animationFrame);
148148
animationFrame = undefined;
149149
}
150150

src/vs/workbench/browser/layout.ts

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,12 @@ import { IBannerService } from 'vs/workbench/services/banner/browser/bannerServi
4747
import { IPaneCompositePartService } from 'vs/workbench/services/panecomposite/browser/panecomposite';
4848
import { AuxiliaryBarPart } from 'vs/workbench/browser/parts/auxiliarybar/auxiliaryBarPart';
4949
import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
50-
import { IAuxiliaryWindowService, isAuxiliaryWindow } from 'vs/workbench/services/auxiliaryWindow/browser/auxiliaryWindowService';
50+
import { IAuxiliaryWindowService } from 'vs/workbench/services/auxiliaryWindow/browser/auxiliaryWindowService';
5151

5252
//#region Layout Implementation
5353

5454
interface ILayoutRuntimeState {
55-
activeContainerId: 'main' | number /* window ID */;
55+
activeContainerId: number;
5656
fullscreen: boolean;
5757
maximized: boolean;
5858
hasFocus: boolean;
@@ -466,16 +466,10 @@ export abstract class Layout extends Disposable implements IWorkbenchLayoutServi
466466
}
467467
}
468468

469-
private getActiveContainerId(): 'main' | number {
469+
private getActiveContainerId(): number {
470470
const activeContainer = this.activeContainer;
471-
if (activeContainer !== this.container) {
472-
const containerWindow = getWindow(activeContainer);
473-
if (isAuxiliaryWindow(containerWindow)) {
474-
return containerWindow.vscodeWindowId;
475-
}
476-
}
477471

478-
return 'main';
472+
return getWindow(activeContainer).vscodeWindowId;
479473
}
480474

481475
private doUpdateLayoutConfiguration(skipLayout?: boolean): void {

src/vs/workbench/contrib/terminal/electron-sandbox/localTerminalBackend.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import * as terminalEnvironment from 'vs/workbench/contrib/terminal/common/termi
2626
import { IProductService } from 'vs/platform/product/common/productService';
2727
import { IEnvironmentVariableService } from 'vs/workbench/contrib/terminal/common/environmentVariable';
2828
import { BaseTerminalBackend } from 'vs/workbench/contrib/terminal/browser/baseTerminalBackend';
29-
import { INativeWorkbenchEnvironmentService } from 'vs/workbench/services/environment/electron-sandbox/environmentService';
29+
import { INativeHostService } from 'vs/platform/native/common/native';
3030
import { Client as MessagePortClient } from 'vs/base/parts/ipc/common/ipc.mp';
3131
import { acquirePort } from 'vs/base/parts/ipc/electron-sandbox/ipc.mp';
3232
import { getDelayedChannel, ProxyChannel } from 'vs/base/parts/ipc/common/ipc';
@@ -85,7 +85,7 @@ class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBacke
8585
@ITerminalProfileResolverService private readonly _terminalProfileResolverService: ITerminalProfileResolverService,
8686
@IEnvironmentVariableService private readonly _environmentVariableService: IEnvironmentVariableService,
8787
@IHistoryService historyService: IHistoryService,
88-
@INativeWorkbenchEnvironmentService private readonly _environmentService: INativeWorkbenchEnvironmentService,
88+
@INativeHostService private readonly _nativeHostService: INativeHostService,
8989
@IStatusbarService statusBarService: IStatusbarService,
9090
@IRemoteAgentService private readonly _remoteAgentService: IRemoteAgentService,
9191
) {
@@ -129,7 +129,7 @@ class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBacke
129129
// _localPtyService, and one directly via message port _ptyHostDirectProxy. The former is
130130
// used for pty host management messages, it would make sense in the future to use a
131131
// separate interface/service for this one.
132-
const client = new MessagePortClient(port, `window:${this._environmentService.window.id}`);
132+
const client = new MessagePortClient(port, `window:${this._nativeHostService.windowId}`);
133133
directProxyClientEventually.complete(client);
134134
this._onPtyHostConnected.fire();
135135

src/vs/workbench/electron-sandbox/actions/windowActions.ts

Lines changed: 3 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@ import { isMacintosh } from 'vs/base/common/platform';
2929
import { IEditorService } from 'vs/workbench/services/editor/common/editorService';
3030
import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editorGroupsService';
3131
import { getActiveWindow } from 'vs/base/browser/dom';
32-
import { isAuxiliaryWindow } from 'vs/workbench/services/auxiliaryWindow/browser/auxiliaryWindowService';
33-
import { INativeWorkbenchEnvironmentService } from 'vs/workbench/services/environment/electron-sandbox/environmentService';
3432
import { IOpenedAuxiliaryWindow, IOpenedMainWindow, isOpenedAuxiliaryWindow } from 'vs/platform/window/common/window';
3533

3634
export class CloseWindowAction extends Action2 {
@@ -63,12 +61,7 @@ export class CloseWindowAction extends Action2 {
6361
override async run(accessor: ServicesAccessor): Promise<void> {
6462
const nativeHostService = accessor.get(INativeHostService);
6563

66-
const window = getActiveWindow();
67-
if (isAuxiliaryWindow(window)) {
68-
return nativeHostService.closeWindowById(window.vscodeWindowId);
69-
}
70-
71-
return nativeHostService.closeWindow();
64+
return nativeHostService.closeWindowById(getActiveWindow().vscodeWindowId);
7265
}
7366
}
7467

@@ -217,13 +210,7 @@ abstract class BaseSwitchWindow extends Action2 {
217210
const languageService = accessor.get(ILanguageService);
218211
const nativeHostService = accessor.get(INativeHostService);
219212

220-
let currentWindowId: number;
221-
const activeWindow = getActiveWindow();
222-
if (isAuxiliaryWindow(activeWindow)) {
223-
currentWindowId = activeWindow.vscodeWindowId;
224-
} else {
225-
currentWindowId = nativeHostService.windowId;
226-
}
213+
const currentWindowId = getActiveWindow().vscodeWindowId;
227214

228215
const windows = await nativeHostService.getWindows({ includeAuxiliaryWindows: true });
229216

@@ -411,15 +398,8 @@ export class ExperimentalSplitWindowAction extends Action2 {
411398
const editorService = accessor.get(IEditorService);
412399
const editorGroupService = accessor.get(IEditorGroupsService);
413400
const nativeHostService = accessor.get(INativeHostService);
414-
const environmentService = accessor.get(INativeWorkbenchEnvironmentService);
415401

416-
let activeWindowId: number;
417402
const activeWindow = getActiveWindow();
418-
if (isAuxiliaryWindow(activeWindow)) {
419-
activeWindowId = activeWindow.vscodeWindowId;
420-
} else {
421-
activeWindowId = environmentService.window.id;
422-
}
423403

424404
// First position the active window which may involve
425405
// leaving fullscreen mode and then split it.
@@ -428,7 +408,7 @@ export class ExperimentalSplitWindowAction extends Action2 {
428408
y: 0,
429409
width: activeWindow.screen.availWidth / 2,
430410
height: activeWindow.screen.availHeight
431-
}, { targetWindowId: activeWindowId });
411+
}, { targetWindowId: activeWindow.vscodeWindowId });
432412

433413
// Then create a new window next to the active window
434414
const auxiliaryEditorPart = await editorGroupService.createAuxiliaryEditorPart({

0 commit comments

Comments
 (0)