Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf(platform-browser): resolve memory leak when using animations with shadow DOM #47903

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 10 additions & 4 deletions packages/platform-browser/animations/src/animation_renderer.ts
Expand Up @@ -50,7 +50,11 @@ export class AnimationRendererFactory implements RendererFactory2 {
if (!hostElement || !type || !type.data || !type.data['animation']) {
let renderer: BaseAnimationRenderer|undefined = this._rendererCache.get(delegate);
if (!renderer) {
renderer = new BaseAnimationRenderer(EMPTY_NAMESPACE_ID, delegate, this.engine);
// Ensure that the renderer is removed from the cache on destroy
// since it may contain references to detached DOM nodes.
const onRendererDestroy = () => this._rendererCache.delete(delegate);
crisbeto marked this conversation as resolved.
Show resolved Hide resolved
renderer =
new BaseAnimationRenderer(EMPTY_NAMESPACE_ID, delegate, this.engine, onRendererDestroy);
// only cache this result when the base renderer is used
this._rendererCache.set(delegate, renderer);
}
Expand Down Expand Up @@ -135,7 +139,8 @@ export class AnimationRendererFactory implements RendererFactory2 {

export class BaseAnimationRenderer implements Renderer2 {
constructor(
protected namespaceId: string, public delegate: Renderer2, public engine: AnimationEngine) {
protected namespaceId: string, public delegate: Renderer2, public engine: AnimationEngine,
private _onDestroy?: () => void) {
this.destroyNode = this.delegate.destroyNode ? (n) => delegate.destroyNode!(n) : null;
}

Expand All @@ -148,6 +153,7 @@ export class BaseAnimationRenderer implements Renderer2 {
destroy(): void {
this.engine.destroy(this.namespaceId, this.delegate);
this.delegate.destroy();
this._onDestroy?.();
}

createElement(name: string, namespace?: string|null|undefined) {
Expand Down Expand Up @@ -237,8 +243,8 @@ export class BaseAnimationRenderer implements Renderer2 {
export class AnimationRenderer extends BaseAnimationRenderer implements Renderer2 {
constructor(
public factory: AnimationRendererFactory, namespaceId: string, delegate: Renderer2,
engine: AnimationEngine) {
super(namespaceId, delegate, engine);
engine: AnimationEngine, onDestroy?: () => void) {
super(namespaceId, delegate, engine, onDestroy);
this.namespaceId = namespaceId;
}

Expand Down