From d57db50463f36391598c4513eca79bd0c5b69091 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Wed, 5 Jun 2019 15:30:23 -0700 Subject: [PATCH] Fixing a few possible leaked disposables --- .../editor/contrib/find/simpleFindWidget.ts | 12 +++++------ .../editor/contrib/suggest/suggestWidget.ts | 10 +++++---- src/vs/platform/actions/common/menuService.ts | 21 +++++++------------ .../browser/parts/editor/tabsTitleControl.ts | 2 +- .../electron-browser/extensionsList.ts | 6 +++--- .../quickopen/browser/commandsHandler.ts | 14 +++++++++---- .../electron-browser/webviewElement.ts | 7 +++++-- 7 files changed, 39 insertions(+), 33 deletions(-) diff --git a/src/vs/editor/contrib/find/simpleFindWidget.ts b/src/vs/editor/contrib/find/simpleFindWidget.ts index c7833dcc253..2c1cb22025f 100644 --- a/src/vs/editor/contrib/find/simpleFindWidget.ts +++ b/src/vs/editor/contrib/find/simpleFindWidget.ts @@ -99,29 +99,29 @@ export abstract class SimpleFindWidget extends Widget { } })); - const prevBtn = new SimpleButton({ + const prevBtn = this._register(new SimpleButton({ label: NLS_PREVIOUS_MATCH_BTN_LABEL, className: 'previous', onTrigger: () => { this.find(true); } - }); + })); - const nextBtn = new SimpleButton({ + const nextBtn = this._register(new SimpleButton({ label: NLS_NEXT_MATCH_BTN_LABEL, className: 'next', onTrigger: () => { this.find(false); } - }); + })); - const closeBtn = new SimpleButton({ + const closeBtn = this._register(new SimpleButton({ label: NLS_CLOSE_BTN_LABEL, className: 'close-fw', onTrigger: () => { this.hide(); } - }); + })); this._innerDomNode = document.createElement('div'); this._innerDomNode.classList.add('simple-find-part'); diff --git a/src/vs/editor/contrib/suggest/suggestWidget.ts b/src/vs/editor/contrib/suggest/suggestWidget.ts index b81fbd2526d..265ccca9ab8 100644 --- a/src/vs/editor/contrib/suggest/suggestWidget.ts +++ b/src/vs/editor/contrib/suggest/suggestWidget.ts @@ -103,7 +103,9 @@ class Renderer implements IListRenderer renderTemplate(container: HTMLElement): ISuggestionTemplateData { const data = Object.create(null); - data.disposables = []; + const disposables = new DisposableStore(); + data.disposables = [disposables]; + data.root = container; addClass(data.root, 'show-file-icons'); @@ -114,7 +116,7 @@ class Renderer implements IListRenderer const main = append(text, $('.main')); data.iconLabel = new IconLabel(main, { supportHighlights: true }); - data.disposables.push(data.iconLabel); + disposables.add(data.iconLabel); data.typeLabel = append(main, $('span.type-label')); @@ -142,9 +144,9 @@ class Renderer implements IListRenderer configureFont(); - Event.chain(this.editor.onDidChangeConfiguration.bind(this.editor)) + disposables.add(Event.chain(this.editor.onDidChangeConfiguration.bind(this.editor)) .filter(e => e.fontInfo || e.contribInfo) - .on(configureFont, null, data.disposables); + .on(configureFont, null)); return data; } diff --git a/src/vs/platform/actions/common/menuService.ts b/src/vs/platform/actions/common/menuService.ts index 1523ae10337..e0ee881b773 100644 --- a/src/vs/platform/actions/common/menuService.ts +++ b/src/vs/platform/actions/common/menuService.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { Emitter, Event } from 'vs/base/common/event'; -import { dispose, IDisposable } from 'vs/base/common/lifecycle'; +import { Disposable } from 'vs/base/common/lifecycle'; import { IMenu, IMenuActionOptions, IMenuItem, IMenuService, isIMenuItem, ISubmenuItem, MenuId, MenuItemAction, MenuRegistry, SubmenuItemAction } from 'vs/platform/actions/common/actions'; import { ICommandService } from 'vs/platform/commands/common/commands'; import { ContextKeyExpr, IContextKeyService, IContextKeyChangeEvent } from 'vs/platform/contextkey/common/contextkey'; @@ -27,10 +27,9 @@ export class MenuService implements IMenuService { type MenuItemGroup = [string, Array]; -class Menu implements IMenu { +class Menu extends Disposable implements IMenu { - private readonly _onDidChange = new Emitter(); - private readonly _disposables: IDisposable[] = []; + private readonly _onDidChange = this._register(new Emitter()); private _menuGroups: MenuItemGroup[]; private _contextKeys: Set; @@ -40,23 +39,24 @@ class Menu implements IMenu { @ICommandService private readonly _commandService: ICommandService, @IContextKeyService private readonly _contextKeyService: IContextKeyService ) { + super(); this._build(); // rebuild this menu whenever the menu registry reports an // event for this MenuId - Event.debounce( + this._register(Event.debounce( Event.filter(MenuRegistry.onDidChangeMenu, menuId => menuId === this._id), () => { }, 50 - )(this._build, this, this._disposables); + )(this._build, this)); // when context keys change we need to check if the menu also // has changed - Event.debounce( + this._register(Event.debounce( this._contextKeyService.onDidChangeContext, (last, event) => last || event.affectsSome(this._contextKeys), 50 - )(e => e && this._onDidChange.fire(undefined), this, this._disposables); + )(e => e && this._onDidChange.fire(undefined), this)); } private _build(): void { @@ -95,11 +95,6 @@ class Menu implements IMenu { this._onDidChange.fire(this); } - dispose() { - dispose(this._disposables); - this._onDidChange.dispose(); - } - get onDidChange(): Event { return this._onDidChange.event; } diff --git a/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts b/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts index 66c7c480c7a..7578bda2fc2 100644 --- a/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts +++ b/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts @@ -107,7 +107,7 @@ export class TabsTitleControl extends TitleControl { this.registerTabsContainerListeners(); // Tabs Scrollbar - this.tabsScrollbar = this.createTabsScrollbar(this.tabsContainer); + this.tabsScrollbar = this._register(this.createTabsScrollbar(this.tabsContainer)); tabsAndActionsContainer.appendChild(this.tabsScrollbar.getDomNode()); // Editor Toolbar Container diff --git a/src/vs/workbench/contrib/extensions/electron-browser/extensionsList.ts b/src/vs/workbench/contrib/extensions/electron-browser/extensionsList.ts index aac51bff673..585a81141ec 100644 --- a/src/vs/workbench/contrib/extensions/electron-browser/extensionsList.ts +++ b/src/vs/workbench/contrib/extensions/electron-browser/extensionsList.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { append, $, addClass, removeClass, toggleClass } from 'vs/base/browser/dom'; -import { IDisposable, dispose } from 'vs/base/common/lifecycle'; +import { IDisposable, dispose, combinedDisposable } from 'vs/base/common/lifecycle'; import { Action } from 'vs/base/common/actions'; import { ActionBar } from 'vs/base/browser/ui/actionbar/actionbar'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; @@ -116,10 +116,10 @@ export class Renderer implements IPagedRenderer { const extensionContainers: ExtensionContainers = this.instantiationService.createInstance(ExtensionContainers, [...actions, ...widgets, disabledLabelAction]); actionbar.push(actions, actionOptions); - const disposables = [...actions, ...widgets, actionbar, extensionContainers]; + const disposables = combinedDisposable(...actions, ...widgets, actionbar, extensionContainers); return { - root, element, icon, name, installCount, ratings, author, description, disposables, actionbar, + root, element, icon, name, installCount, ratings, author, description, disposables: [disposables], actionbar, extensionDisposables: [], set extension(extension: IExtension) { extensionContainers.extension = extension; diff --git a/src/vs/workbench/contrib/quickopen/browser/commandsHandler.ts b/src/vs/workbench/contrib/quickopen/browser/commandsHandler.ts index 8d72b3c257a..67fa33738d1 100644 --- a/src/vs/workbench/contrib/quickopen/browser/commandsHandler.ts +++ b/src/vs/workbench/contrib/quickopen/browser/commandsHandler.ts @@ -30,7 +30,7 @@ import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { INotificationService } from 'vs/platform/notification/common/notification'; import { CancellationToken } from 'vs/base/common/cancellation'; import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions'; -import { Disposable } from 'vs/base/common/lifecycle'; +import { Disposable, DisposableStore, IDisposable } from 'vs/base/common/lifecycle'; import { timeout } from 'vs/base/common/async'; export const ALL_COMMANDS_PREFIX = '>'; @@ -377,12 +377,14 @@ class ActionCommandEntry extends BaseCommandEntry { const wordFilter = or(matchesPrefix, matchesWords, matchesContiguousSubString); -export class CommandsHandler extends QuickOpenHandler { +export class CommandsHandler extends QuickOpenHandler implements IDisposable { static readonly ID = 'workbench.picker.commands'; private commandHistoryEnabled: boolean; - private commandsHistory: CommandsHistory; + private readonly commandsHistory: CommandsHistory; + + private readonly disposables = new DisposableStore(); private waitedForExtensionsRegistered: boolean; @@ -396,7 +398,7 @@ export class CommandsHandler extends QuickOpenHandler { ) { super(); - this.commandsHistory = this.instantiationService.createInstance(CommandsHistory); + this.commandsHistory = this.disposables.add(this.instantiationService.createInstance(CommandsHistory)); this.extensionService.whenInstalledExtensionsRegistered().then(() => this.waitedForExtensionsRegistered = true); @@ -588,6 +590,10 @@ export class CommandsHandler extends QuickOpenHandler { getEmptyLabel(searchString: string): string { return nls.localize('noCommandsMatching', "No commands matching"); } + + dispose() { + this.disposables.dispose(); + } } registerEditorAction(CommandPaletteEditorAction); diff --git a/src/vs/workbench/contrib/webview/electron-browser/webviewElement.ts b/src/vs/workbench/contrib/webview/electron-browser/webviewElement.ts index 48e77cdadce..148b2cb8300 100644 --- a/src/vs/workbench/contrib/webview/electron-browser/webviewElement.ts +++ b/src/vs/workbench/contrib/webview/electron-browser/webviewElement.ts @@ -532,10 +532,13 @@ export class WebviewElement extends Disposable implements Webview { if (this._webview.parentElement) { this._webview.parentElement.removeChild(this._webview); } + this._webview = undefined; } - this._webview = undefined; - this._webviewFindWidget = undefined; + if (this._webviewFindWidget) { + this._webviewFindWidget.dispose(); + this._webviewFindWidget = undefined; + } super.dispose(); } -- GitLab