fix(animations): always cleanup players after they have finished internally (#13334)

Closes #13333
Closes #13334

Author: matsko

modules/@angular/compiler/src/animation/animation_compiler.ts

@@ -199,8 +199,9 @@ class _AnimationBuilder implements AnimationAstVisitor {
                         .set(_ANIMATION_FACTORY_VIEW_CONTEXT.callMethod(
                             'getAnimationPlayers',
                             [
-                              _ANIMATION_FACTORY_ELEMENT_VAR, o.literal(this.animationName),
+                              _ANIMATION_FACTORY_ELEMENT_VAR,
                               _ANIMATION_NEXT_STATE_VAR.equals(o.literal(EMPTY_STATE))
+                                  .conditional(o.NULL_EXPR, o.literal(this.animationName))
                             ]))
                         .toDeclStmt());
 

modules/@angular/core/src/animation/view_animation_map.ts

@@ -44,16 +44,18 @@ export class ViewAnimationMap {
 
   getAllPlayers(): AnimationPlayer[] { return this._allPlayers; }
 
-  remove(element: any, animationName: string): void {
+  remove(element: any, animationName: string, targetPlayer: AnimationPlayer = null): void {
     const playersByAnimation = this._map.get(element);
     if (playersByAnimation) {
       const player = playersByAnimation[animationName];
-      delete playersByAnimation[animationName];
-      const index = this._allPlayers.indexOf(player);
-      this._allPlayers.splice(index, 1);
-
-      if (Object.keys(playersByAnimation).length === 0) {
-        this._map.delete(element);
+      if (!targetPlayer || player === targetPlayer) {
+        delete playersByAnimation[animationName];
+        const index = this._allPlayers.indexOf(player);
+        this._allPlayers.splice(index, 1);
+
+        if (Object.keys(playersByAnimation).length === 0) {
+          this._map.delete(element);
+        }
       }
     }
   }

modules/@angular/core/src/linker/animation_view_context.ts

@@ -29,19 +29,19 @@ export class AnimationViewContext {
   queueAnimation(element: any, animationName: string, player: AnimationPlayer): void {
     queueAnimationGlobally(player);
     this._players.set(element, animationName, player);
+    player.onDone(() => this._players.remove(element, animationName, player));
   }
 
-  getAnimationPlayers(element: any, animationName: string, removeAllAnimations: boolean = false):
-      AnimationPlayer[] {
+  getAnimationPlayers(element: any, animationName: string = null): AnimationPlayer[] {
     const players: AnimationPlayer[] = [];
-    if (removeAllAnimations) {
-      this._players.findAllPlayersByElement(element).forEach(
-          player => { _recursePlayers(player, players); });
-    } else {
+    if (animationName) {
       const currentPlayer = this._players.find(element, animationName);
       if (currentPlayer) {
         _recursePlayers(currentPlayer, players);
       }
+    } else {
+      this._players.findAllPlayersByElement(element).forEach(
+          player => _recursePlayers(player, players));
     }
     return players;
   }

modules/@angular/core/test/animation/animation_integration_spec.ts

@@ -2121,6 +2121,43 @@ function declareTests({useJit}: {useJit: boolean}) {
            expect(kf[1]).toEqual([1, {'height': '333px', 'opacity': AUTO_STYLE, 'width': '200px'}]);
          });
     });
+
+    it('should not use the previous animation\'s styling if the previous animation has already finished',
+       fakeAsync(() => {
+         TestBed.overrideComponent(DummyIfCmp, {
+           set: {
+             template: `
+            <div [@myAnimation]="exp"></div>
+          `,
+             animations: [trigger(
+                 'myAnimation',
+                 [
+                   state('a', style({color: 'red'})), state('b', style({color: 'red'})),
+                   transition('* => *', animate(1000))
+                 ])]
+           }
+         });
+
+         const driver = TestBed.get(AnimationDriver) as MockAnimationDriver;
+         const fixture = TestBed.createComponent(DummyIfCmp);
+         const cmp = fixture.componentInstance;
+
+         cmp.exp = 'a';
+         fixture.detectChanges();
+         flushMicrotasks();
+
+         const animation1 = driver.log.shift();
+         expect(animation1['previousStyles']).toEqual({});
+
+         animation1['player'].finish();
+
+         cmp.exp = 'b';
+         fixture.detectChanges();
+         flushMicrotasks();
+
+         const animation2 = driver.log.shift();
+         expect(animation2['previousStyles']).toEqual({});
+       }));
   });
 
   describe('full animation integration tests', () => {

modules/@angular/core/test/linker/animation_view_context_spec.ts

@@ -0,0 +1,61 @@
+/**
+ * @license
+ * Copyright Google Inc. All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import {el} from '@angular/platform-browser/testing/browser_util';
+
+import {NoOpAnimationPlayer} from '../../src/animation/animation_player';
+import {AnimationViewContext} from '../../src/linker/animation_view_context';
+import {fakeAsync, flushMicrotasks} from '../../testing';
+import {describe, expect, iit, it} from '../../testing/testing_internal';
+
+export function main() {
+  describe('AnimationViewContext', function() {
+    let viewContext: AnimationViewContext;
+    let elm: any;
+    beforeEach(() => {
+      viewContext = new AnimationViewContext();
+      elm = el('<div></div>');
+    });
+
+    function getPlayers() { return viewContext.getAnimationPlayers(elm); }
+
+    it('should remove the player from the registry once the animation is complete',
+       fakeAsync(() => {
+         const player = new NoOpAnimationPlayer();
+
+         expect(getPlayers().length).toEqual(0);
+         viewContext.queueAnimation(elm, 'someAnimation', player);
+         expect(getPlayers().length).toEqual(1);
+         player.finish();
+         expect(getPlayers().length).toEqual(0);
+       }));
+
+    it('should not remove a follow-up player from the registry if another player is queued',
+       fakeAsync(() => {
+         const player1 = new NoOpAnimationPlayer();
+         const player2 = new NoOpAnimationPlayer();
+
+         viewContext.queueAnimation(elm, 'someAnimation', player1);
+         expect(getPlayers().length).toBe(1);
+         expect(getPlayers()[0]).toBe(player1);
+
+         viewContext.queueAnimation(elm, 'someAnimation', player2);
+         expect(getPlayers().length).toBe(1);
+         expect(getPlayers()[0]).toBe(player2);
+
+         player1.finish();
+
+         expect(getPlayers().length).toBe(1);
+         expect(getPlayers()[0]).toBe(player2);
+
+         player2.finish();
+
+         expect(getPlayers().length).toBe(0);
+       }));
+  });
+}