Revert "fix(animations): always fire inner trigger callbacks even if blocked by parent animations (#19753)"

This reverts commit 5a9ed2de27.
As it broke an internal test.

packages/animations/browser/src/render/transition_animation_engine.ts

@@ -986,15 +986,11 @@ export class TransitionAnimationEngine {
     }
 
     const allPreviousPlayersMap = new Map<any, TransitionAnimationPlayer[]>();
-    // this map works to tell which element in the DOM tree is contained by
+    let sortedParentElements: any[] = [];
-    // which animation. Further down below this map will get populated once
-    // the players are built and in doing so it can efficiently figure out
-    // if a sub player is skipped due to a parent player having priority.
-    const animationElementMap = new Map<any, any>();
     queuedInstructions.forEach(entry => {
       const element = entry.element;
       if (subTimelines.has(element)) {
-        animationElementMap.set(element, element);
+        sortedParentElements.unshift(element);
         this._beforeAnimationBuild(
             entry.player.namespaceId, entry.instruction, allPreviousPlayersMap);
       }
@@ -1045,7 +1041,6 @@ export class TransitionAnimationEngine {
 
     const rootPlayers: TransitionAnimationPlayer[] = [];
     const subPlayers: TransitionAnimationPlayer[] = [];
-    const NO_PARENT_ANIMATION_ELEMENT_DETECTED = {};
     queuedInstructions.forEach(entry => {
       const {element, player, instruction} = entry;
       // this means that it was never consumed by a parent animation which
@@ -1056,41 +1051,29 @@ export class TransitionAnimationEngine {
           return;
         }
 
-        // this will flow up the DOM and query the map to figure out
-        // if a parent animation has priority over it. In the situation
-        // that a parent is detected then it will cancel the loop. If
-        // nothing is detected, or it takes a few hops to find a parent,
-        // then it will fill in the missing nodes and signal them as having
-        // a detected parent (or a NO_PARENT value via a special constant).
-        let parentWithAnimation: any = NO_PARENT_ANIMATION_ELEMENT_DETECTED;
-        if (animationElementMap.size > 1) {
-          let elm = element;
-          const parentsToAdd: any[] = [];
-          while (elm = elm.parentNode) {
-            const detectedParent = animationElementMap.get(elm);
-            if (detectedParent) {
-              parentWithAnimation = detectedParent;
-              break;
-            }
-            parentsToAdd.push(elm);
-          }
-          parentsToAdd.forEach(parent => animationElementMap.set(parent, parentWithAnimation));
-        }
-
         const innerPlayer = this._buildAnimation(
             player.namespaceId, instruction, allPreviousPlayersMap, skippedPlayersMap, preStylesMap,
             postStylesMap);
-
         player.setRealPlayer(innerPlayer);
 
-        if (parentWithAnimation === NO_PARENT_ANIMATION_ELEMENT_DETECTED) {
+        let parentHasPriority: any = null;
-          rootPlayers.push(player);
+        for (let i = 0; i < sortedParentElements.length; i++) {
-        } else {
+          const parent = sortedParentElements[i];
-          const parentPlayers = this.playersByElement.get(parentWithAnimation);
+          if (parent === element) break;
+          if (this.driver.containsElement(parent, element)) {
+            parentHasPriority = parent;
+            break;
+          }
+        }
+
+        if (parentHasPriority) {
+          const parentPlayers = this.playersByElement.get(parentHasPriority);
           if (parentPlayers && parentPlayers.length) {
             player.parentPlayer = optimizeGroupPlayer(parentPlayers);
           }
           skippedPlayers.push(player);
+        } else {
+          rootPlayers.push(player);
         }
       } else {
         eraseStyles(element, instruction.fromStyles);
@@ -1121,7 +1104,7 @@ export class TransitionAnimationEngine {
     // fire the start/done transition callback events
     skippedPlayers.forEach(player => {
       if (player.parentPlayer) {
-        player.syncPlayerEvents(player.parentPlayer);
+        player.parentPlayer.onDestroy(() => player.destroy());
       } else {
         player.destroy();
       }
@@ -1382,15 +1365,6 @@ export class TransitionAnimationPlayer implements AnimationPlayer {
 
   getRealPlayer() { return this._player; }
 
-  syncPlayerEvents(player: AnimationPlayer) {
-    const p = this._player as any;
-    if (p.triggerCallback) {
-      player.onStart(() => p.triggerCallback('start'));
-    }
-    player.onDone(() => this.finish());
-    player.onDestroy(() => this.destroy());
-  }
-
   private _queueEvent(name: string, callback: (event: any) => any): void {
     getOrSetAsInMap(this._queuedCallbacks, name, []).push(callback);
   }
@@ -1444,14 +1418,6 @@ export class TransitionAnimationPlayer implements AnimationPlayer {
   getPosition(): number { return this.queued ? 0 : this._player.getPosition(); }
 
   get totalTime(): number { return this._player.totalTime; }
-
-  /* @internal */
-  triggerCallback(phaseName: string): void {
-    const p = this._player as any;
-    if (p.triggerCallback) {
-      p.triggerCallback(phaseName);
-    }
-  }
 }
 
 function deleteOrUnsetInMap(map: Map<any, any[]>| {[key: string]: any}, key: any, value: any) {

packages/animations/browser/src/render/web_animations/web_animations_player.ts

@@ -184,13 +184,6 @@ export class WebAnimationsPlayer implements AnimationPlayer {
     }
     this.currentSnapshot = styles;
   }
-
-  /* @internal */
-  triggerCallback(phaseName: string): void {
-    const methods = phaseName == 'start' ? this._onStartFns : this._onDoneFns;
-    methods.forEach(fn => fn());
-    methods.length = 0;
-  }
 }
 
 function _computeStyle(element: any, prop: string): string {

packages/animations/browser/test/render/web_animations/web_animations_player_spec.ts

@@ -45,26 +45,6 @@ export function main() {
       const p = innerPlayer !;
       expect(p.log).toEqual(['play']);
     });
-
-    it('should fire start/done callbacks manually when called directly', () => {
-      const log: string[] = [];
-
-      const player = new WebAnimationsPlayer(element, [], {duration: 1000});
-      player.onStart(() => log.push('started'));
-      player.onDone(() => log.push('done'));
-
-      player.triggerCallback('start');
-      expect(log).toEqual(['started']);
-
-      player.play();
-      expect(log).toEqual(['started']);
-
-      player.triggerCallback('done');
-      expect(log).toEqual(['started', 'done']);
-
-      player.finish();
-      expect(log).toEqual(['started', 'done']);
-    });
   });
 }
 

packages/animations/src/players/animation_group_player.ts

@@ -140,11 +140,4 @@ export class AnimationGroupPlayer implements AnimationPlayer {
       }
     });
   }
-
-  /* @internal */
-  triggerCallback(phaseName: string): void {
-    const methods = phaseName == 'start' ? this._onStartFns : this._onDoneFns;
-    methods.forEach(fn => fn());
-    methods.length = 0;
-  }
 }

packages/animations/src/players/animation_player.ts

@@ -31,8 +31,6 @@ export interface AnimationPlayer {
   parentPlayer: AnimationPlayer|null;
   readonly totalTime: number;
   beforeDestroy?: () => any;
-  /* @internal */
-  triggerCallback?: (phaseName: string) => void;
 }
 
 /**
@@ -93,11 +91,4 @@ export class NoopAnimationPlayer implements AnimationPlayer {
   reset(): void {}
   setPosition(p: number): void {}
   getPosition(): number { return 0; }
-
-  /* @internal */
-  triggerCallback(phaseName: string): void {
-    const methods = phaseName == 'start' ? this._onStartFns : this._onDoneFns;
-    methods.forEach(fn => fn());
-    methods.length = 0;
-  }
 }

packages/animations/test/animation_player_spec.ts

@@ -40,29 +40,5 @@ export function main() {
       player.destroy();
       expect(log).toEqual(['started', 'done', 'destroy']);
     });
-
-    it('should fire start/done callbacks manually when called directly', fakeAsync(() => {
-         const log: string[] = [];
-
-         const player = new NoopAnimationPlayer();
-         player.onStart(() => log.push('started'));
-         player.onDone(() => log.push('done'));
-         flushMicrotasks();
-
-         player.triggerCallback('start');
-         expect(log).toEqual(['started']);
-
-         player.play();
-         expect(log).toEqual(['started']);
-
-         player.triggerCallback('done');
-         expect(log).toEqual(['started', 'done']);
-
-         player.finish();
-         expect(log).toEqual(['started', 'done']);
-
-         flushMicrotasks();
-         expect(log).toEqual(['started', 'done']);
-       }));
   });
 }

packages/core/test/animation/animation_query_integration_spec.ts

@@ -409,39 +409,39 @@ export function main() {
            const fixture = TestBed.createComponent(Cmp);
            const cmp = fixture.componentInstance;
 
+           cmp.exp0 = 0;
+
            cmp.exp1 = 0;
            cmp.exp2 = 0;
            cmp.exp3 = 0;
            cmp.exp4 = 0;
            cmp.exp5 = 0;
            fixture.detectChanges();
-
+           engine.flush();
-           cmp.exp0 = 0;
-           fixture.detectChanges();
 
            let players = engine.players;
            cancelAllPlayers(players);
 
+           cmp.exp0 = 1;
+
            cmp.exp2 = 1;
            cmp.exp4 = 1;
            fixture.detectChanges();
-
+           engine.flush();
-           cmp.exp0 = 1;
-           fixture.detectChanges();
 
            players = engine.players;
            cancelAllPlayers(players);
            expect(players.length).toEqual(3);
 
+           cmp.exp0 = 2;
+
            cmp.exp1 = 2;
            cmp.exp2 = 2;
            cmp.exp3 = 2;
            cmp.exp4 = 2;
            cmp.exp5 = 2;
            fixture.detectChanges();
-
+           engine.flush();
-           cmp.exp0 = 2;
-           fixture.detectChanges();
 
            players = engine.players;
            cancelAllPlayers(players);
@@ -449,6 +449,7 @@ export function main() {
 
            cmp.exp0 = 3;
            fixture.detectChanges();
+           engine.flush();
 
            players = engine.players;
            cancelAllPlayers(players);
@@ -2287,15 +2288,19 @@ export function main() {
            }
 
            TestBed.configureTestingModule({declarations: [ParentCmp, ChildCmp]});
+
+           const engine = TestBed.get(ɵAnimationEngine);
            const fixture = TestBed.createComponent(ParentCmp);
            const cmp = fixture.componentInstance;
 
            fixture.detectChanges();
+           engine.flush();
 
            const childCmp = cmp.childElm;
 
            cmp.exp = false;
            fixture.detectChanges();
+           engine.flush();
            flushMicrotasks();
 
            expect(cmp.childEvent.toState).toEqual('void');
@@ -2723,106 +2728,6 @@ export function main() {
         expect(engine.players[0].getRealPlayer()).toBe(players[1]);
       });
 
-      it('should fire and synchronize the start/done callbacks on sub triggers even if they are not allowed to animate within the animation',
-         fakeAsync(() => {
-           @Component({
-             selector: 'parent-cmp',
-             animations: [
-               trigger(
-                   'parent',
-                   [
-                     transition(
-                         '* => go',
-                         [
-                           style({height: '0px'}),
-                           animate(1000, style({height: '100px'})),
-                         ]),
-                   ]),
-             ],
-             template: `
-            <div *ngIf="!remove"
-                 [@parent]="exp"
-                 (@parent.start)="track($event)"
-                 (@parent.done)="track($event)">
-                 <child-cmp #child></child-cmp>
-            </div>
-          `
-           })
-           class ParentCmp {
-             @ViewChild('child') public childCmp: any;
-
-             public exp: any;
-             public log: string[] = [];
-             public remove = false;
-
-             track(event: any) { this.log.push(`${event.triggerName}-${event.phaseName}`); }
-           }
-
-           @Component({
-             selector: 'child-cmp',
-             animations: [
-               trigger(
-                   'child',
-                   [
-                     transition(
-                         '* => go',
-                         [
-                           style({width: '0px'}),
-                           animate(1000, style({width: '100px'})),
-                         ]),
-                   ]),
-             ],
-             template: `
-            <div [@child]="exp"
-                 (@child.start)="track($event)"
-                 (@child.done)="track($event)"></div>
-          `
-           })
-           class ChildCmp {
-             public exp: any;
-             public log: string[] = [];
-             track(event: any) { this.log.push(`${event.triggerName}-${event.phaseName}`); }
-           }
-
-           TestBed.configureTestingModule({declarations: [ParentCmp, ChildCmp]});
-           const engine = TestBed.get(ɵAnimationEngine);
-           const fixture = TestBed.createComponent(ParentCmp);
-           fixture.detectChanges();
-           flushMicrotasks();
-
-           const cmp = fixture.componentInstance;
-           const child = cmp.childCmp;
-
-           expect(cmp.log).toEqual(['parent-start', 'parent-done']);
-           expect(child.log).toEqual(['child-start', 'child-done']);
-
-           cmp.log = [];
-           child.log = [];
-           cmp.exp = 'go';
-           cmp.childCmp.exp = 'go';
-           fixture.detectChanges();
-           flushMicrotasks();
-
-           expect(cmp.log).toEqual(['parent-start']);
-           expect(child.log).toEqual(['child-start']);
-
-           const players = engine.players;
-           expect(players.length).toEqual(1);
-           players[0].finish();
-
-           expect(cmp.log).toEqual(['parent-start', 'parent-done']);
-           expect(child.log).toEqual(['child-start', 'child-done']);
-
-           cmp.log = [];
-           child.log = [];
-           cmp.remove = true;
-           fixture.detectChanges();
-           flushMicrotasks();
-
-           expect(cmp.log).toEqual(['parent-start', 'parent-done']);
-           expect(child.log).toEqual(['child-start', 'child-done']);
-         }));
-
       it('should stretch the starting keyframe of a child animation queries are issued by the parent',
          () => {
            @Component({