revert decoration dispose changes (#4215)

src/browser/decorations/BufferDecorationRenderer.ts

@@ -98,7 +98,6 @@ export class BufferDecorationRenderer extends Disposable {
     } else {
       let element = this._decorationElements.get(decoration);
       if (!element) {
-        decoration.onDispose(() => this._removeDecoration(decoration));
         element = this._createElement(decoration);
         decoration.element = element;
         this._decorationElements.set(decoration, element);
@@ -125,5 +124,6 @@ export class BufferDecorationRenderer extends Disposable {
   private _removeDecoration(decoration: IInternalDecoration): void {
     this._decorationElements.get(decoration)?.remove();
     this._decorationElements.delete(decoration);
+    decoration.dispose();
   }
 }

src/common/Lifecycle.ts

@@ -17,18 +17,15 @@ export abstract class Disposable implements IDisposable {
   }
 
   /**
-   * Disposes the object, triggering the `dispose` method on all registered IDisposables. This is a
-   * readonly property instead of a method to prevent subclasses overriding it which is an easy
-   * mistake that can introduce memory leaks. If a class extends Disposable, all dispose calls
-   * should be done via {@link register}.
+   * Disposes the object, triggering the `dispose` method on all registered IDisposables.
    */
-  public readonly dispose = (): void => {
+  public dispose(): void {
     this._isDisposed = true;
     for (const d of this._disposables) {
       d.dispose();
     }
     this._disposables.length = 0;
-  };
+  }
 
   /**
    * Registers a disposable object.

src/common/services/DecorationService.ts

@@ -42,6 +42,7 @@ export class DecorationService extends Disposable implements IDecorationService
       this.reset();
     }));
   }
+
   public registerDecoration(options: IDecorationOptions): IDecoration | undefined {
     if (options.marker.isDisposed) {
       return undefined;
@@ -91,19 +92,25 @@ export class DecorationService extends Disposable implements IDecorationService
       }
     });
   }
+
+  public dispose(): void {
+    for (const d of this._decorations.values()) {
+      this._onDecorationRemoved.fire(d);
+    }
+    this.reset();
+  }
 }
 
 class Decoration extends Disposable implements IInternalDecoration {
   public readonly marker: IMarker;
   public element: HTMLElement | undefined;
+  public isDisposed: boolean = false;
 
   public readonly onRenderEmitter = this.register(new EventEmitter<HTMLElement>());
   public readonly onRender = this.onRenderEmitter.event;
   private readonly _onDispose = this.register(new EventEmitter<void>());
   public readonly onDispose = this._onDispose.event;
 
-  public get isDisposed(): boolean { return this._isDisposed; }
-
   private _cachedBg: IColor | undefined | null = null;
   public get backgroundColorRGB(): IColor | undefined {
     if (this._cachedBg === null) {
@@ -136,12 +143,10 @@ class Decoration extends Disposable implements IInternalDecoration {
     if (this.options.overviewRulerOptions && !this.options.overviewRulerOptions.position) {
       this.options.overviewRulerOptions.position = 'full';
     }
+  }
 
-    this.register(toDisposable(() => {
-      if (this._isDisposed) {
-        return;
-      }
-      this._onDispose.fire();
-    }));
+  public override dispose(): void {
+    this._onDispose.fire();
+    super.dispose();
   }
 }