From c8024c841781b8a255f95a4eb5250bf3fc3d94c4 Mon Sep 17 00:00:00 2001 From: BeniBenj Date: Thu, 7 Sep 2023 11:59:35 +0200 Subject: [PATCH 1/4] fix floating scrollbar, remove hardcoded value --- src/vs/workbench/browser/parts/editor/noTabsTitleControl.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/vs/workbench/browser/parts/editor/noTabsTitleControl.ts b/src/vs/workbench/browser/parts/editor/noTabsTitleControl.ts index 0772d57a3ce..460698f6749 100644 --- a/src/vs/workbench/browser/parts/editor/noTabsTitleControl.ts +++ b/src/vs/workbench/browser/parts/editor/noTabsTitleControl.ts @@ -26,8 +26,6 @@ interface IRenderedEditorLabel { export class NoTabsTitleControl extends TitleControl { - private static readonly HEIGHT = 35; - private titleContainer: HTMLElement | undefined; private editorLabel: IResourceLabel | undefined; private activeLabel: IRenderedEditorLabel = Object.create(null); @@ -352,7 +350,7 @@ export class NoTabsTitleControl extends TitleControl { getHeight(): IEditorGroupTitleHeight { return { - total: NoTabsTitleControl.HEIGHT, + total: this.titleHeight, offset: 0 }; } From ed0d3d50f1118621da5ca0adda0fff499f9aff2d Mon Sep 17 00:00:00 2001 From: Johannes Date: Thu, 7 Sep 2023 12:58:16 +0200 Subject: [PATCH 2/4] a bit more `ensureNoDisposablesAreLeakedInTestSuite` work --- .../browser/services/openerService.test.ts | 21 +++++++++++-------- .../test/common/uriIdentityService.test.ts | 7 +++++++ .../test/node/commonProperties.test.ts | 3 +++ 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/vs/editor/test/browser/services/openerService.test.ts b/src/vs/editor/test/browser/services/openerService.test.ts index d823f89d285..4f1b579d5e7 100644 --- a/src/vs/editor/test/browser/services/openerService.test.ts +++ b/src/vs/editor/test/browser/services/openerService.test.ts @@ -5,6 +5,7 @@ import * as assert from 'assert'; import { Disposable } from 'vs/base/common/lifecycle'; import { URI } from 'vs/base/common/uri'; +import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils'; import { OpenerService } from 'vs/editor/browser/services/openerService'; import { TestCodeEditorService } from 'vs/editor/test/browser/editorTestServices'; import { CommandsRegistry, ICommandService } from 'vs/platform/commands/common/commands'; @@ -33,6 +34,8 @@ suite('OpenerService', function () { lastCommand = undefined; }); + const store = ensureNoDisposablesAreLeakedInTestSuite(); + test('delegate to editorService, scheme:///fff', async function () { const openerService = new OpenerService(editorService, NullCommandService); await openerService.open(URI.parse('another:///somepath')); @@ -83,7 +86,7 @@ suite('OpenerService', function () { const openerService = new OpenerService(editorService, commandService); const id = `aCommand${Math.random()}`; - CommandsRegistry.registerCommand(id, function () { }); + store.add(CommandsRegistry.registerCommand(id, function () { })); assert.strictEqual(lastCommand, undefined); await openerService.open(URI.parse('command:' + id)); @@ -91,11 +94,11 @@ suite('OpenerService', function () { }); - test('delegate to commandsService, command:someid', async function () { + test('delegate to commandsService, command:someid, 2', async function () { const openerService = new OpenerService(editorService, commandService); const id = `aCommand${Math.random()}`; - CommandsRegistry.registerCommand(id, function () { }); + store.add(CommandsRegistry.registerCommand(id, function () { })); await openerService.open(URI.parse('command:' + id).with({ query: '\"123\"' }), { allowCommands: true }); assert.strictEqual(lastCommand!.id, id); @@ -121,7 +124,7 @@ suite('OpenerService', function () { test('links are protected by validators', async function () { const openerService = new OpenerService(editorService, commandService); - openerService.registerValidator({ shouldOpen: () => Promise.resolve(false) }); + store.add(openerService.registerValidator({ shouldOpen: () => Promise.resolve(false) })); const httpResult = await openerService.open(URI.parse('https://www.microsoft.com')); const httpsResult = await openerService.open(URI.parse('https://www.microsoft.com')); @@ -132,15 +135,15 @@ suite('OpenerService', function () { test('links validated by validators go to openers', async function () { const openerService = new OpenerService(editorService, commandService); - openerService.registerValidator({ shouldOpen: () => Promise.resolve(true) }); + store.add(openerService.registerValidator({ shouldOpen: () => Promise.resolve(true) })); let openCount = 0; - openerService.registerOpener({ + store.add(openerService.registerOpener({ open: (resource: URI) => { openCount++; return Promise.resolve(true); } - }); + })); await openerService.open(URI.parse('http://microsoft.com')); assert.strictEqual(openCount, 1); @@ -151,13 +154,13 @@ suite('OpenerService', function () { test('links aren\'t manipulated before being passed to validator: PR #118226', async function () { const openerService = new OpenerService(editorService, commandService); - openerService.registerValidator({ + store.add(openerService.registerValidator({ shouldOpen: (resource) => { // We don't want it to convert strings into URIs assert.strictEqual(resource instanceof URI, false); return Promise.resolve(false); } - }); + })); await openerService.open('https://wwww.microsoft.com'); await openerService.open('https://www.microsoft.com??params=CountryCode%3DUSA%26Name%3Dvscode"'); }); diff --git a/src/vs/platform/uriIdentity/test/common/uriIdentityService.test.ts b/src/vs/platform/uriIdentity/test/common/uriIdentityService.test.ts index caea0117d08..4b20f54a4db 100644 --- a/src/vs/platform/uriIdentity/test/common/uriIdentityService.test.ts +++ b/src/vs/platform/uriIdentity/test/common/uriIdentityService.test.ts @@ -9,6 +9,7 @@ import { mock } from 'vs/base/test/common/mock'; import { IFileService, FileSystemProviderCapabilities } from 'vs/platform/files/common/files'; import { URI } from 'vs/base/common/uri'; import { Event } from 'vs/base/common/event'; +import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils'; suite('URI Identity', function () { @@ -38,6 +39,12 @@ suite('URI Identity', function () { ]))); }); + teardown(function () { + _service.dispose(); + }); + + ensureNoDisposablesAreLeakedInTestSuite(); + function assertCanonical(input: URI, expected: URI, service: UriIdentityService = _service) { const actual = service.asCanonicalUri(input); assert.strictEqual(actual.toString(), expected.toString()); diff --git a/src/vs/workbench/services/telemetry/test/node/commonProperties.test.ts b/src/vs/workbench/services/telemetry/test/node/commonProperties.test.ts index 8759a80f67d..8600bd13f15 100644 --- a/src/vs/workbench/services/telemetry/test/node/commonProperties.test.ts +++ b/src/vs/workbench/services/telemetry/test/node/commonProperties.test.ts @@ -8,6 +8,7 @@ import { release, hostname } from 'os'; import { resolveWorkbenchCommonProperties } from 'vs/workbench/services/telemetry/common/workbenchCommonProperties'; import { IStorageService, StorageScope, InMemoryStorageService, StorageTarget } from 'vs/platform/storage/common/storage'; import { timeout } from 'vs/base/common/async'; +import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils'; suite('Telemetry - common properties', function () { const commit: string = (undefined)!; @@ -18,6 +19,8 @@ suite('Telemetry - common properties', function () { testStorageService = new InMemoryStorageService(); }); + ensureNoDisposablesAreLeakedInTestSuite(); + test('default', function () { const props = resolveWorkbenchCommonProperties(testStorageService, release(), hostname(), commit, version, 'someMachineId', false, process); assert.ok('commitHash' in props); From 4a6e5c124d2d541afcc363551459d252ae54fb5a Mon Sep 17 00:00:00 2001 From: Johannes Date: Thu, 7 Sep 2023 14:16:36 +0200 Subject: [PATCH 3/4] update notebook milesones --- .vscode/notebooks/api.github-issues | 2 +- .vscode/notebooks/my-work.github-issues | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.vscode/notebooks/api.github-issues b/.vscode/notebooks/api.github-issues index 706cb4d5d38..3c234735491 100644 --- a/.vscode/notebooks/api.github-issues +++ b/.vscode/notebooks/api.github-issues @@ -7,7 +7,7 @@ { "kind": 2, "language": "github-issues", - "value": "$repo=repo:microsoft/vscode\n$milestone=milestone:\"August 2023\"" + "value": "$repo=repo:microsoft/vscode\n$milestone=milestone:\"September 2023\"" }, { "kind": 1, diff --git a/.vscode/notebooks/my-work.github-issues b/.vscode/notebooks/my-work.github-issues index 06efd345018..e48617f8fde 100644 --- a/.vscode/notebooks/my-work.github-issues +++ b/.vscode/notebooks/my-work.github-issues @@ -7,7 +7,7 @@ { "kind": 2, "language": "github-issues", - "value": "// list of repos we work in\n$repos=repo:microsoft/vscode repo:microsoft/vscode-remote-release repo:microsoft/vscode-js-debug repo:microsoft/vscode-pull-request-github repo:microsoft/vscode-github-issue-notebooks repo:microsoft/vscode-internalbacklog repo:microsoft/vscode-dev repo:microsoft/vscode-unpkg repo:microsoft/vscode-references-view repo:microsoft/vscode-anycode repo:microsoft/vscode-hexeditor repo:microsoft/vscode-extension-telemetry repo:microsoft/vscode-livepreview repo:microsoft/vscode-remotehub repo:microsoft/vscode-settings-sync-server repo:microsoft/vscode-remote-repositories-github repo:microsoft/monaco-editor repo:microsoft/vscode-vsce repo:microsoft/vscode-dev-chrome-launcher repo:microsoft/vscode-emmet-helper repo:microsoft/vscode-python repo:microsoft/vscode-jupyter repo:microsoft/vscode-jupyter-internal repo:microsoft/vscode-github-issue-notebooks repo:microsoft/vscode-l10n repo:microsoft/vscode-remote-tunnels repo:microsoft/vscode-markdown-tm-grammar repo:microsoft/vscode-markdown-languageservice repo:microsoft/vscode-copilot repo:microsoft/vscode-copilot-release\n\n// current milestone name\n$milestone=milestone:\"August 2023\"" + "value": "// list of repos we work in\n$repos=repo:microsoft/vscode repo:microsoft/vscode-remote-release repo:microsoft/vscode-js-debug repo:microsoft/vscode-pull-request-github repo:microsoft/vscode-github-issue-notebooks repo:microsoft/vscode-internalbacklog repo:microsoft/vscode-dev repo:microsoft/vscode-unpkg repo:microsoft/vscode-references-view repo:microsoft/vscode-anycode repo:microsoft/vscode-hexeditor repo:microsoft/vscode-extension-telemetry repo:microsoft/vscode-livepreview repo:microsoft/vscode-remotehub repo:microsoft/vscode-settings-sync-server repo:microsoft/vscode-remote-repositories-github repo:microsoft/monaco-editor repo:microsoft/vscode-vsce repo:microsoft/vscode-dev-chrome-launcher repo:microsoft/vscode-emmet-helper repo:microsoft/vscode-python repo:microsoft/vscode-jupyter repo:microsoft/vscode-jupyter-internal repo:microsoft/vscode-github-issue-notebooks repo:microsoft/vscode-l10n repo:microsoft/vscode-remote-tunnels repo:microsoft/vscode-markdown-tm-grammar repo:microsoft/vscode-markdown-languageservice repo:microsoft/vscode-copilot repo:microsoft/vscode-copilot-release\n\n// current milestone name\n$milestone=milestone:\"September 2023\"" }, { "kind": 1, From df9171e630222778149cf1cc1edf3e7fd66ad124 Mon Sep 17 00:00:00 2001 From: Alex Ross Date: Thu, 7 Sep 2023 14:36:47 +0200 Subject: [PATCH 4/4] `vscode.TreeView.dispose` doesn't clean up in the renderer (#192403) Fixes #192126 --- .../api/browser/mainThreadTreeViews.ts | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadTreeViews.ts b/src/vs/workbench/api/browser/mainThreadTreeViews.ts index 279a4f05c28..5451d23f352 100644 --- a/src/vs/workbench/api/browser/mainThreadTreeViews.ts +++ b/src/vs/workbench/api/browser/mainThreadTreeViews.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { Disposable } from 'vs/base/common/lifecycle'; +import { Disposable, DisposableStore } from 'vs/base/common/lifecycle'; import { ExtHostContext, MainThreadTreeViewsShape, ExtHostTreeViewsShape, MainContext, CheckboxUpdate } from 'vs/workbench/api/common/extHost.protocol'; import { ITreeViewDataProvider, ITreeItem, IViewsService, ITreeView, IViewsRegistry, ITreeViewDescriptor, IRevealOptions, Extensions, ResolvableTreeItem, ITreeViewDragAndDropController, IViewBadge, NoTreeViewError } from 'vs/workbench/common/views'; import { extHostNamedCustomer, IExtHostContext } from 'vs/workbench/services/extensions/common/extHostCustomers'; @@ -24,7 +24,7 @@ import { IMarkdownString } from 'vs/base/common/htmlContent'; export class MainThreadTreeViews extends Disposable implements MainThreadTreeViewsShape { private readonly _proxy: ExtHostTreeViewsShape; - private readonly _dataProviders: Map = new Map(); + private readonly _dataProviders: Map = new Map(); private readonly _dndControllers = new Map(); constructor( @@ -43,7 +43,9 @@ export class MainThreadTreeViews extends Disposable implements MainThreadTreeVie this.extensionService.whenInstalledExtensionsRegistered().then(() => { const dataProvider = new TreeViewDataProvider(treeViewId, this._proxy, this.notificationService); - this._dataProviders.set(treeViewId, dataProvider); + const disposables = new DisposableStore(); + this._register(disposables); + this._dataProviders.set(treeViewId, { dataProvider, disposables }); const dndController = (options.hasHandleDrag || options.hasHandleDrop) ? new TreeViewDragAndDropController(treeViewId, options.dropMimeTypes, options.dragMimeTypes, options.hasHandleDrag, this._proxy) : undefined; const viewer = this.getTreeView(treeViewId); @@ -58,7 +60,7 @@ export class MainThreadTreeViews extends Disposable implements MainThreadTreeVie this._dndControllers.set(treeViewId, dndController); } viewer.dataProvider = dataProvider; - this.registerListeners(treeViewId, viewer); + this.registerListeners(treeViewId, viewer, disposables); this._proxy.$setVisible(treeViewId, viewer.visible); } else { this.notificationService.error('No view is registered with id: ' + treeViewId); @@ -73,7 +75,7 @@ export class MainThreadTreeViews extends Disposable implements MainThreadTreeVie .then(() => { const viewer = this.getTreeView(treeViewId); if (viewer && itemInfo) { - return this.reveal(viewer, this._dataProviders.get(treeViewId)!, itemInfo.item, itemInfo.parentChain, options); + return this.reveal(viewer, this._dataProviders.get(treeViewId)!.dataProvider, itemInfo.item, itemInfo.parentChain, options); } return undefined; }); @@ -85,7 +87,7 @@ export class MainThreadTreeViews extends Disposable implements MainThreadTreeVie const viewer = this.getTreeView(treeViewId); const dataProvider = this._dataProviders.get(treeViewId); if (viewer && dataProvider) { - const itemsToRefresh = dataProvider.getItemsToRefresh(itemsToRefreshByHandle); + const itemsToRefresh = dataProvider.dataProvider.getItemsToRefresh(itemsToRefreshByHandle); return viewer.refresh(itemsToRefresh.length ? itemsToRefresh : undefined); } return Promise.resolve(); @@ -132,6 +134,11 @@ export class MainThreadTreeViews extends Disposable implements MainThreadTreeVie if (viewer) { viewer.dataProvider = undefined; } + const dataProvider = this._dataProviders.get(treeViewId); + if (dataProvider) { + dataProvider.disposables.dispose(); + this._dataProviders.delete(treeViewId); + } } private async reveal(treeView: ITreeView, dataProvider: TreeViewDataProvider, itemIn: ITreeItem, parentChain: ITreeItem[], options: IRevealOptions): Promise { @@ -175,12 +182,12 @@ export class MainThreadTreeViews extends Disposable implements MainThreadTreeVie } } - private registerListeners(treeViewId: string, treeView: ITreeView): void { - this._register(treeView.onDidExpandItem(item => this._proxy.$setExpanded(treeViewId, item.handle, true))); - this._register(treeView.onDidCollapseItem(item => this._proxy.$setExpanded(treeViewId, item.handle, false))); - this._register(treeView.onDidChangeSelectionAndFocus(items => this._proxy.$setSelectionAndFocus(treeViewId, items.selection.map(({ handle }) => handle), items.focus.handle))); - this._register(treeView.onDidChangeVisibility(isVisible => this._proxy.$setVisible(treeViewId, isVisible))); - this._register(treeView.onDidChangeCheckboxState(items => { + private registerListeners(treeViewId: string, treeView: ITreeView, disposables: DisposableStore): void { + disposables.add(treeView.onDidExpandItem(item => this._proxy.$setExpanded(treeViewId, item.handle, true))); + disposables.add(treeView.onDidCollapseItem(item => this._proxy.$setExpanded(treeViewId, item.handle, false))); + disposables.add(treeView.onDidChangeSelectionAndFocus(items => this._proxy.$setSelectionAndFocus(treeViewId, items.selection.map(({ handle }) => handle), items.focus.handle))); + disposables.add(treeView.onDidChangeVisibility(isVisible => this._proxy.$setVisible(treeViewId, isVisible))); + disposables.add(treeView.onDidChangeCheckboxState(items => { this._proxy.$changeCheckboxState(treeViewId, items.map(item => { return { treeItemHandle: item.handle, newState: item.checkbox?.isChecked ?? false }; }));