Merge pull request microsoft#107887 from microsoft/alexr00/issue#107801

roblourens · web-flow · commit 1137ae35459c · 2020-10-01T09:50:05.000-07:00
Listener leak when scrolling PULL REQUESTS view
diff --git a/src/vs/platform/contextkey/browser/contextKeyService.ts b/src/vs/platform/contextkey/browser/contextKeyService.ts
@@ -328,7 +328,7 @@ export abstract class AbstractContextKeyService implements IContextKeyService {
 	public abstract getContextValuesContainer(contextId: number): Context;
 	public abstract createChildContext(parentContextId?: number): number;
 	public abstract disposeContext(contextId: number): void;
-	public abstract updateParent(parentContextKeyService: IContextKeyService): void;
+	public abstract updateParent(parentContextKeyService?: IContextKeyService): void;
 }
 
 export class ContextKeyService extends AbstractContextKeyService implements IContextKeyService {
@@ -423,6 +423,7 @@ class ScopedContextKeyService extends AbstractContextKeyService {
 	public dispose(): void {
 		this._isDisposed = true;
 		this._parent.disposeContext(this._myContextId);
+		this._parentChangeListener?.dispose();
 		if (this._domNode) {
 			this._domNode.removeAttribute(KEYBINDING_CONTEXT_ATTR);
 			this._domNode = undefined;
@@ -454,7 +455,11 @@ class ScopedContextKeyService extends AbstractContextKeyService {
 		this._parent.disposeContext(contextId);
 	}
 
-	public updateParent(parentContextKeyService: AbstractContextKeyService): void {
+	public updateParent(parentContextKeyService?: AbstractContextKeyService): void {
+		if (!parentContextKeyService) {
+			this._parentChangeListener?.dispose();
+			return;
+		}
 		const thisContainer = this._parent.getContextValuesContainer(this._myContextId);
 		const oldAllValues = thisContainer.collectAllValues();
 		this._parent = parentContextKeyService;
diff --git a/src/vs/platform/contextkey/common/contextkey.ts b/src/vs/platform/contextkey/common/contextkey.ts
@@ -1139,7 +1139,7 @@ export interface IContextKeyService {
 	createScoped(target?: IContextKeyServiceTarget): IContextKeyService;
 	getContext(target: IContextKeyServiceTarget | null): IContext;
 
-	updateParent(parentContextKeyService: IContextKeyService): void;
+	updateParent(parentContextKeyService?: IContextKeyService): void;
 }
 
 export const SET_CONTEXT_COMMAND_ID = 'setContext';
diff --git a/src/vs/workbench/contrib/views/browser/treeView.ts b/src/vs/workbench/contrib/views/browser/treeView.ts
@@ -1027,7 +1027,9 @@ class TreeMenus extends Disposable implements IDisposable {
 		createAndFillInContextMenuActions(menu, { shouldForwardArgs: true }, result, this.contextMenuService, g => /^inline/.test(g));
 
 		menu.dispose();
-
+		// When called without a parameter, updateParent will dispose the parent change listener.
+		// We cannot call dispose on the contextKeyService because it will break submenus.
+		contextKeyService.updateParent();
 		return result;
 	}
 }

Original file line number	Diff line number	Diff line change
`@@ -1139,7 +1139,7 @@ export interface IContextKeyService {`
`1139`	`1139`	`createScoped(target?: IContextKeyServiceTarget): IContextKeyService;`
`1140`	`1140`	`getContext(target: IContextKeyServiceTarget \| null): IContext;`
`1141`	`1141`
`1142`		`- updateParent(parentContextKeyService: IContextKeyService): void;`
	`1142`	`+ updateParent(parentContextKeyService?: IContextKeyService): void;`
`1143`	`1143`	`}`
`1144`	`1144`
`1145`	`1145`	`export const SET_CONTEXT_COMMAND_ID = 'setContext';`
Original file line number	Diff line number	Diff line change
`@@ -1027,7 +1027,9 @@ class TreeMenus extends Disposable implements IDisposable {`
`1027`	`1027`	`createAndFillInContextMenuActions(menu, { shouldForwardArgs: true }, result, this.contextMenuService, g => /^inline/.test(g));`
`1028`	`1028`
`1029`	`1029`	`menu.dispose();`
`1030`		`-`
	`1030`	`+ // When called without a parameter, updateParent will dispose the parent change listener.`
	`1031`	`+ // We cannot call dispose on the contextKeyService because it will break submenus.`
	`1032`	`+ contextKeyService.updateParent();`
`1031`	`1033`	`return result;`
`1032`	`1034`	`}`
`1033`	`1035`	`}`