From 835ed0f35fa187f4051129ab30b7b635f15aeb57 Mon Sep 17 00:00:00 2001
From: crisbeto <crisbeto@abv.bg>
Date: Sat, 14 Dec 2019 17:29:35 +0100
Subject: [PATCH] fix(animations): leaking detached nodes when parent has a
 leave transition (#34409)

In the TransitionAnimationEngine we keep track of the existing elements with animations and we clear the cached data when they're removed. We also have some logic where we transition away the child elements when a parent is removed, however in that case we never cleared the cached element data which resulted in a memory leak. The leak is particularly visible in Material where whenever there's an animated overlay with a component inside of it that has an animation, the child component would always be retained in memory.

Fixes #25744.

PR Close #34409
---
 .../src/render/transition_animation_engine.ts | 13 +++++--
 .../transition_animation_engine_spec.ts       | 34 +++++++++++++++++++
 2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/packages/animations/browser/src/render/transition_animation_engine.ts b/packages/animations/browser/src/render/transition_animation_engine.ts
index b26fd8308b..b3ec5bc052 100644
--- a/packages/animations/browser/src/render/transition_animation_engine.ts
+++ b/packages/animations/browser/src/render/transition_animation_engine.ts
@@ -308,11 +308,13 @@ export class AnimationTransitionNamespace {
     }
   }
 
-  private _signalRemovalForInnerTriggers(rootElement: any, context: any, animate: boolean = false) {
+  private _signalRemovalForInnerTriggers(rootElement: any, context: any) {
+    const elements = this._engine.driver.query(rootElement, NG_TRIGGER_SELECTOR, true);
+
     // emulate a leave animation for all inner nodes within this node.
     // If there are no animations found for any of the nodes then clear the cache
     // for the element.
-    this._engine.driver.query(rootElement, NG_TRIGGER_SELECTOR, true).forEach(elm => {
+    elements.forEach(elm => {
       // this means that an inner remove() operation has already kicked off
       // the animation on this element...
       if (elm[REMOVAL_FLAG]) return;
@@ -324,6 +326,11 @@ export class AnimationTransitionNamespace {
         this.clearElementCache(elm);
       }
     });
+
+    // If the child elements were removed along with the parent, their animations might not
+    // have completed. Clear all the elements from the cache so we don't end up with a memory leak.
+    this._engine.afterFlushAnimationsDone(
+        () => elements.forEach(elm => this.clearElementCache(elm)));
   }
 
   triggerLeaveAnimation(
@@ -388,7 +395,7 @@ export class AnimationTransitionNamespace {
     const engine = this._engine;
 
     if (element.childElementCount) {
-      this._signalRemovalForInnerTriggers(element, context, true);
+      this._signalRemovalForInnerTriggers(element, context);
     }
 
     // this means that a * => VOID animation was detected and kicked off
diff --git a/packages/animations/browser/test/render/transition_animation_engine_spec.ts b/packages/animations/browser/test/render/transition_animation_engine_spec.ts
index bfeb656916..98d80c294d 100644
--- a/packages/animations/browser/test/render/transition_animation_engine_spec.ts
+++ b/packages/animations/browser/test/render/transition_animation_engine_spec.ts
@@ -142,6 +142,40 @@ const DEFAULT_NAMESPACE_ID = 'id';
                       .duration)
                .toEqual(1234);
          });
+
+      it('should clear child node data when a parent node with leave transition is removed', () => {
+        const engine = makeEngine();
+        const child = document.createElement('div');
+        const parentTrigger = trigger('parent', [
+          transition(':leave', [style({height: '0px'}), animate(1000, style({height: '100px'}))])
+        ]);
+        const childTrigger = trigger(
+            'child',
+            [transition(':enter', [style({opacity: '0'}), animate(1000, style({opacity: '1'}))])]);
+
+        registerTrigger(element, engine, parentTrigger);
+        registerTrigger(child, engine, childTrigger);
+
+        element.appendChild(child);
+        engine.insertNode(DEFAULT_NAMESPACE_ID, child, element, true);
+
+        setProperty(element, engine, 'parent', 'value');
+        setProperty(child, engine, 'child', 'visible');
+        engine.flush();
+
+        expect(engine.statesByElement.has(element))
+            .toBe(true, 'Expected parent data to be defined.');
+        expect(engine.statesByElement.has(child)).toBe(true, 'Expected child data to be defined.');
+
+        engine.removeNode(DEFAULT_NAMESPACE_ID, element, true, true);
+        engine.flush();
+        engine.players[0].finish();
+
+        expect(engine.statesByElement.has(element))
+            .toBe(false, 'Expected parent data to be cleared.');
+        expect(engine.statesByElement.has(child)).toBe(false, 'Expected child data to be cleared.');
+      });
+
     });
 
     describe('event listeners', () => {