fix(animations): allow animations to be destroyed manually (#12719)

Closes #12456
Closes #12719

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

@@ -251,17 +251,22 @@ class _AnimationBuilder implements AnimationAstVisitor {
         _ANIMATION_PLAYER_VAR
             .callMethod(
                 'onDone',
-                [o.fn(
+                [o
-                    [],
+                     .fn([],
-                    [RENDER_STYLES_FN
+                         [
-                         .callFn([
+                           _ANIMATION_PLAYER_VAR.callMethod('destroy', []).toStmt(),
-                           _ANIMATION_FACTORY_ELEMENT_VAR, _ANIMATION_FACTORY_RENDERER_VAR,
+                           RENDER_STYLES_FN
-                           o.importExpr(resolveIdentifier(Identifiers.prepareFinalAnimationStyles))
                                .callFn([
-                                 _ANIMATION_START_STATE_STYLES_VAR, _ANIMATION_END_STATE_STYLES_VAR
+                                 _ANIMATION_FACTORY_ELEMENT_VAR, _ANIMATION_FACTORY_RENDERER_VAR,
+                                 o.importExpr(
+                                      resolveIdentifier(Identifiers.prepareFinalAnimationStyles))
+                                     .callFn([
+                                       _ANIMATION_START_STATE_STYLES_VAR,
+                                       _ANIMATION_END_STATE_STYLES_VAR
+                                     ])
                                ])
-                         ])
+                               .toStmt()
-                         .toStmt()])])
+                         ])])
             .toStmt());
 
     statements.push(_ANIMATION_FACTORY_VIEW_CONTEXT

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

@@ -15,6 +15,7 @@ export class AnimationGroupPlayer implements AnimationPlayer {
   private _onStartFns: Function[] = [];
   private _finished = false;
   private _started = false;
+  private _destroyed = false;
 
   public parentPlayer: AnimationPlayer = null;
 
@@ -38,9 +39,6 @@ export class AnimationGroupPlayer implements AnimationPlayer {
   private _onFinish() {
     if (!this._finished) {
       this._finished = true;
-      if (!isPresent(this.parentPlayer)) {
-        this.destroy();
-      }
       this._onDoneFns.forEach(fn => fn());
       this._onDoneFns = [];
     }
@@ -76,11 +74,19 @@ export class AnimationGroupPlayer implements AnimationPlayer {
   }
 
   destroy(): void {
-    this._onFinish();
+    if (!this._destroyed) {
-    this._players.forEach(player => player.destroy());
+      this._onFinish();
+      this._players.forEach(player => player.destroy());
+      this._destroyed = true;
+    }
   }
 
-  reset(): void { this._players.forEach(player => player.reset()); }
+  reset(): void {
+    this._players.forEach(player => player.reset());
+    this._destroyed = false;
+    this._finished = false;
+    this._started = false;
+  }
 
   setPosition(p: any /** TODO #9100 */): void {
     this._players.forEach(player => { player.setPosition(p); });

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

@@ -16,7 +16,8 @@ export class AnimationSequencePlayer implements AnimationPlayer {
   private _onDoneFns: Function[] = [];
   private _onStartFns: Function[] = [];
   private _finished = false;
-  private _started: boolean = false;
+  private _started = false;
+  private _destroyed = false;
 
   public parentPlayer: AnimationPlayer = null;
 
@@ -48,9 +49,6 @@ export class AnimationSequencePlayer implements AnimationPlayer {
   private _onFinish() {
     if (!this._finished) {
       this._finished = true;
-      if (!isPresent(this.parentPlayer)) {
-        this.destroy();
-      }
       this._onDoneFns.forEach(fn => fn());
       this._onDoneFns = [];
     }
@@ -79,13 +77,18 @@ export class AnimationSequencePlayer implements AnimationPlayer {
   pause(): void { this._activePlayer.pause(); }
 
   restart(): void {
+    this.reset();
     if (this._players.length > 0) {
-      this.reset();
       this._players[0].restart();
     }
   }
 
-  reset(): void { this._players.forEach(player => player.reset()); }
+  reset(): void {
+    this._players.forEach(player => player.reset());
+    this._destroyed = false;
+    this._finished = false;
+    this._started = false;
+  }
 
   finish(): void {
     this._onFinish();
@@ -93,8 +96,11 @@ export class AnimationSequencePlayer implements AnimationPlayer {
   }
 
   destroy(): void {
-    this._onFinish();
+    if (!this._destroyed) {
-    this._players.forEach(player => player.destroy());
+      this._onFinish();
+      this._players.forEach(player => player.destroy());
+      this._destroyed = true;
+    }
   }
 
   setPosition(p: any /** TODO #9100 */): void { this._players[0].setPosition(p); }

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

@@ -95,7 +95,7 @@ export function main() {
       assertLastStatus(players[2], 'restart', true);
     });
 
-    it('should finish all the players', () => {
+    it('should not destroy the inner the players when finished', () => {
       var group = new AnimationGroupPlayer(players);
 
       var completed = false;
@@ -113,55 +113,36 @@ export function main() {
 
       group.finish();
 
-      assertLastStatus(players[0], 'finish', true, -1);
+      assertLastStatus(players[0], 'finish', true);
-      assertLastStatus(players[1], 'finish', true, -1);
+      assertLastStatus(players[1], 'finish', true);
-      assertLastStatus(players[2], 'finish', true, -1);
+      assertLastStatus(players[2], 'finish', true);
-
-      assertLastStatus(players[0], 'destroy', true);
-      assertLastStatus(players[1], 'destroy', true);
-      assertLastStatus(players[2], 'destroy', true);
 
       expect(completed).toBeTruthy();
     });
 
-    it('should call destroy automatically when finished if no parent player is present', () => {
+    it('should not call destroy automatically when finished even if a parent player finishes',
-      var group = new AnimationGroupPlayer(players);
+       () => {
+         var group = new AnimationGroupPlayer(players);
+         var parent = new AnimationGroupPlayer([group, new MockAnimationPlayer()]);
 
-      group.play();
+         group.play();
 
-      assertLastStatus(players[0], 'destroy', false);
+         assertLastStatus(players[0], 'destroy', false);
-      assertLastStatus(players[1], 'destroy', false);
+         assertLastStatus(players[1], 'destroy', false);
-      assertLastStatus(players[2], 'destroy', false);
+         assertLastStatus(players[2], 'destroy', false);
 
-      group.finish();
+         group.finish();
 
-      assertLastStatus(players[0], 'destroy', true);
+         assertLastStatus(players[0], 'destroy', false);
-      assertLastStatus(players[1], 'destroy', true);
+         assertLastStatus(players[1], 'destroy', false);
-      assertLastStatus(players[2], 'destroy', true);
+         assertLastStatus(players[2], 'destroy', false);
-    });
 
-    it('should not call destroy automatically when finished if a parent player is present', () => {
+         parent.finish();
-      var group = new AnimationGroupPlayer(players);
-      var parent = new AnimationGroupPlayer([group, new MockAnimationPlayer()]);
 
-      group.play();
+         assertLastStatus(players[0], 'destroy', false);
-
+         assertLastStatus(players[1], 'destroy', false);
-      assertLastStatus(players[0], 'destroy', false);
+         assertLastStatus(players[2], 'destroy', false);
-      assertLastStatus(players[1], 'destroy', false);
+       });
-      assertLastStatus(players[2], 'destroy', false);
-
-      group.finish();
-
-      assertLastStatus(players[0], 'destroy', false);
-      assertLastStatus(players[1], 'destroy', false);
-      assertLastStatus(players[2], 'destroy', false);
-
-      parent.finish();
-
-      assertLastStatus(players[0], 'destroy', true);
-      assertLastStatus(players[1], 'destroy', true);
-      assertLastStatus(players[2], 'destroy', true);
-    });
 
     it('should function without any players', () => {
       var group = new AnimationGroupPlayer([]);
@@ -193,5 +174,31 @@ export function main() {
          flushMicrotasks();
          expect(completed).toEqual(true);
        }));
+
+    it('should not allow the player to be destroyed if it already has been destroyed unless reset',
+       fakeAsync(() => {
+         var p1 = new MockAnimationPlayer();
+         var p2 = new MockAnimationPlayer();
+         var innerPlayers = [p1, p2];
+
+         var groupPlayer = new AnimationGroupPlayer(innerPlayers);
+         expect(p1.log[p1.log.length - 1]).not.toContain('destroy');
+         expect(p2.log[p2.log.length - 1]).not.toContain('destroy');
+
+         groupPlayer.destroy();
+         expect(p1.log[p1.log.length - 1]).toContain('destroy');
+         expect(p2.log[p2.log.length - 1]).toContain('destroy');
+
+         p1.log = p2.log = [];
+
+         groupPlayer.destroy();
+         expect(p1.log[p1.log.length - 1]).not.toContain('destroy');
+         expect(p2.log[p2.log.length - 1]).not.toContain('destroy');
+
+         groupPlayer.reset();
+         groupPlayer.destroy();
+         expect(p1.log[p1.log.length - 1]).toContain('destroy');
+         expect(p2.log[p2.log.length - 1]).toContain('destroy');
+       }));
   });
 }

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

@@ -135,55 +135,36 @@ export function main() {
 
       sequence.finish();
 
-      assertLastStatus(players[0], 'finish', true, -1);
+      assertLastStatus(players[0], 'finish', true);
-      assertLastStatus(players[1], 'finish', true, -1);
+      assertLastStatus(players[1], 'finish', true);
-      assertLastStatus(players[2], 'finish', true, -1);
+      assertLastStatus(players[2], 'finish', true);
-
-      assertLastStatus(players[0], 'destroy', true);
-      assertLastStatus(players[1], 'destroy', true);
-      assertLastStatus(players[2], 'destroy', true);
 
       expect(completed).toBeTruthy();
     });
 
-    it('should call destroy automatically when finished if no parent player is present', () => {
+    it('should not call destroy automatically when finished even if a parent player is present',
-      var sequence = new AnimationSequencePlayer(players);
+       () => {
+         var sequence = new AnimationSequencePlayer(players);
+         var parent = new AnimationSequencePlayer([sequence, new MockAnimationPlayer()]);
 
-      sequence.play();
+         sequence.play();
 
-      assertLastStatus(players[0], 'destroy', false);
+         assertLastStatus(players[0], 'destroy', false);
-      assertLastStatus(players[1], 'destroy', false);
+         assertLastStatus(players[1], 'destroy', false);
-      assertLastStatus(players[2], 'destroy', false);
+         assertLastStatus(players[2], 'destroy', false);
 
-      sequence.finish();
+         sequence.finish();
 
-      assertLastStatus(players[0], 'destroy', true);
+         assertLastStatus(players[0], 'destroy', false);
-      assertLastStatus(players[1], 'destroy', true);
+         assertLastStatus(players[1], 'destroy', false);
-      assertLastStatus(players[2], 'destroy', true);
+         assertLastStatus(players[2], 'destroy', false);
-    });
 
-    it('should not call destroy automatically when finished if a parent player is present', () => {
+         parent.finish();
-      var sequence = new AnimationSequencePlayer(players);
-      var parent = new AnimationSequencePlayer([sequence, new MockAnimationPlayer()]);
 
-      sequence.play();
+         assertLastStatus(players[0], 'destroy', false);
-
+         assertLastStatus(players[1], 'destroy', false);
-      assertLastStatus(players[0], 'destroy', false);
+         assertLastStatus(players[2], 'destroy', false);
-      assertLastStatus(players[1], 'destroy', false);
+       });
-      assertLastStatus(players[2], 'destroy', false);
-
-      sequence.finish();
-
-      assertLastStatus(players[0], 'destroy', false);
-      assertLastStatus(players[1], 'destroy', false);
-      assertLastStatus(players[2], 'destroy', false);
-
-      parent.finish();
-
-      assertLastStatus(players[0], 'destroy', true);
-      assertLastStatus(players[1], 'destroy', true);
-      assertLastStatus(players[2], 'destroy', true);
-    });
 
     it('should function without any players', () => {
       var sequence = new AnimationSequencePlayer([]);
@@ -215,5 +196,31 @@ export function main() {
          flushMicrotasks();
          expect(completed).toEqual(true);
        }));
+
+    it('should not allow the player to be destroyed if it already has been destroyed unless reset',
+       fakeAsync(() => {
+         var p1 = new MockAnimationPlayer();
+         var p2 = new MockAnimationPlayer();
+         var innerPlayers = [p1, p2];
+
+         var sequencePlayer = new AnimationSequencePlayer(innerPlayers);
+         expect(p1.log[p1.log.length - 1]).not.toContain('destroy');
+         expect(p2.log[p2.log.length - 1]).not.toContain('destroy');
+
+         sequencePlayer.destroy();
+         expect(p1.log[p1.log.length - 1]).toContain('destroy');
+         expect(p2.log[p2.log.length - 1]).toContain('destroy');
+
+         p1.log = p2.log = [];
+
+         sequencePlayer.destroy();
+         expect(p1.log[p1.log.length - 1]).not.toContain('destroy');
+         expect(p2.log[p2.log.length - 1]).not.toContain('destroy');
+
+         sequencePlayer.reset();
+         sequencePlayer.destroy();
+         expect(p1.log[p1.log.length - 1]).toContain('destroy');
+         expect(p2.log[p2.log.length - 1]).toContain('destroy');
+       }));
   });
 }

modules/@angular/core/testing/mock_animation_player.ts

@@ -7,14 +7,13 @@
  */
 
 import {AnimationPlayer} from '@angular/core';
-import {isPresent} from './facade/lang';
 
 export class MockAnimationPlayer implements AnimationPlayer {
   private _onDoneFns: Function[] = [];
   private _onStartFns: Function[] = [];
   private _finished = false;
   private _destroyed = false;
-  private _started: boolean = false;
+  private _started = false;
 
   public parentPlayer: AnimationPlayer = null;
 
@@ -27,9 +26,6 @@ export class MockAnimationPlayer implements AnimationPlayer {
 
       this._onDoneFns.forEach(fn => fn());
       this._onDoneFns = [];
-      if (!isPresent(this.parentPlayer)) {
-        this.destroy();
-      }
     }
   }
 
@@ -56,7 +52,12 @@ export class MockAnimationPlayer implements AnimationPlayer {
 
   finish(): void { this._onFinish(); }
 
-  reset(): void { this.log.push('reset'); }
+  reset(): void {
+    this.log.push('reset');
+    this._destroyed = false;
+    this._finished = false;
+    this._started = false;
+  }
 
   destroy(): void {
     if (!this._destroyed) {

modules/@angular/platform-browser/src/dom/web_animations_player.ts

@@ -7,7 +7,6 @@
  */
 
 import {AUTO_STYLE} from '@angular/core';
-import {isPresent} from '../facade/lang';
 import {AnimationPlayer} from '../private_import_core';
 
 import {getDOM} from './dom_adapter';
@@ -16,11 +15,12 @@ import {DomAnimatePlayer} from './dom_animate_player';
 export class WebAnimationsPlayer implements AnimationPlayer {
   private _onDoneFns: Function[] = [];
   private _onStartFns: Function[] = [];
-  private _finished = false;
-  private _initialized = false;
   private _player: DomAnimatePlayer;
-  private _started: boolean = false;
   private _duration: number;
+  private _initialized = false;
+  private _finished = false;
+  private _started = false;
+  private _destroyed = false;
 
   public parentPlayer: AnimationPlayer = null;
 
@@ -33,9 +33,6 @@ export class WebAnimationsPlayer implements AnimationPlayer {
   private _onFinish() {
     if (!this._finished) {
       this._finished = true;
-      if (!isPresent(this.parentPlayer)) {
-        this.destroy();
-      }
       this._onDoneFns.forEach(fn => fn());
       this._onDoneFns = [];
     }
@@ -57,7 +54,7 @@ export class WebAnimationsPlayer implements AnimationPlayer {
     this._player = this._triggerWebAnimation(this.element, keyframes, this.options);
 
     // this is required so that the player doesn't start to animate right away
-    this.reset();
+    this._resetDomPlayerState();
     this._player.onfinish = () => this._onFinish();
   }
 
@@ -91,7 +88,14 @@ export class WebAnimationsPlayer implements AnimationPlayer {
     this._player.finish();
   }
 
-  reset(): void { this._player.cancel(); }
+  reset(): void {
+    this._resetDomPlayerState();
+    this._destroyed = false;
+    this._finished = false;
+    this._started = false;
+  }
+
+  private _resetDomPlayerState() { this._player.cancel(); }
 
   restart(): void {
     this.reset();
@@ -101,8 +105,11 @@ export class WebAnimationsPlayer implements AnimationPlayer {
   hasStarted(): boolean { return this._started; }
 
   destroy(): void {
-    this.reset();
+    if (!this._destroyed) {
-    this._onFinish();
+      this._resetDomPlayerState();
+      this._onFinish();
+      this._destroyed = true;
+    }
   }
 
   get totalTime(): number { return this._duration; }

modules/@angular/platform-browser/test/dom/web_animations_player_spec.ts

@@ -110,12 +110,12 @@ export function main() {
          expect(count).toEqual(2);
        });
 
-    it('should destroy itself automatically if a parent player is not present', () => {
+    it('should not destroy itself automatically if a parent player is not present', () => {
       captures['cancel'] = [];
       player.finish();
 
       expect(captures['finish'].length).toEqual(1);
-      expect(captures['cancel'].length).toEqual(1);
+      expect(captures['cancel'].length).toEqual(0);
 
       var next = makePlayer();
       var player2 = next['player'];
@@ -139,5 +139,20 @@ export function main() {
       player.play();
       expect(calls).toEqual(1);
     });
+
+    it('should not allow the player to be cancelled via destroy if it has already been destroyed unless reset',
+       () => {
+         captures['cancel'] = [];
+         expect(captures['cancel'].length).toBe(0);
+         player.destroy();
+         expect(captures['cancel'].length).toBe(1);
+         captures['cancel'] = [];
+         player.destroy();
+         expect(captures['cancel'].length).toBe(0);
+         player.reset();
+         captures['cancel'] = [];
+         player.destroy();
+         expect(captures['cancel'].length).toBe(1);
+       });
   });
 }