Adding MutableDisposable

This class can help manage a disposable property that is optional or may be changed. It prevents errors when you forget to release the previously held disposable when setting a new one

src/vs/base/common/lifecycle.ts

@@ -144,6 +144,48 @@ export abstract class Disposable implements IDisposable {
 	}
 }
 
+/**
+ * Manages the lifecycle of a disposable value that may be changed.
+ *
+ * This ensures that when the the disposable value is changed, the previously held disposable is disposed of. You can
+ * also register a `MutableDisposable` on a `Disposable` to ensure it is automatically cleaned up.
+ */
+export class MutableDisposable<T extends IDisposable> implements IDisposable {
+	private _value?: T;
+	private _isDisposed = false;
+
+	constructor() {
+		trackDisposable(this);
+	}
+
+	get value(): T | undefined {
+		return this._isDisposed ? undefined : this._value;
+	}
+
+	set value(value: T | undefined) {
+		if (this._isDisposed || value === this._value) {
+			return;
+		}
+
+		if (this._value) {
+			this._value.dispose();
+		}
+		if (value) {
+			markTracked(value);
+		}
+		this._value = value;
+	}
+
+	dispose(): void {
+		this._isDisposed = true;
+		markTracked(this);
+		if (this._value) {
+			this._value.dispose();
+		}
+		this._value = undefined;
+	}
+}
+
 export interface IReference<T> extends IDisposable {
 	readonly object: T;
 }

src/vs/editor/contrib/codeAction/codeActionModel.ts

@@ -5,7 +5,7 @@
 
 import { CancelablePromise, createCancelablePromise, TimeoutTimer } from 'vs/base/common/async';
 import { Emitter } from 'vs/base/common/event';
-import { dispose, Disposable } from 'vs/base/common/lifecycle';
+import { Disposable, MutableDisposable } from 'vs/base/common/lifecycle';
 import { URI } from 'vs/base/common/uri';
 import { ICodeEditor } from 'vs/editor/browser/editorBrowser';
 import { Position } from 'vs/editor/common/core/position';
@@ -156,7 +156,7 @@ export namespace CodeActionsState {
 
 export class CodeActionModel extends Disposable {
 
-	private _codeActionOracle?: CodeActionOracle;
+	private readonly _codeActionOracle = this._register(new MutableDisposable<CodeActionOracle>());
 	private _state: CodeActionsState.State = CodeActionsState.Empty;
 	private readonly _supportedCodeActions: IContextKey<string>;
 
@@ -181,15 +181,11 @@ export class CodeActionModel extends Disposable {
 
 	dispose(): void {
 		super.dispose();
-		dispose(this._codeActionOracle);
 		this.setState(CodeActionsState.Empty, true);
 	}
 
 	private _update(): void {
-		if (this._codeActionOracle) {
-			this._codeActionOracle.dispose();
-			this._codeActionOracle = undefined;
-		}
+		this._codeActionOracle.value = undefined;
 
 		this.setState(CodeActionsState.Empty);
 
@@ -207,7 +203,7 @@ export class CodeActionModel extends Disposable {
 
 			this._supportedCodeActions.set(supportedActions.join(' '));
 
-			this._codeActionOracle = new CodeActionOracle(this._editor, this._markerService, trigger => {
+			this._codeActionOracle.value = new CodeActionOracle(this._editor, this._markerService, trigger => {
 				if (!trigger) {
 					this.setState(CodeActionsState.Empty);
 					return;
@@ -221,15 +217,15 @@ export class CodeActionModel extends Disposable {
 				this.setState(new CodeActionsState.Triggered(trigger.trigger, trigger.selection, trigger.position, actions));
 
 			}, undefined);
-			this._codeActionOracle.trigger({ type: 'auto' });
+			this._codeActionOracle.value.trigger({ type: 'auto' });
 		} else {
 			this._supportedCodeActions.reset();
 		}
 	}
 
 	public trigger(trigger: CodeActionTrigger) {
-		if (this._codeActionOracle) {
-			this._codeActionOracle.trigger(trigger);
+		if (this._codeActionOracle.value) {
+			this._codeActionOracle.value.trigger(trigger);
 		}
 	}
 

src/vs/platform/actions/browser/menuEntryActionViewItem.ts

@@ -9,7 +9,7 @@ import { ActionViewItem, Separator } from 'vs/base/browser/ui/actionbar/actionba
 import { IAction } from 'vs/base/common/actions';
 import { Emitter } from 'vs/base/common/event';
 import { IdGenerator } from 'vs/base/common/idGenerator';
-import { dispose, IDisposable, toDisposable } from 'vs/base/common/lifecycle';
+import { dispose, IDisposable, toDisposable, MutableDisposable } from 'vs/base/common/lifecycle';
 import { isLinux, isWindows } from 'vs/base/common/platform';
 import { localize } from 'vs/nls';
 import { ICommandAction, IMenu, IMenuActionOptions, MenuItemAction, SubmenuItemAction } from 'vs/platform/actions/common/actions';
@@ -127,7 +127,7 @@ export class MenuEntryActionViewItem extends ActionViewItem {
 	static readonly ICON_PATH_TO_CSS_RULES: Map<string /* path*/, string /* CSS rule */> = new Map<string, string>();
 
 	private _wantsAltCommand: boolean;
-	private _itemClassDispose?: IDisposable;
+	private readonly _itemClassDispose = this._register(new MutableDisposable());
 	private readonly _altKey: AlternativeKeyEmitter;
 
 	constructor(
@@ -222,8 +222,7 @@ export class MenuEntryActionViewItem extends ActionViewItem {
 	}
 
 	_updateItemClass(item: ICommandAction): void {
-		dispose(this._itemClassDispose);
-		this._itemClassDispose = undefined;
+		this._itemClassDispose.value = undefined;
 
 		if (item.iconLocation) {
 			let iconClass: string;
@@ -240,18 +239,9 @@ export class MenuEntryActionViewItem extends ActionViewItem {
 			}
 
 			addClasses(this.label, 'icon', iconClass);
-			this._itemClassDispose = toDisposable(() => removeClasses(this.label, 'icon', iconClass));
+			this._itemClassDispose.value = toDisposable(() => removeClasses(this.label, 'icon', iconClass));
 		}
 	}
-
-	dispose(): void {
-		if (this._itemClassDispose) {
-			dispose(this._itemClassDispose);
-			this._itemClassDispose = undefined;
-		}
-
-		super.dispose();
-	}
 }
 
 // Need to subclass MenuEntryActionViewItem in order to respect

src/vs/workbench/contrib/webview/browser/webviewEditor.ts

@@ -6,7 +6,7 @@
 import * as DOM from 'vs/base/browser/dom';
 import { CancellationToken } from 'vs/base/common/cancellation';
 import { Emitter, Event } from 'vs/base/common/event';
-import { IDisposable, DisposableStore } from 'vs/base/common/lifecycle';
+import { DisposableStore, MutableDisposable } from 'vs/base/common/lifecycle';
 import { URI } from 'vs/base/common/uri';
 import { IContextKey, IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
 import { IStorageService } from 'vs/platform/storage/common/storage';
@@ -33,8 +33,8 @@ export class WebviewEditor extends BaseEditor {
 	private _content?: HTMLElement;
 	private _webviewContent: HTMLElement | undefined;
 
-	private readonly _webviewFocusTrackerDisposables = new DisposableStore();
-	private _onFocusWindowHandler?: IDisposable;
+	private readonly _webviewFocusTrackerDisposables = this._register(new DisposableStore());
+	private readonly _onFocusWindowHandler = this._register(new MutableDisposable());
 
 	private readonly _onDidFocusWebview = this._register(new Emitter<void>());
 	public get onDidFocus(): Event<any> { return this._onDidFocusWebview.event; }
@@ -89,12 +89,6 @@ export class WebviewEditor extends BaseEditor {
 			this._content = undefined;
 		}
 
-		this._webviewFocusTrackerDisposables.dispose();
-
-		if (this._onFocusWindowHandler) {
-			this._onFocusWindowHandler.dispose();
-		}
-
 		super.dispose();
 	}
 
@@ -136,10 +130,10 @@ export class WebviewEditor extends BaseEditor {
 
 	public focus(): void {
 		super.focus();
-		if (!this._onFocusWindowHandler) {
+		if (!this._onFocusWindowHandler.value) {
 
 			// Make sure we restore focus when switching back to a VS Code window
-			this._onFocusWindowHandler = this._windowService.onDidChangeFocus(focused => {
+			this._onFocusWindowHandler.value = this._windowService.onDidChangeFocus(focused => {
 				if (focused && this._editorService.activeControl === this) {
 					this.focus();
 				}