From 84ef5e0ca237bb15fa20a88d7b081fb9925a31d0 Mon Sep 17 00:00:00 2001 From: Dmitriy Vasyura Date: Wed, 19 Aug 2026 12:59:26 -0700 Subject: [PATCH 1/5] update: respect metered connections Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../electron-main/abstractUpdateService.ts | 45 +++++++++- .../abstractUpdateService.test.ts | 82 ++++++++++++++++++- .../browser/meteredConnectionStatus.ts | 2 +- .../update/browser/postUpdateWidget.ts | 6 ++ .../contrib/update/browser/updateTooltip.ts | 17 ++-- .../test/browser/postUpdateWidget.test.ts | 77 +++++++++++++++++ .../test/browser/updateTitleBarEntry.test.ts | 12 +-- 7 files changed, 217 insertions(+), 24 deletions(-) create mode 100644 src/vs/workbench/contrib/update/test/browser/postUpdateWidget.test.ts diff --git a/src/vs/platform/update/electron-main/abstractUpdateService.ts b/src/vs/platform/update/electron-main/abstractUpdateService.ts index 09971caf6b3e33..b249b9372a462a 100644 --- a/src/vs/platform/update/electron-main/abstractUpdateService.ts +++ b/src/vs/platform/update/electron-main/abstractUpdateService.ts @@ -165,6 +165,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(); + } + })); } /** @@ -310,6 +316,24 @@ 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) { + void this.downloadUpdate(false); + return; + } + + this.scheduleCheckForUpdates(0, updateMode === 'default'); + } + private async trackVersionChange(): Promise { await this.applicationStorageMainService.whenReady; @@ -408,6 +432,11 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat return; } + if (!explicit && this.meteredConnectionService.isConnectionMetered) { + this.logService.info('update#checkForUpdates - skipping automatic check because connection is metered'); + return; + } + this.doCheckForUpdates(explicit); } @@ -487,6 +516,11 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat return false; } + if (!explicit && this.meteredConnectionService.isConnectionMetered) { + this.logService.info('update#checkForOverwriteUpdates - skipping automatic check because connection is metered'); + return false; + } + const pendingUpdateCommit = this._state.update.version; if (!pendingUpdateCommit || pendingUpdateCommit === 'unknown') { @@ -498,7 +532,7 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat try { const cts = new CancellationTokenSource(); const timeoutPromise = timeout(2000).then(() => { cts.cancel(); return undefined; }); - isLatest = await Promise.race([this.isLatestVersion(pendingUpdateCommit, cts.token), timeoutPromise]); + isLatest = await Promise.race([this.doIsLatestVersion(pendingUpdateCommit, cts.token), timeoutPromise]); cts.dispose(); } catch (error) { this.logService.warn('update#checkForOverwriteUpdates(): failed to check for updates, proceeding with restart'); @@ -527,6 +561,15 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat } async isLatestVersion(commit?: string, token: CancellationToken = CancellationToken.None): Promise { + 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 { if (!this.quality) { return undefined; } diff --git a/src/vs/platform/update/test/electron-main/abstractUpdateService.test.ts b/src/vs/platform/update/test/electron-main/abstractUpdateService.test.ts index 2766d433ed5379..4e3878bfe6a088 100644 --- a/src/vs/platform/update/test/electron-main/abstractUpdateService.test.ts +++ b/src/vs/platform/update/test/electron-main/abstractUpdateService.test.ts @@ -6,7 +6,8 @@ import assert from 'assert'; import * as sinon from 'sinon'; import { DeferredPromise, timeout } from '../../../../base/common/async.js'; -import { Event } from '../../../../base/common/event.js'; +import { Emitter, Event } from '../../../../base/common/event.js'; +import { Disposable } from '../../../../base/common/lifecycle.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; import { IConfigurationChangeEvent, IConfigurationOverrides, IConfigurationValue } from '../../../configuration/common/configuration.js'; import { TestConfigurationService } from '../../../configuration/test/common/testConfigurationService.js'; @@ -21,6 +22,22 @@ import { NullTelemetryService } from '../../../telemetry/common/telemetryUtils.j import { DisablementReason, State, StateType } from '../../common/update.js'; import { AbstractUpdateService, IUpdateURLOptions } from '../../electron-main/abstractUpdateService.js'; +class TestMeteredConnectionService extends Disposable implements IMeteredConnectionService { + declare readonly _serviceBrand: undefined; + + private readonly _onDidChangeIsConnectionMetered = this._register(new Emitter()); + readonly onDidChangeIsConnectionMetered = this._onDidChangeIsConnectionMetered.event; + + constructor(public isConnectionMetered: boolean) { + super(); + } + + setIsConnectionMetered(isConnectionMetered: boolean): void { + this.isConnectionMetered = isConnectionMetered; + this._onDidChangeIsConnectionMetered.fire(isConnectionMetered); + } +} + class TestUpdateService extends AbstractUpdateService { private readonly _initialized = new DeferredPromise(); @@ -32,6 +49,9 @@ class TestUpdateService extends AbstractUpdateService { private _cancelCount = 0; get cancelCount(): number { return this._cancelCount; } + private _downloadCount = 0; + get downloadCount(): number { return this._downloadCount; } + /** When set, `cancelUpdate` blocks on this promise so tests can observe the transient Cancelling state. */ private _cancelGate: Promise | undefined; blockCancelUpdate(gate: Promise): void { this._cancelGate = gate; } @@ -57,6 +77,14 @@ class TestUpdateService extends AbstractUpdateService { this._checkCount++; } + protected override async doDownloadUpdate(): Promise { + this._downloadCount++; + } + + checkLatestVersionExplicitly(): Promise { + return this.doIsLatestVersion(); + } + protected override async cancelUpdate(): Promise { this._cancelCount++; if (this._cancelGate) { @@ -91,10 +119,13 @@ suite('AbstractUpdateService', () => { } let configurationService: PolicyTestConfigurationService; + let requestCount: number; + let meteredConnectionService: TestMeteredConnectionService; - function createService(mode: string, options?: { isBuilt?: boolean; disableUpdates?: boolean; updateUrl?: string }): TestUpdateService { + function createService(mode: string, options?: { isBuilt?: boolean; disableUpdates?: boolean; updateUrl?: string; isConnectionMetered?: boolean }): TestUpdateService { configurationService = new PolicyTestConfigurationService(); configurationService.setUserConfiguration('update.mode', mode); + requestCount = 0; const lifecycleMainService = { when: () => Promise.resolve(), @@ -109,7 +140,10 @@ suite('AbstractUpdateService', () => { } as unknown as IEnvironmentMainService; const requestService = { - request: () => Promise.reject(new Error('not expected')) + request: () => { + requestCount++; + return Promise.reject(new Error('not expected')); + } } as unknown as IRequestService; const productService = { @@ -126,7 +160,7 @@ suite('AbstractUpdateService', () => { store: () => { } } as unknown as IApplicationStorageMainService; - const meteredConnectionService = { isConnectionMetered: false } as unknown as IMeteredConnectionService; + meteredConnectionService = store.add(new TestMeteredConnectionService(options?.isConnectionMetered ?? false)); const service = new TestUpdateService( lifecycleMainService, @@ -223,6 +257,46 @@ suite('AbstractUpdateService', () => { } }); + test('metered connections skip automatic update requests but allow explicit actions', async () => { + const service = createService('default', { isConnectionMetered: true }); + await service.whenInitialized; + + await service.checkForUpdates(false); + await service.isLatestVersion(); + await service.checkForUpdates(true); + await service.checkLatestVersionExplicitly(); + + service.forceState(State.AvailableForDownload({ version: '1.1.0', productVersion: '1.1.0', url: 'https://update.example/download' })); + await service.downloadUpdate(false); + await service.downloadUpdate(true); + + assert.deepStrictEqual({ + checkCount: service.checkCount, + downloadCount: service.downloadCount, + requestCount, + }, { + checkCount: 1, + downloadCount: 1, + requestCount: 1, + }); + }); + + test('automatic checks resume when the connection is no longer metered', async () => { + const clock = sinon.useFakeTimers(); + try { + const service = createService('start', { isConnectionMetered: true }); + await service.whenInitialized; + await clock.tickAsync(30 * 1000); + assert.strictEqual(service.checkCount, 0); + + meteredConnectionService.setIsConnectionMetered(false); + await clock.tickAsync(0); + assert.strictEqual(service.checkCount, 1); + } finally { + clock.restore(); + } + }); + test('permanent disablement ignores runtime mode changes', async () => { const service = createService('default', { isBuilt: false }); await service.whenInitialized; diff --git a/src/vs/workbench/contrib/meteredConnection/browser/meteredConnectionStatus.ts b/src/vs/workbench/contrib/meteredConnection/browser/meteredConnectionStatus.ts index 677ddbc62521b2..466419e06561fb 100644 --- a/src/vs/workbench/contrib/meteredConnection/browser/meteredConnectionStatus.ts +++ b/src/vs/workbench/contrib/meteredConnection/browser/meteredConnectionStatus.ts @@ -48,7 +48,7 @@ export class MeteredConnectionStatusContribution extends Disposable implements I name: localize('status.meteredConnection', "Metered Connection"), text: '$(radio-tower)', ariaLabel: localize('status.meteredConnection.ariaLabel', "Metered Connection Enabled"), - tooltip: localize('status.meteredConnection.tooltip', "Metered connection enabled. Some automatic features like extension updates, Settings Sync, and automatic Git operations are paused to reduce data usage."), + tooltip: localize('status.meteredConnection.tooltip', "Metered connection enabled. Background network activity including updates, Settings Sync, inline completions, telemetry, and automatic Git operations is paused to reduce data usage."), command: { id: 'workbench.action.configureMeteredConnection', title: localize('status.meteredConnection.configure', "Configure") diff --git a/src/vs/workbench/contrib/update/browser/postUpdateWidget.ts b/src/vs/workbench/contrib/update/browser/postUpdateWidget.ts index 6e0df0ff7d16c1..dae0b9462b679e 100644 --- a/src/vs/workbench/contrib/update/browser/postUpdateWidget.ts +++ b/src/vs/workbench/contrib/update/browser/postUpdateWidget.ts @@ -15,6 +15,7 @@ import { IConfigurationService } from '../../../../platform/configuration/common import { IHoverService } from '../../../../platform/hover/browser/hover.js'; import { ILayoutService } from '../../../../platform/layout/browser/layoutService.js'; import { IMarkdownRendererService, openLinkFromMarkdown } from '../../../../platform/markdown/browser/markdownRenderer.js'; +import { IMeteredConnectionService } from '../../../../platform/meteredConnection/common/meteredConnection.js'; import { IOpenerService } from '../../../../platform/opener/common/opener.js'; import { IProductService } from '../../../../platform/product/common/productService.js'; import { asTextOrError, IRequestService } from '../../../../platform/request/common/request.js'; @@ -52,6 +53,7 @@ export class PostUpdateWidgetContribution extends Disposable implements IWorkben @IHoverService private readonly hoverService: IHoverService, @ILayoutService private readonly layoutService: ILayoutService, @IMarkdownRendererService private readonly markdownRendererService: IMarkdownRendererService, + @IMeteredConnectionService private readonly meteredConnectionService: IMeteredConnectionService, @IOpenerService private readonly openerService: IOpenerService, @IProductService private readonly productService: IProductService, @IRequestService private readonly requestService: IRequestService, @@ -73,6 +75,10 @@ export class PostUpdateWidgetContribution extends Disposable implements IWorkben return; } + if (this.meteredConnectionService.isConnectionMetered) { + return; + } + if (!this.detectVersionChange()) { return; } diff --git a/src/vs/workbench/contrib/update/browser/updateTooltip.ts b/src/vs/workbench/contrib/update/browser/updateTooltip.ts index e85e477beb4b81..1a69c83c3a011f 100644 --- a/src/vs/workbench/contrib/update/browser/updateTooltip.ts +++ b/src/vs/workbench/contrib/update/browser/updateTooltip.ts @@ -12,7 +12,6 @@ import { IClipboardService } from '../../../../platform/clipboard/common/clipboa import { ICommandService } from '../../../../platform/commands/common/commands.js'; import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js'; import { IHoverService } from '../../../../platform/hover/browser/hover.js'; -import { IMeteredConnectionService } from '../../../../platform/meteredConnection/common/meteredConnection.js'; import { IProductService } from '../../../../platform/product/common/productService.js'; import { AvailableForDownload, Disabled, DisablementReason, Downloaded, Downloading, Idle, IUpdate, Overwriting, Ready, Restarting, State, StateType, Updating } from '../../../../platform/update/common/update.js'; import { ShowCurrentReleaseNotesActionId } from '../common/update.js'; @@ -65,7 +64,6 @@ export class UpdateTooltip extends Disposable { @ICommandService private readonly commandService: ICommandService, @IConfigurationService private readonly configurationService: IConfigurationService, @IHoverService private readonly hoverService: IHoverService, - @IMeteredConnectionService private readonly meteredConnectionService: IMeteredConnectionService, @IProductService private readonly productService: IProductService, ) { super(); @@ -275,8 +273,9 @@ export class UpdateTooltip extends Disposable { return; } + const updateMode = this.configurationService.getValue('update.mode'); this.renderTitleAndInfo(localize('updateTooltip.upToDateTitle', "Up to Date")); - switch (this.configurationService.getValue('update.mode')) { + switch (updateMode) { case 'none': this.renderMessage(localize('updateTooltip.autoUpdateNone', "Automatic updates are disabled."), Codicon.warning); break; @@ -287,15 +286,9 @@ export class UpdateTooltip extends Disposable { this.renderMessage(localize('updateTooltip.autoUpdateStart', "Updates will be applied on restart.")); break; case 'default': - if (this.meteredConnectionService.isConnectionMetered) { - this.renderMessage( - localize('updateTooltip.meteredConnectionMessage', "Automatic updates are paused because the network connection is metered."), - Codicon.radioTower); - } else { - this.renderMessage( - localize('updateTooltip.autoUpdateDefault', "Automatic updates are enabled. Happy Coding!"), - Codicon.smiley); - } + this.renderMessage( + localize('updateTooltip.autoUpdateDefault', "Automatic updates are enabled. Happy Coding!"), + Codicon.smiley); break; } } diff --git a/src/vs/workbench/contrib/update/test/browser/postUpdateWidget.test.ts b/src/vs/workbench/contrib/update/test/browser/postUpdateWidget.test.ts new file mode 100644 index 00000000000000..4774eccb7f99fa --- /dev/null +++ b/src/vs/workbench/contrib/update/test/browser/postUpdateWidget.test.ts @@ -0,0 +1,77 @@ +/*--------------------------------------------------------------------------------------------- + * 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 { timeout } from '../../../../../base/common/async.js'; +import { bufferToStream, VSBuffer } from '../../../../../base/common/buffer.js'; +import { IRequestContext } from '../../../../../base/parts/request/common/request.js'; +import { mock } from '../../../../../base/test/common/mock.js'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; +import { CommandsRegistry, ICommandService } from '../../../../../platform/commands/common/commands.js'; +import { TestConfigurationService } from '../../../../../platform/configuration/test/common/testConfigurationService.js'; +import { IHoverService } from '../../../../../platform/hover/browser/hover.js'; +import { ILayoutService } from '../../../../../platform/layout/browser/layoutService.js'; +import { IMarkdownRendererService } from '../../../../../platform/markdown/browser/markdownRenderer.js'; +import { IMeteredConnectionService } from '../../../../../platform/meteredConnection/common/meteredConnection.js'; +import { IOpenerService } from '../../../../../platform/opener/common/opener.js'; +import { IProductService } from '../../../../../platform/product/common/productService.js'; +import { IRequestService } from '../../../../../platform/request/common/request.js'; +import { IStorageService } from '../../../../../platform/storage/common/storage.js'; +import { ITelemetryService } from '../../../../../platform/telemetry/common/telemetry.js'; +import { IHostService } from '../../../../services/host/browser/host.js'; +import { PostUpdateWidgetContribution } from '../../browser/postUpdateWidget.js'; + +class TestRequestService extends mock() { + requestCount = 0; + + override async request(): Promise { + this.requestCount++; + return { + res: { statusCode: 200, headers: {} }, + stream: bufferToStream(VSBuffer.fromString('')), + }; + } +} + +suite('PostUpdateWidgetContribution', () => { + const store = ensureNoDisposablesAreLeakedInTestSuite(); + + test('skips the automatic request while metered but preserves the explicit command', async () => { + const requestService = new TestRequestService(); + const configurationService = new TestConfigurationService(); + store.add(configurationService.onDidChangeConfigurationEmitter); + store.add(new PostUpdateWidgetContribution( + new class extends mock() { }, + configurationService, + new class extends mock() { + override hadLastFocus(): Promise { + return Promise.resolve(true); + } + }, + new class extends mock() { }, + new class extends mock() { }, + new class extends mock() { }, + new class extends mock() { + override readonly isConnectionMetered = true; + }, + new class extends mock() { }, + new class extends mock() { + override readonly version = '1.135.0'; + override readonly commit = 'current'; + }, + requestService, + new class extends mock() { }, + new class extends mock() { }, + )); + + await timeout(0); + assert.strictEqual(requestService.requestCount, 0); + + const command = CommandsRegistry.getCommand('_update.showUpdateInfo'); + assert.ok(command); + await command.handler(undefined as never); + assert.strictEqual(requestService.requestCount, 1); + }); +}); diff --git a/src/vs/workbench/contrib/update/test/browser/updateTitleBarEntry.test.ts b/src/vs/workbench/contrib/update/test/browser/updateTitleBarEntry.test.ts index 4c4a8d0c3ae542..b5ee28008af27d 100644 --- a/src/vs/workbench/contrib/update/test/browser/updateTitleBarEntry.test.ts +++ b/src/vs/workbench/contrib/update/test/browser/updateTitleBarEntry.test.ts @@ -18,7 +18,6 @@ import { TestConfigurationService } from '../../../../../platform/configuration/ import { ContextKeyExpression } from '../../../../../platform/contextkey/common/contextkey.js'; import { IHoverService } from '../../../../../platform/hover/browser/hover.js'; import { MockContextKeyService } from '../../../../../platform/keybinding/test/common/mockKeybindingService.js'; -import { IMeteredConnectionService } from '../../../../../platform/meteredConnection/common/meteredConnection.js'; import { IProductService } from '../../../../../platform/product/common/productService.js'; import { ITelemetryService } from '../../../../../platform/telemetry/common/telemetry.js'; import { IUpdateService, State } from '../../../../../platform/update/common/update.js'; @@ -145,23 +144,24 @@ suite('UpdateGlobalActivityBadgeVisibleContext', () => { suite('UpdateTooltip', () => { const store = ensureNoDisposablesAreLeakedInTestSuite(); - test('removes hidden actions from the tab order', () => { + function createTooltip(): UpdateTooltip { const configurationService = new TestConfigurationService({ 'update.mode': 'default' }); store.add(configurationService.onDidChangeConfigurationEmitter); - const tooltip = store.add(new UpdateTooltip( + return store.add(new UpdateTooltip( new class extends mock() { }, store.add(new TestCommandService()), configurationService, new TestHoverService(), - new class extends mock() { - override readonly isConnectionMetered = false; - }, new class extends mock() { override readonly nameLong = 'Code - OSS Dev'; override readonly version = '1.134.0'; override readonly commit = 'current'; }, )); + } + + test('removes hidden actions from the tab order', () => { + const tooltip = createTooltip(); tooltip.renderState(State.Ready({ version: 'next', productVersion: '1.135.0' }, false, false)); From 3b7c6f1de09e24d2afcc7ab7e922cc0f03e51afc Mon Sep 17 00:00:00 2001 From: Dmitriy Vasyura Date: Wed, 19 Aug 2026 13:26:53 -0700 Subject: [PATCH 2/5] Fix deferred metered update handling Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../electron-main/abstractUpdateService.ts | 42 ++++++++++-- .../abstractUpdateService.test.ts | 67 ++++++++++++++++++- .../test/browser/postUpdateWidget.test.ts | 24 ++++++- 3 files changed, 124 insertions(+), 9 deletions(-) diff --git a/src/vs/platform/update/electron-main/abstractUpdateService.ts b/src/vs/platform/update/electron-main/abstractUpdateService.ts index b249b9372a462a..5cf2f2d130d1fa 100644 --- a/src/vs/platform/update/electron-main/abstractUpdateService.ts +++ b/src/vs/platform/update/electron-main/abstractUpdateService.ts @@ -112,6 +112,9 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat private _disabledPermanently: boolean = false; /** Whether one-time platform init (e.g. background update GC, pending update resume) has run. */ private _postInitialized: boolean = false; + private _automaticCheckDeferred = false; + private _automaticDownloadDeferred = false; + private _automaticOverwriteCheckDeferred = false; /** Cancels the pending scheduled update check, if any. */ private readonly scheduler = this._register(new MutableDisposable()); /** Serializes reconfiguration so overlapping `update.mode` changes settle on the latest value. */ @@ -131,6 +134,12 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat this.logService.info('update#setState', state.type); } this._state = state; + if (state.type !== StateType.AvailableForDownload) { + this._automaticDownloadDeferred = false; + } + if (state.type !== StateType.Ready) { + this._automaticOverwriteCheckDeferred = false; + } this._onStateChange.fire(state); // Clear transient one-time properties from Idle state after delivering the event. @@ -299,6 +308,7 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat private scheduleAccordingToMode(updateMode: 'none' | 'manual' | 'start' | 'default'): void { this.scheduler.clear(); + this._automaticCheckDeferred = false; if (updateMode === 'manual') { this.logService.info('update#ctor - manual checks only; automatic updates are disabled by user preference'); @@ -327,10 +337,27 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat } if (this.state.type === StateType.AvailableForDownload) { - void this.downloadUpdate(false); + if (this._automaticDownloadDeferred) { + void this.downloadUpdate(false); + } + return; + } + + if (this.state.type === StateType.Ready) { + if (this._automaticOverwriteCheckDeferred) { + void this.checkForOverwriteUpdates(); + } return; } + if (this.state.type !== StateType.Idle) { + return; + } + + if (updateMode === 'start' && !this._automaticCheckDeferred) { + return; + } + this._automaticCheckDeferred = false; this.scheduleCheckForUpdates(0, updateMode === 'default'); } @@ -433,10 +460,12 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat } if (!explicit && this.meteredConnectionService.isConnectionMetered) { + this._automaticCheckDeferred = true; this.logService.info('update#checkForUpdates - skipping automatic check because connection is metered'); return; } + this._automaticCheckDeferred = false; this.doCheckForUpdates(explicit); } @@ -448,10 +477,12 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat } if (!explicit && this.meteredConnectionService.isConnectionMetered) { + this._automaticDownloadDeferred = true; this.logService.info('update#downloadUpdate - skipping download because connection is metered'); return; } + this._automaticDownloadDeferred = false; await this.doDownloadUpdate(this.state); } @@ -517,10 +548,12 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat } if (!explicit && this.meteredConnectionService.isConnectionMetered) { + this._automaticOverwriteCheckDeferred = true; this.logService.info('update#checkForOverwriteUpdates - skipping automatic check because connection is metered'); return false; } + this._automaticOverwriteCheckDeferred = false; const pendingUpdateCommit = this._state.update.version; if (!pendingUpdateCommit || pendingUpdateCommit === 'unknown') { @@ -529,15 +562,16 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat let isLatest: boolean | undefined; + const cts = new CancellationTokenSource(); try { - const cts = new CancellationTokenSource(); - const timeoutPromise = timeout(2000).then(() => { cts.cancel(); return undefined; }); + const timeoutPromise = timeout(2000, cts.token).then(() => { cts.cancel(); return undefined; }); isLatest = await Promise.race([this.doIsLatestVersion(pendingUpdateCommit, cts.token), timeoutPromise]); - cts.dispose(); } 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) { diff --git a/src/vs/platform/update/test/electron-main/abstractUpdateService.test.ts b/src/vs/platform/update/test/electron-main/abstractUpdateService.test.ts index 4e3878bfe6a088..bf181dabc62616 100644 --- a/src/vs/platform/update/test/electron-main/abstractUpdateService.test.ts +++ b/src/vs/platform/update/test/electron-main/abstractUpdateService.test.ts @@ -122,7 +122,7 @@ suite('AbstractUpdateService', () => { let requestCount: number; let meteredConnectionService: TestMeteredConnectionService; - function createService(mode: string, options?: { isBuilt?: boolean; disableUpdates?: boolean; updateUrl?: string; isConnectionMetered?: boolean }): TestUpdateService { + function createService(mode: string, options?: { isBuilt?: boolean; disableUpdates?: boolean; updateUrl?: string; isConnectionMetered?: boolean; supportsUpdateOverwrite?: boolean }): TestUpdateService { configurationService = new PolicyTestConfigurationService(); configurationService.setUserConfiguration('update.mode', mode); requestCount = 0; @@ -172,7 +172,7 @@ suite('AbstractUpdateService', () => { NullTelemetryService, applicationStorageMainService, meteredConnectionService, - false + options?.supportsUpdateOverwrite ?? false ); return store.add(service); @@ -269,6 +269,8 @@ suite('AbstractUpdateService', () => { service.forceState(State.AvailableForDownload({ version: '1.1.0', productVersion: '1.1.0', url: 'https://update.example/download' })); await service.downloadUpdate(false); await service.downloadUpdate(true); + meteredConnectionService.setIsConnectionMetered(false); + await timeout(0); assert.deepStrictEqual({ checkCount: service.checkCount, @@ -297,6 +299,67 @@ suite('AbstractUpdateService', () => { } }); + test('completed startup checks do not run again after a metered transition', async () => { + const clock = sinon.useFakeTimers(); + try { + const service = createService('start'); + await service.whenInitialized; + await clock.tickAsync(30 * 1000); + assert.strictEqual(service.checkCount, 1); + + meteredConnectionService.setIsConnectionMetered(true); + meteredConnectionService.setIsConnectionMetered(false); + await clock.tickAsync(0); + + assert.strictEqual(service.checkCount, 1); + } finally { + clock.restore(); + } + }); + + test('only resumes automatic downloads that were deferred by metering', async () => { + const service = createService('default'); + await service.whenInitialized; + service.forceState(State.AvailableForDownload({ version: '1.1.0', productVersion: '1.1.0', url: 'https://update.example/download' })); + + meteredConnectionService.setIsConnectionMetered(true); + meteredConnectionService.setIsConnectionMetered(false); + await timeout(0); + const downloadsWithoutDeferredIntent = service.downloadCount; + + meteredConnectionService.setIsConnectionMetered(true); + await service.downloadUpdate(false); + meteredConnectionService.setIsConnectionMetered(false); + await timeout(0); + + assert.deepStrictEqual({ + downloadsWithoutDeferredIntent, + downloadsAfterDeferredIntent: service.downloadCount, + }, { + downloadsWithoutDeferredIntent: 0, + downloadsAfterDeferredIntent: 1, + }); + }); + + test('resumes overwrite checks that were deferred by metering', async () => { + const clock = sinon.useFakeTimers(); + try { + const service = createService('default', { isConnectionMetered: true, supportsUpdateOverwrite: true }); + await service.whenInitialized; + service.forceState(State.Ready({ version: 'pending' }, false, false)); + + await clock.tickAsync(5 * 60 * 1000); + assert.strictEqual(requestCount, 0); + + meteredConnectionService.setIsConnectionMetered(false); + await clock.tickAsync(0); + + assert.strictEqual(requestCount, 1); + } finally { + clock.restore(); + } + }); + test('permanent disablement ignores runtime mode changes', async () => { const service = createService('default', { isBuilt: false }); await service.whenInitialized; diff --git a/src/vs/workbench/contrib/update/test/browser/postUpdateWidget.test.ts b/src/vs/workbench/contrib/update/test/browser/postUpdateWidget.test.ts index 4774eccb7f99fa..4735c6a176e421 100644 --- a/src/vs/workbench/contrib/update/test/browser/postUpdateWidget.test.ts +++ b/src/vs/workbench/contrib/update/test/browser/postUpdateWidget.test.ts @@ -38,7 +38,7 @@ class TestRequestService extends mock() { suite('PostUpdateWidgetContribution', () => { const store = ensureNoDisposablesAreLeakedInTestSuite(); - test('skips the automatic request while metered but preserves the explicit command', async () => { + function createContribution(isConnectionMetered: boolean): TestRequestService { const requestService = new TestRequestService(); const configurationService = new TestConfigurationService(); store.add(configurationService.onDidChangeConfigurationEmitter); @@ -54,7 +54,7 @@ suite('PostUpdateWidgetContribution', () => { new class extends mock() { }, new class extends mock() { }, new class extends mock() { - override readonly isConnectionMetered = true; + override readonly isConnectionMetered = isConnectionMetered; }, new class extends mock() { }, new class extends mock() { @@ -62,9 +62,27 @@ suite('PostUpdateWidgetContribution', () => { override readonly commit = 'current'; }, requestService, - new class extends mock() { }, + new class extends mock() { + override getObject(): T | undefined { + return { version: '1.134.0', commit: 'previous', timestamp: 0 } as T; + } + override store(): void { } + }, new class extends mock() { }, )); + return requestService; + } + + test('requests update info automatically after a version change when unmetered', async () => { + const requestService = createContribution(false); + + await timeout(0); + + assert.strictEqual(requestService.requestCount, 1); + }); + + test('skips the automatic request while metered but preserves the explicit command', async () => { + const requestService = createContribution(true); await timeout(0); assert.strictEqual(requestService.requestCount, 0); From 017fbafd52ec65c50d262ee707644e3e058e4032 Mon Sep 17 00:00:00 2001 From: Dmitriy Vasyura Date: Wed, 19 Aug 2026 17:34:22 -0700 Subject: [PATCH 3/5] Fix deferred metered update resumption Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../meteredConnectionService.ts | 11 ++- .../meteredConnectionService.test.ts | 51 +++++++++++++ .../electron-main/abstractUpdateService.ts | 74 ++++++++++--------- .../electron-main/updateService.win32.ts | 7 +- .../abstractUpdateService.test.ts | 13 +++- .../browser/meteredConnectionStatus.ts | 2 +- .../postUpdateWidget.test.ts | 2 +- 7 files changed, 119 insertions(+), 41 deletions(-) create mode 100644 src/vs/platform/meteredConnection/test/electron-browser/meteredConnectionService.test.ts rename src/vs/workbench/contrib/update/test/{browser => electron-browser}/postUpdateWidget.test.ts (98%) diff --git a/src/vs/platform/meteredConnection/electron-browser/meteredConnectionService.ts b/src/vs/platform/meteredConnection/electron-browser/meteredConnectionService.ts index cf0276efc2e927..4f1607adc73feb 100644 --- a/src/vs/platform/meteredConnection/electron-browser/meteredConnectionService.ts +++ b/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)); diff --git a/src/vs/platform/meteredConnection/test/electron-browser/meteredConnectionService.test.ts b/src/vs/platform/meteredConnection/test/electron-browser/meteredConnectionService.test.ts new file mode 100644 index 00000000000000..36355c464b94c7 --- /dev/null +++ b/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(command: string, arg?: unknown, _cancellationToken?: CancellationToken): Promise { + this.calls.push({ command, argument: arg }); + return Promise.resolve(undefined as T); + } + + listen(_event: string, _arg?: unknown): Event { + 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() { + 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, + }]); + }); +}); diff --git a/src/vs/platform/update/electron-main/abstractUpdateService.ts b/src/vs/platform/update/electron-main/abstractUpdateService.ts index 5cf2f2d130d1fa..dd81388e833206 100644 --- a/src/vs/platform/update/electron-main/abstractUpdateService.ts +++ b/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()); @@ -112,9 +117,6 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat private _disabledPermanently: boolean = false; /** Whether one-time platform init (e.g. background update GC, pending update resume) has run. */ private _postInitialized: boolean = false; - private _automaticCheckDeferred = false; - private _automaticDownloadDeferred = false; - private _automaticOverwriteCheckDeferred = false; /** Cancels the pending scheduled update check, if any. */ private readonly scheduler = this._register(new MutableDisposable()); /** Serializes reconfiguration so overlapping `update.mode` changes settle on the latest value. */ @@ -124,28 +126,22 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat readonly onStateChange: Event = 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; - if (state.type !== StateType.AvailableForDownload) { - this._automaticDownloadDeferred = false; - } - if (state.type !== StateType.Ready) { - this._automaticOverwriteCheckDeferred = false; - } + 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 @@ -158,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, @@ -240,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; } @@ -257,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())); } @@ -278,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); } @@ -308,7 +310,9 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat private scheduleAccordingToMode(updateMode: 'none' | 'manual' | 'start' | 'default'): void { this.scheduler.clear(); - this._automaticCheckDeferred = false; + 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'); @@ -337,14 +341,14 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat } if (this.state.type === StateType.AvailableForDownload) { - if (this._automaticDownloadDeferred) { - void this.downloadUpdate(false); + if (this._state.deferred) { + this.resumeDeferredDownload(); } return; } if (this.state.type === StateType.Ready) { - if (this._automaticOverwriteCheckDeferred) { + if (this._state.deferred) { void this.checkForOverwriteUpdates(); } return; @@ -354,10 +358,10 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat return; } - if (updateMode === 'start' && !this._automaticCheckDeferred) { + if (updateMode === 'start' && !this._state.deferred) { return; } - this._automaticCheckDeferred = false; + this.setDeferred(false); this.scheduleCheckForUpdates(0, updateMode === 'default'); } @@ -460,12 +464,12 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat } if (!explicit && this.meteredConnectionService.isConnectionMetered) { - this._automaticCheckDeferred = true; + this.setDeferred(true); this.logService.info('update#checkForUpdates - skipping automatic check because connection is metered'); return; } - this._automaticCheckDeferred = false; + this.setDeferred(false); this.doCheckForUpdates(explicit); } @@ -477,12 +481,12 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat } if (!explicit && this.meteredConnectionService.isConnectionMetered) { - this._automaticDownloadDeferred = true; + this.setDeferred(true); this.logService.info('update#downloadUpdate - skipping download because connection is metered'); return; } - this._automaticDownloadDeferred = false; + this.setDeferred(false); await this.doDownloadUpdate(this.state); } @@ -490,6 +494,10 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat // noop } + protected resumeDeferredDownload(): void { + void this.downloadUpdate(false); + } + async applyUpdate(): Promise { this.logService.trace('update#applyUpdate, state = ', this.state.type); @@ -543,18 +551,18 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat } private async checkForOverwriteUpdates(explicit: boolean = false): Promise { - if (this._state.type !== StateType.Ready) { + if (this.state.type !== StateType.Ready) { return false; } if (!explicit && this.meteredConnectionService.isConnectionMetered) { - this._automaticOverwriteCheckDeferred = true; + this.setDeferred(true); this.logService.info('update#checkForOverwriteUpdates - skipping automatic check because connection is metered'); return false; } - this._automaticOverwriteCheckDeferred = false; - const pendingUpdateCommit = this._state.update.version; + this.setDeferred(false); + const pendingUpdateCommit = this.state.update.version; if (!pendingUpdateCommit || pendingUpdateCommit === 'unknown') { return false; @@ -574,7 +582,7 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat 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 { @@ -586,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; } diff --git a/src/vs/platform/update/electron-main/updateService.win32.ts b/src/vs/platform/update/electron-main/updateService.win32.ts index 2d3aa8bd3f88bd..58078bd4077237 100644 --- a/src/vs/platform/update/electron-main/updateService.win32.ts +++ b/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 { const cachePath = await this.cachePath; return path.join(cachePath, `CodeSetup-${this.productService.quality}-${version}.exe`); diff --git a/src/vs/platform/update/test/electron-main/abstractUpdateService.test.ts b/src/vs/platform/update/test/electron-main/abstractUpdateService.test.ts index bf181dabc62616..bea89646e020e3 100644 --- a/src/vs/platform/update/test/electron-main/abstractUpdateService.test.ts +++ b/src/vs/platform/update/test/electron-main/abstractUpdateService.test.ts @@ -57,7 +57,7 @@ class TestUpdateService extends AbstractUpdateService { blockCancelUpdate(gate: Promise): void { this._cancelGate = gate; } /** Forces the service into a given state so tests can exercise cancellation from a cancellable state. */ - forceState(state: State): void { this.setState(state); } + forceState(state: State, options?: { deferred?: boolean }): void { this.setState(state, options); } feedUrl: string | undefined = 'https://update.example/feed'; @@ -341,6 +341,17 @@ suite('AbstractUpdateService', () => { }); }); + test('resumes an automatic download deferred after an update check', async () => { + const service = createService('default', { isConnectionMetered: true }); + await service.whenInitialized; + service.forceState(State.AvailableForDownload({ version: '1.1.0', productVersion: '1.1.0', url: 'https://update.example/download' }), { deferred: true }); + + meteredConnectionService.setIsConnectionMetered(false); + await timeout(0); + + assert.strictEqual(service.downloadCount, 1); + }); + test('resumes overwrite checks that were deferred by metering', async () => { const clock = sinon.useFakeTimers(); try { diff --git a/src/vs/workbench/contrib/meteredConnection/browser/meteredConnectionStatus.ts b/src/vs/workbench/contrib/meteredConnection/browser/meteredConnectionStatus.ts index 466419e06561fb..eea62f66bfd193 100644 --- a/src/vs/workbench/contrib/meteredConnection/browser/meteredConnectionStatus.ts +++ b/src/vs/workbench/contrib/meteredConnection/browser/meteredConnectionStatus.ts @@ -48,7 +48,7 @@ export class MeteredConnectionStatusContribution extends Disposable implements I name: localize('status.meteredConnection', "Metered Connection"), text: '$(radio-tower)', ariaLabel: localize('status.meteredConnection.ariaLabel', "Metered Connection Enabled"), - tooltip: localize('status.meteredConnection.tooltip', "Metered connection enabled. Background network activity including updates, Settings Sync, inline completions, telemetry, and automatic Git operations is paused to reduce data usage."), + tooltip: localize('status.meteredConnection.tooltip', "Metered connection enabled. Some background network activity, including updates, Settings Sync, inline completions, telemetry, and automatic Git operations, is paused to reduce data usage."), command: { id: 'workbench.action.configureMeteredConnection', title: localize('status.meteredConnection.configure', "Configure") diff --git a/src/vs/workbench/contrib/update/test/browser/postUpdateWidget.test.ts b/src/vs/workbench/contrib/update/test/electron-browser/postUpdateWidget.test.ts similarity index 98% rename from src/vs/workbench/contrib/update/test/browser/postUpdateWidget.test.ts rename to src/vs/workbench/contrib/update/test/electron-browser/postUpdateWidget.test.ts index 4735c6a176e421..e29834c8f9d18b 100644 --- a/src/vs/workbench/contrib/update/test/browser/postUpdateWidget.test.ts +++ b/src/vs/workbench/contrib/update/test/electron-browser/postUpdateWidget.test.ts @@ -35,7 +35,7 @@ class TestRequestService extends mock() { } } -suite('PostUpdateWidgetContribution', () => { +suite('PostUpdateWidgetContribution (Electron)', () => { const store = ensureNoDisposablesAreLeakedInTestSuite(); function createContribution(isConnectionMetered: boolean): TestRequestService { From fb03e92bb6d1ed91265e928234aff6767a2d03ef Mon Sep 17 00:00:00 2001 From: Dmitriy Vasyura Date: Thu, 20 Aug 2026 14:57:37 -0700 Subject: [PATCH 4/5] Fix metered update ordering races Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../common/meteredConnection.ts | 5 + .../meteredConnectionService.ts | 2 +- .../meteredConnectionMainService.ts | 8 + .../meteredConnectionMainService.test.ts | 36 +++++ .../electron-main/abstractUpdateService.ts | 53 +++++-- .../electron-main/updateService.win32.ts | 12 +- .../abstractUpdateService.test.ts | 138 +++++++++++++++++- 7 files changed, 234 insertions(+), 20 deletions(-) create mode 100644 src/vs/platform/meteredConnection/test/electron-main/meteredConnectionMainService.test.ts diff --git a/src/vs/platform/meteredConnection/common/meteredConnection.ts b/src/vs/platform/meteredConnection/common/meteredConnection.ts index 2eb6796aae722d..9448920e163844 100644 --- a/src/vs/platform/meteredConnection/common/meteredConnection.ts +++ b/src/vs/platform/meteredConnection/common/meteredConnection.ts @@ -23,6 +23,11 @@ export interface IMeteredConnectionService { */ readonly isConnectionMetered: boolean; + /** + * Resolves once the initial connection state is available, when initialization is asynchronous. + */ + readonly whenConnectionStateInitialized?: Promise; + /** * Event that fires when the metered connection status changes. */ diff --git a/src/vs/platform/meteredConnection/electron-browser/meteredConnectionService.ts b/src/vs/platform/meteredConnection/electron-browser/meteredConnectionService.ts index 4f1607adc73feb..728ce40afccf40 100644 --- a/src/vs/platform/meteredConnection/electron-browser/meteredConnectionService.ts +++ b/src/vs/platform/meteredConnection/electron-browser/meteredConnectionService.ts @@ -45,4 +45,4 @@ export class NativeMeteredConnectionService extends AbstractMeteredConnectionSer } } -registerSingleton(IMeteredConnectionService, new SyncDescriptor(NativeMeteredConnectionService, [getIsBrowserConnectionMetered], true)); +registerSingleton(IMeteredConnectionService, new SyncDescriptor(NativeMeteredConnectionService, [getIsBrowserConnectionMetered], false)); diff --git a/src/vs/platform/meteredConnection/electron-main/meteredConnectionMainService.ts b/src/vs/platform/meteredConnection/electron-main/meteredConnectionMainService.ts index 864e9f09693aed..c77a2fed7ef906 100644 --- a/src/vs/platform/meteredConnection/electron-main/meteredConnectionMainService.ts +++ b/src/vs/platform/meteredConnection/electron-main/meteredConnectionMainService.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { DeferredPromise } from '../../../base/common/async.js'; import { IConfigurationService } from '../../configuration/common/configuration.js'; import { ITelemetryService } from '../../telemetry/common/telemetry.js'; import { AbstractMeteredConnectionService } from '../common/meteredConnection.js'; @@ -13,6 +14,8 @@ import { AbstractMeteredConnectionService } from '../common/meteredConnection.js */ export class MeteredConnectionMainService extends AbstractMeteredConnectionService { private telemetryService: ITelemetryService | undefined; + private readonly connectionStateInitialized = new DeferredPromise(); + readonly whenConnectionStateInitialized = this.connectionStateInitialized.p; constructor(@IConfigurationService configurationService: IConfigurationService) { super(configurationService, false); @@ -22,6 +25,11 @@ export class MeteredConnectionMainService extends AbstractMeteredConnectionServi this.telemetryService = telemetryService; } + public override setIsBrowserConnectionMetered(value: boolean): void { + super.setIsBrowserConnectionMetered(value); + this.connectionStateInitialized.complete(); + } + protected override onChangeBrowserConnection() { // Fire event after sending telemetry if switching to metered since telemetry will be paused. const fireAfter = this.isBrowserConnectionMetered; diff --git a/src/vs/platform/meteredConnection/test/electron-main/meteredConnectionMainService.test.ts b/src/vs/platform/meteredConnection/test/electron-main/meteredConnectionMainService.test.ts new file mode 100644 index 00000000000000..6834a5d80f814d --- /dev/null +++ b/src/vs/platform/meteredConnection/test/electron-main/meteredConnectionMainService.test.ts @@ -0,0 +1,36 @@ +/*--------------------------------------------------------------------------------------------- + * 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 { timeout } from '../../../../base/common/async.js'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; +import { TestConfigurationService } from '../../../configuration/test/common/testConfigurationService.js'; +import { MeteredConnectionMainService } from '../../electron-main/meteredConnectionMainService.js'; + +suite('MeteredConnectionMainService', () => { + const store = ensureNoDisposablesAreLeakedInTestSuite(); + + test('initialization waits for the initial browser connection state', async () => { + const configurationService = new TestConfigurationService(); + store.add(configurationService.onDidChangeConfigurationEmitter); + const service = store.add(new MeteredConnectionMainService(configurationService)); + let initialized = false; + void service.whenConnectionStateInitialized.then(() => initialized = true); + + await timeout(0); + assert.strictEqual(initialized, false); + + service.setIsBrowserConnectionMetered(true); + await service.whenConnectionStateInitialized; + + assert.deepStrictEqual({ + initialized, + isConnectionMetered: service.isConnectionMetered, + }, { + initialized: true, + isConnectionMetered: true, + }); + }); +}); diff --git a/src/vs/platform/update/electron-main/abstractUpdateService.ts b/src/vs/platform/update/electron-main/abstractUpdateService.ts index dd81388e833206..2749d7beb453cb 100644 --- a/src/vs/platform/update/electron-main/abstractUpdateService.ts +++ b/src/vs/platform/update/electron-main/abstractUpdateService.ts @@ -21,7 +21,7 @@ import { IRequestService } from '../../request/common/request.js'; import { StorageScope, StorageTarget } from '../../storage/common/storage.js'; import { IApplicationStorageMainService } from '../../storage/electron-main/storageMainService.js'; import { ITelemetryService } from '../../telemetry/common/telemetry.js'; -import { AvailableForDownload, DisablementReason, IUpdateService, State, StateType, UpdateType } from '../common/update.js'; +import { AvailableForDownload, DisablementReason, IUpdate, IUpdateService, State, StateType, UpdateType } from '../common/update.js'; const LAST_KNOWN_VERSION_STORAGE_KEY = 'abstractUpdateService/lastKnownVersion'; @@ -209,6 +209,8 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat return; } + await this.meteredConnectionService.whenConnectionStateInitialized; + // React to runtime `update.mode`/policy changes so switching to/from `none` applies without a restart. this._register(this.configurationService.onDidChangeConfiguration(e => { if (e.affectsConfiguration('update.mode')) { @@ -265,8 +267,8 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat // One-time platform init, gated behind updates being enabled so a pending update is never resumed under `none`. if (!this._postInitialized) { - this._postInitialized = true; await this.postInitialize(); + this._postInitialized = true; } this.scheduleAccordingToMode(updateMode); @@ -310,15 +312,22 @@ 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'); return; } + if (this._state.deferred && !this.meteredConnectionService.isConnectionMetered) { + this.resumeAutomaticUpdates(); + return; + } + + if (this.state.type !== StateType.Idle) { + return; + } + this.setDeferred(false); + if (updateMode === 'start') { this.logService.info('update#ctor - startup checks only; automatic updates are disabled by user preference'); @@ -331,7 +340,7 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat } private resumeAutomaticUpdates(): void { - if (this._disabledPermanently || !this.quality) { + if (this._disabledPermanently || !this._postInitialized || !this.quality) { return; } @@ -498,6 +507,16 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat void this.downloadUpdate(false); } + protected deferAutomaticDownload(update: IUpdate, explicit: boolean): boolean { + if (explicit || !this.meteredConnectionService.isConnectionMetered) { + return false; + } + + this.logService.info('update#deferAutomaticDownload - deferring download because connection is metered'); + this.setState(State.AvailableForDownload(update), { deferred: true }); + return true; + } + async applyUpdate(): Promise { this.logService.trace('update#applyUpdate, state = ', this.state.type); @@ -555,9 +574,7 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat return false; } - if (!explicit && this.meteredConnectionService.isConnectionMetered) { - this.setDeferred(true); - this.logService.info('update#checkForOverwriteUpdates - skipping automatic check because connection is metered'); + if (this.deferOverwriteCheckIfMetered(explicit)) { return false; } @@ -583,6 +600,10 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat } if (isLatest === false && this.state.type === StateType.Ready) { + if (this.deferOverwriteCheckIfMetered(explicit)) { + return false; + } + this.logService.info('update#readyStateCheck: newer update available, restarting update machinery'); try { @@ -593,6 +614,10 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat return false; } + if (this.deferOverwriteCheckIfMetered(explicit)) { + return false; + } + this._overwrite = true; this.setState(State.Overwriting(this.state.update, explicit)); this.doCheckForUpdates(explicit, pendingUpdateCommit); @@ -602,6 +627,16 @@ export abstract class AbstractUpdateService extends Disposable implements IUpdat return false; } + private deferOverwriteCheckIfMetered(explicit: boolean): boolean { + if (explicit || !this.meteredConnectionService.isConnectionMetered) { + return false; + } + + this.setDeferred(true); + this.logService.info('update#checkForOverwriteUpdates - deferring overwrite because connection is metered'); + return true; + } + async isLatestVersion(commit?: string, token: CancellationToken = CancellationToken.None): Promise { if (this.meteredConnectionService.isConnectionMetered) { this.logService.info('update#isLatestVersion - skipping automatic check because connection is metered'); diff --git a/src/vs/platform/update/electron-main/updateService.win32.ts b/src/vs/platform/update/electron-main/updateService.win32.ts index 58078bd4077237..7628ad1ef04078 100644 --- a/src/vs/platform/update/electron-main/updateService.win32.ts +++ b/src/vs/platform/update/electron-main/updateService.win32.ts @@ -272,11 +272,7 @@ export class Win32UpdateService extends AbstractUpdateService implements IRelaun return Promise.resolve(null); } - // When connection is metered and this is not an explicit check, - // 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), { deferred: true }); + if (this.deferAutomaticDownload(update, explicit)) { return Promise.resolve(null); } @@ -290,6 +286,10 @@ export class Win32UpdateService extends AbstractUpdateService implements IRelaun return Promise.resolve(updatePackagePath); } + if (this.deferAutomaticDownload(update, explicit)) { + return undefined; + } + const downloadPath = `${updatePackagePath}.tmp`; return this.requestService.request({ url: update.url, callSite: 'updateService.win32.downloadUpdate' }, token) @@ -324,7 +324,7 @@ export class Win32UpdateService extends AbstractUpdateService implements IRelaun .then(() => updatePackagePath); }); }).then(packagePath => { - if (token.isCancellationRequested) { + if (!packagePath || token.isCancellationRequested) { return; } diff --git a/src/vs/platform/update/test/electron-main/abstractUpdateService.test.ts b/src/vs/platform/update/test/electron-main/abstractUpdateService.test.ts index bea89646e020e3..a493bdb603d777 100644 --- a/src/vs/platform/update/test/electron-main/abstractUpdateService.test.ts +++ b/src/vs/platform/update/test/electron-main/abstractUpdateService.test.ts @@ -6,6 +6,7 @@ import assert from 'assert'; import * as sinon from 'sinon'; import { DeferredPromise, timeout } from '../../../../base/common/async.js'; +import { CancellationToken } from '../../../../base/common/cancellation.js'; import { Emitter, Event } from '../../../../base/common/event.js'; import { Disposable } from '../../../../base/common/lifecycle.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; @@ -19,7 +20,7 @@ import { IProductService } from '../../../product/common/productService.js'; import { IRequestService } from '../../../request/common/request.js'; import { IApplicationStorageMainService } from '../../../storage/electron-main/storageMainService.js'; import { NullTelemetryService } from '../../../telemetry/common/telemetryUtils.js'; -import { DisablementReason, State, StateType } from '../../common/update.js'; +import { DisablementReason, IUpdate, State, StateType } from '../../common/update.js'; import { AbstractUpdateService, IUpdateURLOptions } from '../../electron-main/abstractUpdateService.js'; class TestMeteredConnectionService extends Disposable implements IMeteredConnectionService { @@ -28,7 +29,10 @@ class TestMeteredConnectionService extends Disposable implements IMeteredConnect private readonly _onDidChangeIsConnectionMetered = this._register(new Emitter()); readonly onDidChangeIsConnectionMetered = this._onDidChangeIsConnectionMetered.event; - constructor(public isConnectionMetered: boolean) { + constructor( + public isConnectionMetered: boolean, + readonly whenConnectionStateInitialized?: Promise, + ) { super(); } @@ -42,6 +46,10 @@ class TestUpdateService extends AbstractUpdateService { private readonly _initialized = new DeferredPromise(); get whenInitialized(): Promise { return this._initialized.p; } + private readonly _postInitializeStarted = new DeferredPromise(); + get whenPostInitializeStarted(): Promise { return this._postInitializeStarted.p; } + private _postInitializeGate: Promise | undefined; + blockPostInitialize(gate: Promise): void { this._postInitializeGate = gate; } private _checkCount = 0; get checkCount(): number { return this._checkCount; } @@ -51,6 +59,11 @@ class TestUpdateService extends AbstractUpdateService { private _downloadCount = 0; get downloadCount(): number { return this._downloadCount; } + private _latestVersionResult: Promise | undefined; + setLatestVersionResult(result: Promise): void { this._latestVersionResult = result; } + deferDownload(update: IUpdate, explicit: boolean): boolean { + return this.deferAutomaticDownload(update, explicit); + } /** When set, `cancelUpdate` blocks on this promise so tests can observe the transient Cancelling state. */ private _cancelGate: Promise | undefined; @@ -85,6 +98,15 @@ class TestUpdateService extends AbstractUpdateService { return this.doIsLatestVersion(); } + protected override doIsLatestVersion(commit?: string, token?: CancellationToken): Promise { + return this._latestVersionResult ?? super.doIsLatestVersion(commit, token); + } + + protected override async postInitialize(): Promise { + this._postInitializeStarted.complete(); + await this._postInitializeGate; + } + protected override async cancelUpdate(): Promise { this._cancelCount++; if (this._cancelGate) { @@ -122,7 +144,7 @@ suite('AbstractUpdateService', () => { let requestCount: number; let meteredConnectionService: TestMeteredConnectionService; - function createService(mode: string, options?: { isBuilt?: boolean; disableUpdates?: boolean; updateUrl?: string; isConnectionMetered?: boolean; supportsUpdateOverwrite?: boolean }): TestUpdateService { + function createService(mode: string, options?: { isBuilt?: boolean; disableUpdates?: boolean; updateUrl?: string; isConnectionMetered?: boolean; meteredConnectionInitialization?: Promise; postInitializeGate?: Promise; supportsUpdateOverwrite?: boolean }): TestUpdateService { configurationService = new PolicyTestConfigurationService(); configurationService.setUserConfiguration('update.mode', mode); requestCount = 0; @@ -160,7 +182,7 @@ suite('AbstractUpdateService', () => { store: () => { } } as unknown as IApplicationStorageMainService; - meteredConnectionService = store.add(new TestMeteredConnectionService(options?.isConnectionMetered ?? false)); + meteredConnectionService = store.add(new TestMeteredConnectionService(options?.isConnectionMetered ?? false, options?.meteredConnectionInitialization)); const service = new TestUpdateService( lifecycleMainService, @@ -174,6 +196,9 @@ suite('AbstractUpdateService', () => { meteredConnectionService, options?.supportsUpdateOverwrite ?? false ); + if (options?.postInitializeGate) { + service.blockPostInitialize(options.postInitializeGate); + } return store.add(service); } @@ -257,6 +282,52 @@ suite('AbstractUpdateService', () => { } }); + test('automatic scheduling waits for the initial metered connection state', async () => { + const clock = sinon.useFakeTimers(); + try { + const connectionInitialized = new DeferredPromise(); + const service = createService('default', { meteredConnectionInitialization: connectionInitialized.p }); + + await clock.tickAsync(30 * 1000); + assert.strictEqual(service.checkCount, 0); + + meteredConnectionService.setIsConnectionMetered(true); + connectionInitialized.complete(); + await service.whenInitialized; + await clock.tickAsync(30 * 1000); + assert.strictEqual(service.checkCount, 0); + + meteredConnectionService.setIsConnectionMetered(false); + await clock.tickAsync(0); + assert.strictEqual(service.checkCount, 1); + } finally { + clock.restore(); + } + }); + + test('unmetering during post-initialization does not start a check', async () => { + const clock = sinon.useFakeTimers(); + try { + const postInitializeGate = new DeferredPromise(); + const service = createService('default', { isConnectionMetered: true, postInitializeGate: postInitializeGate.p }); + await service.whenPostInitializeStarted; + + meteredConnectionService.setIsConnectionMetered(false); + await clock.tickAsync(0); + assert.strictEqual(service.checkCount, 0); + + postInitializeGate.complete(); + await service.whenInitialized; + await clock.tickAsync(0); + assert.strictEqual(service.checkCount, 0); + + await clock.tickAsync(30 * 1000); + assert.strictEqual(service.checkCount, 1); + } finally { + clock.restore(); + } + }); + test('metered connections skip automatic update requests but allow explicit actions', async () => { const service = createService('default', { isConnectionMetered: true }); await service.whenInitialized; @@ -352,6 +423,42 @@ suite('AbstractUpdateService', () => { assert.strictEqual(service.downloadCount, 1); }); + test('defers an automatic download when connection becomes metered during preparation', async () => { + const service = createService('default'); + await service.whenInitialized; + const update = { version: '1.1.0', productVersion: '1.1.0', url: 'https://update.example/download' }; + service.forceState(State.Downloading(update, false, false)); + + meteredConnectionService.setIsConnectionMetered(true); + const deferred = service.deferDownload(update, false); + assert.deepStrictEqual({ deferred, state: service.state.type, downloadCount: service.downloadCount }, { + deferred: true, + state: StateType.AvailableForDownload, + downloadCount: 0, + }); + + meteredConnectionService.setIsConnectionMetered(false); + await timeout(0); + assert.strictEqual(service.downloadCount, 1); + }); + + test('resumes deferred work when automatic mode is re-enabled while unmetered', async () => { + const service = createService('manual', { isConnectionMetered: true }); + await service.whenInitialized; + service.forceState(State.AvailableForDownload({ version: '1.1.0', productVersion: '1.1.0', url: 'https://update.example/download' }), { deferred: true }); + + meteredConnectionService.setIsConnectionMetered(false); + await timeout(0); + assert.strictEqual(service.downloadCount, 0); + + configurationService.setUserConfiguration('update.mode', 'default'); + configurationService.onDidChangeConfigurationEmitter.fire({ affectsConfiguration: () => true } as unknown as IConfigurationChangeEvent); + await timeout(0); + await timeout(0); + + assert.strictEqual(service.downloadCount, 1); + }); + test('resumes overwrite checks that were deferred by metering', async () => { const clock = sinon.useFakeTimers(); try { @@ -371,6 +478,29 @@ suite('AbstractUpdateService', () => { } }); + test('defers overwrite continuation when connection becomes metered during latest-version probe', async () => { + const clock = sinon.useFakeTimers(); + try { + const latestVersionResult = new DeferredPromise(); + const service = createService('default', { supportsUpdateOverwrite: true }); + await service.whenInitialized; + service.setLatestVersionResult(latestVersionResult.p); + service.forceState(State.Ready({ version: 'pending' }, false, false)); + + await clock.tickAsync(5 * 60 * 1000); + meteredConnectionService.setIsConnectionMetered(true); + latestVersionResult.complete(false); + await clock.tickAsync(0); + assert.deepStrictEqual({ checkCount: service.checkCount, state: service.state.type }, { checkCount: 0, state: StateType.Ready }); + + meteredConnectionService.setIsConnectionMetered(false); + await clock.tickAsync(0); + assert.deepStrictEqual({ checkCount: service.checkCount, state: service.state.type }, { checkCount: 1, state: StateType.Overwriting }); + } finally { + clock.restore(); + } + }); + test('permanent disablement ignores runtime mode changes', async () => { const service = createService('default', { isBuilt: false }); await service.whenInitialized; From b726f11b8d0828f7aebffc37aa4a37bd1a10dd29 Mon Sep 17 00:00:00 2001 From: Dmitriy Vasyura Date: Thu, 20 Aug 2026 15:16:17 -0700 Subject: [PATCH 5/5] chore: create empty commit Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>