refactor(animations): do not inherit past styles into sub timelines

modules/@angular/animation/src/dsl/animation_timeline_visitor.ts

@@ -51,17 +51,10 @@ import {AnimationTimelineInstruction, createTimelineInstruction} from './animati
  *
  * [TimelineBuilder]
  * This class is responsible for tracking the styles and building a series of keyframe objects for a
- * timeline between a start and end time. There is always one top-level timeline and sub timelines
+ * timeline between a start and end time. The builder starts off with an initial timeline and each
- * are forked in two specific cases:
+ * time the AST comes across a `group()`, `keyframes()` or a combination of the two wihtin a
- *
+ * `sequence()` then it will generate a sub timeline for each step as well as a new one after
- * 1. When keyframes() is used it will create a sub timeline. Upon creation, ALL OF THE COLLECTED
+ * they are complete.
- * STYLES from the parent timeline up until this point will be inherited into the keyframes
- * timeline.
- *
- * 2. When group() is used it will create a sub timeline. Upon creation, NONE OF THE COLLECTED
- * STYLES from the parent timeline will be inherited. Although, if the sub timeline does reference a
- * style that was previously used within the parent then it will be copied over into the sub
- * timeline.
  *
  * As the AST is traversed, the timing state on each of the timelines will be incremented. If a sub
  * timeline was created (based on one of the cases above) then the parent timeline will attempt to
@@ -96,22 +89,14 @@ import {AnimationTimelineInstruction, createTimelineInstruction} from './animati
  * keyframes. Therefore the missing `height` value will be properly filled into the already
  * processed keyframes.
  *
+ * When a sub-timeline is created it will have its own backFill property. This is done so that
+ * styles present within the sub-timeline do not accidentally seep into the previous/future timeline
+ * keyframes
+ *
  * (For prototypically-inherited contents to be detected a `for(i in obj)` loop must be used.)
  *
- * Based on whether the styles are inherited into a sub timeline (depending on the two cases
- * mentioned above), the functionality of the backFill will behave differently:
- *
- * 1. If the styles are inherited from the parent then the backFill property will also be inherited
- * and therefore any newly added styles to the backFill will be propagated to the parent timeline
- * and its already processed keyframes.
- *
- * 2. If the styles are not inherited from the parent then the sub timeline will have its own
- * backFill. Then if the sub timeline comes across a property that was not defined already then it
- * will read that from the parent's styles and pass that into its own backFill (which will then
- * propagate the missing styles across the sub timeline only).
- *
  * [Validation]
- * The code in this file is not responsible for validation. That functionaliy happens with within
+ * The code in this file is not responsible for validation. That functionality happens with within
  * the `AnimationValidatorVisitor` code.
  */
 export function buildAnimationKeyframes(
@@ -139,9 +124,9 @@ export class AnimationTimelineContext {
     timelines.push(this.currentTimeline);
   }
 
-  createSubContext(inherit: boolean = false): AnimationTimelineContext {
+  createSubContext(): AnimationTimelineContext {
-    const context = new AnimationTimelineContext(
+    const context =
-        this.errors, this.timelines, this.currentTimeline.fork(inherit));
+        new AnimationTimelineContext(this.errors, this.timelines, this.currentTimeline.fork());
     context.previousNode = this.previousNode;
     context.currentAnimateTimings = this.currentAnimateTimings;
     this.subContextCount++;
@@ -154,7 +139,7 @@ export class AnimationTimelineContext {
     if (newTime > 0) {
       oldTimeline.time = newTime;
     }
-    this.currentTimeline = oldTimeline.fork(true);
+    this.currentTimeline = oldTimeline.fork();
     oldTimeline.time = oldTime;
     this.timelines.push(this.currentTimeline);
     return this.currentTimeline;
@@ -217,35 +202,33 @@ export class AnimationTimelineVisitor implements AnimationDslVisitor {
       context.currentTimeline.snapshotCurrentStyles();
     }
     ast.steps.map(s => visitAnimationNode(this, s, context));
-    context.previousNode = ast;
+
+    // this means that some animation function within the sequence
+    // ended up creating a sub timeline (which means the current
+    // timeline cannot overlap with the contents of the sequence)
     if (context.subContextCount > subContextCount) {
       context.transformIntoNewTimeline();
-      context.currentTimeline.snapshotCurrentStyles();
     }
+
+    context.previousNode = ast;
   }
 
   visitGroup(ast: meta.AnimationGroupMetadata, context: AnimationTimelineContext) {
     const innerTimelines: TimelineBuilder[] = [];
     let furthestTime = context.currentTimeline.currentTime;
     ast.steps.map(s => {
-      const innerContext = context.createSubContext(false);
+      const innerContext = context.createSubContext();
-      innerContext.currentTimeline.snapshotCurrentStyles();
       visitAnimationNode(this, s, innerContext);
       furthestTime = Math.max(furthestTime, innerContext.currentTimeline.currentTime);
       innerTimelines.push(innerContext.currentTimeline);
     });
 
-    context.transformIntoNewTimeline(furthestTime);
-
     // this operation is run after the AST loop because otherwise
     // if the parent timeline's collected styles were updated then
     // it would pass in invalid data into the new-to-be forked items
     innerTimelines.forEach(
         timeline => context.currentTimeline.mergeTimelineCollectedStyles(timeline));
-
+    context.transformIntoNewTimeline(furthestTime);
-    // we do this because the window between this timeline and the sub timeline
-    // should ensure that the styles within are exactly the same as they were before
-    context.currentTimeline.snapshotCurrentStyles();
     context.previousNode = ast;
   }
 
@@ -307,16 +290,10 @@ export class AnimationTimelineVisitor implements AnimationDslVisitor {
     }
 
     const keyframeDuration = context.currentAnimateTimings.duration;
-    const innerContext = context.createSubContext(true);
+    const innerContext = context.createSubContext();
     const innerTimeline = innerContext.currentTimeline;
     innerTimeline.easing = context.currentAnimateTimings.easing;
 
-    // this will ensure that all collected styles so far
-    // are populated into the first keyframe of the keyframes()
-    // timeline (even if there exists a starting keyframe then
-    // it will override the contents of the first frame later)
-    innerTimeline.snapshotCurrentStyles();
-
     ast.steps.map((step: meta.AnimationStyleMetadata, i: number) => {
       const normalizedStyles = normalizeStyles(new AnimationStyles(step.styles));
       const offset = containsOffsets ? <number>normalizedStyles['offset'] :
@@ -332,7 +309,6 @@ export class AnimationTimelineVisitor implements AnimationDslVisitor {
     // we do this because the window between this timeline and the sub timeline
     // should ensure that the styles within are exactly the same as they were before
     context.transformIntoNewTimeline(context.currentTimeline.time + keyframeDuration);
-    context.currentTimeline.snapshotCurrentStyles();
     context.previousNode = ast;
   }
 }
@@ -346,18 +322,8 @@ export class TimelineBuilder {
   private _localTimelineStyles: StyleData;
   private _backFill: StyleData = {};
 
-  constructor(
+  constructor(public startTime: number, private _globalTimelineStyles: StyleData = null) {
-      public startTime: number, private _globalTimelineStyles: StyleData = null,
-      inheritedBackFill: StyleData = null, inheritedStyles: StyleData = null) {
-    if (inheritedBackFill) {
-      this._backFill = inheritedBackFill;
-    }
-
     this._localTimelineStyles = Object.create(this._backFill, {});
-    if (inheritedStyles) {
-      this._localTimelineStyles = copyStyles(inheritedStyles, false, this._localTimelineStyles);
-    }
-
     if (!this._globalTimelineStyles) {
       this._globalTimelineStyles = this._localTimelineStyles;
     }
@@ -368,11 +334,8 @@ export class TimelineBuilder {
 
   get currentTime() { return this.startTime + this.time; }
 
-  fork(inherit: boolean = false): TimelineBuilder {
+  fork(): TimelineBuilder {
-    let inheritedBackFill = inherit ? this._backFill : null;
+    return new TimelineBuilder(this.currentTime, this._globalTimelineStyles);
-    let inheritedStyles = inherit ? this._localTimelineStyles : null;
-    return new TimelineBuilder(
-        this.currentTime, this._globalTimelineStyles, inheritedBackFill, inheritedStyles);
   }
 
   private _loadKeyframe() {
@@ -395,9 +358,6 @@ export class TimelineBuilder {
 
   private _updateStyle(prop: string, value: string|number) {
     if (prop != 'easing') {
-      if (!this._localTimelineStyles[prop]) {
-        this._backFill[prop] = this._globalTimelineStyles[prop] || meta.AUTO_STYLE;
-      }
       this._localTimelineStyles[prop] = value;
       this._globalTimelineStyles[prop] = value;
       this._styleSummary[prop] = {time: this.currentTime, value};
@@ -409,6 +369,9 @@ export class TimelineBuilder {
       if (prop !== 'offset') {
         const val = styles[prop];
         this._currentKeyframe[prop] = val;
+        if (prop !== 'easing' && !this._localTimelineStyles[prop]) {
+          this._backFill[prop] = this._globalTimelineStyles[prop] || meta.AUTO_STYLE;
+        }
         this._updateStyle(prop, val);
       }
     });

modules/@angular/animation/src/engine/web_animations/web_animations_player.ts

@@ -16,11 +16,13 @@ export class WebAnimationsPlayer implements AnimationPlayer {
   private _onDestroyFns: Function[] = [];
   private _player: DOMAnimation;
   private _duration: number;
+  private _delay: number;
   private _initialized = false;
   private _finished = false;
   private _started = false;
   private _destroyed = false;
   private _finalKeyframe: {[key: string]: string | number};
+  public time = 0;
 
   public parentPlayer: AnimationPlayer = null;
   public previousStyles: {[styleName: string]: string | number};
@@ -30,6 +32,8 @@ export class WebAnimationsPlayer implements AnimationPlayer {
       public options: {[key: string]: string | number},
       previousPlayers: WebAnimationsPlayer[] = []) {
     this._duration = <number>options['duration'];
+    this._delay = <number>options['delay'] || 0;
+    this.time = this._duration + this._delay;
 
     this.previousStyles = {};
     previousPlayers.forEach(player => {
@@ -157,11 +161,9 @@ export class WebAnimationsPlayer implements AnimationPlayer {
     }
   }
 
-  get totalTime(): number { return this._duration; }
+  setPosition(p: number): void { this._player.currentTime = p * this.time; }
 
-  setPosition(p: number): void { this._player.currentTime = p * this.totalTime; }
+  getPosition(): number { return this._player.currentTime / this.time; }
-
-  getPosition(): number { return this._player.currentTime / this.totalTime; }
 
   private _captureStyles(): {[prop: string]: string | number} {
     const styles: {[key: string]: string | number} = {};

modules/@angular/animation/test/dsl/animation_spec.ts

@@ -227,8 +227,8 @@ export function main() {
           const steps = [
             animate(1000, style({opacity: .5})), animate(1000, style({opacity: 1})),
             animate(
-                1000, keyframes([style({height: 0}), style({height: 100}), style({height: 0})])),
+                1000, keyframes([style({height: 0}), style({height: 100}), style({height: 50})])),
-            animate(1000, style({opacity: 0}))
+            animate(1000, style({height: 0, opacity: 0}))
           ];
 
           const players = invokeAnimationSequence(steps);
@@ -237,23 +237,23 @@ export function main() {
           const player0 = players[0];
           expect(player0.delay).toEqual(0);
           expect(player0.keyframes).toEqual([
-            {opacity: AUTO_STYLE, height: AUTO_STYLE, offset: 0},
+            {opacity: AUTO_STYLE, offset: 0},
-            {opacity: .5, height: AUTO_STYLE, offset: .5},
+            {opacity: .5, offset: .5},
-            {opacity: 1, height: AUTO_STYLE, offset: 1},
+            {opacity: 1, offset: 1},
           ]);
 
           const subPlayer = players[1];
           expect(subPlayer.delay).toEqual(2000);
           expect(subPlayer.keyframes).toEqual([
-            {opacity: 1, height: 0, offset: 0},
+            {height: 0, offset: 0},
-            {opacity: 1, height: 100, offset: .5},
+            {height: 100, offset: .5},
-            {opacity: 1, height: 0, offset: 1},
+            {height: 50, offset: 1},
           ]);
 
           const player1 = players[2];
           expect(player1.delay).toEqual(3000);
           expect(player1.keyframes).toEqual([
-            {opacity: 1, height: 0, offset: 0}, {opacity: 0, height: 0, offset: 1}
+            {opacity: 1, height: 50, offset: 0}, {opacity: 0, height: 0, offset: 1}
           ]);
         });
 
@@ -322,6 +322,59 @@ export function main() {
              const player = invokeAnimationSequence(steps)[1];
              expect(player.delay).toEqual(2500);
            });
+
+        it('should not leak in additional styles used later on after keyframe styles have already been declared',
+           () => {
+             const steps = [
+               animate(1000, style({height: '50px'})),
+               animate(
+                   2000, keyframes([
+                     style({left: '0', transform: 'rotate(0deg)', offset: 0}),
+                     style({
+                       left: '40%',
+                       transform: 'rotate(250deg) translateY(-200px)',
+                       offset: .33
+                     }),
+                     style(
+                         {left: '60%', transform: 'rotate(180deg) translateY(200px)', offset: .66}),
+                     style({left: 'calc(100% - 100px)', transform: 'rotate(0deg)', offset: 1}),
+                   ])),
+               group([animate('2s', style({width: '200px'}))]),
+               animate('2s', style({height: '300px'})),
+               group([animate('2s', style({height: '500px', width: '500px'}))])
+             ];
+
+             const players = invokeAnimationSequence(steps);
+             expect(players.length).toEqual(5);
+
+             const firstPlayerKeyframes = players[0].keyframes;
+             expect(firstPlayerKeyframes[0]['width']).toBeFalsy();
+             expect(firstPlayerKeyframes[1]['width']).toBeFalsy();
+             expect(firstPlayerKeyframes[0]['height']).toEqual(AUTO_STYLE);
+             expect(firstPlayerKeyframes[1]['height']).toEqual('50px');
+
+             const keyframePlayerKeyframes = players[1].keyframes;
+             expect(keyframePlayerKeyframes[0]['width']).toBeFalsy();
+             expect(keyframePlayerKeyframes[0]['height']).toBeFalsy();
+
+             const groupPlayerKeyframes = players[2].keyframes;
+             expect(groupPlayerKeyframes[0]['width']).toEqual(AUTO_STYLE);
+             expect(groupPlayerKeyframes[1]['width']).toEqual('200px');
+             expect(groupPlayerKeyframes[0]['height']).toBeFalsy();
+             expect(groupPlayerKeyframes[1]['height']).toBeFalsy();
+
+             const secondToFinalAnimatePlayerKeyframes = players[3].keyframes;
+             expect(secondToFinalAnimatePlayerKeyframes[0]['width']).toBeFalsy();
+             expect(secondToFinalAnimatePlayerKeyframes[1]['width']).toBeFalsy();
+             expect(secondToFinalAnimatePlayerKeyframes[0]['height']).toEqual('50px');
+             expect(secondToFinalAnimatePlayerKeyframes[1]['height']).toEqual('300px');
+
+             const finalAnimatePlayerKeyframes = players[4].keyframes;
+             expect(finalAnimatePlayerKeyframes[0]['width']).toEqual('200px');
+             expect(finalAnimatePlayerKeyframes[1]['width']).toEqual('500px');
+             expect(finalAnimatePlayerKeyframes[0]['height']).toEqual('300px');
+             expect(finalAnimatePlayerKeyframes[1]['height']).toEqual('500px');
+           });
       });
 
       describe('group()', () => {