Pause automatic updates on metered connections

src/vs/platform/meteredConnection/electron-browser/meteredConnectionService.ts

@@ -6,7 +6,8 @@
 import { toDisposable } from '../../../base/common/lifecycle.js';
 import { IChannel } from '../../../base/parts/ipc/common/ipc.js';
 import { IConfigurationService } from '../../configuration/common/configuration.js';
-import { InstantiationType, registerSingleton } from '../../instantiation/common/extensions.js';
+import { SyncDescriptor } from '../../instantiation/common/descriptors.js';
+import { registerSingleton } from '../../instantiation/common/extensions.js';
 import { IMainProcessService } from '../../ipc/common/mainProcessService.js';
 import { AbstractMeteredConnectionService, getIsBrowserConnectionMetered, IMeteredConnectionService, NavigatorWithConnection } from '../common/meteredConnection.js';
 import { METERED_CONNECTION_CHANNEL, MeteredConnectionCommand } from '../common/meteredConnectionIpc.js';
@@ -19,15 +20,17 @@ export class NativeMeteredConnectionService extends AbstractMeteredConnectionSer
 	private readonly _channel: IChannel;
 
 	constructor(
+		private readonly connectionMeteredDetector: () => boolean,
 		@IConfigurationService configurationService: IConfigurationService,
 		@IMainProcessService mainProcessService: IMainProcessService
 	) {
-		super(configurationService, getIsBrowserConnectionMetered());
+		super(configurationService, connectionMeteredDetector());
 		this._channel = mainProcessService.getChannel(METERED_CONNECTION_CHANNEL);
+		void this._channel.call(MeteredConnectionCommand.SetIsBrowserConnectionMetered, this.isBrowserConnectionMetered);
 
 		const connection = (navigator as NavigatorWithConnection).connection;
 		if (connection) {
-			const onChange = () => this.setIsBrowserConnectionMetered(getIsBrowserConnectionMetered());
+			const onChange = () => this.setIsBrowserConnectionMetered(this.connectionMeteredDetector());
 			connection.addEventListener('change', onChange);
 			this._register(toDisposable(() => connection.removeEventListener('change', onChange)));
 		}
@@ -42,4 +45,4 @@ export class NativeMeteredConnectionService extends AbstractMeteredConnectionSer
 	}
 }
 
-registerSingleton(IMeteredConnectionService, NativeMeteredConnectionService, InstantiationType.Delayed);
+registerSingleton(IMeteredConnectionService, new SyncDescriptor(NativeMeteredConnectionService, [getIsBrowserConnectionMetered], true));

src/vs/platform/meteredConnection/test/electron-browser/meteredConnectionService.test.ts

@@ -0,0 +1,51 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import assert from 'assert';
+import { CancellationToken } from '../../../../base/common/cancellation.js';
+import { Event } from '../../../../base/common/event.js';
+import { IChannel } from '../../../../base/parts/ipc/common/ipc.js';
+import { mock } from '../../../../base/test/common/mock.js';
+import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js';
+import { TestConfigurationService } from '../../../configuration/test/common/testConfigurationService.js';
+import { IMainProcessService } from '../../../ipc/common/mainProcessService.js';
+import { METERED_CONNECTION_CHANNEL, MeteredConnectionCommand } from '../../common/meteredConnectionIpc.js';
+import { NativeMeteredConnectionService } from '../../electron-browser/meteredConnectionService.js';
+
+class TestChannel implements IChannel {
+	readonly calls: { command: string; argument: unknown }[] = [];
+
+	call<T>(command: string, arg?: unknown, _cancellationToken?: CancellationToken): Promise<T> {
+		this.calls.push({ command, argument: arg });
+		return Promise.resolve(undefined as T);
+	}
+
+	listen<T>(_event: string, _arg?: unknown): Event<T> {
+		return Event.None;
+	}
+}
+
+suite('NativeMeteredConnectionService', () => {
+	const store = ensureNoDisposablesAreLeakedInTestSuite();
+
+	test('reports the initial browser connection state to the main process', () => {
+		const channel = new TestChannel();
+		const mainProcessService = new class extends mock<IMainProcessService>() {
+			override getChannel(channelName: string): IChannel {
+				assert.strictEqual(channelName, METERED_CONNECTION_CHANNEL);
+				return channel;
+			}
+		};
+		const configurationService = new TestConfigurationService();
+		store.add(configurationService.onDidChangeConfigurationEmitter);
+
+		store.add(new NativeMeteredConnectionService(() => true, configurationService, mainProcessService));
+
+		assert.deepStrictEqual(channel.calls, [{
+			command: MeteredConnectionCommand.SetIsBrowserConnectionMetered,
+			argument: true,
+		}]);
+	});
+});

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

@@ -96,13 +96,18 @@ function isCancellableState(type: StateType): boolean {
 	}
 }
 
+interface IInternalUpdateState {
+	readonly state: State;
+	readonly deferred: boolean;
+}
+
 export abstract class AbstractUpdateService extends Disposable implements IUpdateService {
 
 	declare readonly _serviceBrand: undefined;
 
 	protected quality: string | undefined;
 
-	private _state: State = State.Uninitialized;
+	private _state: IInternalUpdateState = { state: State.Uninitialized, deferred: false };
 	protected _overwrite: boolean = false;
 	private _hasCheckedForOverwriteOnQuit: boolean = false;
 	private readonly overwriteUpdatesCheckInterval = this._register(new IntervalTimer());
@@ -121,22 +126,22 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat
 	readonly onStateChange: Event<State> = this._onStateChange.event;
 
 	get state(): State {
-		return this._state;
+		return this._state.state;
 	}
 
-	protected setState(state: State): void {
+	protected setState(state: State, options?: { deferred?: boolean }): void {
 		if (state.type === StateType.Updating) {
 			this.logService.trace('update#setState', state.type);
 		} else {
 			this.logService.info('update#setState', state.type);
 		}
-		this._state = state;
+		this._state = { state, deferred: options?.deferred ?? false };
 		this._onStateChange.fire(state);
 
 		// Clear transient one-time properties from Idle state after delivering the event.
 		// This prevents new windows from seeing stale error/notAvailable messages.
 		if (state.type === StateType.Idle && (state.error || state.notAvailable)) {
-			this._state = State.Idle(state.updateType);
+			this._state = { state: State.Idle(state.updateType), deferred: false };
 		}
 
 		// Schedule 5-minute checks when in Ready state and overwrite is supported
@@ -149,6 +154,12 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat
 		}
 	}
 
+	private setDeferred(deferred: boolean): void {
+		if (this._state.deferred !== deferred) {
+			this._state = { ...this._state, deferred };
+		}
+	}
+
 	constructor(
 		@ILifecycleMainService protected readonly lifecycleMainService: ILifecycleMainService,
 		@IConfigurationService protected configurationService: IConfigurationService,
@@ -165,6 +176,12 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat
 
 		lifecycleMainService.when(LifecycleMainPhase.AfterWindowOpen)
 			.finally(() => this.initialize());
+
+		this._register(this.meteredConnectionService.onDidChangeIsConnectionMetered(isMetered => {
+			if (!isMetered) {
+				this.resumeAutomaticUpdates();
+			}
+		}));
 	}
 
 	/**
@@ -225,7 +242,7 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat
 			const reason = policyDisablesUpdates ? DisablementReason.Policy : DisablementReason.ManuallyDisabled;
 
 			// Skip if already disabled for this reason, so a repeated write or policy refresh is a no-op.
-			if (this._state.type === StateType.Disabled && this._state.reason === reason) {
+			if (this.state.type === StateType.Disabled && this.state.reason === reason) {
 				return;
 			}
 
@@ -242,7 +259,7 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat
 		this.quality = quality;
 
 		// Move to Idle so one-time platform init (which may resume a pending update) can act; it requires Idle.
-		if (this._state.type === StateType.Disabled || this._state.type === StateType.Uninitialized) {
+		if (this.state.type === StateType.Disabled || this.state.type === StateType.Uninitialized) {
 			this.setState(State.Idle(this.getUpdateType()));
 		}
 
@@ -263,7 +280,7 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat
 		this.scheduler.clear();
 
 		// Show a transient Cancelling state only when there is in-flight or pending work to tear down.
-		if (isCancellableState(this._state.type)) {
+		if (isCancellableState(this.state.type)) {
 			this.setState(State.Cancelling);
 		}
 
@@ -293,6 +310,9 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat
 
 	private scheduleAccordingToMode(updateMode: 'none' | 'manual' | 'start' | 'default'): void {
 		this.scheduler.clear();
+		if (this.state.type === StateType.Idle) {
+			this.setDeferred(false);
+		}
 
 		if (updateMode === 'manual') {
 			this.logService.info('update#ctor - manual checks only; automatic updates are disabled by user preference');
@@ -310,6 +330,41 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat
 		}
 	}
 
+	private resumeAutomaticUpdates(): void {
+		if (this._disabledPermanently || !this.quality) {
+			return;
+		}
+
+		const updateMode = this.configurationService.getValue<'none' | 'manual' | 'start' | 'default'>('update.mode');
+		if (updateMode === 'none' || updateMode === 'manual') {
+			return;
+		}
+
+		if (this.state.type === StateType.AvailableForDownload) {
+			if (this._state.deferred) {
+				this.resumeDeferredDownload();
+			}
+			return;
+		}
+
+		if (this.state.type === StateType.Ready) {
+			if (this._state.deferred) {
+				void this.checkForOverwriteUpdates();
+			}
+			return;
+		}
+
+		if (this.state.type !== StateType.Idle) {
+			return;
+		}
+
+		if (updateMode === 'start' && !this._state.deferred) {
+			return;
+		}
+		this.setDeferred(false);
+		this.scheduleCheckForUpdates(0, updateMode === 'default');
+	}
+
 	private async trackVersionChange(): Promise<void> {
 		await this.applicationStorageMainService.whenReady;
 
@@ -408,6 +463,13 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat
 			return;
 		}
 
+		if (!explicit && this.meteredConnectionService.isConnectionMetered) {
+			this.setDeferred(true);
+			this.logService.info('update#checkForUpdates - skipping automatic check because connection is metered');
+			return;
+		}
+
+		this.setDeferred(false);
 		this.doCheckForUpdates(explicit);
 	}
 
@@ -419,17 +481,23 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat
 		}
 
 		if (!explicit && this.meteredConnectionService.isConnectionMetered) {
+			this.setDeferred(true);
 			this.logService.info('update#downloadUpdate - skipping download because connection is metered');
 			return;
 		}
 
+		this.setDeferred(false);
 		await this.doDownloadUpdate(this.state);
 	}
 
 	protected async doDownloadUpdate(state: AvailableForDownload): Promise<void> {
 		// noop
 	}
 
+	protected resumeDeferredDownload(): void {
+		void this.downloadUpdate(false);
+	}
+
 	async applyUpdate(): Promise<void> {
 		this.logService.trace('update#applyUpdate, state = ', this.state.type);
 
@@ -483,30 +551,38 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat
 	}
 
 	private async checkForOverwriteUpdates(explicit: boolean = false): Promise<boolean> {
-		if (this._state.type !== StateType.Ready) {
+		if (this.state.type !== StateType.Ready) {
+			return false;
+		}
+
+		if (!explicit && this.meteredConnectionService.isConnectionMetered) {
+			this.setDeferred(true);
+			this.logService.info('update#checkForOverwriteUpdates - skipping automatic check because connection is metered');
 			return false;
 		}
 
-		const pendingUpdateCommit = this._state.update.version;
+		this.setDeferred(false);
+		const pendingUpdateCommit = this.state.update.version;
 
 		if (!pendingUpdateCommit || pendingUpdateCommit === 'unknown') {
 			return false;
 		}
 
 		let isLatest: boolean | undefined;
 
+		const cts = new CancellationTokenSource();
 		try {
-			const cts = new CancellationTokenSource();
-			const timeoutPromise = timeout(2000).then(() => { cts.cancel(); return undefined; });
-			isLatest = await Promise.race([this.isLatestVersion(pendingUpdateCommit, cts.token), timeoutPromise]);
-			cts.dispose();
+			const timeoutPromise = timeout(2000, cts.token).then(() => { cts.cancel(); return undefined; });
+			isLatest = await Promise.race([this.doIsLatestVersion(pendingUpdateCommit, cts.token), timeoutPromise]);
 		} catch (error) {
 			this.logService.warn('update#checkForOverwriteUpdates(): failed to check for updates, proceeding with restart');
 			this.logService.warn(error);
 			return false;
+		} finally {
+			cts.dispose(true);
 		}
 
-		if (isLatest === false && this._state.type === StateType.Ready) {
+		if (isLatest === false && this.state.type === StateType.Ready) {
 			this.logService.info('update#readyStateCheck: newer update available, restarting update machinery');
 
 			try {
@@ -518,7 +594,7 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat
 			}
 
 			this._overwrite = true;
-			this.setState(State.Overwriting(this._state.update, explicit));
+			this.setState(State.Overwriting(this.state.update, explicit));
 			this.doCheckForUpdates(explicit, pendingUpdateCommit);
 			return true;
 		}
@@ -527,6 +603,15 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat
 	}
 
 	async isLatestVersion(commit?: string, token: CancellationToken = CancellationToken.None): Promise<boolean | undefined> {
+		if (this.meteredConnectionService.isConnectionMetered) {
+			this.logService.info('update#isLatestVersion - skipping automatic check because connection is metered');
+			return undefined;
+		}
+
+		return this.doIsLatestVersion(commit, token);
+	}
+
+	protected async doIsLatestVersion(commit?: string, token: CancellationToken = CancellationToken.None): Promise<boolean | undefined> {
 		if (!this.quality) {
 			return undefined;
 		}

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

@@ -276,7 +276,7 @@ export class Win32UpdateService extends AbstractUpdateService implements IRelaun
 				// show update is available but don't start downloading
 				if (!explicit && this.meteredConnectionService.isConnectionMetered) {
 					this.logService.info('update#doCheckForUpdates - update available but skipping download because connection is metered');
-					this.setState(State.AvailableForDownload(update));
+					this.setState(State.AvailableForDownload(update), { deferred: true });
 					return Promise.resolve(null);
 				}
 
@@ -383,6 +383,11 @@ export class Win32UpdateService extends AbstractUpdateService implements IRelaun
 		this.setState(State.Idle(getUpdateType()));
 	}
 
+	protected override resumeDeferredDownload(): void {
+		this.setState(State.Idle(getUpdateType()));
+		void this.checkForUpdates(false);
+	}
+
 	private async getUpdatePackagePath(version: string): Promise<string> {
 		const cachePath = await this.cachePath;
 		return path.join(cachePath, `CodeSetup-${this.productService.quality}-${version}.exe`);