Update: Make sure restarting VS Code after settings change uses autoUpdater.quitAndInstall (fix #187025) (#189041)

* Update: Make sure restarting VS Code after settings change uses `autoUpdater.quitAndInstall` (fix #187025)

* use a real handler

* 💄

* Update src/vs/platform/lifecycle/electron-main/lifecycleMainService.ts

Co-authored-by: João Moreno <joao.moreno@microsoft.com>

---------

Co-authored-by: João Moreno <joao.moreno@microsoft.com>

src/vs/platform/lifecycle/electron-main/lifecycleMainService.ts

@@ -68,6 +68,20 @@ export interface ShutdownEvent {
 	join(id: string, promise: Promise<void>): void;
 }
 
+export interface IRelaunchHandler {
+
+	/**
+	 * Allows a handler to deal with relaunching the application. The return
+	 * value indicates if the relaunch is handled or not.
+	 */
+	handleRelaunch(options?: IRelaunchOptions): boolean;
+}
+
+export interface IRelaunchOptions {
+	readonly addArgs?: string[];
+	readonly removeArgs?: string[];
+}
+
 export interface ILifecycleMainService {
 
 	readonly _serviceBrand: undefined;
@@ -130,7 +144,12 @@ export interface ILifecycleMainService {
 	/**
 	 * Restart the application with optional arguments (CLI). All lifecycle event handlers are triggered.
 	 */
-	relaunch(options?: { addArgs?: string[]; removeArgs?: string[] }): Promise<void>;
+	relaunch(options?: IRelaunchOptions): Promise<void>;
+
+	/**
+	 * Sets a custom handler for relaunching the application.
+	 */
+	setRelaunchHandler(handler: IRelaunchHandler): void;
 
 	/**
 	 * Shutdown the application normally. All lifecycle event handlers are triggered.
@@ -224,6 +243,8 @@ export class LifecycleMainService extends Disposable implements ILifecycleMainSe
 
 	private readonly phaseWhen = new Map<LifecycleMainPhase, Barrier>();
 
+	private relaunchHandler: IRelaunchHandler | undefined = undefined;
+
 	constructor(
 		@ILogService private readonly logService: ILogService,
 		@IStateService private readonly stateService: IStateService,
@@ -553,6 +574,29 @@ export class LifecycleMainService extends Disposable implements ILifecycleMainSe
 	}
 
 	quit(willRestart?: boolean): Promise<boolean /* veto */> {
+		return this.doQuit(willRestart).then(veto => {
+			if (!veto && willRestart) {
+				// Windows: we are about to restart and as such we need to restore the original
+				// current working directory we had on startup to get the exact same startup
+				// behaviour. As such, we briefly change back to that directory and then when
+				// Code starts it will set it back to the installation directory again.
+				try {
+					if (isWindows) {
+						const currentWorkingDir = cwd();
+						if (currentWorkingDir !== process.cwd()) {
+							process.chdir(currentWorkingDir);
+						}
+					}
+				} catch (err) {
+					this.logService.error(err);
+				}
+			}
+
+			return veto;
+		});
+	}
+
+	private doQuit(willRestart?: boolean): Promise<boolean /* veto */> {
 		this.trace(`Lifecycle#quit() - begin (willRestart: ${willRestart})`);
 
 		if (this.pendingQuitPromise) {
@@ -588,7 +632,11 @@ export class LifecycleMainService extends Disposable implements ILifecycleMainSe
 		}
 	}
 
-	async relaunch(options?: { addArgs?: string[]; removeArgs?: string[] }): Promise<void> {
+	setRelaunchHandler(handler: IRelaunchHandler): void {
+		this.relaunchHandler = handler;
+	}
+
+	async relaunch(options?: IRelaunchOptions): Promise<void> {
 		this.trace('Lifecycle#relaunch()');
 
 		const args = process.argv.slice(1);
@@ -606,24 +654,10 @@ export class LifecycleMainService extends Disposable implements ILifecycleMainSe
 		}
 
 		const quitListener = () => {
-			// Windows: we are about to restart and as such we need to restore the original
-			// current working directory we had on startup to get the exact same startup
-			// behaviour. As such, we briefly change back to that directory and then when
-			// Code starts it will set it back to the installation directory again.
-			try {
-				if (isWindows) {
-					const currentWorkingDir = cwd();
-					if (currentWorkingDir !== process.cwd()) {
-						process.chdir(currentWorkingDir);
-					}
-				}
-			} catch (err) {
-				this.logService.error(err);
+			if (!this.relaunchHandler?.handleRelaunch(options)) {
+				this.trace('Lifecycle#relaunch() - calling app.relaunch()');
+				app.relaunch({ args });
 			}
-
-			// relaunch after we are sure there is no veto
-			this.trace('Lifecycle#relaunch() - calling app.relaunch()');
-			app.relaunch({ args });
 		};
 		app.once('quit', quitListener);
 

src/vs/platform/native/electron-main/nativeHostMainService.ts

@@ -26,7 +26,7 @@ import { INativeOpenDialogOptions } from 'vs/platform/dialogs/common/dialogs';
 import { IDialogMainService } from 'vs/platform/dialogs/electron-main/dialogMainService';
 import { IEnvironmentMainService } from 'vs/platform/environment/electron-main/environmentMainService';
 import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
-import { ILifecycleMainService } from 'vs/platform/lifecycle/electron-main/lifecycleMainService';
+import { ILifecycleMainService, IRelaunchOptions } from 'vs/platform/lifecycle/electron-main/lifecycleMainService';
 import { ILogService } from 'vs/platform/log/common/log';
 import { ICommonNativeHostService, IOSProperties, IOSStatistics } from 'vs/platform/native/common/native';
 import { IProductService } from 'vs/platform/product/common/productService';
@@ -650,7 +650,7 @@ export class NativeHostMainService extends Disposable implements INativeHostMain
 		window?.setReady();
 	}
 
-	async relaunch(windowId: number | undefined, options?: { addArgs?: string[]; removeArgs?: string[] }): Promise<void> {
+	async relaunch(windowId: number | undefined, options?: IRelaunchOptions): Promise<void> {
 		return this.lifecycleMainService.relaunch(options);
 	}
 

src/vs/platform/test/electron-main/workbenchTestServices.ts

@@ -6,7 +6,7 @@
 import { Promises } from 'vs/base/common/async';
 import { Event, Emitter } from 'vs/base/common/event';
 import { NativeParsedArgs } from 'vs/platform/environment/common/argv';
-import { ILifecycleMainService, LifecycleMainPhase, ShutdownEvent, ShutdownReason } from 'vs/platform/lifecycle/electron-main/lifecycleMainService';
+import { ILifecycleMainService, IRelaunchHandler, LifecycleMainPhase, ShutdownEvent, ShutdownReason } from 'vs/platform/lifecycle/electron-main/lifecycleMainService';
 import { IStateService } from 'vs/platform/state/node/state';
 import { ICodeWindow, UnloadReason } from 'vs/platform/window/electron-main/window';
 
@@ -43,6 +43,7 @@ export class TestLifecycleMainService implements ILifecycleMainService {
 	registerWindow(window: ICodeWindow): void { }
 	async reload(window: ICodeWindow, cli?: NativeParsedArgs): Promise<void> { }
 	async unload(window: ICodeWindow, reason: UnloadReason): Promise<boolean> { return true; }
+	setRelaunchHandler(handler: IRelaunchHandler): void { }
 	async relaunch(options?: { addArgs?: string[] | undefined; removeArgs?: string[] | undefined }): Promise<void> { }
 	async quit(willRestart?: boolean): Promise<boolean> { return true; }
 	async kill(code?: number): Promise<void> { }

src/vs/platform/update/electron-main/abstractUpdateService.ts

@@ -46,7 +46,7 @@ export abstract class AbstractUpdateService implements IUpdateService {
 	}
 
 	constructor(
-		@ILifecycleMainService private readonly lifecycleMainService: ILifecycleMainService,
+		@ILifecycleMainService protected readonly lifecycleMainService: ILifecycleMainService,
 		@IConfigurationService protected configurationService: IConfigurationService,
 		@IEnvironmentMainService private readonly environmentMainService: IEnvironmentMainService,
 		@IRequestService protected requestService: IRequestService,

src/vs/platform/update/electron-main/updateService.darwin.ts

@@ -9,7 +9,7 @@ import { Event } from 'vs/base/common/event';
 import { DisposableStore } from 'vs/base/common/lifecycle';
 import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
 import { IEnvironmentMainService } from 'vs/platform/environment/electron-main/environmentMainService';
-import { ILifecycleMainService } from 'vs/platform/lifecycle/electron-main/lifecycleMainService';
+import { ILifecycleMainService, IRelaunchHandler, IRelaunchOptions } from 'vs/platform/lifecycle/electron-main/lifecycleMainService';
 import { ILogService } from 'vs/platform/log/common/log';
 import { IProductService } from 'vs/platform/product/common/productService';
 import { IRequestService } from 'vs/platform/request/common/request';
@@ -17,7 +17,7 @@ import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
 import { IUpdate, State, StateType, UpdateType } from 'vs/platform/update/common/update';
 import { AbstractUpdateService, createUpdateURL, UpdateNotAvailableClassification } from 'vs/platform/update/electron-main/abstractUpdateService';
 
-export class DarwinUpdateService extends AbstractUpdateService {
+export class DarwinUpdateService extends AbstractUpdateService implements IRelaunchHandler {
 
 	private readonly disposables = new DisposableStore();
 
@@ -36,6 +36,23 @@ export class DarwinUpdateService extends AbstractUpdateService {
 		@IProductService productService: IProductService
 	) {
 		super(lifecycleMainService, configurationService, environmentMainService, requestService, logService, productService);
+
+		lifecycleMainService.setRelaunchHandler(this);
+	}
+
+	handleRelaunch(options?: IRelaunchOptions): boolean {
+		if (options?.addArgs || options?.removeArgs) {
+			return false; // we cannot apply an update and restart with different args
+		}
+
+		if (this.state.type !== StateType.Ready) {
+			return false; // we only handle the relaunch when we have a pending update
+		}
+
+		this.logService.trace('update#handleRelaunch(): running raw#quitAndInstall()');
+		this.doQuitAndInstall();
+
+		return true;
 	}
 
 	protected override async initialize(): Promise<void> {

src/vs/platform/update/electron-main/updateService.win32.ts

@@ -16,7 +16,7 @@ import * as pfs from 'vs/base/node/pfs';
 import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
 import { IEnvironmentMainService } from 'vs/platform/environment/electron-main/environmentMainService';
 import { IFileService } from 'vs/platform/files/common/files';
-import { ILifecycleMainService } from 'vs/platform/lifecycle/electron-main/lifecycleMainService';
+import { ILifecycleMainService, IRelaunchHandler, IRelaunchOptions } from 'vs/platform/lifecycle/electron-main/lifecycleMainService';
 import { ILogService } from 'vs/platform/log/common/log';
 import { INativeHostMainService } from 'vs/platform/native/electron-main/nativeHostMainService';
 import { IProductService } from 'vs/platform/product/common/productService';
@@ -47,7 +47,7 @@ function getUpdateType(): UpdateType {
 	return _updateType;
 }
 
-export class Win32UpdateService extends AbstractUpdateService {
+export class Win32UpdateService extends AbstractUpdateService implements IRelaunchHandler {
 
 	private availableUpdate: IAvailableUpdate | undefined;
 
@@ -69,6 +69,23 @@ export class Win32UpdateService extends AbstractUpdateService {
 		@IProductService productService: IProductService
 	) {
 		super(lifecycleMainService, configurationService, environmentMainService, requestService, logService, productService);
+
+		lifecycleMainService.setRelaunchHandler(this);
+	}
+
+	handleRelaunch(options?: IRelaunchOptions): boolean {
+		if (options?.addArgs || options?.removeArgs) {
+			return false; // we cannot apply an update and restart with different args
+		}
+
+		if (this.state.type !== StateType.Ready || !this.availableUpdate) {
+			return false; // we only handle the relaunch when we have a pending update
+		}
+
+		this.logService.trace('update#handleRelaunch(): running raw#quitAndInstall()');
+		this.doQuitAndInstall();
+
+		return true;
 	}
 
 	protected override async initialize(): Promise<void> {