fix(animations): Ensure elements are removed from the cache after leave animation. (#50929)

JeanMeche · dylhunn · commit a14bdfe8591a · 2023-07-11T08:28:10.000-07:00
This commit fixes a memory leak. `_namespaceLookup` was cleared before the call to `processLeaveNode()` which was using the lookup. Without that lookup `clearElementCache()` wasn't called thus keeping a reference to the element. Fixes #24197 & #50533 PR Close #50929
diff --git a/packages/animations/browser/src/render/transition_animation_engine.ts b/packages/animations/browser/src/render/transition_animation_engine.ts
@@ -508,14 +508,6 @@ export class AnimationTransitionNamespace {
     this.players.forEach(p => p.destroy());
     this._signalRemovalForInnerTriggers(this.hostElement, context);
   }
-
-  elementContainsData(element: any): boolean {
-    let containsData = false;
-    if (this._elementListeners.has(element)) containsData = true;
-    containsData =
-        (this._queue.find(entry => entry.element === element) ? true : false) || containsData;
-    return containsData;
-  }
 }
 
 export interface QueuedTransition {
@@ -640,19 +632,18 @@ export class TransitionAnimationEngine {
 
   destroy(namespaceId: string, context: any) {
     if (!namespaceId) return;
+    this.afterFlush(() => {});
 
-    const ns = this._fetchNamespace(namespaceId);
-
-    this.afterFlush(() => {
+    this.afterFlushAnimationsDone(() => {
+      const ns = this._fetchNamespace(namespaceId);
       this.namespacesByHostElement.delete(ns.hostElement);
-      delete this._namespaceLookup[namespaceId];
       const index = this._namespaceList.indexOf(ns);
       if (index >= 0) {
         this._namespaceList.splice(index, 1);
       }
+      ns.destroy(context);
+      delete this._namespaceLookup[namespaceId];
     });
-
-    this.afterFlushAnimationsDone(() => ns.destroy(context));
   }
 
   private _fetchNamespace(id: string) {
@@ -1314,16 +1305,6 @@ export class TransitionAnimationEngine {
     return rootPlayers;
   }
 
-  elementContainsData(namespaceId: string, element: any) {
-    let containsData = false;
-    const details = element[REMOVAL_FLAG] as ElementAnimationState;
-    if (details && details.setForRemoval) containsData = true;
-    if (this.playersByElement.has(element)) containsData = true;
-    if (this.playersByQueriedElement.has(element)) containsData = true;
-    if (this.statesByElement.has(element)) containsData = true;
-    return this._fetchNamespace(namespaceId).elementContainsData(element) || containsData;
-  }
-
   afterFlush(callback: () => any) {
     this._flushFns.push(callback);
   }
diff --git a/packages/animations/browser/test/render/transition_animation_engine_spec.ts b/packages/animations/browser/test/render/transition_animation_engine_spec.ts
@@ -113,13 +113,20 @@ describe('TransitionAnimationEngine', () => {
       registerTrigger(element, engine, trig);
       setProperty(element, engine, 'myTrigger', 'value');
       engine.flush();
+      expect(engine.statesByElement.has(element))
+          .toBe(true, 'Expected element data to be defined.');
+      expect(engine.playersByElement.has(element))
+          .toBe(true, 'Expected element data to be defined.');
 
-      expect(engine.elementContainsData(DEFAULT_NAMESPACE_ID, element)).toBeTruthy();
-
+      engine.destroy(DEFAULT_NAMESPACE_ID, null);
       engine.removeNode(DEFAULT_NAMESPACE_ID, element, true);
       engine.flush();
+      engine.players[0].finish();
 
-      expect(engine.elementContainsData(DEFAULT_NAMESPACE_ID, element)).toBeTruthy();
+      expect(engine.statesByElement.has(element))
+          .toBe(false, 'Expected element data to be undefined.');
+      expect(engine.playersByElement.has(element))
+          .toBe(false, 'Expected element data to be undefined.');
     });
 
     it('should create and recreate a namespace for a host element with the same component source',
