Skip to content

Commit

Permalink
GH-6530: Set the shutdown hook to beforeunload.
Browse files Browse the repository at this point in the history 
 - `unload` was not triggered in electron. (See: electron/electron#7278 (comment))
 - Exposed `ShellLayoutRestorer` APIs.
 - Added logging.
 - Fixed typos.

Closes #6530.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
  • Loading branch information
Akos Kitta committed Nov 12, 2019
1 parent a8f750a commit afe6b66
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 17 deletions.
8 changes: 7 additions & 1 deletion packages/core/src/browser/frontend-application-state.ts
View file
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
View file
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
View file
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: Command = {
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_COMMAND, {
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_COMMAND.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

0 comments on commit afe6b66

Please sign in to comment.