Skip to content

Commit 98467c3

Browse files
author
Benjamin Pasero
committed
web - storage/lifecycle 💄
1 parent 70dc559 commit 98467c3

6 files changed

Lines changed: 34 additions & 22 deletions

File tree

‎src/vs/base/browser/dom.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -855,6 +855,7 @@ export const EventType = {
855855
KEY_UP: 'keyup',
856856
// HTML Document
857857
LOAD: 'load',
858+
BEFORE_UNLOAD: 'beforeunload',
858859
UNLOAD: 'unload',
859860
ABORT: 'abort',
860861
ERROR: 'error',

‎src/vs/platform/lifecycle/browser/lifecycleService.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { ILogService } from 'vs/platform/log/common/log';
88
import { AbstractLifecycleService } from 'vs/platform/lifecycle/common/lifecycleService';
99
import { localize } from 'vs/nls';
1010
import { ServiceIdentifier } from 'vs/platform/instantiation/common/instantiation';
11+
import { addDisposableListener, EventType } from 'vs/base/browser/dom';
1112

1213
export class BrowserLifecycleService extends AbstractLifecycleService {
1314

@@ -22,10 +23,10 @@ export class BrowserLifecycleService extends AbstractLifecycleService {
2223
}
2324

2425
private registerListeners(): void {
25-
window.onbeforeunload = () => this.beforeUnload();
26+
addDisposableListener(window, EventType.BEFORE_UNLOAD, () => this.onBeforeUnload());
2627
}
2728

28-
private beforeUnload(): string | null {
29+
private onBeforeUnload(): string | null {
2930
let veto = false;
3031

3132
// Before Shutdown
@@ -34,7 +35,7 @@ export class BrowserLifecycleService extends AbstractLifecycleService {
3435
if (value === true) {
3536
veto = true;
3637
} else if (value instanceof Promise && !veto) {
37-
console.warn(new Error('Long running onBeforeShutdown currently not supported'));
38+
console.warn(new Error('Long running onBeforeShutdown currently not supported in the web'));
3839
veto = true;
3940
}
4041
},
@@ -49,11 +50,14 @@ export class BrowserLifecycleService extends AbstractLifecycleService {
4950
// No Veto: continue with Will Shutdown
5051
this._onWillShutdown.fire({
5152
join() {
52-
console.warn(new Error('Long running onWillShutdown currently not supported'));
53+
console.warn(new Error('Long running onWillShutdown currently not supported in the web'));
5354
},
5455
reason: ShutdownReason.QUIT
5556
});
5657

58+
// Finally end with Shutdown event
59+
this._onShutdown.fire();
60+
5761
return null;
5862
}
5963
}

‎src/vs/platform/storage/browser/storageService.ts‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ export class BrowserStorageService extends Disposable implements IStorageService
3737
private workspaceStorageFile: URI;
3838

3939
private initializePromise: Promise<void>;
40-
private periodicSaveScheduler = this._register(new RunOnceScheduler(() => this.saveState(), 5000));
40+
private periodicSaveScheduler = this._register(new RunOnceScheduler(() => this.collectState(), 5000));
4141

4242
get hasPendingUpdate(): boolean {
4343
return this.globalStorageDatabase.hasPendingUpdate || this.workspaceStorageDatabase.hasPendingUpdate;
@@ -57,14 +57,19 @@ export class BrowserStorageService extends Disposable implements IStorageService
5757
this.periodicSaveScheduler.schedule();
5858
}
5959

60-
private saveState(): void {
60+
private collectState(): void {
6161
runWhenIdle(() => {
6262

6363
// this event will potentially cause new state to be stored
6464
// since new state will only be created while the document
6565
// has focus, one optimization is to not run this when the
6666
// document has no focus, assuming that state has not changed
67-
if (document.hasFocus()) {
67+
//
68+
// another optimization is to not collect more state if we
69+
// have a pending update already running which indicates
70+
// that the connection is either slow or disconnected and
71+
// thus unhealthy.
72+
if (document.hasFocus() && !this.hasPendingUpdate) {
6873
this._onWillSaveState.fire({ reason: WillSaveStateReason.NONE });
6974
}
7075

@@ -197,7 +202,7 @@ export class FileStorageDatabase extends Disposable implements IStorageDatabase
197202
this._register(this.fileService.watch(this.file));
198203
this._register(this.fileService.onFileChanges(e => {
199204
if (document.hasFocus()) {
200-
return; // ignore changes from ourselves by checking for focus
205+
return; // optimization: ignore changes from ourselves by checking for focus
201206
}
202207

203208
if (!e.contains(this.file, FileChangeType.UPDATED)) {
@@ -251,15 +256,17 @@ export class FileStorageDatabase extends Disposable implements IStorageDatabase
251256

252257
await this.pendingUpdate;
253258

254-
this._hasPendingUpdate = true;
259+
this.pendingUpdate = (async () => {
260+
try {
261+
this._hasPendingUpdate = true;
262+
263+
await this.fileService.writeFile(this.file, VSBuffer.fromString(JSON.stringify(mapToSerializable(items))));
255264

256-
this.pendingUpdate = this.fileService.writeFile(this.file, VSBuffer.fromString(JSON.stringify(mapToSerializable(items))))
257-
.then(() => {
258265
this.ensureWatching(); // now that the file must exist, ensure we watch it for changes
259-
})
260-
.finally(() => {
266+
} finally {
261267
this._hasPendingUpdate = false;
262-
});
268+
}
269+
})();
263270

264271
return this.pendingUpdate;
265272
}

‎src/vs/workbench/browser/workbench.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,12 @@ export class Workbench extends Layout {
5050
private readonly _onBeforeShutdown = this._register(new Emitter<BeforeShutdownEvent>());
5151
readonly onBeforeShutdown: Event<BeforeShutdownEvent> = this._onBeforeShutdown.event;
5252

53-
private readonly _onShutdown = this._register(new Emitter<void>());
54-
readonly onShutdown: Event<void> = this._onShutdown.event;
55-
5653
private readonly _onWillShutdown = this._register(new Emitter<WillShutdownEvent>());
5754
readonly onWillShutdown: Event<WillShutdownEvent> = this._onWillShutdown.event;
5855

56+
private readonly _onShutdown = this._register(new Emitter<void>());
57+
readonly onShutdown: Event<void> = this._onShutdown.event;
58+
5959
constructor(
6060
parent: HTMLElement,
6161
private readonly serviceCollection: ServiceCollection,

‎src/vs/workbench/services/textfile/browser/textFileService.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,15 @@ export class BrowserTextFileService extends TextFileService {
1717
}
1818
};
1919

20-
protected beforeShutdown(reason: ShutdownReason): boolean {
20+
protected onBeforeShutdown(reason: ShutdownReason): boolean {
2121
// Web: we cannot perform long running in the shutdown phase
2222
// As such we need to check sync if there are any dirty files
2323
// that have not been backed up yet and then prevent the shutdown
2424
// if that is the case.
25-
return this.doBeforeShutdownSync(reason);
25+
return this.doBeforeShutdownSync();
2626
}
2727

28-
private doBeforeShutdownSync(reason: ShutdownReason): boolean {
28+
private doBeforeShutdownSync(): boolean {
2929
const dirtyResources = this.getDirty();
3030
if (!dirtyResources.length) {
3131
return false; // no dirty: no veto

‎src/vs/workbench/services/textfile/common/textFileService.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ export abstract class TextFileService extends Disposable implements ITextFileSer
107107
private registerListeners(): void {
108108

109109
// Lifecycle
110-
this.lifecycleService.onBeforeShutdown(event => event.veto(this.beforeShutdown(event.reason)));
110+
this.lifecycleService.onBeforeShutdown(event => event.veto(this.onBeforeShutdown(event.reason)));
111111
this.lifecycleService.onShutdown(this.dispose, this);
112112

113113
// Files configuration changes
@@ -118,7 +118,7 @@ export abstract class TextFileService extends Disposable implements ITextFileSer
118118
}));
119119
}
120120

121-
protected beforeShutdown(reason: ShutdownReason): boolean | Promise<boolean> {
121+
protected onBeforeShutdown(reason: ShutdownReason): boolean | Promise<boolean> {
122122

123123
// Dirty files need treatment on shutdown
124124
const dirty = this.getDirty();

0 commit comments

Comments
 (0)