Bug 1108055 - Part 2: Refine ComputedTiming. r=birtles

Do some minor revisions in struct ComputedTiming.
1. Use Nullable<double> mProgress, so remove the static const kNullProgress.
   The generated ComputedTimingProperties dictionary uses "Nullable" variable,
   so we replace the origin type in ComputedTiming to make it more consistent
   with that in ComputedTimingProperties dictionary.
2. Use scoped enums for AnimationPhase.

--HG--
extra : rebase_source : 31280c867a30e7bcdcfe831cbc72ca08c8ddc762

dom/animation/KeyframeEffect.cpp

@@ -105,9 +105,6 @@ ComputedTimingFunction::AppendToString(nsAString& aResult) const
   }
 }
 
-// In the Web Animations model, the iteration progress can be outside the range
-// [0.0, 1.0] but it shouldn't be Infinity.
-const double ComputedTiming::kNullProgress = PositiveInfinity<double>();
 
 namespace dom {
 
@@ -206,10 +203,10 @@ KeyframeEffectReadOnly::GetComputedTimingAt(
   // Get the normalized time within the active interval.
   StickyTimeDuration activeTime;
   if (localTime >= aTiming.mDelay + result.mActiveDuration) {
-    result.mPhase = ComputedTiming::AnimationPhase_After;
+    result.mPhase = ComputedTiming::AnimationPhase::After;
     if (!aTiming.FillsForwards()) {
       // The animation isn't active or filling at this time.
-      result.mProgress = ComputedTiming::kNullProgress;
+      result.mProgress.SetNull();
       return result;
     }
     activeTime = result.mActiveDuration;
@@ -219,17 +216,17 @@ KeyframeEffectReadOnly::GetComputedTimingAt(
       aTiming.mIterationCount != 0.0 &&
       aTiming.mIterationCount == floor(aTiming.mIterationCount);
   } else if (localTime < aTiming.mDelay) {
-    result.mPhase = ComputedTiming::AnimationPhase_Before;
+    result.mPhase = ComputedTiming::AnimationPhase::Before;
     if (!aTiming.FillsBackwards()) {
       // The animation isn't active or filling at this time.
-      result.mProgress = ComputedTiming::kNullProgress;
+      result.mProgress.SetNull();
       return result;
     }
     // activeTime is zero
   } else {
     MOZ_ASSERT(result.mActiveDuration != zeroDuration,
                "How can we be in the middle of a zero-duration interval?");
-    result.mPhase = ComputedTiming::AnimationPhase_Active;
+    result.mPhase = ComputedTiming::AnimationPhase::Active;
     activeTime = localTime - aTiming.mDelay;
   }
 
@@ -254,7 +251,7 @@ KeyframeEffectReadOnly::GetComputedTimingAt(
     // iteration duration of zero that is filling forwards (but we're not at
     // the exact end of an iteration since we deal with that above).
     result.mCurrentIteration =
-      result.mPhase == ComputedTiming::AnimationPhase_After
+      result.mPhase == ComputedTiming::AnimationPhase::After
       ? static_cast<uint64_t>(aTiming.mIterationCount) // floor
       : 0;
   } else {
@@ -263,19 +260,21 @@ KeyframeEffectReadOnly::GetComputedTimingAt(
   }
 
   // Normalize the iteration time into a fraction of the iteration duration.
-  if (result.mPhase == ComputedTiming::AnimationPhase_Before) {
-    result.mProgress = 0.0;
-  } else if (result.mPhase == ComputedTiming::AnimationPhase_After) {
-    result.mProgress = isEndOfFinalIteration
-                       ? 1.0
-                       : fmod(aTiming.mIterationCount, 1.0f);
+  if (result.mPhase == ComputedTiming::AnimationPhase::Before) {
+    result.mProgress.SetValue(0.0);
+  } else if (result.mPhase == ComputedTiming::AnimationPhase::After) {
+    double progress = isEndOfFinalIteration
+                      ? 1.0
+                      : fmod(aTiming.mIterationCount, 1.0f);
+    result.mProgress.SetValue(progress);
   } else {
     // We are in the active phase so the iteration duration can't be zero.
     MOZ_ASSERT(aTiming.mIterationDuration != zeroDuration,
                "In the active phase of a zero-duration animation?");
-    result.mProgress = aTiming.mIterationDuration == TimeDuration::Forever()
-                       ? 0.0
-                       : iterationTime / aTiming.mIterationDuration;
+    double progress = aTiming.mIterationDuration == TimeDuration::Forever()
+                      ? 0.0
+                      : iterationTime / aTiming.mIterationDuration;
+    result.mProgress.SetValue(progress);
   }
 
   bool thisIterationReverse = false;
@@ -294,7 +293,7 @@ KeyframeEffectReadOnly::GetComputedTimingAt(
       break;
   }
   if (thisIterationReverse) {
-    result.mProgress = 1.0 - result.mProgress;
+    result.mProgress.SetValue(1.0 - result.mProgress.Value());
   }
 
   return result;
@@ -324,7 +323,7 @@ KeyframeEffectReadOnly::IsInPlay() const
     return false;
   }
 
-  return GetComputedTiming().mPhase == ComputedTiming::AnimationPhase_Active;
+  return GetComputedTiming().mPhase == ComputedTiming::AnimationPhase::Active;
 }
 
 // https://w3c.github.io/web-animations/#current
@@ -336,8 +335,8 @@ KeyframeEffectReadOnly::IsCurrent() const
   }
 
   ComputedTiming computedTiming = GetComputedTiming();
-  return computedTiming.mPhase == ComputedTiming::AnimationPhase_Before ||
-         computedTiming.mPhase == ComputedTiming::AnimationPhase_Active;
+  return computedTiming.mPhase == ComputedTiming::AnimationPhase::Before ||
+         computedTiming.mPhase == ComputedTiming::AnimationPhase::Active;
 }
 
 // https://w3c.github.io/web-animations/#in-effect
@@ -345,7 +344,7 @@ bool
 KeyframeEffectReadOnly::IsInEffect() const
 {
   ComputedTiming computedTiming = GetComputedTiming();
-  return computedTiming.mProgress != ComputedTiming::kNullProgress;
+  return !computedTiming.mProgress.IsNull();
 }
 
 void
@@ -391,12 +390,13 @@ KeyframeEffectReadOnly::ComposeStyle(RefPtr<AnimValuesStyleRule>& aStyleRule,
 
   // If the progress is null, we don't have fill data for the current
   // time so we shouldn't animate.
-  if (computedTiming.mProgress == ComputedTiming::kNullProgress) {
+  if (computedTiming.mProgress.IsNull()) {
     return;
   }
 
-  MOZ_ASSERT(0.0 <= computedTiming.mProgress &&
-             computedTiming.mProgress <= 1.0,
+  MOZ_ASSERT(!computedTiming.mProgress.IsNull() &&
+             0.0 <= computedTiming.mProgress.Value() &&
+             computedTiming.mProgress.Value() <= 1.0,
              "iteration progress should be in [0-1]");
 
   for (size_t propIdx = 0, propEnd = mProperties.Length();
@@ -434,7 +434,7 @@ KeyframeEffectReadOnly::ComposeStyle(RefPtr<AnimValuesStyleRule>& aStyleRule,
     // FIXME: Maybe cache the current segment?
     const AnimationPropertySegment *segment = prop.mSegments.Elements(),
                                 *segmentEnd = segment + prop.mSegments.Length();
-    while (segment->mToKey < computedTiming.mProgress) {
+    while (segment->mToKey < computedTiming.mProgress.Value()) {
       MOZ_ASSERT(segment->mFromKey < segment->mToKey, "incorrect keys");
       ++segment;
       if (segment == segmentEnd) {
@@ -458,7 +458,7 @@ KeyframeEffectReadOnly::ComposeStyle(RefPtr<AnimValuesStyleRule>& aStyleRule,
     }
 
     double positionInSegment =
-      (computedTiming.mProgress - segment->mFromKey) /
+      (computedTiming.mProgress.Value() - segment->mFromKey) /
       (segment->mToKey - segment->mFromKey);
     double valuePosition =
       segment->mTimingFunction.GetValue(positionInSegment);

dom/animation/KeyframeEffect.h

@@ -72,38 +72,25 @@ struct AnimationTiming
  */
 struct ComputedTiming
 {
-  ComputedTiming()
-    : mProgress(kNullProgress)
-    , mCurrentIteration(0)
-    , mPhase(AnimationPhase_Null)
-  { }
-
-  static const double kNullProgress;
-
   // The total duration of the animation including all iterations.
   // Will equal StickyTimeDuration::Forever() if the animation repeats
   // indefinitely.
-  StickyTimeDuration mActiveDuration;
-
+  StickyTimeDuration  mActiveDuration;
   // Progress towards the end of the current iteration. If the effect is
   // being sampled backwards, this will go from 1.0 to 0.0.
-  // Will be kNullProgress if the animation is neither animating nor
+  // Will be null if the animation is neither animating nor
   // filling at the sampled time.
-  double mProgress;
+  Nullable<double>    mProgress;
+  // Zero-based iteration index (meaningless if mProgress is null).
+  uint64_t            mCurrentIteration = 0;
 
-  // Zero-based iteration index (meaningless if mProgress is kNullProgress).
-  uint64_t mCurrentIteration;
-
-  enum {
-    // Not sampled (null sample time)
-    AnimationPhase_Null,
-    // Sampled prior to the start of the active interval
-    AnimationPhase_Before,
-    // Sampled within the active interval
-    AnimationPhase_Active,
-    // Sampled after (or at) the end of the active interval
-    AnimationPhase_After
-  } mPhase;
+  enum class AnimationPhase {
+    Null,   // Not sampled (null sample time)
+    Before, // Sampled prior to the start of the active interval
+    Active, // Sampled within the active interval
+    After   // Sampled after (or at) the end of the active interval
+  };
+  AnimationPhase      mPhase = AnimationPhase::Null;
 };
 
 class ComputedTimingFunction
@@ -270,8 +257,8 @@ public:
   // active duration are calculated. All other members of the returned object
   // are given a null/initial value.
   //
-  // This function returns ComputedTiming::kNullProgress for the mProgress
-  // member of the return value if the animation should not be run
+  // This function returns a null mProgress member of the return value
+  // if the animation should not be run
   // (because it is not currently active and is not filling at this time).
   static ComputedTiming
   GetComputedTimingAt(const Nullable<TimeDuration>& aLocalTime,
@@ -279,8 +266,9 @@ public:
 
   // Shortcut for that gets the computed timing using the current local time as
   // calculated from the timeline time.
-  ComputedTiming GetComputedTiming(const AnimationTiming* aTiming
-                                     = nullptr) const {
+  ComputedTiming
+  GetComputedTiming(const AnimationTiming* aTiming = nullptr) const
+  {
     return GetComputedTimingAt(GetLocalTime(), aTiming ? *aTiming : mTiming);
   }
 

gfx/layers/composite/AsyncCompositionManager.cpp

@@ -589,19 +589,20 @@ SampleAnimations(Layer* aLayer, TimeStamp aPoint)
       dom::KeyframeEffectReadOnly::GetComputedTimingAt(
         Nullable<TimeDuration>(elapsedDuration), timing);
 
-    MOZ_ASSERT(0.0 <= computedTiming.mProgress &&
-               computedTiming.mProgress <= 1.0,
+    MOZ_ASSERT(!computedTiming.mProgress.IsNull() &&
+               0.0 <= computedTiming.mProgress.Value() &&
+               computedTiming.mProgress.Value() <= 1.0,
                "iteration progress should be in [0-1]");
 
     int segmentIndex = 0;
     AnimationSegment* segment = animation.segments().Elements();
-    while (segment->endPortion() < computedTiming.mProgress) {
+    while (segment->endPortion() < computedTiming.mProgress.Value()) {
       ++segment;
       ++segmentIndex;
     }
 
     double positionInSegment =
-      (computedTiming.mProgress - segment->startPortion()) /
+      (computedTiming.mProgress.Value() - segment->startPortion()) /
       (segment->endPortion() - segment->startPortion());
 
     double portion =

layout/style/nsAnimationManager.cpp

@@ -198,7 +198,7 @@ CSSAnimation::QueueEvents()
 
   ComputedTiming computedTiming = mEffect->GetComputedTiming();
 
-  if (computedTiming.mPhase == ComputedTiming::AnimationPhase_Null) {
+  if (computedTiming.mPhase == ComputedTiming::AnimationPhase::Null) {
     return; // do nothing
   }
 
@@ -212,23 +212,23 @@ CSSAnimation::QueueEvents()
   bool wasActive = mPreviousPhaseOrIteration != PREVIOUS_PHASE_BEFORE &&
                    mPreviousPhaseOrIteration != PREVIOUS_PHASE_AFTER;
   bool isActive =
-         computedTiming.mPhase == ComputedTiming::AnimationPhase_Active;
+         computedTiming.mPhase == ComputedTiming::AnimationPhase::Active;
   bool isSameIteration =
          computedTiming.mCurrentIteration == mPreviousPhaseOrIteration;
   bool skippedActivePhase =
     (mPreviousPhaseOrIteration == PREVIOUS_PHASE_BEFORE &&
-     computedTiming.mPhase == ComputedTiming::AnimationPhase_After) ||
+     computedTiming.mPhase == ComputedTiming::AnimationPhase::After) ||
     (mPreviousPhaseOrIteration == PREVIOUS_PHASE_AFTER &&
-     computedTiming.mPhase == ComputedTiming::AnimationPhase_Before);
+     computedTiming.mPhase == ComputedTiming::AnimationPhase::Before);
 
   MOZ_ASSERT(!skippedActivePhase || (!isActive && !wasActive),
              "skippedActivePhase only makes sense if we were & are inactive");
 
-  if (computedTiming.mPhase == ComputedTiming::AnimationPhase_Before) {
+  if (computedTiming.mPhase == ComputedTiming::AnimationPhase::Before) {
     mPreviousPhaseOrIteration = PREVIOUS_PHASE_BEFORE;
-  } else if (computedTiming.mPhase == ComputedTiming::AnimationPhase_Active) {
+  } else if (computedTiming.mPhase == ComputedTiming::AnimationPhase::Active) {
     mPreviousPhaseOrIteration = computedTiming.mCurrentIteration;
-  } else if (computedTiming.mPhase == ComputedTiming::AnimationPhase_After) {
+  } else if (computedTiming.mPhase == ComputedTiming::AnimationPhase::After) {
     mPreviousPhaseOrIteration = PREVIOUS_PHASE_AFTER;
   }
 

layout/style/nsTransitionManager.cpp

@@ -57,14 +57,14 @@ ElementPropertyTransition::CurrentValuePortion() const
   timingToUse.mFillMode = NS_STYLE_ANIMATION_FILL_MODE_BOTH;
   ComputedTiming computedTiming = GetComputedTiming(&timingToUse);
 
-  MOZ_ASSERT(computedTiming.mProgress != ComputedTiming::kNullProgress,
+  MOZ_ASSERT(!computedTiming.mProgress.IsNull(),
              "Got a null progress for a fill mode of 'both'");
   MOZ_ASSERT(mProperties.Length() == 1,
              "Should have one animation property for a transition");
   MOZ_ASSERT(mProperties[0].mSegments.Length() == 1,
              "Animation property should have one segment for a transition");
   return mProperties[0].mSegments[0].mTimingFunction
-         .GetValue(computedTiming.mProgress);
+         .GetValue(computedTiming.mProgress.Value());
 }
 
 ////////////////////////// CSSTransition ////////////////////////////