make leaking menus less severe, #126365

src/vs/platform/actions/common/menuService.ts

@@ -36,10 +36,10 @@ type MenuItemGroup = [string, Array<IMenuItem | ISubmenuItem>];
 
 class Menu implements IMenu {
 
-	private readonly _dispoables = new DisposableStore();
+	private readonly _disposables = new DisposableStore();
 
-	private readonly _onDidChange = new Emitter<IMenu>();
-	readonly onDidChange: Event<IMenu> = this._onDidChange.event;
+	private readonly _onDidChange: Emitter<IMenu>;
+	readonly onDidChange: Event<IMenu>;
 
 	private _menuGroups: MenuItemGroup[] = [];
 	private _contextKeys: Set<string> = new Set();
@@ -53,29 +53,45 @@ class Menu implements IMenu {
 	) {
 		this._build();
 
-		// rebuild this menu whenever the menu registry reports an
-		// event for this MenuId
-		const rebuildMenuSoon = new RunOnceScheduler(() => this._build(), 50);
-		this._dispoables.add(rebuildMenuSoon);
-		this._dispoables.add(MenuRegistry.onDidChangeMenu(e => {
+		// Rebuild this menu whenever the menu registry reports an event for this MenuId.
+		// This usually happen while code and extensions are loaded and affects the over
+		// structure of the menu
+		const rebuildMenuSoon = new RunOnceScheduler(() => {
+			this._build();
+			this._onDidChange.fire(this);
+		}, 50);
+		this._disposables.add(rebuildMenuSoon);
+		this._disposables.add(MenuRegistry.onDidChangeMenu(e => {
 			if (e.has(_id)) {
 				rebuildMenuSoon.schedule();
 			}
 		}));
 
-		// when context keys change we need to check if the menu also
-		// has changed
-		const fireChangeSoon = new RunOnceScheduler(() => this._onDidChange.fire(this), 50);
-		this._dispoables.add(fireChangeSoon);
-		this._dispoables.add(_contextKeyService.onDidChangeContext(e => {
-			if (e.affectsSome(this._contextKeys)) {
-				fireChangeSoon.schedule();
-			}
-		}));
+		// When context keys change we need to check if the menu also has changed. However,
+		// we only do that when someone listens on this menu because (1) context key events are
+		// firing often and (2) menu are often leaked
+		const contextKeyListener = this._disposables.add(new DisposableStore());
+		const startContextKeyListener = () => {
+			const fireChangeSoon = new RunOnceScheduler(() => this._onDidChange.fire(this), 50);
+			contextKeyListener.add(fireChangeSoon);
+			contextKeyListener.add(_contextKeyService.onDidChangeContext(e => {
+				if (e.affectsSome(this._contextKeys)) {
+					fireChangeSoon.schedule();
+				}
+			}));
+		};
+
+		this._onDidChange = new Emitter({
+			// start/stop context key listener
+			onFirstListenerAdd: startContextKeyListener,
+			onLastListenerRemove: contextKeyListener.clear.bind(contextKeyListener)
+		});
+		this.onDidChange = this._onDidChange.event;
+
 	}
 
 	dispose(): void {
-		this._dispoables.dispose();
+		this._disposables.dispose();
 		this._onDidChange.dispose();
 	}
 
@@ -90,7 +106,7 @@ class Menu implements IMenu {
 		let group: MenuItemGroup | undefined;
 		menuItems.sort(Menu._compareMenuItems);
 
-		for (let item of menuItems) {
+		for (const item of menuItems) {
 			// group by groupId
 			const groupName = item.group || '';
 			if (!group || group[0] !== groupName) {
@@ -102,7 +118,6 @@ class Menu implements IMenu {
 			// keep keys for eventing
 			this._collectContextKeys(item);
 		}
-		this._onDidChange.fire(this);
 	}
 
 	private _collectContextKeys(item: IMenuItem | ISubmenuItem): void {