Skip to content

Commit

Permalink
perf(platform-browser): resolve memory leak when using animations wit…
Browse files Browse the repository at this point in the history
…h shadow DOM (#47903)

`AnimationRendererFactory` maintains a map between a renderer delegate and the animations renderer it corresponds to, but the renderers are never removed from the map. This leads to memory leaks when used with the `ShadowDom` view encapsulation, because the specific renderer keeps a references to its shadow root which in turn references all the elements in the view.

These changes resolve the leak by clearing the reference when the animations renderer is destroyed.

Fixes #47892.

PR Close #47903
  • Loading branch information
crisbeto authored and alxhub committed Nov 1, 2022
1 parent 65a3338 commit 92d28bd
Showing 1 changed file with 10 additions and 4 deletions.
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);
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

0 comments on commit 92d28bd

Please sign in to comment.