Validate backup workspaces on startup

src/vs/code/electron-main/lifecycle.ts

@@ -27,9 +27,6 @@ export class LifecycleService implements ILifecycleMainService {
 	private oneTimeListenerTokenGenerator: number;
 	private _wasUpdated: boolean;
 
-	private _onBeforeUnload = new Emitter<IVSCodeWindow>();
-	onBeforeUnload: Event<IVSCodeWindow> = this._onBeforeUnload.event;
-
 	private _onBeforeQuit = new Emitter<void>();
 	onBeforeQuit: Event<void> = this._onBeforeQuit.event;
 
@@ -133,8 +130,6 @@ export class LifecycleService implements ILifecycleMainService {
 			const oneTimeCancelEvent = 'vscode:cancel' + oneTimeEventToken;
 
 			ipc.once(oneTimeOkEvent, () => {
-				this._onBeforeUnload.fire(vscodeWindow);
-
 				c(false); // no veto
 			});
 

src/vs/code/electron-main/windows.ts

@@ -357,11 +357,6 @@ export class WindowsManager implements IWindowsMainService {
 		if (openConfig.initialStartup && !openConfig.cli.extensionDevelopmentPath) {
 			const workspacesWithBackups = this.backupService.getWorkspaceBackupPaths();
 			workspacesWithBackups.forEach(workspacePath => {
-				if (!fs.existsSync(workspacePath)) {
-					this.backupService.removeWorkspaceBackupPathSync(Uri.file(workspacePath));
-					return;
-				}
-
 				const configuration = this.toConfiguration(openConfig, workspacePath);
 				const browserWindow = this.openInBrowserWindow(configuration, true /* new window */);
 				usedWindows.push(browserWindow);

src/vs/code/test/electron-main/servicesTestUtils.ts

@@ -11,9 +11,6 @@ import Event, { Emitter } from 'vs/base/common/event';
 export class TestLifecycleService implements ILifecycleMainService {
 	public _serviceBrand: any;
 
-	private _onBeforeUnload = new Emitter<IVSCodeWindow>();
-	onBeforeUnload: Event<IVSCodeWindow> = this._onBeforeUnload.event;
-
 	private _onBeforeQuit = new Emitter<void>();
 	onBeforeQuit: Event<void> = this._onBeforeQuit.event;
 

src/vs/platform/backup/common/backup.ts

@@ -28,20 +28,4 @@ export interface IBackupMainService {
 	 * @param workspaces The workspaces to add.
 	 */
 	pushWorkspaceBackupPathsSync(workspaces: Uri[]): void;
-
-	/**
-	 * Removes a workspace backup path being tracked for restoration.
-	 *
-	 * @param workspace The workspace to remove.
-	 */
-	removeWorkspaceBackupPathSync(workspace: Uri): void;
-
-	/**
-	 * Gets whether the workspace has backup(s) associated with it (ie. if the workspace backup
-	 * directory exists).
-	 *
-	 * @param workspace The workspace to evaluate.
-	 * @return Whether the workspace has backups.
-	 */
-	hasWorkspaceBackup(workspace: Uri): boolean;
 }

src/vs/platform/backup/electron-main/backupMainService.ts

@@ -3,14 +3,13 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import * as crypto from 'crypto';
 import * as fs from 'fs';
 import * as path from 'path';
+import * as crypto from 'crypto';
+import * as extfs from 'vs/base/node/extfs';
 import Uri from 'vs/base/common/uri';
 import { IBackupWorkspacesFormat, IBackupMainService } from 'vs/platform/backup/common/backup';
 import { IEnvironmentService } from 'vs/platform/environment/common/environment';
-import { ILifecycleMainService } from 'vs/platform/lifecycle/common/mainLifecycle';
-import { VSCodeWindow } from 'vs/code/electron-main/window';
 
 export class BackupMainService implements IBackupMainService {
 
@@ -19,86 +18,107 @@ export class BackupMainService implements IBackupMainService {
 	protected backupHome: string;
 	protected workspacesJsonPath: string;
 
-	private workspacesJsonContent: IBackupWorkspacesFormat;
+	private backups: IBackupWorkspacesFormat;
 
 	constructor(
-		@IEnvironmentService environmentService: IEnvironmentService,
-		@ILifecycleMainService lifecycleService: ILifecycleMainService
+		@IEnvironmentService environmentService: IEnvironmentService
 	) {
 		this.backupHome = environmentService.backupHome;
 		this.workspacesJsonPath = environmentService.backupWorkspacesPath;
 
-		lifecycleService.onBeforeUnload(this.onBeforeUnloadWindow.bind(this));
-
 		this.loadSync();
 	}
 
-	private onBeforeUnloadWindow(vscodeWindow: VSCodeWindow) {
-		if (vscodeWindow.openedWorkspacePath) {
-			// Clear out workspace from workspaces.json if it doesn't have any backups
-			const workspaceResource = Uri.file(vscodeWindow.openedWorkspacePath);
-			if (!this.hasWorkspaceBackup(workspaceResource)) {
-				this.removeWorkspaceBackupPathSync(workspaceResource);
-			}
-		}
-	}
-
 	public getWorkspaceBackupPaths(): string[] {
-		return this.workspacesJsonContent.folderWorkspaces;
+		return this.backups.folderWorkspaces;
 	}
 
 	public pushWorkspaceBackupPathsSync(workspaces: Uri[]): void {
+		let needsSaving = false;
 		workspaces.forEach(workspace => {
-			// Hot exit is disabled for empty workspaces
-			if (!workspace) {
-				return;
-			}
-
-			if (this.workspacesJsonContent.folderWorkspaces.indexOf(workspace.fsPath) === -1) {
-				this.workspacesJsonContent.folderWorkspaces.push(workspace.fsPath);
+			if (this.backups.folderWorkspaces.indexOf(workspace.fsPath) === -1) {
+				this.backups.folderWorkspaces.push(workspace.fsPath);
+				needsSaving = true;
 			}
 		});
-		this.saveSync();
+
+		if (needsSaving) {
+			this.saveSync();
+		}
 	}
 
-	public removeWorkspaceBackupPathSync(workspace: Uri): void {
-		if (!this.workspacesJsonContent.folderWorkspaces) {
+	protected removeWorkspaceBackupPathSync(workspace: Uri): void {
+		if (!this.backups.folderWorkspaces) {
 			return;
 		}
-		const index = this.workspacesJsonContent.folderWorkspaces.indexOf(workspace.fsPath);
+		const index = this.backups.folderWorkspaces.indexOf(workspace.fsPath);
 		if (index === -1) {
 			return;
 		}
-		this.workspacesJsonContent.folderWorkspaces.splice(index, 1);
+		this.backups.folderWorkspaces.splice(index, 1);
 		this.saveSync();
 	}
 
-	public hasWorkspaceBackup(workspace: Uri): boolean {
-		return fs.existsSync(this.getWorkspaceBackupDirectory(workspace));
-	}
-
-	private getWorkspaceBackupDirectory(workspace: Uri): string {
-		const workspaceHash = crypto.createHash('md5').update(workspace.fsPath).digest('hex');
-		return path.join(this.backupHome, workspaceHash);
-	}
-
 	protected loadSync(): void {
+		let backups: IBackupWorkspacesFormat;
 		try {
-			this.workspacesJsonContent = JSON.parse(fs.readFileSync(this.workspacesJsonPath, 'utf8').toString()); // invalid JSON or permission issue can happen here
+			backups = JSON.parse(fs.readFileSync(this.workspacesJsonPath, 'utf8').toString()); // invalid JSON or permission issue can happen here
 		} catch (error) {
-			this.workspacesJsonContent = Object.create(null);
+			backups = Object.create(null);
 		}
 
 		// Ensure folderWorkspaces is a string[]
-		if (this.workspacesJsonContent.folderWorkspaces) {
-			const fws = this.workspacesJsonContent.folderWorkspaces;
+		if (backups.folderWorkspaces) {
+			const fws = backups.folderWorkspaces;
 			if (!Array.isArray(fws) || fws.some(f => typeof f !== 'string')) {
-				this.workspacesJsonContent = Object.create(null);
+				backups = Object.create(null);
 			}
 		}
 
-		if (!this.workspacesJsonContent.folderWorkspaces) {
-			this.workspacesJsonContent.folderWorkspaces = [];
+		if (!backups.folderWorkspaces) {
+			backups.folderWorkspaces = [];
+		}
+
+		this.backups = backups;
+
+		// Validate backup workspaces
+		this.validateBackupWorkspaces(backups);
+	}
+
+	private validateBackupWorkspaces(backups: IBackupWorkspacesFormat): void {
+		const staleBackupWorkspaces: { workspacePath: string; backupPath: string; }[] = [];
+
+		const backupWorkspaces = backups.folderWorkspaces;
+		backupWorkspaces.forEach(workspacePath => {
+			const backupPath = this.toBackupPath(workspacePath);
+			if (!this.hasBackupsSync(backupPath)) {
+				staleBackupWorkspaces.push({ workspacePath, backupPath });
+			}
+		});
+
+		staleBackupWorkspaces.forEach(staleBackupWorkspace => {
+			const {backupPath, workspacePath} = staleBackupWorkspace;
+			extfs.delSync(backupPath);
+			this.removeWorkspaceBackupPathSync(Uri.file(workspacePath));
+		});
+	}
+
+	private hasBackupsSync(backupPath): boolean {
+		try {
+			const backupSchemas = extfs.readdirSync(backupPath);
+			if (backupSchemas.length === 0) {
+				return false; // empty backups
+			}
+
+			return backupSchemas.some(backupSchema => {
+				try {
+					return extfs.readdirSync(path.join(backupPath, backupSchema)).length > 0;
+				} catch (error) {
+					return false; // invalid folder
+				}
+			});
+		} catch (error) {
+			return false; // backup path does not exist
 		}
 	}
 
@@ -108,9 +128,15 @@ export class BackupMainService implements IBackupMainService {
 			if (!fs.existsSync(this.backupHome)) {
 				fs.mkdirSync(this.backupHome);
 			}
-			fs.writeFileSync(this.workspacesJsonPath, JSON.stringify(this.workspacesJsonContent));
+			fs.writeFileSync(this.workspacesJsonPath, JSON.stringify(this.backups));
 		} catch (ex) {
 			console.error('Could not save workspaces.json', ex);
 		}
 	}
+
+	protected toBackupPath(workspacePath: string): string {
+		const workspaceHash = crypto.createHash('md5').update(workspacePath).digest('hex');
+
+		return path.join(this.backupHome, workspaceHash);
+	}
 }

src/vs/platform/backup/test/electron-main/backupMainService.test.ts

@@ -7,7 +7,6 @@
 
 import * as assert from 'assert';
 import * as platform from 'vs/base/common/platform';
-import crypto = require('crypto');
 import fs = require('fs');
 import os = require('os');
 import path = require('path');
@@ -15,13 +14,12 @@ import extfs = require('vs/base/node/extfs');
 import pfs = require('vs/base/node/pfs');
 import Uri from 'vs/base/common/uri';
 import { TestEnvironmentService } from 'vs/test/utils/servicesTestUtils';
-import { TestLifecycleService } from 'vs/code/test/electron-main/servicesTestUtils';
 import { BackupMainService } from 'vs/platform/backup/electron-main/backupMainService';
 import { IBackupWorkspacesFormat } from 'vs/platform/backup/common/backup';
 
 class TestBackupMainService extends BackupMainService {
 	constructor(backupHome: string, backupWorkspacesPath: string) {
-		super(TestEnvironmentService, new TestLifecycleService());
+		super(TestEnvironmentService);
 
 		this.backupHome = backupHome;
 		this.workspacesJsonPath = backupWorkspacesPath;
@@ -29,6 +27,18 @@ class TestBackupMainService extends BackupMainService {
 		// Force a reload with the new paths
 		this.loadSync();
 	}
+
+	public removeWorkspaceBackupPathSync(workspace: Uri): void {
+		return super.removeWorkspaceBackupPathSync(workspace);
+	}
+
+	public loadSync(): void {
+		super.loadSync();
+	}
+
+	public toBackupPath(workspacePath: string): string {
+		return super.toBackupPath(workspacePath);
+	}
 }
 
 suite('BackupMainService', () => {
@@ -39,9 +49,7 @@ suite('BackupMainService', () => {
 	const fooFile = Uri.file(platform.isWindows ? 'C:\\foo' : '/foo');
 	const barFile = Uri.file(platform.isWindows ? 'C:\\bar' : '/bar');
 
-	const fooWorkspaceBackupDir = path.join(backupHome, crypto.createHash('md5').update(fooFile.fsPath).digest('hex'));
-
-	let service: BackupMainService;
+	let service: TestBackupMainService;
 
 	setup(done => {
 		service = new TestBackupMainService(backupHome, backupWorkspacesPath);
@@ -123,9 +131,33 @@ suite('BackupMainService', () => {
 		});
 	});
 
-	test('doesWorkspaceHaveBackups should return whether the workspace\'s backup exists', () => {
-		assert.equal(service.hasWorkspaceBackup(fooFile), false);
-		fs.mkdirSync(fooWorkspaceBackupDir);
-		assert.equal(service.hasWorkspaceBackup(fooFile), true);
+	test('service validates backup workspaces on startup and cleans up', done => {
+
+		// 1) backup workspace path does not exist
+		service.pushWorkspaceBackupPathsSync([fooFile, barFile]);
+		service.loadSync();
+		assert.equal(service.getWorkspaceBackupPaths().length, 0);
+
+		// 2) backup workspace path exists with empty contents within
+		fs.mkdirSync(service.toBackupPath(fooFile.fsPath));
+		fs.mkdirSync(service.toBackupPath(barFile.fsPath));
+		service.pushWorkspaceBackupPathsSync([fooFile, barFile]);
+		service.loadSync();
+		assert.equal(service.getWorkspaceBackupPaths().length, 0);
+		assert.ok(!fs.exists(service.toBackupPath(fooFile.fsPath)));
+		assert.ok(!fs.exists(service.toBackupPath(barFile.fsPath)));
+
+		// 3) backup workspace path exists with empty folders within
+		fs.mkdirSync(service.toBackupPath(fooFile.fsPath));
+		fs.mkdirSync(service.toBackupPath(barFile.fsPath));
+		fs.mkdirSync(path.join(service.toBackupPath(fooFile.fsPath), 'file'));
+		fs.mkdirSync(path.join(service.toBackupPath(barFile.fsPath), 'untitled'));
+		service.pushWorkspaceBackupPathsSync([fooFile, barFile]);
+		service.loadSync();
+		assert.equal(service.getWorkspaceBackupPaths().length, 0);
+		assert.ok(!fs.exists(service.toBackupPath(fooFile.fsPath)));
+		assert.ok(!fs.exists(service.toBackupPath(barFile.fsPath)));
+
+		done();
 	});
 });
\ No newline at end of file

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

@@ -19,12 +19,6 @@ export interface ILifecycleMainService {
 	 */
 	wasUpdated: boolean;
 
-	/**
-	 * Fired before the window unloads. This can either happen as a matter of closing the
-	 * window or when the window is being reloaded.
-	 */
-	onBeforeUnload: Event<IVSCodeWindow>;
-
 	/**
 	 * Due to the way we handle lifecycle with eventing, the general app.on('before-quit')
 	 * event cannot be used because it can be called twice on shutdown. Instead the onBeforeQuit