From 3b6bbf54039eefc6837b64d5216e787df39b01e8 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` - Moved the `beforeunload` handling logic to the frontend. - Removed the `attachWillPreventUnload` from the main electron app. Signed-off-by: Akos Kitta --- CHANGELOG.md | 1 + .../core/src/browser/frontend-application.ts | 7 ++++-- .../browser/window/default-window-service.ts | 21 ++++++++++++++++ .../window/test/mock-window-service.ts | 2 ++ .../core/src/browser/window/window-service.ts | 8 +++++++ .../window/electron-window-service.ts | 24 +++++++++++++++++-- .../electron-main-application.ts | 23 +----------------- 7 files changed, 60 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c46b143eece8e..4716142a42989 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ [Breaking Changes:](#breaking_changes_1.8.0) - [file-search] Deprecate dependency on `@theia/process` and replaced its usage by node's `child_process` api. +- [electron] Removed `attachWillPreventUnload` method from the Electron main application. The `confirmExit` logic is handled on the frontend. [#8732](https://github.com/eclipse-theia/theia/pull/8732) ## v1.7.0 - 29/10/2020 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..bf8ce32e22a0d 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'; @@ -23,6 +24,9 @@ import { WindowService } from './window-service'; @injectable() export class DefaultWindowService implements WindowService, FrontendApplicationContribution { + private didFireUnload = false; + private onUnloadEmitter = new Emitter(); + protected frontendApplication: FrontendApplication; @inject(CorePreferences) @@ -38,7 +42,9 @@ export class DefaultWindowService implements WindowService, FrontendApplicationC if (!this.canUnload()) { return this.preventUnload(event); } + this.onUnloadEmitter.fire(); }); + this.registerUnloadListener(); } openNewWindow(url: string): undefined { @@ -61,6 +67,21 @@ export class DefaultWindowService implements WindowService, FrontendApplicationC return confirmExit !== 'always'; } + protected registerUnloadListener(): void { + window.addEventListener('unload', this.fireUnload.bind(this)); + } + + protected fireUnload(): void { + if (!this.didFireUnload) { + this.didFireUnload = true; + 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/test/mock-window-service.ts b/packages/core/src/browser/window/test/mock-window-service.ts index a0662fcfde2a1..a3cca33908fa3 100644 --- a/packages/core/src/browser/window/test/mock-window-service.ts +++ b/packages/core/src/browser/window/test/mock-window-service.ts @@ -14,10 +14,12 @@ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 ********************************************************************************/ import { injectable } from 'inversify'; +import { Event } from '../../../common/event'; import { WindowService } from '../window-service'; @injectable() export class MockWindowService implements WindowService { openNewWindow(): undefined { return undefined; } canUnload(): boolean { return true; } + get onUnload(): Event { return Event.None; } } diff --git a/packages/core/src/browser/window/window-service.ts b/packages/core/src/browser/window/window-service.ts index 1300b5af249c0..cec3b1e9a25f7 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. The unload event is inevitable. On this event, the frontend application can save its state and release resource. + * Saving the state and releasing any resources must be a synchronous call. Any asynchronous calls invoked after emitting this event might be ignored. + */ + 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..c5a42a37cda46 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,7 @@ ********************************************************************************/ import { injectable, inject } from 'inversify'; +import { remote } from 'electron'; 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,9 +31,28 @@ export class ElectronWindowService extends DefaultWindowService { return undefined; } + registerUnloadListener(): void { + // NOOP. The unload logic is handled in the `preventUnload` when running the app in electron env. + } + 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; + const electronWindow = remote.getCurrentWindow(); + const response = remote.dialog.showMessageBoxSync(electronWindow, { + type: 'question', + buttons: ['Yes', 'No'], + title: 'Confirm', + message: 'Are you sure you want to quit?', + detail: 'Any unsaved changes will not be saved.' + }); + if (response === 0) { // 'Yes', close the window. + this.fireUnload(); + // The absence of a `returnValue` property on the event will guarantee the browser `unload` happens. + // See: https://developer.mozilla.org/en-US/docs/Web/API/WindowEventHandlers/onbeforeunload + delete event.returnValue; + } else { + event.preventDefault(); + event.returnValue = true; + } } } diff --git a/packages/core/src/electron-main/electron-main-application.ts b/packages/core/src/electron-main/electron-main-application.ts index e1a7194599c5c..dc7304c35c8e8 100644 --- a/packages/core/src/electron-main/electron-main-application.ts +++ b/packages/core/src/electron-main/electron-main-application.ts @@ -15,7 +15,7 @@ ********************************************************************************/ import { inject, injectable, named } from 'inversify'; -import { session, screen, globalShortcut, app, BrowserWindow, BrowserWindowConstructorOptions, Event as ElectronEvent, dialog } from 'electron'; +import { session, screen, globalShortcut, app, BrowserWindow, BrowserWindowConstructorOptions, Event as ElectronEvent } from 'electron'; import * as path from 'path'; import { Argv } from 'yargs'; import { AddressInfo } from 'net'; @@ -213,7 +213,6 @@ export class ElectronMainApplication { const electronWindow = new BrowserWindow(options); this.attachReadyToShow(electronWindow); this.attachSaveWindowState(electronWindow); - this.attachWillPreventUnload(electronWindow); this.attachGlobalShortcuts(electronWindow); this.restoreMaximizedState(electronWindow, options); return electronWindow; @@ -339,26 +338,6 @@ export class ElectronMainApplication { electronWindow.on('move', saveWindowStateDelayed); } - /** - * Catch window closing event and display a confirmation window. - */ - 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, { - type: 'question', - buttons: ['Yes', 'No'], - title: 'Confirm', - message: 'Are you sure you want to quit?', - detail: 'Any unsaved changes will not be saved.' - }); - if (response === 0) { // 'Yes' - // This ignores the beforeunload callback, allowing the page to unload - event.preventDefault(); - } - }); - } - /** * Catch certain keybindings to prevent reloading the window using keyboard shortcuts. */