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 · Nov 3, 2021 · d319b59 · d319b59
1 parent e91bea4
commit d319b59
Show file tree

Hide file tree

Showing 11 changed files with 221 additions and 89 deletions.
diff --git a/packages/core/src/browser/common-frontend-contribution.ts b/packages/core/src/browser/common-frontend-contribution.ts
@@ -21,7 +21,7 @@ import { injectable, inject, optional } from 'inversify';
 import { TabBar, Widget } from '@phosphor/widgets';
 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,7 @@ import { FormatType } from './saveable';
 import { QuickInputService, QuickPick, QuickPickItem } from './quick-input';
 import { AsyncLocalizationProvider } from '../common/i18n/localization';
 import { nls } from '../common/nls';
+import { confirmExit } from './dialogs';
 
 export namespace CommonMenus {
 
@@ -1024,10 +1025,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;

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: 'Are you sure you want to quit?',
+        msg: 'Any unsaved changes will not be saved.',
+        ok: 'Yes',
+        cancel: 'No',
+    }).open();
+    return safeToExit === true;
 }
 
 @injectable()

diff --git a/packages/core/src/browser/frontend-application.ts b/packages/core/src/browser/frontend-application.ts
@@ -55,7 +55,7 @@ export interface FrontendApplicationContribution {
      * Return `true` 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.safeToShutDown()) {
+            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, '');
         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,9 +18,10 @@ 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 {
@@ -53,33 +54,69 @@ export class DefaultWindowService implements WindowService, FrontendApplicationC
         this.openNewWindow(DEFAULT_WINDOW_HASH);
     }
 
-    canUnload(): boolean {
-        const confirmExit = this.corePreferences['application.confirmExit'];
-        let preventUnload = confirmExit === 'always';
+    protected collectContributionUnloadVetoes(): OnWillStopAction[] {
+        const vetoes = [];
+        const shouldConfirmExit = this.corePreferences['application.confirmExit'];
         for (const contribution of this.contributions.getContributions()) {
-            if (contribution.onWillStop?.(this.frontendApplication)) {
-                preventUnload = true;
+            const veto = contribution.onWillStop?.(this.frontendApplication);
+            if (veto && shouldConfirmExit !== 'never') { // Ignore vetoes if we should not prompt for exit.
+                if (OnWillStopAction.is(veto)) {
+                    vetoes.push(veto);
+                } else {
+                    vetoes.push({ reason: 'No reason given', action: () => false });
+                }
             }
         }
-        return confirmExit === 'never' || !preventUnload;
+        if (vetoes.length === 0 && shouldConfirmExit === 'always') {
+            vetoes.push({ reason: 'application.confirmExit preference', action: () => confirmExit() });
+        }
+        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 safeToShutDown(): 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');
+            return true;
+        } else {
+            return 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 +132,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,7 @@ import { WindowService } from '../window-service';
 export class MockWindowService implements WindowService {
     openNewWindow(): undefined { return undefined; }
     openNewDefaultWindow(): void { }
-    canUnload(): boolean { return true; }
+    reload(): void { }
+    safeToShutDown(): Promise<boolean> { return Promise.resolve(true); }
     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,20 @@ 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.
+     */
+    safeToShutDown(): Promise<boolean>;
+
+    /**
+     * Reloads the window according to platform.
+     */
+    reload(): void;
 }
diff --git a/packages/core/src/electron-browser/window/electron-window-service.ts b/packages/core/src/electron-browser/window/electron-window-service.ts
@@ -15,11 +15,12 @@
  ********************************************************************************/
 
 import { injectable, inject, postConstruct } from 'inversify';
-import { remote } from '../../../shared/electron';
+import * as electron from '../../../shared/electron';
 import { NewWindowOptions } from '../../common/window';
 import { DefaultWindowService } from '../../browser/window/default-window-service';
 import { ElectronMainWindowService } from '../../electron-common/electron-main-window-service';
 import { ElectronWindowPreferences } from './electron-window-preferences';
+import { CloseRequestArguments, CLOSE_REQUESTED_SIGNAL, RELOAD_REQUESTED_SIGNAL, StopReason } from '../../electron-common/messaging/electron-messages';
 
 @injectable()
 export class ElectronWindowService extends DefaultWindowService {
@@ -59,59 +60,38 @@ export class ElectronWindowService extends DefaultWindowService {
         });
     }
 
-    registerUnloadListeners(): void {
-        window.addEventListener('beforeunload', event => {
-            if (this.isUnloading) {
-                // Unloading process ongoing, do nothing:
-                return this.preventUnload(event);
-            } else if (this.closeOnUnload || this.canUnload()) {
-                // Let the window close and notify clients:
-                delete event.returnValue;
-                this.onUnloadEmitter.fire();
-                return;
-            } else {
-                this.isUnloading = true;
-                // Fix https://github.com/eclipse-theia/theia/issues/8186#issuecomment-742624480
-                // On Electron/Linux doing `showMessageBoxSync` does not seems to block the closing
-                // process long enough and closes the window no matter what you click on (yes/no).
-                // Instead we'll prevent closing right away, ask for confirmation and finally close.
-                setTimeout(() => {
-                    if (this.shouldUnload()) {
-                        this.closeOnUnload = true;
-                        window.close();
-                    }
-                    this.isUnloading = false;
-                });
-                return this.preventUnload(event);
-            }
-        });
+    protected registerUnloadListeners(): void {
+        electron.ipcRenderer.on(CLOSE_REQUESTED_SIGNAL, (_event, closeRequestEvent: CloseRequestArguments) => this.handleCloseRequestedEvent(closeRequestEvent));
+        window.addEventListener('unload', () => this.onUnloadEmitter.fire());
     }
 
     /**
-     * When preventing `beforeunload` on Electron, no popup is shown.
-     *
-     * This method implements a modal to ask the user if he wants to quit the page.
+     * Run when ElectronMain detects a `close` event and emits a `close-requested` event.
+     * Should send an event to `electron.ipcRenderer` on the event's `confirmChannel` if it is safe to exit
+     * after running FrontentApplication `onWillStop` handlers or on the `cancelChannel` if it is not safe to exit.
      */
-    protected shouldUnload(): boolean {
-        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.'
-        });
-        return response === 0; // 'Yes', close the window.
+    protected async handleCloseRequestedEvent(event: CloseRequestArguments): Promise<void> {
+        const safeToClose = await this.safeToShutDown();
+        if (safeToClose) {
+            console.debug(`Shutting down because of ${StopReason[event.reason]} request.`);
+            electron.ipcRenderer.send(event.confirmChannel);
+        } else {
+            electron.ipcRenderer.send(event.cancelChannel);
+        }
     }
 
     /**
      * Updates the window zoom level based on the preference value.
      */
     protected updateWindowZoomLevel(): void {
         const preferredZoomLevel = this.electronWindowPreferences['window.zoomLevel'];
-        const webContents = remote.getCurrentWindow().webContents;
+        const webContents = electron.remote.getCurrentWindow().webContents;
         if (webContents.getZoomLevel() !== preferredZoomLevel) {
             webContents.setZoomLevel(preferredZoomLevel);
         }
     }
+
+    reload(): void {
+        electron.ipcRenderer.send(RELOAD_REQUESTED_SIGNAL);
+    }
 }