From eeae98789371a24481a0690f116137b3e098206e Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Sat, 7 Nov 2020 18:29:30 +0100 Subject: [PATCH] GH-8188: Workaround for `application.confirmExit` From now on, the browser and electron frontends listen on different events when it comes to saving the app state and releasing the resources. The browser behavior is unchanged. Made the `will-prevent-unload` handling sync. Signed-off-by: Akos Kitta --- packages/core/src/browser/frontend-application.ts | 7 +++++-- .../src/browser/window/default-window-service.ts | 11 +++++++++++ packages/core/src/browser/window/window-service.ts | 8 ++++++++ .../window/electron-window-service.ts | 14 ++++++++++++++ .../src/electron-main/electron-main-application.ts | 4 ++-- 5 files changed, 40 insertions(+), 4 deletions(-) diff --git a/packages/core/src/browser/frontend-application.ts b/packages/core/src/browser/frontend-application.ts index 25997c737a692..1d7c6a1ae870b 100644 --- a/packages/core/src/browser/frontend-application.ts +++ b/packages/core/src/browser/frontend-application.ts @@ -24,6 +24,7 @@ import { ShellLayoutRestorer, ApplicationShellLayoutMigrationError } from './she import { FrontendApplicationStateService } from './frontend-application-state'; import { preventNavigation, parseCssTime, animationFrame } from './browser'; import { CorePreferences } from './core-preferences'; +import { WindowService } from './window/window-service'; /** * Clients can implement to get a callback for contributing widgets to a shell on start. @@ -96,6 +97,9 @@ export class FrontendApplication { @inject(CorePreferences) protected readonly corePreferences: CorePreferences; + @inject(WindowService) + protected readonly windowsService: WindowService; + constructor( @inject(CommandRegistry) protected readonly commands: CommandRegistry, @inject(MenuModelRegistry) protected readonly menus: MenuModelRegistry, @@ -182,8 +186,7 @@ export class FrontendApplication { */ protected registerEventListeners(): void { this.registerCompositionEventListeners(); /* Hotfix. See above. */ - - window.addEventListener('beforeunload', () => { + this.windowsService.onUnload(() => { this.stateService.state = 'closing_window'; this.layoutRestorer.storeLayout(this); this.stopContributions(); diff --git a/packages/core/src/browser/window/default-window-service.ts b/packages/core/src/browser/window/default-window-service.ts index a2948fb7a351d..548a4af22b6d5 100644 --- a/packages/core/src/browser/window/default-window-service.ts +++ b/packages/core/src/browser/window/default-window-service.ts @@ -15,6 +15,7 @@ ********************************************************************************/ import { inject, injectable, named } from 'inversify'; +import { Event, Emitter } from '../../common'; import { CorePreferences } from '../core-preferences'; import { ContributionProvider } from '../../common/contribution-provider'; import { FrontendApplicationContribution, FrontendApplication } from '../frontend-application'; @@ -24,6 +25,7 @@ import { WindowService } from './window-service'; export class DefaultWindowService implements WindowService, FrontendApplicationContribution { protected frontendApplication: FrontendApplication; + protected onUnloadEmitter = new Emitter(); @inject(CorePreferences) protected readonly corePreferences: CorePreferences; @@ -39,6 +41,7 @@ export class DefaultWindowService implements WindowService, FrontendApplicationC return this.preventUnload(event); } }); + this.registerUnloadListener(); } openNewWindow(url: string): undefined { @@ -61,6 +64,14 @@ export class DefaultWindowService implements WindowService, FrontendApplicationC return confirmExit !== 'always'; } + protected registerUnloadListener(): void { + window.addEventListener('unload', () => this.onUnloadEmitter.fire()); + } + + get onUnload(): Event { + return this.onUnloadEmitter.event; + } + /** * Ask the user to confirm if they want to unload the window. Prevent it if they do not. * @param event The beforeunload event diff --git a/packages/core/src/browser/window/window-service.ts b/packages/core/src/browser/window/window-service.ts index 1300b5af249c0..1f6b107aa4886 100644 --- a/packages/core/src/browser/window/window-service.ts +++ b/packages/core/src/browser/window/window-service.ts @@ -14,6 +14,8 @@ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 ********************************************************************************/ +import { Event } from '../../common/event'; + export interface NewWindowOptions { readonly external?: boolean; } @@ -38,4 +40,10 @@ export interface WindowService { */ canUnload(): boolean; + /** + * Fires when the `window` unloads. On this event, the frontend application can save its state and release resource. + * Saving the state and releasing any resources must be synchronous. + */ + readonly onUnload: Event; + } diff --git a/packages/core/src/electron-browser/window/electron-window-service.ts b/packages/core/src/electron-browser/window/electron-window-service.ts index 0bbda274987b9..68c5d6e4b1658 100644 --- a/packages/core/src/electron-browser/window/electron-window-service.ts +++ b/packages/core/src/electron-browser/window/electron-window-service.ts @@ -15,6 +15,8 @@ ********************************************************************************/ import { injectable, inject } from 'inversify'; +import { remote } from 'electron'; +import { Event } from '../../common/event'; import { NewWindowOptions } from '../../browser/window/window-service'; import { DefaultWindowService } from '../../browser/window/default-window-service'; import { ElectronMainWindowService } from '../../electron-common/electron-main-window-service'; @@ -30,6 +32,18 @@ export class ElectronWindowService extends DefaultWindowService { return undefined; } + registerUnloadListener(): void { + remote.getCurrentWindow().on('closed', () => this.onUnloadEmitter.fire()); + } + + /** + * Unlike the default implementation, this even is fires when the `ElectronWindow` + * [closes](https://github.com/electron/electron/blob/master/docs/api/browser-window.md#event-close). + */ + get onUnload(): Event { + return this.onUnloadEmitter.event; + } + protected preventUnload(event: BeforeUnloadEvent): string | void { // The user will be shown a confirmation dialog by the will-prevent-unload handler in the Electron main script event.returnValue = false; diff --git a/packages/core/src/electron-main/electron-main-application.ts b/packages/core/src/electron-main/electron-main-application.ts index e1a7194599c5c..85fd2e9562a1c 100644 --- a/packages/core/src/electron-main/electron-main-application.ts +++ b/packages/core/src/electron-main/electron-main-application.ts @@ -344,8 +344,8 @@ export class ElectronMainApplication { */ protected attachWillPreventUnload(electronWindow: BrowserWindow): void { // Fired when a beforeunload handler tries to prevent the page unloading - electronWindow.webContents.on('will-prevent-unload', async event => { - const { response } = await dialog.showMessageBox(electronWindow, { + electronWindow.webContents.on('will-prevent-unload', event => { + const response = dialog.showMessageBoxSync(electronWindow, { type: 'question', buttons: ['Yes', 'No'], title: 'Confirm',