From 7bc7686565ccc4e4e871614fc0a61d0379707ba7 Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Mon, 4 Dec 2023 10:33:03 -0800 Subject: [PATCH] aux window: add barrier waiting for styles to load (#199882) * wip * aux window: add barrier waiting for styles to load The debug toolbar needs to compute its size and positioning, which without this fails if executed in onDidChangeActiveContainer. The result of these changes is a new property on the ILayoutService. * address comments * address comments --------- Co-authored-by: Benjamin Pasero --- src/vs/base/common/async.ts | 1 - .../quickInput/standaloneQuickInputService.ts | 1 + .../browser/standaloneLayoutService.ts | 7 +-- .../platform/layout/browser/layoutService.ts | 7 +++ src/vs/workbench/browser/layout.ts | 10 +++- .../browser/auxiliaryWindowService.ts | 48 +++++++++++++++---- .../auxiliaryWindowService.ts | 10 ++-- .../test/browser/workbenchTestServices.ts | 1 + 8 files changed, 66 insertions(+), 19 deletions(-) diff --git a/src/vs/base/common/async.ts b/src/vs/base/common/async.ts index fe7b653a580..abc2aed919a 100644 --- a/src/vs/base/common/async.ts +++ b/src/vs/base/common/async.ts @@ -426,7 +426,6 @@ export class ThrottledDelayer { * A barrier that is initially closed and then becomes opened permanently. */ export class Barrier { - private _isOpen: boolean; private _promise: Promise; private _completePromise!: (v: boolean) => void; diff --git a/src/vs/editor/standalone/browser/quickInput/standaloneQuickInputService.ts b/src/vs/editor/standalone/browser/quickInput/standaloneQuickInputService.ts index 8fbc1ec6300..9897d433286 100644 --- a/src/vs/editor/standalone/browser/quickInput/standaloneQuickInputService.ts +++ b/src/vs/editor/standalone/browser/quickInput/standaloneQuickInputService.ts @@ -50,6 +50,7 @@ class EditorScopedQuickInputService extends QuickInputService { get onDidLayoutContainer() { return Event.map(editor.onDidLayoutChange, dimension => ({ container: widget.getDomNode(), dimension })); }, get onDidChangeActiveContainer() { return Event.None; }, get onDidAddContainer() { return Event.None; }, + get whenActiveContainerStylesLoaded() { return Promise.resolve(); }, get mainContainerOffset() { return { top: 0, quickPickTop: 0 }; }, get activeContainerOffset() { return { top: 0, quickPickTop: 0 }; }, focus: () => editor.focus() diff --git a/src/vs/editor/standalone/browser/standaloneLayoutService.ts b/src/vs/editor/standalone/browser/standaloneLayoutService.ts index c928577670f..cc946b22b97 100644 --- a/src/vs/editor/standalone/browser/standaloneLayoutService.ts +++ b/src/vs/editor/standalone/browser/standaloneLayoutService.ts @@ -4,12 +4,12 @@ *--------------------------------------------------------------------------------------------*/ import * as dom from 'vs/base/browser/dom'; +import { mainWindow } from 'vs/base/browser/window'; +import { coalesce, firstOrDefault } from 'vs/base/common/arrays'; import { Event } from 'vs/base/common/event'; -import { ILayoutService, ILayoutOffsetInfo } from 'vs/platform/layout/browser/layoutService'; import { ICodeEditorService } from 'vs/editor/browser/services/codeEditorService'; import { InstantiationType, registerSingleton } from 'vs/platform/instantiation/common/extensions'; -import { coalesce, firstOrDefault } from 'vs/base/common/arrays'; -import { mainWindow } from 'vs/base/browser/window'; +import { ILayoutOffsetInfo, ILayoutService } from 'vs/platform/layout/browser/layoutService'; class StandaloneLayoutService implements ILayoutService { declare readonly _serviceBrand: undefined; @@ -19,6 +19,7 @@ class StandaloneLayoutService implements ILayoutService { readonly onDidLayoutContainer = Event.None; readonly onDidChangeActiveContainer = Event.None; readonly onDidAddContainer = Event.None; + readonly whenActiveContainerStylesLoaded = Promise.resolve(); get mainContainer(): HTMLElement { return firstOrDefault(this._codeEditorService.listCodeEditors())?.getContainerDomNode() ?? mainWindow.document.body; diff --git a/src/vs/platform/layout/browser/layoutService.ts b/src/vs/platform/layout/browser/layoutService.ts index 3f9b354ae22..6fad206c73e 100644 --- a/src/vs/platform/layout/browser/layoutService.ts +++ b/src/vs/platform/layout/browser/layoutService.ts @@ -94,6 +94,13 @@ export interface ILayoutService { */ readonly activeContainerOffset: ILayoutOffsetInfo; + /** + * A promise resolved when the stylesheets for the active container have been + * loaded. Aux windows load their styles asynchronously, so there may be + * an initial delay before resolution happens. + */ + readonly whenActiveContainerStylesLoaded: Promise; + /** * Focus the primary component of the active container. */ diff --git a/src/vs/workbench/browser/layout.ts b/src/vs/workbench/browser/layout.ts index 5ba8b083b88..4d6849924e4 100644 --- a/src/vs/workbench/browser/layout.ts +++ b/src/vs/workbench/browser/layout.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { Disposable, DisposableStore } from 'vs/base/common/lifecycle'; +import { Disposable, DisposableStore, toDisposable } from 'vs/base/common/lifecycle'; import { Event, Emitter } from 'vs/base/common/event'; import { EventType, addDisposableListener, getClientArea, position, size, IDimension, isAncestorUsingFlowTo, computeScreenAwareSize, getActiveDocument, getWindows, getActiveWindow, focusWindow, isActiveDocument, getWindow, getWindowId, getActiveElement } from 'vs/base/browser/dom'; import { onDidChangeFullscreen, isFullscreen, isWCOEnabled } from 'vs/base/browser/browser'; @@ -212,6 +212,11 @@ export abstract class Layout extends Disposable implements IWorkbenchLayoutServi return this.computeContainerOffset(getWindow(this.activeContainer)); } + get whenActiveContainerStylesLoaded() { + const active = this.activeContainer; + return this.auxWindowStylesLoaded.get(active) || Promise.resolve(); + } + private computeContainerOffset(targetWindow: Window) { let top = 0; let quickPickTop = 0; @@ -240,6 +245,7 @@ export abstract class Layout extends Disposable implements IWorkbenchLayoutServi //#endregion private readonly parts = new Map(); + private readonly auxWindowStylesLoaded = new Map>(); private initialized = false; private workbenchGrid!: SerializableGrid; @@ -382,9 +388,11 @@ export abstract class Layout extends Disposable implements IWorkbenchLayoutServi // Auxiliary windows this._register(this.auxiliaryWindowService.onDidOpenAuxiliaryWindow(({ window, disposables }) => { const eventDisposables = disposables.add(new DisposableStore()); + this.auxWindowStylesLoaded.set(window.container, window.whenStylesHaveLoaded); this._onDidAddContainer.fire({ container: window.container, disposables: eventDisposables }); disposables.add(window.onDidLayout(dimension => this.handleContainerDidLayout(window.container, dimension))); + disposables.add(toDisposable(() => this.auxWindowStylesLoaded.delete(window.container))); })); } diff --git a/src/vs/workbench/services/auxiliaryWindow/browser/auxiliaryWindowService.ts b/src/vs/workbench/services/auxiliaryWindow/browser/auxiliaryWindowService.ts index a4639125b9b..fe1e0c19c90 100644 --- a/src/vs/workbench/services/auxiliaryWindow/browser/auxiliaryWindowService.ts +++ b/src/vs/workbench/services/auxiliaryWindow/browser/auxiliaryWindowService.ts @@ -20,6 +20,7 @@ import Severity from 'vs/base/common/severity'; import { BaseWindow } from 'vs/workbench/browser/window'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; +import { Barrier } from 'vs/base/common/async'; export const IAuxiliaryWindowService = createDecorator('auxiliaryWindowService'); @@ -45,6 +46,7 @@ export interface IAuxiliaryWindow extends IDisposable { readonly onDidLayout: Event; readonly onWillClose: Event; + readonly whenStylesHaveLoaded: Promise; readonly window: CodeWindow; readonly container: HTMLElement; @@ -63,13 +65,17 @@ export class AuxiliaryWindow extends BaseWindow implements IAuxiliaryWindow { private readonly _onWillDispose = this._register(new Emitter()); readonly onWillDispose = this._onWillDispose.event; + readonly whenStylesHaveLoaded: Promise; + constructor( readonly window: CodeWindow, readonly container: HTMLElement, + stylesHaveLoaded: Barrier, @IConfigurationService private readonly configurationService: IConfigurationService, ) { super(window); + this.whenStylesHaveLoaded = stylesHaveLoaded.wait().then(() => { }); this.registerListeners(); } @@ -165,9 +171,9 @@ export class BrowserAuxiliaryWindowService extends Disposable implements IAuxili ensureCodeWindow(targetWindow, resolvedWindowId); const containerDisposables = new DisposableStore(); - const container = this.createContainer(targetWindow, containerDisposables); + const { container, stylesLoaded } = this.createContainer(targetWindow, containerDisposables); - const auxiliaryWindow = this.createAuxiliaryWindow(targetWindow, container); + const auxiliaryWindow = this.createAuxiliaryWindow(targetWindow, container, stylesLoaded); const registryDisposables = new DisposableStore(); this.windows.set(targetWindow.vscodeWindowId, auxiliaryWindow); @@ -201,8 +207,8 @@ export class BrowserAuxiliaryWindowService extends Disposable implements IAuxili return auxiliaryWindow; } - protected createAuxiliaryWindow(targetWindow: CodeWindow, container: HTMLElement): AuxiliaryWindow { - return new AuxiliaryWindow(targetWindow, container, this.configurationService); + protected createAuxiliaryWindow(targetWindow: CodeWindow, container: HTMLElement, stylesLoaded: Barrier): AuxiliaryWindow { + return new AuxiliaryWindow(targetWindow, container, stylesLoaded, this.configurationService); } private async openWindow(options?: IAuxiliaryWindowOpenOptions): Promise { @@ -257,13 +263,13 @@ export class BrowserAuxiliaryWindowService extends Disposable implements IAuxili return BrowserAuxiliaryWindowService.WINDOW_IDS++; } - protected createContainer(auxiliaryWindow: CodeWindow, disposables: DisposableStore): HTMLElement { + protected createContainer(auxiliaryWindow: CodeWindow, disposables: DisposableStore): { stylesLoaded: Barrier; container: HTMLElement } { this.patchMethods(auxiliaryWindow); this.applyMeta(auxiliaryWindow); - this.applyCSS(auxiliaryWindow, disposables); - - return this.applyHTML(auxiliaryWindow, disposables); + const { stylesLoaded } = this.applyCSS(auxiliaryWindow, disposables); + const container = this.applyHTML(auxiliaryWindow, disposables); + return { stylesLoaded, container }; } protected patchMethods(auxiliaryWindow: CodeWindow): void { @@ -292,24 +298,44 @@ export class BrowserAuxiliaryWindowService extends Disposable implements IAuxili } } - protected applyCSS(auxiliaryWindow: CodeWindow, disposables: DisposableStore): void { + protected applyCSS(auxiliaryWindow: CodeWindow, disposables: DisposableStore) { mark('code/auxiliaryWindow/willApplyCSS'); const mapOriginalToClone = new Map(); - function cloneNode(originalNode: Node): void { + const stylesLoaded = new Barrier(); + stylesLoaded.wait().then(() => mark('code/auxiliaryWindow/didLoadCSSStyles')); + + let pendingLinkSettles = 0; + function onLinkSettled(_event?: globalThis.Event) { + // network errors from loading stylesheets will be written to the console + // already, we probably don't need to log them manually. + if (!--pendingLinkSettles) { + stylesLoaded.open(); + } + } + + function cloneNode(originalNode: Element): void { if (isGlobalStylesheet(originalNode)) { return; // global stylesheets are handled by `cloneGlobalStylesheets` below } const clonedNode = auxiliaryWindow.document.head.appendChild(originalNode.cloneNode(true)); + if (originalNode.tagName === 'LINK') { + pendingLinkSettles++; + disposables.add(addDisposableListener(clonedNode, 'load', onLinkSettled)); + disposables.add(addDisposableListener(clonedNode, 'error', onLinkSettled)); + } + mapOriginalToClone.set(originalNode, clonedNode); } // Clone all style elements and stylesheet links from the window to the child window + pendingLinkSettles++; // outer increment handles cases where there's nothing to load, and ensures it can't settle prematurely for (const originalNode of mainWindow.document.head.querySelectorAll('link[rel="stylesheet"], style')) { cloneNode(originalNode); } + onLinkSettled(); // Global stylesheets in are cloned in a special way because the mutation // observer is not firing for changes done via `style.sheet` API. Only text changes @@ -356,6 +382,8 @@ export class BrowserAuxiliaryWindowService extends Disposable implements IAuxili })); mark('code/auxiliaryWindow/didApplyCSS'); + + return { stylesLoaded }; } private applyHTML(auxiliaryWindow: CodeWindow, disposables: DisposableStore): HTMLElement { diff --git a/src/vs/workbench/services/auxiliaryWindow/electron-sandbox/auxiliaryWindowService.ts b/src/vs/workbench/services/auxiliaryWindow/electron-sandbox/auxiliaryWindowService.ts index 96b3075ad95..10251fbb2c9 100644 --- a/src/vs/workbench/services/auxiliaryWindow/electron-sandbox/auxiliaryWindowService.ts +++ b/src/vs/workbench/services/auxiliaryWindow/electron-sandbox/auxiliaryWindowService.ts @@ -18,6 +18,7 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti import { NativeWindow } from 'vs/workbench/electron-sandbox/window'; import { ShutdownReason } from 'vs/workbench/services/lifecycle/common/lifecycle'; import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; +import { Barrier } from 'vs/base/common/async'; type NativeCodeWindow = CodeWindow & { readonly vscode: ISandboxGlobals; @@ -30,11 +31,12 @@ export class NativeAuxiliaryWindow extends AuxiliaryWindow { constructor( window: CodeWindow, container: HTMLElement, + stylesHaveLoaded: Barrier, @IConfigurationService configurationService: IConfigurationService, @INativeHostService private readonly nativeHostService: INativeHostService, @IInstantiationService private readonly instantiationService: IInstantiationService ) { - super(window, container, configurationService); + super(window, container, stylesHaveLoaded, configurationService); } protected override async confirmBeforeClose(e: BeforeUnloadEvent): Promise { @@ -73,7 +75,7 @@ export class NativeAuxiliaryWindowService extends BrowserAuxiliaryWindowService return windowId; } - protected override createContainer(auxiliaryWindow: NativeCodeWindow, disposables: DisposableStore): HTMLElement { + protected override createContainer(auxiliaryWindow: NativeCodeWindow, disposables: DisposableStore) { // Zoom level const windowConfig = this.configurationService.getValue(); @@ -100,8 +102,8 @@ export class NativeAuxiliaryWindowService extends BrowserAuxiliaryWindowService }; } - protected override createAuxiliaryWindow(targetWindow: CodeWindow, container: HTMLElement): AuxiliaryWindow { - return new NativeAuxiliaryWindow(targetWindow, container, this.configurationService, this.nativeHostService, this.instantiationService); + protected override createAuxiliaryWindow(targetWindow: CodeWindow, container: HTMLElement, stylesHaveLoaded: Barrier,): AuxiliaryWindow { + return new NativeAuxiliaryWindow(targetWindow, container, stylesHaveLoaded, this.configurationService, this.nativeHostService, this.instantiationService); } } diff --git a/src/vs/workbench/test/browser/workbenchTestServices.ts b/src/vs/workbench/test/browser/workbenchTestServices.ts index c3d93488f4d..ca32d3629f8 100644 --- a/src/vs/workbench/test/browser/workbenchTestServices.ts +++ b/src/vs/workbench/test/browser/workbenchTestServices.ts @@ -593,6 +593,7 @@ export class TestLayoutService implements IWorkbenchLayoutService { activeContainerDimension: IDimension = { width: 800, height: 600 }; mainContainerOffset: ILayoutOffsetInfo = { top: 0, quickPickTop: 0 }; activeContainerOffset: ILayoutOffsetInfo = { top: 0, quickPickTop: 0 }; + whenActiveContainerStylesLoaded = Promise.resolve(); mainContainer: HTMLElement = mainWindow.document.body; containers = [mainWindow.document.body];