Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/browser/decorations/BufferDecorationRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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();
}
}
9 changes: 3 additions & 6 deletions src/common/Lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
23 changes: 14 additions & 9 deletions src/common/services/DecorationService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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();
}
}
}
Comment thread
meganrogge marked this conversation as resolved.
Outdated