Skip to content

Commit eeb34e0

Browse files
author
Benjamin Pasero
committed
web - only prevent unload if backups have not completed yet
1 parent 66cdaa1 commit eeb34e0

15 files changed

Lines changed: 143 additions & 50 deletions

File tree

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export class BrowserLifecycleService extends AbstractLifecycleService {
2626
}
2727

2828
private beforeUnload(): string | null {
29-
let veto: boolean = false;
29+
let veto = false;
3030

3131
// Before Shutdown
3232
this._onBeforeShutdown.fire({
@@ -35,6 +35,7 @@ export class BrowserLifecycleService extends AbstractLifecycleService {
3535
veto = true;
3636
} else if (value instanceof Promise && !veto) {
3737
console.warn(new Error('Long running onBeforeShutdown currently not supported'));
38+
veto = true;
3839
}
3940
},
4041
reason: ShutdownReason.QUIT

‎src/vs/platform/lifecycle/common/lifecycle.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { Event } from 'vs/base/common/event';
7-
import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
7+
import { createDecorator, ServiceIdentifier } from 'vs/platform/instantiation/common/instantiation';
88
import { isThenable } from 'vs/base/common/async';
99

1010
export const ILifecycleService = createDecorator<ILifecycleService>('lifecycleService');
@@ -123,7 +123,7 @@ export function LifecyclePhaseToString(phase: LifecyclePhase) {
123123
*/
124124
export interface ILifecycleService {
125125

126-
_serviceBrand: any;
126+
_serviceBrand: ServiceIdentifier<any>;
127127

128128
/**
129129
* Value indicates how this window got loaded.
@@ -166,12 +166,16 @@ export interface ILifecycleService {
166166
}
167167

168168
export const NullLifecycleService: ILifecycleService = {
169-
_serviceBrand: null,
169+
170+
_serviceBrand: null as any,
171+
170172
onBeforeShutdown: Event.None,
171173
onWillShutdown: Event.None,
172174
onShutdown: Event.None,
175+
173176
phase: LifecyclePhase.Restored,
174177
startupKind: StartupKind.NewWindow,
178+
175179
when() { return Promise.resolve(); }
176180
};
177181

‎src/vs/workbench/common/editor/untitledEditorInput.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,14 @@ export class UntitledEditorInput extends EditorInput implements IEncodingSupport
139139
return this.hasAssociatedFilePath;
140140
}
141141

142+
hasBackup(): boolean {
143+
if (this.cachedModel) {
144+
return this.cachedModel.hasBackup();
145+
}
146+
147+
return false;
148+
}
149+
142150
confirmSave(): Promise<ConfirmResult> {
143151
return this.textFileService.confirmSave([this.resource]);
144152
}

‎src/vs/workbench/common/editor/untitledEditorModel.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,10 @@ export class UntitledEditorModel extends BaseTextEditorModel implements IEncodin
133133
return Promise.resolve();
134134
}
135135

136+
hasBackup(): boolean {
137+
return this.backupFileService.hasBackupSync(this.resource, this.versionId);
138+
}
139+
136140
async load(): Promise<UntitledEditorModel & IResolvedTextEditorModel> {
137141

138142
// Check for backups first

‎src/vs/workbench/contrib/backup/common/backupModelTracker.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@ const AUTO_SAVE_AFTER_DELAY_DISABLED_TIME = CONTENT_CHANGE_EVENT_BUFFER_DELAY +
1616

1717
export class BackupModelTracker extends Disposable implements IWorkbenchContribution {
1818

19-
_serviceBrand: any;
20-
2119
private configuredAutoSaveAfterDelay: boolean;
2220

2321
constructor(

‎src/vs/workbench/services/backup/common/backup.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { URI } from 'vs/base/common/uri';
7-
import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
7+
import { createDecorator, ServiceIdentifier } from 'vs/platform/instantiation/common/instantiation';
88
import { ITextBufferFactory, ITextSnapshot } from 'vs/editor/common/model';
99
import { IEnvironmentService } from 'vs/platform/environment/common/environment';
1010
import { joinPath, relativePath } from 'vs/base/common/resources';
@@ -20,13 +20,19 @@ export interface IResolvedBackup<T extends object> {
2020
* A service that handles any I/O and state associated with the backup system.
2121
*/
2222
export interface IBackupFileService {
23-
_serviceBrand: any;
23+
24+
_serviceBrand: ServiceIdentifier<IBackupFileService>;
2425

2526
/**
2627
* Finds out if there are any backups stored.
2728
*/
2829
hasBackups(): Promise<boolean>;
2930

31+
/**
32+
* Finds out if the provided resource with the given version is backed up.
33+
*/
34+
hasBackupSync(resource: URI, versionId?: number): boolean;
35+
3036
/**
3137
* Loads the backup resource for a particular resource within the current workspace.
3238
*

‎src/vs/workbench/services/backup/common/backupFileService.ts‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ export class BackupFileService implements IBackupFileService {
124124

125125
protected hashPath(resource: URI): string {
126126
const str = resource.scheme === Schemas.file || resource.scheme === Schemas.untitled ? resource.fsPath : resource.toString();
127+
127128
return hash(str).toString(16);
128129
}
129130

@@ -137,6 +138,10 @@ export class BackupFileService implements IBackupFileService {
137138
return this.impl.hasBackups();
138139
}
139140

141+
hasBackupSync(resource: URI, versionId?: number): boolean {
142+
return this.impl.hasBackupSync(resource, versionId);
143+
}
144+
140145
loadBackupResource(resource: URI): Promise<URI | undefined> {
141146
return this.impl.loadBackupResource(resource);
142147
}
@@ -172,14 +177,16 @@ class BackupFileServiceImpl implements IBackupFileService {
172177
private static readonly PREAMBLE_META_SEPARATOR = ' '; // using a character that is know to be escaped in a URI as separator
173178
private static readonly PREAMBLE_MAX_LENGTH = 10000;
174179

175-
_serviceBrand: any;
180+
_serviceBrand: ServiceIdentifier<IBackupFileService>;
176181

177182
private backupWorkspacePath: URI;
178183

179184
private isShuttingDown: boolean;
180-
private ready: Promise<IBackupFilesModel>;
181185
private ioOperationQueues: ResourceQueue; // queue IO operations to ensure write order
182186

187+
private ready: Promise<IBackupFilesModel>;
188+
private model: IBackupFilesModel;
189+
183190
constructor(
184191
backupWorkspaceResource: URI,
185192
private readonly hashPath: (resource: URI) => string,
@@ -198,9 +205,9 @@ class BackupFileServiceImpl implements IBackupFileService {
198205
}
199206

200207
private init(): Promise<IBackupFilesModel> {
201-
const model = new BackupFilesModel(this.fileService);
208+
this.model = new BackupFilesModel(this.fileService);
202209

203-
return model.resolve(this.backupWorkspacePath);
210+
return this.model.resolve(this.backupWorkspacePath);
204211
}
205212

206213
async hasBackups(): Promise<boolean> {
@@ -209,6 +216,12 @@ class BackupFileServiceImpl implements IBackupFileService {
209216
return model.count() > 0;
210217
}
211218

219+
hasBackupSync(resource: URI, versionId?: number): boolean {
220+
const backupResource = this.toBackupResource(resource);
221+
222+
return this.model.has(backupResource, versionId);
223+
}
224+
212225
async loadBackupResource(resource: URI): Promise<URI | undefined> {
213226
const model = await this.ready;
214227

@@ -377,6 +390,12 @@ export class InMemoryBackupFileService implements IBackupFileService {
377390
return Promise.resolve(this.backups.size > 0);
378391
}
379392

393+
hasBackupSync(resource: URI, versionId?: number): boolean {
394+
const backupResource = this.toBackupResource(resource);
395+
396+
return this.backups.has(backupResource.toString());
397+
}
398+
380399
loadBackupResource(resource: URI): Promise<URI | undefined> {
381400
const backupResource = this.toBackupResource(resource);
382401
if (this.backups.has(backupResource.toString())) {

‎src/vs/workbench/services/backup/test/node/backupFileService.test.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,20 +155,32 @@ suite('BackupFileService', () => {
155155
assert.equal(fs.readdirSync(path.join(workspaceBackupPath, 'file')).length, 1);
156156
assert.equal(fs.existsSync(fooBackupPath), true);
157157
assert.equal(fs.readFileSync(fooBackupPath), `${fooFile.toString()}\ntest`);
158+
assert.ok(service.hasBackupSync(fooFile));
159+
});
160+
161+
test('text file (with version)', async () => {
162+
await service.backupResource(fooFile, createTextBufferFactory('test').create(DefaultEndOfLine.LF).createSnapshot(false), 666);
163+
assert.equal(fs.readdirSync(path.join(workspaceBackupPath, 'file')).length, 1);
164+
assert.equal(fs.existsSync(fooBackupPath), true);
165+
assert.equal(fs.readFileSync(fooBackupPath), `${fooFile.toString()}\ntest`);
166+
assert.ok(!service.hasBackupSync(fooFile, 555));
167+
assert.ok(service.hasBackupSync(fooFile, 666));
158168
});
159169

160170
test('text file (with meta)', async () => {
161171
await service.backupResource(fooFile, createTextBufferFactory('test').create(DefaultEndOfLine.LF).createSnapshot(false), undefined, { etag: '678', orphaned: true });
162172
assert.equal(fs.readdirSync(path.join(workspaceBackupPath, 'file')).length, 1);
163173
assert.equal(fs.existsSync(fooBackupPath), true);
164174
assert.equal(fs.readFileSync(fooBackupPath).toString(), `${fooFile.toString()} {"etag":"678","orphaned":true}\ntest`);
175+
assert.ok(service.hasBackupSync(fooFile));
165176
});
166177

167178
test('untitled file', async () => {
168179
await service.backupResource(untitledFile, createTextBufferFactory('test').create(DefaultEndOfLine.LF).createSnapshot(false));
169180
assert.equal(fs.readdirSync(path.join(workspaceBackupPath, 'untitled')).length, 1);
170181
assert.equal(fs.existsSync(untitledBackupPath), true);
171182
assert.equal(fs.readFileSync(untitledBackupPath), `${untitledFile.toString()}\ntest`);
183+
assert.ok(service.hasBackupSync(untitledFile));
172184
});
173185

174186
test('text file (ITextSnapshot)', async () => {
@@ -178,6 +190,8 @@ suite('BackupFileService', () => {
178190
assert.equal(fs.readdirSync(path.join(workspaceBackupPath, 'file')).length, 1);
179191
assert.equal(fs.existsSync(fooBackupPath), true);
180192
assert.equal(fs.readFileSync(fooBackupPath), `${fooFile.toString()}\ntest`);
193+
assert.ok(service.hasBackupSync(fooFile));
194+
181195
model.dispose();
182196
});
183197

@@ -188,6 +202,7 @@ suite('BackupFileService', () => {
188202
assert.equal(fs.readdirSync(path.join(workspaceBackupPath, 'untitled')).length, 1);
189203
assert.equal(fs.existsSync(untitledBackupPath), true);
190204
assert.equal(fs.readFileSync(untitledBackupPath), `${untitledFile.toString()}\ntest`);
205+
191206
model.dispose();
192207
});
193208

@@ -199,6 +214,8 @@ suite('BackupFileService', () => {
199214
assert.equal(fs.readdirSync(path.join(workspaceBackupPath, 'file')).length, 1);
200215
assert.equal(fs.existsSync(fooBackupPath), true);
201216
assert.equal(fs.readFileSync(fooBackupPath), `${fooFile.toString()}\n${largeString}`);
217+
assert.ok(service.hasBackupSync(fooFile));
218+
202219
model.dispose();
203220
});
204221

@@ -210,6 +227,8 @@ suite('BackupFileService', () => {
210227
assert.equal(fs.readdirSync(path.join(workspaceBackupPath, 'untitled')).length, 1);
211228
assert.equal(fs.existsSync(untitledBackupPath), true);
212229
assert.equal(fs.readFileSync(untitledBackupPath), `${untitledFile.toString()}\n${largeString}`);
230+
assert.ok(service.hasBackupSync(untitledFile));
231+
213232
model.dispose();
214233
});
215234
});
@@ -218,9 +237,12 @@ suite('BackupFileService', () => {
218237
test('text file', async () => {
219238
await service.backupResource(fooFile, createTextBufferFactory('test').create(DefaultEndOfLine.LF).createSnapshot(false));
220239
assert.equal(fs.readdirSync(path.join(workspaceBackupPath, 'file')).length, 1);
240+
assert.ok(service.hasBackupSync(fooFile));
241+
221242
await service.discardResourceBackup(fooFile);
222243
assert.equal(fs.existsSync(fooBackupPath), false);
223244
assert.equal(fs.readdirSync(path.join(workspaceBackupPath, 'file')).length, 0);
245+
assert.ok(!service.hasBackupSync(fooFile));
224246
});
225247

226248
test('untitled file', async () => {

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

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { TextFileService } from 'vs/workbench/services/textfile/common/textFileS
77
import { ITextFileService, IResourceEncodings, IResourceEncoding } from 'vs/workbench/services/textfile/common/textfiles';
88
import { registerSingleton } from 'vs/platform/instantiation/common/extensions';
99
import { ShutdownReason } from 'vs/platform/lifecycle/common/lifecycle';
10+
import { Schemas } from 'vs/base/common/network';
1011

1112
export class BrowserTextFileService extends TextFileService {
1213

@@ -16,17 +17,36 @@ export class BrowserTextFileService extends TextFileService {
1617
}
1718
};
1819

19-
protected beforeShutdown(reason: ShutdownReason): boolean | Promise<boolean> {
20-
const veto = super.beforeShutdown(reason);
20+
protected beforeShutdown(reason: ShutdownReason): boolean {
21+
return this.doBeforeShutdownSync(reason);
22+
}
23+
24+
private doBeforeShutdownSync(reason: ShutdownReason): boolean {
25+
const dirtyResources = this.getDirty();
26+
if (!dirtyResources.length) {
27+
return false; // no dirty: no veto
28+
}
29+
30+
if (!this.isHotExitEnabled) {
31+
return true; // dirty without backup: veto
32+
}
33+
34+
for (const dirtyResource of dirtyResources) {
35+
let hasBackup = false;
36+
37+
if (this.fileService.canHandleResource(dirtyResource)) {
38+
const model = this.models.get(dirtyResource);
39+
hasBackup = !!(model && model.hasBackup());
40+
} else if (dirtyResource.scheme === Schemas.untitled) {
41+
hasBackup = this.untitledEditorService.hasBackup(dirtyResource);
42+
}
2143

22-
// Web: there is no support for long running unload handlers. As such
23-
// we need to return a direct boolean veto when we detect that there
24-
// are dirty files around.
25-
if (veto instanceof Promise) {
26-
return this.getDirty().length > 0;
44+
if (!hasBackup) {
45+
return true; // dirty without backup: veto
46+
}
2747
}
2848

29-
return veto;
49+
return false; // dirty with backups: no veto
3050
}
3151
}
3252

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,10 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
237237
}
238238
}
239239

240+
hasBackup(): boolean {
241+
return this.backupFileService.hasBackupSync(this.resource, this.versionId);
242+
}
243+
240244
async revert(soft?: boolean): Promise<void> {
241245
if (!this.isResolved()) {
242246
return;

0 commit comments

Comments
 (0)