Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-6530: Set the shutdown hook to beforeunload. #6532

Merged
merged 1 commit into from
Nov 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## v0.13.0

- [core] Switched the frontend application's shutdown hook from `window.unload` to `window.beforeunload`. [#6530](https://github.com/eclipse-theia/theia/issues/6530)
- [scm] added handling when opening diff-editors to respect preference `workbench.list.openMode` [#6481](https://github.com/eclipse-theia/theia/pull/6481)

Breaking changes:
Expand Down
8 changes: 7 additions & 1 deletion packages/core/src/browser/frontend-application-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { injectable } from 'inversify';
import { injectable, inject } from 'inversify';
import { Emitter, Event } from '../common/event';
import { Deferred } from '../common/promise-util';
import { ILogger } from '../common/logger';

export type FrontendApplicationState =
'init'
Expand All @@ -29,6 +30,9 @@ export type FrontendApplicationState =
@injectable()
export class FrontendApplicationStateService {

@inject(ILogger)
protected readonly logger: ILogger;

private _state: FrontendApplicationState = 'init';

protected deferred: { [state: string]: Deferred<void> } = {};
Expand All @@ -43,11 +47,13 @@ export class FrontendApplicationStateService {
if (this.deferred[this._state] === undefined) {
this.deferred[this._state] = new Deferred();
}
const oldState = this._state;
this._state = state;
if (this.deferred[state] === undefined) {
this.deferred[state] = new Deferred();
}
this.deferred[state].resolve();
this.logger.info(`Changed application state from '${oldState}' to '${this._state}'.`);
this.stateChanged.fire(state);
}
}
Expand Down
6 changes: 4 additions & 2 deletions packages/core/src/browser/frontend-application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export interface FrontendApplicationContribution {
/**
* Called when an application is stopped or unloaded.
*
* Note that this is implemented using `window.unload` which doesn't allow any asynchronous code anymore.
* Note that this is implemented using `window.beforeunload` which doesn't allow any asynchronous code anymore.
* I.e. this is the last tick.
*/
onStop?(app: FrontendApplication): void;
Expand Down Expand Up @@ -161,7 +161,7 @@ export class FrontendApplication {
* Register global event listeners.
*/
protected registerEventListeners(): void {
window.addEventListener('unload', () => {
window.addEventListener('beforeunload', () => {
this.stateService.state = 'closing_window';
this.layoutRestorer.storeLayout(this);
this.stopContributions();
Expand Down Expand Up @@ -320,6 +320,7 @@ export class FrontendApplication {
* Stop the frontend application contributions. This is called when the window is unloaded.
*/
protected stopContributions(): void {
this.logger.info('>>> Stopping contributions....');
for (const contribution of this.contributions.getContributions()) {
if (contribution.onStop) {
try {
Expand All @@ -329,6 +330,7 @@ export class FrontendApplication {
}
}
}
this.logger.info('<<< All contributions have been stopped.');
}

protected async measure<T>(name: string, fn: () => MaybePromise<T>): Promise<T> {
Expand Down
38 changes: 24 additions & 14 deletions packages/core/src/browser/shell/shell-layout-restorer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { FrontendApplication } from '../frontend-application';
import { WidgetManager, WidgetConstructionOptions } from '../widget-manager';
import { StorageService } from '../storage-service';
import { ILogger } from '../../common/logger';
import { CommandContribution, CommandRegistry } from '../../common/command';
import { CommandContribution, CommandRegistry, Command } from '../../common/command';
import { ThemeService } from '../theming';
import { ContributionProvider } from '../../common/contribution-provider';
import { MaybePromise } from '../../common/types';
Expand Down Expand Up @@ -109,16 +109,17 @@ export interface ApplicationShellLayoutMigration {
onWillInflateWidget?(desc: WidgetDescription, context: ApplicationShellLayoutMigrationContext): MaybePromise<WidgetDescription | undefined>;
}

const resetLayoutCommand = {
export const RESET_LAYOUT: Command = {
vince-fugnitto marked this conversation as resolved.
Show resolved Hide resolved
id: 'reset.layout',
category: 'View',
label: 'Reset Workbench Layout'
};

@injectable()
export class ShellLayoutRestorer implements CommandContribution {
private storageKey = 'layout';
private shouldStoreLayout: boolean = true;

protected storageKey = 'layout';
protected shouldStoreLayout: boolean = true;

@inject(ContributionProvider) @named(ApplicationShellLayoutMigration)
protected readonly migrations: ContributionProvider<ApplicationShellLayoutMigration>;
Expand All @@ -129,22 +130,28 @@ export class ShellLayoutRestorer implements CommandContribution {
@inject(StorageService) protected storageService: StorageService) { }

registerCommands(commands: CommandRegistry): void {
commands.registerCommand(resetLayoutCommand, {
execute: async () => {
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.
window.location.reload(true);
}
commands.registerCommand(RESET_LAYOUT, {
execute: async () => this.resetLayout()
});
}

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);
}

storeLayout(app: FrontendApplication): void {
if (this.shouldStoreLayout) {
try {
this.logger.info('>>> Storing the layout...');
const layoutData = app.shell.getLayoutData();
const serializedLayoutData = this.deflate(layoutData);
this.storageService.setData(this.storageKey, serializedLayoutData);
this.logger.info('<<< The layout has been successfully stored.');
} catch (error) {
this.storageService.setData(this.storageKey, undefined);
this.logger.error('Error during serialization of layout data', error);
Expand All @@ -153,12 +160,15 @@ export class ShellLayoutRestorer implements CommandContribution {
}

async restoreLayout(app: FrontendApplication): Promise<boolean> {
this.logger.info('>>> Restoring the layout state...');
const serializedLayoutData = await this.storageService.getData<string>(this.storageKey);
if (serializedLayoutData === undefined) {
this.logger.info('<<< Nothing to restore.');
return false;
}
const layoutData = await this.inflate(serializedLayoutData);
await app.shell.setLayoutData(layoutData);
this.logger.info('<<< The layout has been successfully restored.');
return true;
}

Expand Down Expand Up @@ -227,7 +237,7 @@ export class ShellLayoutRestorer implements CommandContribution {
} else {
console.warn(`Layout version ${layoutVersion} is ahead current layout version ${applicationShellLayoutVersion}, trying to load anyway...`);
}
console.info(`Please use '${resetLayoutCommand.label}' command if the layout looks bogus.`);
console.info(`Please use '${RESET_LAYOUT.label}' command if the layout looks bogus.`);
}

const migrations = this.migrations.getContributions()
Expand All @@ -246,7 +256,7 @@ export class ShellLayoutRestorer implements CommandContribution {
protected async fireWillInflateLayout(context: ShellLayoutRestorer.InflateContext): Promise<void> {
for (const migration of context.migrations) {
if (migration.onWillInflateLayout) {
// don't catch exceptions, if one migrarion fails all should fail.
// don't catch exceptions, if one migration fails all should fail.
await migration.onWillInflateLayout(context);
}
}
Expand Down Expand Up @@ -284,7 +294,7 @@ export class ShellLayoutRestorer implements CommandContribution {
protected async fireWillInflateWidget(desc: WidgetDescription, context: ShellLayoutRestorer.InflateContext): Promise<WidgetDescription> {
for (const migration of context.migrations) {
if (migration.onWillInflateWidget) {
// don't catch exceptions, if one migrarion fails all should fail.
// don't catch exceptions, if one migration fails all should fail.
const migrated = await migration.onWillInflateWidget(desc, context);
if (migrated) {
if (migrated.innerWidgetState && typeof migrated.innerWidgetState !== 'string') {
Expand Down