Skip to content

Commit

Permalink
Extends multi-window support to Electron (eclipse-theia#11642)
Browse files Browse the repository at this point in the history
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.

Contributed on behalf of ST Microelectronics.

Signed-off-by: Thomas Mäder <t.s.maeder@gmail.com>
  • Loading branch information
tsmaeder committed Sep 14, 2022
1 parent 98a0bf6 commit 1b824eb
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 39 deletions.
1 change: 1 addition & 0 deletions examples/electron/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
"@theia/scm": "1.29.0",
"@theia/scm-extra": "1.29.0",
"@theia/search-in-workspace": "1.29.0",
"@theia/secondary-window": "1.29.0",
"@theia/task": "1.29.0",
"@theia/terminal": "1.29.0",
"@theia/timeline": "1.29.0",
Expand Down
18 changes: 11 additions & 7 deletions packages/core/src/browser/secondary-window-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,13 +131,17 @@ 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(
async () => {
await this.applicationShell.closeWidget(widget.id);
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.');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,14 @@ import { inject, injectable, postConstruct } from 'inversify';
import { SecondaryWindowService } from './secondary-window-service';
import { WindowService } from './window-service';

enum ClosingState {
initial,
inProgress,
readyToClose
}
@injectable()
export class DefaultSecondaryWindowService implements SecondaryWindowService {

// secondary-window.html is part of Theia's generated code. It is generated by dev-packages/application-manager/src/generator/frontend-generator.ts
protected static SECONDARY_WINDOW_URL = 'secondary-window.html';

Expand Down Expand Up @@ -46,33 +52,51 @@ export class DefaultSecondaryWindowService implements SecondaryWindowService {
});
}

createSecondaryWindow(onClose?: (closedWin: Window) => void): Window | undefined {
const win = this.doCreateSecondaryWindow(onClose);
createSecondaryWindow(onClosing?: (win: Window) => Promise<boolean>, onClosed?: (closedWin: Window) => void): Window | undefined {
const id = this.nextWindowId();
const win = this.doCreateSecondaryWindow(id);
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');
if (win) {
let closingState = ClosingState.initial;
// 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('unload', () => {
this.handleWindowClosed(win, onClose);
win.addEventListener('closed', () => {
const extIndex = this.secondaryWindows.indexOf(win);
if (extIndex > -1) {
this.secondaryWindows.splice(extIndex, 1);
};
});

win.addEventListener('beforeunload', event => {
if (closingState === ClosingState.initial) {
closingState = ClosingState.inProgress;
if (onClosing) {
onClosing(win).then(shouldClose => {
if (shouldClose) {
closingState = ClosingState.readyToClose;
win.close();
}
});
event.preventDefault();
event.returnValue = false;
return false;
}
} else if (closingState === ClosingState.inProgress) {
// When the extracted widget is disposed programmatically, a dispose listener on it will try to close the window.
// if we dispose the widget because of closing the window, we'll get a recursive call to window.close()
event.preventDefault();
event.returnValue = false;
return false;
}
});
});
}
return win ?? undefined;
return win;
}

protected handleWindowClosed(win: Window, onClose?: (closedWin: Window) => void): void {
const extIndex = this.secondaryWindows.indexOf(win);
if (extIndex > -1) {
this.secondaryWindows.splice(extIndex, 1);
};
onClose?.(win);
protected doCreateSecondaryWindow(id: string): Window | undefined {
const win = window.open(DefaultSecondaryWindowService.SECONDARY_WINDOW_URL, id, 'popup');
return win ?? undefined;
}

focus(win: Window): void {
Expand Down
5 changes: 3 additions & 2 deletions packages/core/src/browser/window/secondary-window-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,11 @@ 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 onClosing optional callback to be executed before closing. 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(onClosing?: (win: Window) => Promise<boolean>, onClosed?: (win: Window) => void): Window | undefined;

/** Handles focussing the given secondary window in the browser and on Electron. */
focus(win: Window): void;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,18 @@ import { DefaultSecondaryWindowService } from '../../browser/window/default-seco

@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();

protected override doCreateSecondaryWindow(id: string): Window | undefined {
electronRemote.getCurrentWindow().webContents.once('did-create-window', newElectronWindow => {
newElectronWindow.setMenuBarVisibility(false);
this.electronWindows.set(id, newElectronWindow);
newElectronWindow.on('closed', () => {
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.`);
};
});
});
const win = window.open(DefaultSecondaryWindowService.SECONDARY_WINDOW_URL, id);
return win ?? undefined;
return super.doCreateSecondaryWindow(id);
}

override focus(win: Window): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,6 @@ export class CustomTextEditorModel implements CustomEditorModel {
private readonly toDispose = new DisposableCollection();
private readonly onDirtyChangedEmitter = new Emitter<void>();
readonly onDirtyChanged = this.onDirtyChangedEmitter.event;
readonly autoSave: 'off' | 'afterDelay' | 'onFocusChange' | 'onWindowChange';

static async create(
viewType: string,
Expand Down Expand Up @@ -568,6 +567,14 @@ export class CustomTextEditorModel implements CustomEditorModel {
return this.model.object;
}

get autoSave(): 'off' | 'afterDelay' | 'onFocusChange' | 'onWindowChange' {
return this.editorTextModel.autoSave;
}

get autoSaveDelay(): number {
return this.editorTextModel.autoSaveDelay;
}

revert(options?: Saveable.RevertOptions): Promise<void> {
return this.editorTextModel.revert(options);
}
Expand Down

0 comments on commit 1b824eb

Please sign in to comment.