Add secondary window support for Electron.

Fixes eclipse-theia#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. Since we cannot reliably prevent closing windows in the browser case, we either save or discard unsaved changes according to the autosave settings Contributed on behalf of ST Microelectronics. Signed-off-by: Thomas Mäder <[email protected]>
tsmaeder · Oct 14, 2022 · ea8db47 · ea8db47
1 parent e96bb5b
commit ea8db47
Show file tree

Hide file tree

Showing 15 changed files with 214 additions and 71 deletions.
diff --git a/dev-packages/application-manager/src/generator/frontend-generator.ts b/dev-packages/application-manager/src/generator/frontend-generator.ts
@@ -230,7 +230,10 @@ module.exports = Promise.resolve()${this.compileElectronMainModuleImports(electr
         // Only process messages from Theia main window
         if (e.source === window.opener) {
             // Delegate message to iframe
-            document.getElementsByTagName('iframe').item(0).contentWindow.postMessage({ ...e.data }, '*');
+            const f = document.getElementsByTagName('iframe');
+            if (f) {
+                f.item(0).contentWindow.postMessage({ ...e.data }, '*');
+            }
         }
     });
     </script>

diff --git a/examples/electron/package.json b/examples/electron/package.json
@@ -48,6 +48,7 @@
     "@theia/scm": "1.30.0",
     "@theia/scm-extra": "1.30.0",
     "@theia/search-in-workspace": "1.30.0",
+    "@theia/secondary-window": "1.30.0",
     "@theia/task": "1.30.0",
     "@theia/terminal": "1.30.0",
     "@theia/timeline": "1.30.0",

diff --git a/examples/electron/tsconfig.json b/examples/electron/tsconfig.json
@@ -107,6 +107,9 @@
     {
       "path": "../../packages/search-in-workspace"
     },
+    {
+      "path": "../../packages/secondary-window"
+    },
     {
       "path": "../../packages/task"
     },

diff --git a/packages/core/src/browser/secondary-window-handler.ts b/packages/core/src/browser/secondary-window-handler.ts
@@ -14,7 +14,6 @@
 // SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
 // *****************************************************************************
 
-import debounce = require('lodash.debounce');
 import { inject, injectable } from 'inversify';
 import { BoxLayout, BoxPanel, ExtractableWidget, Widget } from './widgets';
 import { MessageService } from '../common/message-service';
@@ -23,6 +22,8 @@ import { Emitter } from '../common/event';
 import { SecondaryWindowService } from './window/secondary-window-service';
 import { KeybindingRegistry } from './keybinding';
 import { ColorApplicationContribution } from './color-application-contribution';
+import debounce = require('lodash.debounce');
+import { Saveable } from './saveable';
 
 /** Widget to be contained directly in a secondary window. */
 class SecondaryWindowRootWidget extends Widget {
@@ -40,7 +41,6 @@ class SecondaryWindowRootWidget extends Widget {
 }
 
 /**
- * Offers functionality to move a widget out of the main window to a newly created window.
  * Widgets must explicitly implement the `ExtractableWidget` interface to support this.
  *
  * This handler manages the opened secondary windows and sets up messaging between them and the Theia main window.
@@ -139,13 +139,32 @@ 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(
+            () => {
+                const saveable = Saveable.get(widget);
+                return !!saveable && saveable.dirty && saveable.autoSave === 'off';
+            },
+            async trySaving => {
+                widget.isClosing = true;
+                // if trySaving is true, we should let the widget decide what to do
+                const saveable = Saveable.get(widget);
+                let closeOptions = undefined;
+                if (!trySaving) {
+                    closeOptions = {
+                        save: !!saveable && saveable.dirty && saveable.autoSave !== 'off'
+                    };
+                }
+
+                await this.applicationShell.closeWidget(widget.id, closeOptions);
+                widget.isClosing = false;
+                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.');
@@ -172,6 +191,7 @@ export class SecondaryWindowHandler {
             widget.secondaryWindow = newWindow;
             const rootWidget = new SecondaryWindowRootWidget();
             rootWidget.addClass('secondary-widget-root');
+            rootWidget.id = 'root-' + widget.id;
             Widget.attach(rootWidget, element);
             rootWidget.addWidget(widget);
             widget.show();
@@ -183,9 +203,6 @@ export class SecondaryWindowHandler {
             widget.disposed.connect(() => {
                 unregisterWithColorContribution.dispose();
                 this.removeWidget(widget);
-                if (!newWindow.closed) {
-                    newWindow.close();
-                }
             });
 
             // debounce to avoid rapid updates while resizing the secondary window

diff --git a/packages/core/src/browser/shell/application-shell.ts b/packages/core/src/browser/shell/application-shell.ts
@@ -34,7 +34,7 @@ import { FrontendApplicationStateService } from '../frontend-application-state';
 import { TabBarToolbarRegistry, TabBarToolbarFactory } from './tab-bar-toolbar';
 import { ContextKeyService } from '../context-key-service';
 import { Emitter } from '../../common/event';
-import { waitForRevealed, waitForClosed, PINNED_CLASS } from '../widgets';
+import { waitForRevealed, waitForClosed, PINNED_CLASS, ExtractableWidget } from '../widgets';
 import { CorePreferences } from '../core-preferences';
 import { BreadcrumbsRendererFactory } from '../breadcrumbs/breadcrumbs-renderer';
 import { Deferred } from '../../common/promise-util';
@@ -1561,14 +1561,19 @@ export class ApplicationShell extends Widget {
         if (!current) {
             return undefined;
         }
-        const saveableOptions = options && { shouldSave: () => options.save };
-        const pendingClose = SaveableWidget.is(current)
-            ? current.closeWithSaving(saveableOptions)
-            : (current.close(), waitForClosed(current));
-        await Promise.all([
-            pendingClose,
-            this.pendingUpdates
-        ]);
+
+        if (ExtractableWidget.is(current) && current.secondaryWindow && !current.isClosing) {
+            current.secondaryWindow!.close();
+        } else {
+            const saveableOptions = options && { shouldSave: () => options.save };
+            const pendingClose = SaveableWidget.is(current)
+                ? current.closeWithSaving(saveableOptions)
+                : (current.close(), waitForClosed(current));
+            await Promise.all([
+                pendingClose,
+                this.pendingUpdates
+            ]);
+        }
         return stack[0] || current;
     }
 

diff --git a/packages/core/src/browser/widgets/extractable-widget.ts b/packages/core/src/browser/widgets/extractable-widget.ts
@@ -22,6 +22,9 @@ import { Widget } from './widget';
 export interface ExtractableWidget extends Widget {
     /** Set to `true` to mark the widget to be extractable. */
     isExtractable: boolean;
+
+    /** State variable to keep track of recursive attempty to close the secondary window */
+    isClosing: boolean;
     /** The secondary window that the window was extracted to or `undefined` if it is not yet extracted. */
     secondaryWindow: Window | undefined;
 }

diff --git a/packages/core/src/browser/widgets/widget.ts b/packages/core/src/browser/widgets/widget.ts
@@ -336,16 +336,17 @@ export function waitForHidden(widget: Widget): Promise<void> {
 }
 
 function waitForVisible(widget: Widget, visible: boolean, attached?: boolean): Promise<void> {
+    const win = widget.node.ownerDocument.defaultView || window;
     if ((typeof attached !== 'boolean' || widget.isAttached === attached) &&
         (widget.isVisible === visible || (widget.node.style.visibility !== 'hidden') === visible)
     ) {
-        return new Promise(resolve => window.requestAnimationFrame(() => resolve()));
+        return new Promise(resolve => win.requestAnimationFrame(() => resolve()));
     }
     return new Promise(resolve => {
-        const waitFor = () => window.requestAnimationFrame(() => {
+        const waitFor = () => win.requestAnimationFrame(() => {
             if ((typeof attached !== 'boolean' || widget.isAttached === attached) &&
                 (widget.isVisible === visible || (widget.node.style.visibility !== 'hidden') === visible)) {
-                window.requestAnimationFrame(() => resolve());
+                win.requestAnimationFrame(() => resolve());
             } else {
                 waitFor();
             }

diff --git a/packages/core/src/browser/window/default-secondary-window-service.ts b/packages/core/src/browser/window/default-secondary-window-service.ts
@@ -46,33 +46,44 @@ export class DefaultSecondaryWindowService implements SecondaryWindowService {
         });
     }
 
-    createSecondaryWindow(onClose?: (closedWin: Window) => void): Window | undefined {
-        const win = this.doCreateSecondaryWindow(onClose);
+    createSecondaryWindow(wouldLoseStateOnClosing: () => boolean, tryCloseWidget: (trySaving: boolean) => Promise<boolean>, closed: (win: Window) => void): Window | undefined {
+        const id = this.nextWindowId();
+        const win = this.doCreateSecondaryWindow(id, wouldLoseStateOnClosing, tryCloseWidget, closed);
         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');
+    protected doCreateSecondaryWindow(id: string, wouldLoseStateOnClosing: () => boolean, tryCloseWidget: (trySaving: boolean) => Promise<boolean>,
+        closed: (win: Window) => void): Window | undefined {
+        const win = window.open(DefaultSecondaryWindowService.SECONDARY_WINDOW_URL, id, 'popup');
+
         if (win) {
             // 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('beforeunload', evt => {
+                    if (wouldLoseStateOnClosing()) {
+                        evt.returnValue = '';
+                        evt.preventDefault();
+                        return '';
+                    }
+                }, { capture: true });
                 win.addEventListener('unload', () => {
-                    this.handleWindowClosed(win, onClose);
+                    tryCloseWidget(false);
+                    this.handleWindowClosed(win);
+                    closed(win);
                 });
             });
         }
         return win ?? undefined;
     }
 
-    protected handleWindowClosed(win: Window, onClose?: (closedWin: Window) => void): void {
+    protected handleWindowClosed(win: Window): void {
         const extIndex = this.secondaryWindows.indexOf(win);
         if (extIndex > -1) {
             this.secondaryWindows.splice(extIndex, 1);
         };
-        onClose?.(win);
     }
 
     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
@@ -26,10 +26,12 @@ 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 wouldLoseStateOnClosing callback to determine whether save is necessary
+     * @param tryCloseWidget callback to try and close the widget. 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(wouldLoseStateOnClosing: () => boolean, tryCloseWidget: (trySaving: boolean) => Promise<boolean>, closed: (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
@@ -14,32 +14,53 @@
 // SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
 // *****************************************************************************
 
-import { BrowserWindow } from '../../../electron-shared/electron';
+import { ipcRenderer, BrowserWindow } from '../../../electron-shared/electron';
 import * as electronRemote from '../../../electron-shared/@electron/remote';
-import { injectable } from 'inversify';
+import { injectable, postConstruct } from 'inversify';
 import { DefaultSecondaryWindowService } from '../../browser/window/default-secondary-window-service';
+import { CloseSecondaryRequestArguments, CLOSE_SECONDARY_REQUESTED_SIGNAL } from '../../electron-common/messaging/electron-messages';
 
 @injectable()
 export class ElectronSecondaryWindowService extends DefaultSecondaryWindowService {
-    protected electronWindows: Map<string, BrowserWindow> = new Map();
 
-    protected override doCreateSecondaryWindow(onClose?: (closedWin: Window) => void): Window | undefined {
-        const id = this.nextWindowId();
+    private electronWindows: Map<string, BrowserWindow> = new Map();
+    private electronWindowsById: Map<string, () => Promise<boolean>> = new Map();
+
+    @postConstruct()
+    override init(): void {
+        super.init();
+        ipcRenderer.addListener(CLOSE_SECONDARY_REQUESTED_SIGNAL, (_sender, args: CloseSecondaryRequestArguments) => this.handleCloseRequestedEvent(args));
+    }
+
+    protected async handleCloseRequestedEvent(event: CloseSecondaryRequestArguments): Promise<void> {
+        const safeToClose = await this.safeToClose(event.windowId);
+        if (safeToClose) {
+            ipcRenderer.send(event.confirmChannel);
+        } else {
+            ipcRenderer.send(event.cancelChannel);
+        }
+    }
+
+    protected override doCreateSecondaryWindow(id: string, wouldLoseStateOnClosing: () => boolean, tryCloseWidget: (trySaving: boolean) => Promise<boolean>,
+        closed: (win: Window) => void): Window | undefined {
+        let win: Window | undefined = undefined;
         electronRemote.getCurrentWindow().webContents.once('did-create-window', newElectronWindow => {
-            newElectronWindow.setMenuBarVisibility(false);
+            // newElectronWindow.setMenuBarVisibility(false);
             this.electronWindows.set(id, newElectronWindow);
-            newElectronWindow.on('closed', () => {
+            const electronId = newElectronWindow.id.toString();
+            this.electronWindowsById.set(electronId, () => tryCloseWidget(true));
+            const closedHandler = () => {
+                if (closed) {
+                    closed(win!);
+                }
+
                 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.`);
-                };
-            });
+                this.electronWindowsById.delete(electronId);
+            };
+            newElectronWindow.once('closed', closedHandler);
         });
-        const win = window.open(DefaultSecondaryWindowService.SECONDARY_WINDOW_URL, id);
-        return win ?? undefined;
+        win = window.open(DefaultSecondaryWindowService.SECONDARY_WINDOW_URL, id, 'popup') || undefined;
+        return win;
     }
 
     override focus(win: Window): void {
@@ -54,4 +75,13 @@ export class ElectronSecondaryWindowService extends DefaultSecondaryWindowServic
             console.warn(`There is no known secondary window '${win.name}'. Thus, the window could not be focussed.`);
         }
     }
+
+    safeToClose(windowId: string): Promise<boolean> {
+        const closingHandler = this.electronWindowsById.get(windowId);
+        if (closingHandler) {
+            return closingHandler!();
+        } else {
+            return Promise.resolve(true);
+        }
+    }
 }
diff --git a/packages/core/src/electron-common/messaging/electron-messages.ts b/packages/core/src/electron-common/messaging/electron-messages.ts
@@ -26,6 +26,10 @@ export const Restart = 'restart';
  * Emitted by main when close requested.
  */
 export const CLOSE_REQUESTED_SIGNAL = 'close-requested';
+/**
+ * Emitted by main when close requested.
+ */
+export const CLOSE_SECONDARY_REQUESTED_SIGNAL = 'close-secondary-requested';
 /**
  * Emitted by window when a reload is requested.
  */
@@ -40,3 +44,9 @@ export interface CloseRequestArguments {
     cancelChannel: string;
     reason: StopReason;
 }
+
+export interface CloseSecondaryRequestArguments {
+    windowId: string;
+    confirmChannel: string;
+    cancelChannel: string;
+}