Merge pull request #4689 from Tyriar/4685

Track CursorBlinkStateManager in MutableDisposable
xtermjs · Aug 18, 2023 · 7b7dbd3 · 7b7dbd3
2 parents a35fa61 + d03b5c9
commit 7b7dbd3
Show file tree

Hide file tree

Showing 3 changed files with 99 additions and 16 deletions.
diff --git a/addons/xterm-addon-webgl/src/WebglRenderer.ts b/addons/xterm-addon-webgl/src/WebglRenderer.ts
@@ -16,7 +16,7 @@ import { AttributeData } from 'common/buffer/AttributeData';
 import { CellData } from 'common/buffer/CellData';
 import { Attributes, Content, NULL_CELL_CHAR, NULL_CELL_CODE } from 'common/buffer/Constants';
 import { EventEmitter, forwardEvent } from 'common/EventEmitter';
-import { Disposable, getDisposeArrayDisposable, toDisposable } from 'common/Lifecycle';
+import { Disposable, MutableDisposable, getDisposeArrayDisposable, toDisposable } from 'common/Lifecycle';
 import { ICoreService, IDecorationService, ILogService, IOptionsService } from 'common/services/Services';
 import { CharData, IBufferLine, ICellData } from 'common/Types';
 import { IDisposable, Terminal } from 'xterm';
@@ -31,7 +31,7 @@ import { traceCall } from 'common/services/LogService';
 
 export class WebglRenderer extends Disposable implements IRenderer {
   private _renderLayers: IRenderLayer[];
-  private _cursorBlinkStateManager: CursorBlinkStateManager | undefined;
+  private _cursorBlinkStateManager: MutableDisposable<CursorBlinkStateManager> = new MutableDisposable();
   private _charAtlasDisposable: IDisposable | undefined;
   private _charAtlas: ITextureAtlas | undefined;
   private _devicePixelRatio: number;
@@ -203,7 +203,7 @@ export class WebglRenderer extends Disposable implements IRenderer {
     for (const l of this._renderLayers) {
       l.handleBlur(this._terminal);
     }
-    this._cursorBlinkStateManager?.pause();
+    this._cursorBlinkStateManager.value?.pause();
     // Request a redraw for active/inactive selection background
     this._requestRedrawViewport();
   }
@@ -212,7 +212,7 @@ export class WebglRenderer extends Disposable implements IRenderer {
     for (const l of this._renderLayers) {
       l.handleFocus(this._terminal);
     }
-    this._cursorBlinkStateManager?.resume();
+    this._cursorBlinkStateManager.value?.resume();
     // Request a redraw for active/inactive selection background
     this._requestRedrawViewport();
   }
@@ -229,7 +229,7 @@ export class WebglRenderer extends Disposable implements IRenderer {
     for (const l of this._renderLayers) {
       l.handleCursorMove(this._terminal);
     }
-    this._cursorBlinkStateManager?.restartBlinkAnimation();
+    this._cursorBlinkStateManager.value?.restartBlinkAnimation();
   }
 
   private _handleOptionsChanged(): void {
@@ -312,7 +312,7 @@ export class WebglRenderer extends Disposable implements IRenderer {
       l.reset(this._terminal);
     }
 
-    this._cursorBlinkStateManager?.restartBlinkAnimation();
+    this._cursorBlinkStateManager.value?.restartBlinkAnimation();
     this._updateCursorBlink();
   }
 
@@ -359,21 +359,18 @@ export class WebglRenderer extends Disposable implements IRenderer {
     // Render
     this._rectangleRenderer?.renderBackgrounds();
     this._glyphRenderer?.render(this._model);
-    if (!this._cursorBlinkStateManager || this._cursorBlinkStateManager.isCursorVisible) {
+    if (!this._cursorBlinkStateManager.value || this._cursorBlinkStateManager.value.isCursorVisible) {
       this._rectangleRenderer?.renderCursor();
     }
   }
 
   private _updateCursorBlink(): void {
     if (this._terminal.options.cursorBlink) {
-      if (!this._cursorBlinkStateManager) {
-        this._cursorBlinkStateManager = this.register(new CursorBlinkStateManager(() => {
-          this._requestRedrawCursor();
-        }, this._coreBrowserService));
-      }
+      this._cursorBlinkStateManager.value = new CursorBlinkStateManager(() => {
+        this._requestRedrawCursor();
+      }, this._coreBrowserService);
     } else {
-      this._cursorBlinkStateManager?.dispose();
-      this._cursorBlinkStateManager = undefined;
+      this._cursorBlinkStateManager.clear();
     }
     // Request a refresh from the terminal as management of rendering is being
     // moved back to the terminal
@@ -408,7 +405,7 @@ export class WebglRenderer extends Disposable implements IRenderer {
     const isCursorVisible =
       this._coreService.isCursorInitialized &&
       !this._coreService.isCursorHidden &&
-      (!this._cursorBlinkStateManager || this._cursorBlinkStateManager.isCursorVisible);
+      (!this._cursorBlinkStateManager.value || this._cursorBlinkStateManager.value.isCursorVisible);
     this._model.cursor = undefined;
     let modelUpdated = false;
 

diff --git a/src/common/Lifecycle.test.ts b/src/common/Lifecycle.test.ts
@@ -4,7 +4,8 @@
  */
 
 import { assert } from 'chai';
-import { Disposable } from 'common/Lifecycle';
+import { Disposable, MutableDisposable } from 'common/Lifecycle';
+import { IDisposable } from 'common/Types';
 
 class TestDisposable extends Disposable {
   public get isDisposed(): boolean {
@@ -43,3 +44,52 @@ describe('Disposable', () => {
     });
   });
 });
+
+describe('MutableDisposable', () => {
+  const mutable = new MutableDisposable();
+  class TrackedDisposable extends Disposable {
+    public get isDisposed(): boolean { return this._isDisposed; }
+  }
+  describe('value', () => {
+    it('should set the value', () => {
+      const d1 = new TrackedDisposable();
+      mutable.value = d1;
+      assert.strictEqual(mutable.value, d1);
+      assert.isFalse(d1.isDisposed);
+    });
+    it('should dispose of any previous value', () => {
+      const d1 = new TrackedDisposable();
+      const d2 = new TrackedDisposable();
+      mutable.value = d1;
+      mutable.value = d2;
+      assert.strictEqual(mutable.value, d2);
+      assert.isTrue(d1.isDisposed);
+      assert.isFalse(d2.isDisposed);
+    });
+  });
+  describe('clear', () => {
+    it('should clear and dispose of the object', () => {
+      const d1 = new TrackedDisposable();
+      mutable.value = d1;
+      mutable.clear();
+      assert.strictEqual(mutable.value, undefined);
+      assert.isTrue(d1.isDisposed);
+    });
+  });
+  it('dispose', () => {
+    it('should dispose of the object', () => {
+      const d1 = new TrackedDisposable();
+      mutable.value = d1;
+      mutable.dispose();
+      assert.strictEqual(mutable.value, undefined);
+      assert.isTrue(d1.isDisposed);
+    });
+    it('should prevent using the MutableDisposable again', () => {
+      const d1 = new TrackedDisposable();
+      mutable.value = d1;
+      mutable.dispose();
+      mutable.value = new TrackedDisposable();
+      assert.strictEqual(mutable.value, undefined);
+    });
+  });
+});
diff --git a/src/common/Lifecycle.ts b/src/common/Lifecycle.ts
@@ -50,6 +50,42 @@ export abstract class Disposable implements IDisposable {
   }
 }
 
+export class MutableDisposable<T extends IDisposable> implements IDisposable {
+  private _value?: T;
+  private _isDisposed = false;
+
+  /**
+   * Gets the value if it exists.
+   */
+  public get value(): T | undefined {
+    return this._isDisposed ? undefined : this._value;
+  }
+
+  /**
+   * Sets the value, disposing of the old value if it exists.
+   */
+  public set value(value: T | undefined) {
+    if (this._isDisposed || value === this._value) {
+      return;
+    }
+    this._value?.dispose();
+    this._value = value;
+  }
+
+  /**
+   * Resets the stored value and disposes of the previously stored value.
+   */
+  public clear(): void {
+    this.value = undefined;
+  }
+
+  public dispose(): void {
+    this._isDisposed = true;
+    this._value?.dispose();
+    this._value = undefined;
+  }
+}
+
 /**
  * Wrap a function in a disposable.
  */