From 1b824ebdcac82d3e165597e4387cf9f9c7e6520d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=A4der?= Date: Wed, 14 Sep 2022 14:46:17 +0200 Subject: [PATCH] Extends multi-window support to Electron (#11642) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #11642 The main change is to prevent the secondary window from closing until the extracted widget is removed from the window. This includes waiting until any close handling (including dialogs) are finished. Contributed on behalf of ST Microelectronics. Signed-off-by: Thomas Mäder --- examples/electron/package.json | 1 + .../src/browser/secondary-window-handler.ts | 18 +++--- .../default-secondary-window-service.ts | 60 +++++++++++++------ .../window/secondary-window-service.ts | 5 +- .../electron-secondary-window-service.ts | 15 ++--- .../custom-editors/custom-editors-main.ts | 9 ++- 6 files changed, 69 insertions(+), 39 deletions(-) diff --git a/examples/electron/package.json b/examples/electron/package.json index cfa25c1a9892c..426e1713d582e 100644 --- a/examples/electron/package.json +++ b/examples/electron/package.json @@ -48,6 +48,7 @@ "@theia/scm": "1.29.0", "@theia/scm-extra": "1.29.0", "@theia/search-in-workspace": "1.29.0", + "@theia/secondary-window": "1.29.0", "@theia/task": "1.29.0", "@theia/terminal": "1.29.0", "@theia/timeline": "1.29.0", diff --git a/packages/core/src/browser/secondary-window-handler.ts b/packages/core/src/browser/secondary-window-handler.ts index 2211181a4308f..fdf36b8529106 100644 --- a/packages/core/src/browser/secondary-window-handler.ts +++ b/packages/core/src/browser/secondary-window-handler.ts @@ -131,13 +131,17 @@ export class SecondaryWindowHandler { return; } - const newWindow = this.secondaryWindowService.createSecondaryWindow(closed => { - this.applicationShell.closeWidget(widget.id); - const extIndex = this.secondaryWindows.indexOf(closed); - if (extIndex > -1) { - this.secondaryWindows.splice(extIndex, 1); - } - }); + const newWindow = this.secondaryWindowService.createSecondaryWindow( + async () => { + await this.applicationShell.closeWidget(widget.id); + return widget.isDisposed; + }, + closedWindow => { + const extIndex = this.secondaryWindows.indexOf(closedWindow); + if (extIndex > -1) { + this.secondaryWindows.splice(extIndex, 1); + } + }); if (!newWindow) { this.messageService.error('The widget could not be moved to a secondary window because the window creation failed. Please make sure to allow popups.'); diff --git a/packages/core/src/browser/window/default-secondary-window-service.ts b/packages/core/src/browser/window/default-secondary-window-service.ts index 643afe79d9741..40bfad4f795b8 100644 --- a/packages/core/src/browser/window/default-secondary-window-service.ts +++ b/packages/core/src/browser/window/default-secondary-window-service.ts @@ -17,8 +17,14 @@ import { inject, injectable, postConstruct } from 'inversify'; import { SecondaryWindowService } from './secondary-window-service'; import { WindowService } from './window-service'; +enum ClosingState { + initial, + inProgress, + readyToClose +} @injectable() export class DefaultSecondaryWindowService implements SecondaryWindowService { + // secondary-window.html is part of Theia's generated code. It is generated by dev-packages/application-manager/src/generator/frontend-generator.ts protected static SECONDARY_WINDOW_URL = 'secondary-window.html'; @@ -46,33 +52,51 @@ export class DefaultSecondaryWindowService implements SecondaryWindowService { }); } - createSecondaryWindow(onClose?: (closedWin: Window) => void): Window | undefined { - const win = this.doCreateSecondaryWindow(onClose); + createSecondaryWindow(onClosing?: (win: Window) => Promise, onClosed?: (closedWin: Window) => void): Window | undefined { + const id = this.nextWindowId(); + const win = this.doCreateSecondaryWindow(id); if (win) { this.secondaryWindows.push(win); - } - return win; - } - - protected doCreateSecondaryWindow(onClose?: (closedWin: Window) => void): Window | undefined { - const win = window.open(DefaultSecondaryWindowService.SECONDARY_WINDOW_URL, this.nextWindowId(), 'popup'); - if (win) { + let closingState = ClosingState.initial; // Add the unload listener after the dom content was loaded because otherwise the unload listener is called already on open in some browsers (e.g. Chrome). win.addEventListener('DOMContentLoaded', () => { - win.addEventListener('unload', () => { - this.handleWindowClosed(win, onClose); + win.addEventListener('closed', () => { + const extIndex = this.secondaryWindows.indexOf(win); + if (extIndex > -1) { + this.secondaryWindows.splice(extIndex, 1); + }; + }); + + win.addEventListener('beforeunload', event => { + if (closingState === ClosingState.initial) { + closingState = ClosingState.inProgress; + if (onClosing) { + onClosing(win).then(shouldClose => { + if (shouldClose) { + closingState = ClosingState.readyToClose; + win.close(); + } + }); + event.preventDefault(); + event.returnValue = false; + return false; + } + } else if (closingState === ClosingState.inProgress) { + // When the extracted widget is disposed programmatically, a dispose listener on it will try to close the window. + // if we dispose the widget because of closing the window, we'll get a recursive call to window.close() + event.preventDefault(); + event.returnValue = false; + return false; + } }); }); } - return win ?? undefined; + return win; } - protected handleWindowClosed(win: Window, onClose?: (closedWin: Window) => void): void { - const extIndex = this.secondaryWindows.indexOf(win); - if (extIndex > -1) { - this.secondaryWindows.splice(extIndex, 1); - }; - onClose?.(win); + protected doCreateSecondaryWindow(id: string): Window | undefined { + const win = window.open(DefaultSecondaryWindowService.SECONDARY_WINDOW_URL, id, 'popup'); + return win ?? undefined; } focus(win: Window): void { diff --git a/packages/core/src/browser/window/secondary-window-service.ts b/packages/core/src/browser/window/secondary-window-service.ts index 784e32bf974e2..c72cde8e3ad73 100644 --- a/packages/core/src/browser/window/secondary-window-service.ts +++ b/packages/core/src/browser/window/secondary-window-service.ts @@ -26,10 +26,11 @@ export interface SecondaryWindowService { * Creates a new secondary window for a widget to be extracted from the application shell. * The created window is closed automatically when the current theia instance is closed. * - * @param onClose optional callback that is invoked when the secondary window is closed + * @param onClosing optional callback to be executed before closing. If the promise returns true, the window will be closed. + * @param onClosed optional callback that is invoked when the secondary window is closed * @returns the created window or `undefined` if it could not be created */ - createSecondaryWindow(onClose?: (win: Window) => void): Window | undefined; + createSecondaryWindow(onClosing?: (win: Window) => Promise, onClosed?: (win: Window) => void): Window | undefined; /** Handles focussing the given secondary window in the browser and on Electron. */ focus(win: Window): void; diff --git a/packages/core/src/electron-browser/window/electron-secondary-window-service.ts b/packages/core/src/electron-browser/window/electron-secondary-window-service.ts index 987420e056fd3..9fb3e893fcc4b 100644 --- a/packages/core/src/electron-browser/window/electron-secondary-window-service.ts +++ b/packages/core/src/electron-browser/window/electron-secondary-window-service.ts @@ -21,25 +21,18 @@ import { DefaultSecondaryWindowService } from '../../browser/window/default-seco @injectable() export class ElectronSecondaryWindowService extends DefaultSecondaryWindowService { - protected electronWindows: Map = new Map(); - protected override doCreateSecondaryWindow(onClose?: (closedWin: Window) => void): Window | undefined { - const id = this.nextWindowId(); + private electronWindows: Map = new Map(); + + protected override doCreateSecondaryWindow(id: string): Window | undefined { electronRemote.getCurrentWindow().webContents.once('did-create-window', newElectronWindow => { newElectronWindow.setMenuBarVisibility(false); this.electronWindows.set(id, newElectronWindow); newElectronWindow.on('closed', () => { this.electronWindows.delete(id); - const browserWin = this.secondaryWindows.find(w => w.name === id); - if (browserWin) { - this.handleWindowClosed(browserWin, onClose); - } else { - console.warn(`Could not execute proper close handling for secondary window '${id}' because its frontend window could not be found.`); - }; }); }); - const win = window.open(DefaultSecondaryWindowService.SECONDARY_WINDOW_URL, id); - return win ?? undefined; + return super.doCreateSecondaryWindow(id); } override focus(win: Window): void { diff --git a/packages/plugin-ext/src/main/browser/custom-editors/custom-editors-main.ts b/packages/plugin-ext/src/main/browser/custom-editors/custom-editors-main.ts index c223f908521b2..baa732335acec 100644 --- a/packages/plugin-ext/src/main/browser/custom-editors/custom-editors-main.ts +++ b/packages/plugin-ext/src/main/browser/custom-editors/custom-editors-main.ts @@ -520,7 +520,6 @@ export class CustomTextEditorModel implements CustomEditorModel { private readonly toDispose = new DisposableCollection(); private readonly onDirtyChangedEmitter = new Emitter(); readonly onDirtyChanged = this.onDirtyChangedEmitter.event; - readonly autoSave: 'off' | 'afterDelay' | 'onFocusChange' | 'onWindowChange'; static async create( viewType: string, @@ -568,6 +567,14 @@ export class CustomTextEditorModel implements CustomEditorModel { return this.model.object; } + get autoSave(): 'off' | 'afterDelay' | 'onFocusChange' | 'onWindowChange' { + return this.editorTextModel.autoSave; + } + + get autoSaveDelay(): number { + return this.editorTextModel.autoSaveDelay; + } + revert(options?: Saveable.RevertOptions): Promise { return this.editorTextModel.revert(options); }