Implement more varied onunload handling

Allows Electron to display separate messages from contributions that want to prevent or delay unloading. Breaking: - canUnload() removed from WindowService interface - reload(), safeToShotDown() added to WindowService interface - polling of FrontentContributions moved to collectContributionUnloadVetoes() Signed-off-by: Colin Grant <[email protected]>
eclipse-theia · Dec 9, 2021 · 2a28863 · 2a28863
1 parent 7f954ac
commit 2a28863
Show file tree

Hide file tree

Showing 15 changed files with 333 additions and 103 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,7 @@
 - [scripts] added Electron frontend start-up performance measurement script [#10442](https://github.com/eclipse-theia/theia/pull/10442) - Contributed on behalf of STMicroelectronics
 - [core, editor, editor-preview] additional commands added to tabbar context menu for editor widgets. [#10394](https://github.com/eclipse-theia/theia/pull/10394)
 - [preferences] Updated `AbstractResourcePreferenceProvider` to handle multiple preference settings in the same tick and handle open preference files. It will save the file exactly once, and prompt the user if the file is dirty when a programmatic setting is attempted. [#7775](https://github.com/eclipse-theia/theia/pull/7775)
+- [core] `WindowService` and `ElectronMainApplication` updated to allow for asynchronous pre-exit code in Electron. [#10379](https://github.com/eclipse-theia/theia/pull/10379)
 
 <a name="breaking_changes_1.21.0">[Breaking Changes:](#breaking_changes_1.21.0)</a>
 
@@ -15,6 +16,11 @@
   to your application package's root.
 - [core] `SelectionService` added to constructor arguments of `TabBarRenderer`. [#10394](https://github.com/eclipse-theia/theia/pull/10394)
 - [preferences] Removed `PreferenceProvider#pendingChanges` field. It was previously set unreliably and caused race conditions. If a `PreferenceProvider` needs a mechanism for deferring the resolution of `PreferenceProvider#setPreference`, it should implement its own system. See PR for example implementation in `AbstractResourcePreferenceProvider`. [#7775](https://github.com/eclipse-theia/theia/pull/7775)
+- [core] `WindowService` interface changed considerably [#10379](https://github.com/eclipse-theia/theia/pull/10379)
+  - remove `canUnload(): boolean`- it's replaced by `isSafeToShutDown(): Promise<boolean>` to allow asynchronous handling in Electron.
+  - add `isSafeToShutDown()` - replaces `canUnload()`.
+  - add `setSafeToShutDown()` - ensures that next close event will not be prevented.
+  - add `reload()` - to allow different handling in Electron and browser.
 
 ## v1.20.0 - 11/25/2021
 

diff --git a/packages/core/src/browser/common-frontend-contribution.ts b/packages/core/src/browser/common-frontend-contribution.ts
@@ -20,7 +20,7 @@ import debounce = require('lodash.debounce');
 import { injectable, inject, optional } from 'inversify';
 import { MAIN_MENU_BAR, SETTINGS_MENU, MenuContribution, MenuModelRegistry, ACCOUNTS_MENU } from '../common/menu';
 import { KeybindingContribution, KeybindingRegistry } from './keybinding';
-import { FrontendApplication, FrontendApplicationContribution } from './frontend-application';
+import { FrontendApplication, FrontendApplicationContribution, OnWillStopAction } from './frontend-application';
 import { CommandContribution, CommandRegistry, Command } from '../common/command';
 import { UriAwareCommandHandler } from '../common/uri-command-handler';
 import { SelectionService } from '../common/selection-service';
@@ -55,6 +55,9 @@ import { QuickInputService, QuickPick, QuickPickItem } from './quick-input';
 import { AsyncLocalizationProvider } from '../common/i18n/localization';
 import { nls } from '../common/nls';
 import { CurrentWidgetCommandAdapter } from './shell/current-widget-command-adapter';
+import { ConfirmDialog, confirmExit, Dialog } from './dialogs';
+import { WindowService } from './window/window-service';
+import { FrontendApplicationConfigProvider } from './frontend-application-config-provider';
 
 export namespace CommonMenus {
 
@@ -353,6 +356,9 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
     @inject(AuthenticationService)
     protected readonly authenticationService: AuthenticationService;
 
+    @inject(WindowService)
+    protected readonly windowService: WindowService;
+
     async configure(app: FrontendApplication): Promise<void> {
         const configDirUri = await this.environments.getConfigDirUri();
         // Global settings
@@ -967,10 +973,10 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
         });
     }
 
-    onWillStop(): true | undefined {
+    onWillStop(): OnWillStopAction | undefined {
         try {
             if (this.shouldPreventClose || this.shell.canSaveAll()) {
-                return true;
+                return { reason: 'Dirty editors present', action: () => confirmExit() };
             }
         } finally {
             this.shouldPreventClose = false;
@@ -983,10 +989,11 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
         for (const additionalLanguage of ['en', ...availableLanguages]) {
             items.push({
                 label: additionalLanguage,
-                execute: () => {
-                    if (additionalLanguage !== nls.locale) {
+                execute: async () => {
+                    if (additionalLanguage !== nls.locale && await this.confirmRestart()) {
+                        this.windowService.setSafeToShutDown();
                         window.localStorage.setItem(nls.localeId, additionalLanguage);
-                        window.location.reload();
+                        this.windowService.reload();
                     }
                 }
             });
@@ -998,6 +1005,16 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
             });
     }
 
+    protected async confirmRestart(): Promise<boolean> {
+        const shouldRestart = await new ConfirmDialog({
+            title: nls.localizeByDefault('A restart is required for the change in display language to take effect.'),
+            msg: nls.localizeByDefault('Press the restart button to restart {0} and change the display language.', FrontendApplicationConfigProvider.get().applicationName),
+            ok: nls.localizeByDefault('Restart'),
+            cancel: Dialog.CANCEL,
+        }).open();
+        return shouldRestart === true;
+    }
+
     protected selectIconTheme(): void {
         let resetTo: string | undefined = this.iconThemes.current;
         const previewTheme = debounce((id: string) => this.iconThemes.current = id, 200);

diff --git a/packages/core/src/browser/dialogs.ts b/packages/core/src/browser/dialogs.ts
@@ -385,7 +385,16 @@ export class ConfirmDialog extends AbstractDialog<boolean> {
         }
         return msg;
     }
+}
 
+export async function confirmExit(): Promise<boolean> {
+    const safeToExit = await new ConfirmDialog({
+        title: nls.localize('theia/core/quitTitle', 'Are you sure you want to quit?'),
+        msg: nls.localize('theia/core/quitMessage', 'Any unsaved changes will not be saved.'),
+        ok: Dialog.YES,
+        cancel: Dialog.NO,
+    }).open();
+    return safeToExit === true;
 }
 
 @injectable()

diff --git a/packages/core/src/browser/frontend-application.ts b/packages/core/src/browser/frontend-application.ts
@@ -52,10 +52,10 @@ export interface FrontendApplicationContribution {
 
     /**
      * Called on `beforeunload` event, right before the window closes.
-     * Return `true` in order to prevent exit.
+     * Return `true` or an OnWillStopAction in order to prevent exit.
      * Note: No async code allowed, this function has to run on one tick.
      */
-    onWillStop?(app: FrontendApplication): boolean | void;
+    onWillStop?(app: FrontendApplication): boolean | undefined | OnWillStopAction;
 
     /**
      * Called when an application is stopped or unloaded.
@@ -77,6 +77,23 @@ export interface FrontendApplicationContribution {
     onDidInitializeLayout?(app: FrontendApplication): MaybePromise<void>;
 }
 
+export interface OnWillStopAction {
+    /**
+     * @resolves to `true` if it is safe to close the application; `false` otherwise.
+     */
+    action: () => MaybePromise<boolean>;
+    /**
+     * A descriptive string for the reason preventing close.
+     */
+    reason: string;
+}
+
+export namespace OnWillStopAction {
+    export function is(candidate: unknown): candidate is OnWillStopAction {
+        return typeof candidate === 'object' && !!candidate && 'action' in candidate && 'reason' in candidate;
+    }
+}
+
 const TIMER_WARNING_THRESHOLD = 100;
 
 /**

diff --git a/packages/core/src/browser/shell/shell-layout-restorer.ts b/packages/core/src/browser/shell/shell-layout-restorer.ts
@@ -26,6 +26,7 @@ import { ContributionProvider } from '../../common/contribution-provider';
 import { MaybePromise } from '../../common/types';
 import { ApplicationShell, applicationShellLayoutVersion, ApplicationShellLayoutVersion } from './application-shell';
 import { CommonCommands } from '../common-frontend-contribution';
+import { WindowService } from '../window/window-service';
 
 /**
  * A contract for widgets that want to store and restore their inner state, between sessions.
@@ -125,6 +126,9 @@ export class ShellLayoutRestorer implements CommandContribution {
     @inject(ContributionProvider) @named(ApplicationShellLayoutMigration)
     protected readonly migrations: ContributionProvider<ApplicationShellLayoutMigration>;
 
+    @inject(WindowService)
+    protected readonly windowService: WindowService;
+
     constructor(
         @inject(WidgetManager) protected widgetManager: WidgetManager,
         @inject(ILogger) protected logger: ILogger,
@@ -137,12 +141,14 @@ export class ShellLayoutRestorer implements CommandContribution {
     }
 
     protected async resetLayout(): Promise<void> {
-        this.logger.info('>>> Resetting layout...');
-        this.shouldStoreLayout = false;
-        this.storageService.setData(this.storageKey, undefined);
-        ThemeService.get().reset(); // Theme service cannot use DI, so the current theme ID is stored elsewhere. Hence the explicit reset.
-        this.logger.info('<<< The layout has been successfully reset.');
-        window.location.reload(true);
+        if (await this.windowService.isSafeToShutDown()) {
+            this.logger.info('>>> Resetting layout...');
+            this.shouldStoreLayout = false;
+            this.storageService.setData(this.storageKey, undefined);
+            ThemeService.get().reset(); // Theme service cannot use DI, so the current theme ID is stored elsewhere. Hence the explicit reset.
+            this.logger.info('<<< The layout has been successfully reset.');
+            this.windowService.reload();
+        }
     }
 
     storeLayout(app: FrontendApplication): void {

diff --git a/packages/core/src/browser/window/default-window-service.spec.ts b/packages/core/src/browser/window/default-window-service.spec.ts
@@ -51,7 +51,7 @@ describe('DefaultWindowService', () => {
         ];
         const windowService = setupWindowService('never', frontendContributions);
         assert(frontendContributions.every(contribution => !contribution.onWillStopCalled), 'contributions should not be called yet');
-        assert(windowService.canUnload(), 'canUnload should return true');
+        assert(windowService['collectContributionUnloadVetoes']().length === 0, 'there should be no vetoes');
         assert(frontendContributions.every(contribution => contribution.onWillStopCalled), 'contributions should have been called');
     });
     it('onWillStop should be called on every contribution (ifRequired)', () => {
@@ -62,7 +62,7 @@ describe('DefaultWindowService', () => {
         ];
         const windowService = setupWindowService('ifRequired', frontendContributions);
         assert(frontendContributions.every(contribution => !contribution.onWillStopCalled), 'contributions should not be called yet');
-        assert(!windowService.canUnload(), 'canUnload should return false');
+        assert(windowService['collectContributionUnloadVetoes']().length > 0, 'There should be vetoes');
         assert(frontendContributions.every(contribution => contribution.onWillStopCalled), 'contributions should have been called');
     });
     it('onWillStop should be called on every contribution (always)', () => {
@@ -72,7 +72,7 @@ describe('DefaultWindowService', () => {
         ];
         const windowService = setupWindowService('always', frontendContributions);
         assert(frontendContributions.every(contribution => !contribution.onWillStopCalled), 'contributions should not be called yet');
-        assert(!windowService.canUnload(), 'canUnload should return false');
+        assert(windowService['collectContributionUnloadVetoes']().length > 0, 'there should be vetoes');
         assert(frontendContributions.every(contribution => contribution.onWillStopCalled), 'contributions should have been called');
     });
 });
diff --git a/packages/core/src/browser/window/default-window-service.ts b/packages/core/src/browser/window/default-window-service.ts
@@ -18,14 +18,16 @@ 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';
+import { FrontendApplicationContribution, FrontendApplication, OnWillStopAction } from '../frontend-application';
 import { WindowService } from './window-service';
 import { DEFAULT_WINDOW_HASH } from '../../common/window';
+import { confirmExit } from '../dialogs';
 
 @injectable()
 export class DefaultWindowService implements WindowService, FrontendApplicationContribution {
 
     protected frontendApplication: FrontendApplication;
+    protected allowVetoes = true;
 
     protected onUnloadEmitter = new Emitter<void>();
     get onUnload(): Event<void> {
@@ -53,33 +55,84 @@ export class DefaultWindowService implements WindowService, FrontendApplicationC
         this.openNewWindow(`#${DEFAULT_WINDOW_HASH}`);
     }
 
-    canUnload(): boolean {
-        const confirmExit = this.corePreferences['application.confirmExit'];
-        let preventUnload = confirmExit === 'always';
-        for (const contribution of this.contributions.getContributions()) {
-            if (contribution.onWillStop?.(this.frontendApplication)) {
-                preventUnload = true;
+    /**
+     * Returns a list of actions that {@link FrontendApplicationContribution}s would like to take before shutdown
+     * It is expected that this will succeed - i.e. return an empty array - at most once per session. If no vetoes are received
+     * during any cycle, no further checks will be made. In that case, shutdown should proceed unconditionally.
+     */
+    protected collectContributionUnloadVetoes(): OnWillStopAction[] {
+        const vetoes = [];
+        if (this.allowVetoes) {
+            const shouldConfirmExit = this.corePreferences['application.confirmExit'];
+            for (const contribution of this.contributions.getContributions()) {
+                const veto = contribution.onWillStop?.(this.frontendApplication);
+                if (veto && shouldConfirmExit !== 'never') { // Ignore vetoes if we should not prompt the user on exit.
+                    if (OnWillStopAction.is(veto)) {
+                        vetoes.push(veto);
+                    } else {
+                        vetoes.push({ reason: 'No reason given', action: () => false });
+                    }
+                }
+            }
+            if (vetoes.length === 0 && shouldConfirmExit === 'always') {
+                vetoes.push({ reason: 'application.confirmExit preference', action: () => confirmExit() });
+            }
+            if (vetoes.length === 0) {
+                this.allowVetoes = false;
             }
         }
-        return confirmExit === 'never' || !preventUnload;
+        return vetoes;
     }
 
     /**
      * Implement the mechanism to detect unloading of the page.
      */
     protected registerUnloadListeners(): void {
-        window.addEventListener('beforeunload', event => {
-            if (!this.canUnload()) {
-                return this.preventUnload(event);
-            }
-        });
+        window.addEventListener('beforeunload', event => this.handleBeforeUnloadEvent(event));
         // In a browser, `unload` is correctly fired when the page unloads, unlike Electron.
         // If `beforeunload` is cancelled, the user will be prompted to leave or stay.
         // If the user stays, the page won't be unloaded, so `unload` is not fired.
         // If the user leaves, the page will be unloaded, so `unload` is fired.
         window.addEventListener('unload', () => this.onUnloadEmitter.fire());
     }
 
+    async isSafeToShutDown(): Promise<boolean> {
+        const vetoes = this.collectContributionUnloadVetoes();
+        if (vetoes.length === 0) {
+            return true;
+        }
+        console.debug('Shutdown prevented by', vetoes.map(({ reason }) => reason).join(', '));
+        const resolvedVetoes = await Promise.allSettled(vetoes.map(({ action }) => action()));
+        if (resolvedVetoes.every(resolution => resolution.status === 'rejected' || resolution.value === true)) {
+            console.debug('OnWillStop actions resolved; allowing shutdown');
+            this.allowVetoes = false;
+            return true;
+        } else {
+            return false;
+        }
+    }
+
+    setSafeToShutDown(): void {
+        this.allowVetoes = false;
+    }
+
+    /**
+     * Called when the `window` is about to `unload` its resources.
+     * At this point, the `document` is still visible and the [`BeforeUnloadEvent`](https://developer.mozilla.org/en-US/docs/Web/API/Window/beforeunload_event)
+     * event will be canceled if the return value of this method is `false`.
+     *
+     * In Electron, handleCloseRequestEvent is is run instead.
+     */
+    protected handleBeforeUnloadEvent(event: BeforeUnloadEvent): string | void {
+        const vetoes = this.collectContributionUnloadVetoes();
+        if (vetoes.length) {
+            // In the browser, we don't call the functions because this has to finish in a single tick, so we treat any desired action as a veto.
+            console.debug('Shutdown prevented by', vetoes.map(({ reason }) => reason).join(', '));
+            return this.preventUnload(event);
+        }
+        console.debug('Shutdown will proceed.');
+    }
+
     /**
      * Notify the browser that we do not want to unload.
      *
@@ -95,4 +148,7 @@ export class DefaultWindowService implements WindowService, FrontendApplicationC
         return '';
     }
 
+    reload(): void {
+        window.location.reload();
+    }
 }
diff --git a/packages/core/src/browser/window/test/mock-window-service.ts b/packages/core/src/browser/window/test/mock-window-service.ts
@@ -21,6 +21,8 @@ import { WindowService } from '../window-service';
 export class MockWindowService implements WindowService {
     openNewWindow(): undefined { return undefined; }
     openNewDefaultWindow(): void { }
-    canUnload(): boolean { return true; }
+    reload(): void { }
+    isSafeToShutDown(): Promise<boolean> { return Promise.resolve(true); }
+    setSafeToShutDown(): void { }
     get onUnload(): Event<void> { return Event.None; }
 }
diff --git a/packages/core/src/browser/window/window-service.ts b/packages/core/src/browser/window/window-service.ts
@@ -23,7 +23,6 @@ import { NewWindowOptions } from '../../common/window';
 export const WindowService = Symbol('WindowService');
 
 export interface WindowService {
-
     /**
      * Opens a new window and loads the content from the given URL.
      * In a browser, opening a new Theia tab or open a link is the same thing.
@@ -37,17 +36,32 @@ export interface WindowService {
      */
     openNewDefaultWindow(): void;
 
-    /**
-     * Called when the `window` is about to `unload` its resources.
-     * At this point, the `document` is still visible and the [`BeforeUnloadEvent`](https://developer.mozilla.org/en-US/docs/Web/API/Window/beforeunload_event)
-     * event will be canceled if the return value of this method is `false`.
-     */
-    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<void>;
 
+    /**
+     * Checks `FrontendApplicationContribution#willStop` for impediments to shutdown and runs any actions returned.
+     * Can be used safely in browser and Electron when triggering reload or shutdown programmatically.
+     * Should _only_ be called before a shutdown - if this returns `true`, `FrontendApplicationContribution#willStop`
+     * will not be called again in the current session. I.e. if this return `true`, the shutdown should proceed without
+     * further condition.
+     */
+    isSafeToShutDown(): Promise<boolean>;
+
+    /**
+     * Will prevent subsequent checks of `FrontendApplicationContribution#willStop`. Should only be used after requesting
+     * user confirmation.
+     *
+     * This is primarily intended programmatic restarts due to e.g. change of display language. It allows for a single confirmation
+     * of intent, rather than one warning and then several warnings from other contributions.
+     */
+    setSafeToShutDown(): void;
+
+    /**
+     * Reloads the window according to platform.
+     */
+    reload(): void;
 }