Skip to content

Commit eed1cb5

Browse files
fix: memory leak in composite bar (#280659)
* fix: memory leak in composite bar * fix * simpler fix * use mutable disposables to fix the leak --------- Co-authored-by: BeniBenj <[email protected]>
1 parent 152a97d commit eed1cb5

File tree

1 file changed

+15
-17
lines changed

1 file changed

+15
-17
lines changed

src/vs/workbench/browser/parts/compositeBar.ts

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import { IPaneComposite } from '../../common/panecomposite.js';
2121
import { IComposite } from '../../common/composite.js';
2222
import { CompositeDragAndDropData, CompositeDragAndDropObserver, IDraggedCompositeData, ICompositeDragAndDrop, Before2D, toggleDropEffect, ICompositeDragAndDropObserverCallbacks } from '../dnd.js';
2323
import { Gesture, EventType as TouchEventType, GestureEvent } from '../../../base/browser/touch.js';
24+
import { MutableDisposable } from '../../../base/common/lifecycle.js';
2425

2526
export interface ICompositeBarItem {
2627

@@ -239,8 +240,8 @@ export class CompositeBar extends Widget implements ICompositeBar {
239240
private dimension: Dimension | undefined;
240241

241242
private compositeSwitcherBar: ActionBar | undefined;
242-
private compositeOverflowAction: CompositeOverflowActivityAction | undefined;
243-
private compositeOverflowActionViewItem: CompositeOverflowActivityActionViewItem | undefined;
243+
private compositeOverflowAction = this._register(new MutableDisposable<CompositeOverflowActivityAction>());
244+
private compositeOverflowActionViewItem = this._register(new MutableDisposable<CompositeOverflowActivityActionViewItem>());
244245

245246
private readonly model: CompositeBarModel;
246247
private readonly visibleComposites: string[];
@@ -287,7 +288,7 @@ export class CompositeBar extends Widget implements ICompositeBar {
287288
this.compositeSwitcherBar = this._register(new ActionBar(actionBarDiv, {
288289
actionViewItemProvider: (action, options) => {
289290
if (action instanceof CompositeOverflowActivityAction) {
290-
return this.compositeOverflowActionViewItem;
291+
return this.compositeOverflowActionViewItem.value;
291292
}
292293
const item = this.model.findItem(action.id);
293294
return item && this.instantiationService.createInstance(
@@ -578,14 +579,11 @@ export class CompositeBar extends Widget implements ICompositeBar {
578579
}
579580

580581
// Remove the overflow action if there are no overflows
581-
if (totalComposites === compositesToShow.length && this.compositeOverflowAction) {
582+
if (totalComposites === compositesToShow.length && this.compositeOverflowAction.value) {
582583
compositeSwitcherBar.pull(compositeSwitcherBar.length() - 1);
583584

584-
this.compositeOverflowAction.dispose();
585-
this.compositeOverflowAction = undefined;
586-
587-
this.compositeOverflowActionViewItem?.dispose();
588-
this.compositeOverflowActionViewItem = undefined;
585+
this.compositeOverflowAction.value = undefined;
586+
this.compositeOverflowActionViewItem.value = undefined;
589587
}
590588

591589
// Pull out composites that overflow or got hidden
@@ -615,13 +613,13 @@ export class CompositeBar extends Widget implements ICompositeBar {
615613
});
616614

617615
// Add overflow action as needed
618-
if (totalComposites > compositesToShow.length && !this.compositeOverflowAction) {
619-
this.compositeOverflowAction = this._register(this.instantiationService.createInstance(CompositeOverflowActivityAction, () => {
620-
this.compositeOverflowActionViewItem?.showMenu();
621-
}));
622-
this.compositeOverflowActionViewItem = this._register(this.instantiationService.createInstance(
616+
if (totalComposites > compositesToShow.length && !this.compositeOverflowAction.value) {
617+
this.compositeOverflowAction.value = this.instantiationService.createInstance(CompositeOverflowActivityAction, () => {
618+
this.compositeOverflowActionViewItem.value?.showMenu();
619+
});
620+
this.compositeOverflowActionViewItem.value = this.instantiationService.createInstance(
623621
CompositeOverflowActivityActionViewItem,
624-
this.compositeOverflowAction,
622+
this.compositeOverflowAction.value,
625623
() => this.getOverflowingComposites(),
626624
() => this.model.activeItem ? this.model.activeItem.id : undefined,
627625
compositeId => {
@@ -631,9 +629,9 @@ export class CompositeBar extends Widget implements ICompositeBar {
631629
this.options.getOnCompositeClickAction,
632630
this.options.colors,
633631
this.options.activityHoverOptions
634-
));
632+
);
635633

636-
compositeSwitcherBar.push(this.compositeOverflowAction, { label: false, icon: true });
634+
compositeSwitcherBar.push(this.compositeOverflowAction.value, { label: false, icon: true });
637635
}
638636

639637
if (!donotTrigger) {

0 commit comments

Comments
 (0)