Bug 1237173 - Part2: Change type of duration to Maybe<StickyTimeDuration>. r=birtles

dom/animation/AnimationEffectTiming.cpp

@@ -41,25 +41,18 @@ AnimationEffectTiming::SetEndDelay(double aEndDelay)
 void
 AnimationEffectTiming::SetDuration(const UnrestrictedDoubleOrString& aDuration)
 {
-  if (mTiming.mDuration.IsUnrestrictedDouble() &&
-      aDuration.IsUnrestrictedDouble() &&
-      mTiming.mDuration.GetAsUnrestrictedDouble() ==
-        aDuration.GetAsUnrestrictedDouble()) {
-    return;
-  }
-
-  if (mTiming.mDuration.IsString() && aDuration.IsString() &&
-      mTiming.mDuration.GetAsString() == aDuration.GetAsString()) {
-    return;
-  }
-
+  Maybe<StickyTimeDuration> newDuration;
   if (aDuration.IsUnrestrictedDouble()) {
-    mTiming.mDuration.SetAsUnrestrictedDouble() =
-      aDuration.GetAsUnrestrictedDouble();
-  } else {
-    mTiming.mDuration.SetAsString() = aDuration.GetAsString();
+    newDuration.emplace(StickyTimeDuration::FromMilliseconds(
+                          aDuration.GetAsUnrestrictedDouble()));
   }
 
+  if (mTiming.mDuration == newDuration) {
+    return;
+  }
+
+  mTiming.mDuration = newDuration;
+
   NotifyTimingUpdate();
 }
 

dom/animation/AnimationEffectTimingReadOnly.cpp

@@ -26,6 +26,17 @@ AnimationEffectTimingReadOnly::WrapObject(JSContext* aCx, JS::Handle<JSObject*>
   return AnimationEffectTimingReadOnlyBinding::Wrap(aCx, this, aGivenProto);
 }
 
+void
+AnimationEffectTimingReadOnly::GetDuration(
+    OwningUnrestrictedDoubleOrString& aRetVal) const
+{
+  if (mTiming.mDuration) {
+    aRetVal.SetAsUnrestrictedDouble() = mTiming.mDuration->ToMilliseconds();
+  } else {
+    aRetVal.SetAsString().AssignLiteral("auto");
+  }
+}
+
 void
 AnimationEffectTimingReadOnly::GetEasing(nsString& aRetVal) const
 {

dom/animation/AnimationEffectTimingReadOnly.h

@@ -41,10 +41,7 @@ public:
   FillMode Fill() const { return mTiming.mFill; }
   double IterationStart() const { return mTiming.mIterationStart; }
   double Iterations() const { return mTiming.mIterations; }
-  void GetDuration(OwningUnrestrictedDoubleOrString& aRetVal) const
-  {
-    aRetVal = mTiming.mDuration;
-  }
+  void GetDuration(OwningUnrestrictedDoubleOrString& aRetVal) const;
   PlaybackDirection Direction() const { return mTiming.mDirection; }
   void GetEasing(nsString& aRetVal) const;
 

dom/animation/KeyframeEffect.cpp

@@ -238,12 +238,10 @@ KeyframeEffectReadOnly::GetComputedTimingAt(
   // Always return the same object to benefit from return-value optimization.
   ComputedTiming result;
 
-  if (aTiming.mDuration.IsUnrestrictedDouble()) {
-    double durationMs = aTiming.mDuration.GetAsUnrestrictedDouble();
-    if (!IsNaN(durationMs) && durationMs >= 0.0f) {
-      result.mDuration = StickyTimeDuration::FromMilliseconds(durationMs);
-    }
+  if (aTiming.mDuration && aTiming.mDuration.ref() > zeroDuration) {
+    result.mDuration = aTiming.mDuration.ref();
   }
+
   result.mIterations = IsNaN(aTiming.mIterations) || aTiming.mIterations < 0.0f ?
                        1.0f :
                        aTiming.mIterations;

dom/animation/TimingParams.cpp

@@ -10,20 +10,23 @@ namespace mozilla {
 
 TimingParams::TimingParams(const dom::AnimationEffectTimingProperties& aRhs,
                            const dom::Element* aTarget)
-  : mDuration(aRhs.mDuration)
-  , mDelay(TimeDuration::FromMilliseconds(aRhs.mDelay))
+  : mDelay(TimeDuration::FromMilliseconds(aRhs.mDelay))
   , mEndDelay(TimeDuration::FromMilliseconds(aRhs.mEndDelay))
   , mIterations(aRhs.mIterations)
   , mIterationStart(aRhs.mIterationStart)
   , mDirection(aRhs.mDirection)
   , mFill(aRhs.mFill)
 {
+  if (aRhs.mDuration.IsUnrestrictedDouble()) {
+    mDuration.emplace(StickyTimeDuration::FromMilliseconds(
+                        aRhs.mDuration.GetAsUnrestrictedDouble()));
+  }
   mFunction = AnimationUtils::ParseEasing(aTarget, aRhs.mEasing);
 }
 
 TimingParams::TimingParams(double aDuration)
 {
-  mDuration.SetAsUnrestrictedDouble() = aDuration;
+  mDuration.emplace(StickyTimeDuration::FromMilliseconds(aDuration));
 }
 
 template <class OptionsType>
@@ -95,18 +98,7 @@ TimingParams::FromOptionsUnion(
 bool
 TimingParams::operator==(const TimingParams& aOther) const
 {
-  bool durationEqual;
-  if (mDuration.IsUnrestrictedDouble()) {
-    durationEqual = aOther.mDuration.IsUnrestrictedDouble() &&
-                    (mDuration.GetAsUnrestrictedDouble() ==
-                     aOther.mDuration.GetAsUnrestrictedDouble());
-  } else {
-    // We consider all string values and uninitialized values as meaning "auto".
-    // Since mDuration is either a string or uninitialized, we consider it equal
-    // if aOther.mDuration is also either a string or uninitialized.
-    durationEqual = !aOther.mDuration.IsUnrestrictedDouble();
-  }
-  return durationEqual &&
+  return mDuration == aOther.mDuration &&
          mDelay == aOther.mDelay &&
          mIterations == aOther.mIterations &&
          mIterationStart == aOther.mIterationStart &&

dom/animation/TimingParams.h

@@ -44,9 +44,8 @@ struct TimingParams
     const dom::UnrestrictedDoubleOrKeyframeAnimationOptions& aOptions,
     const Nullable<dom::ElementOrCSSPseudoElement>& aTarget);
 
-  // The unitialized state of mDuration represents "auto".
-  // Bug 1237173: We will replace this with Maybe<TimeDuration>.
-  dom::OwningUnrestrictedDoubleOrString mDuration;
+  // mDuration.isNothing() represents the "auto" value
+  Maybe<StickyTimeDuration> mDuration;
   TimeDuration mDelay;      // Initializes to zero
   TimeDuration mEndDelay;
   double mIterations = 1.0; // Can be NaN, negative, +/-Infinity

gfx/layers/composite/AsyncCompositionManager.cpp

@@ -583,8 +583,7 @@ SampleAnimations(Layer* aLayer, TimeStamp aPoint)
     }
 
     TimingParams timing;
-    timing.mDuration.SetAsUnrestrictedDouble() =
-      animation.duration().ToMilliseconds();
+    timing.mDuration.emplace(animation.duration());
     // Currently animations run on the compositor have their delay factored
     // into their start time, hence the delay is effectively zero.
     timing.mDelay = TimeDuration(0);
@@ -833,7 +832,7 @@ AsyncCompositionManager::ApplyAsyncContentTransformToTree(Layer *aLayer,
   // async transformed for async scrolls of this scroll frame's ancestor
   // scroll frames, not for async scrolls of this scroll frame itself.
   // In the loop below, we iterate over scroll frames from inside to outside.
-  // At each iteration, this array contains the layer's ancestor mask layers 
+  // At each iteration, this array contains the layer's ancestor mask layers
   // of all scroll frames inside the current one.
   nsTArray<Layer*> ancestorMaskLayers;
 

layout/base/nsDisplayList.cpp

@@ -2473,7 +2473,7 @@ nsDisplayBackgroundImage::AppendBackgroundItemsToTop(nsDisplayListBuilder* aBuil
 
   const DisplayItemScrollClip* scrollClip =
     aBuilder->ClipState().GetCurrentInnermostScrollClip();
- 
+
   bool needBlendContainer = false;
 
   // Passing bg == nullptr in this macro will result in one iteration with
@@ -2963,7 +2963,7 @@ static void CheckForBorderItem(nsDisplayItem *aItem, uint32_t& aFlags)
   while (nextItem && nextItem->GetType() == nsDisplayItem::TYPE_BACKGROUND) {
     nextItem = nextItem->GetAbove();
   }
-  if (nextItem && 
+  if (nextItem &&
       nextItem->Frame() == aItem->Frame() &&
       nextItem->GetType() == nsDisplayItem::TYPE_BORDER) {
     aFlags |= nsCSSRendering::PAINTBG_WILL_PAINT_BORDER;
@@ -3608,8 +3608,8 @@ nsDisplayBorder::IsInvisibleInRect(const nsRect& aRect)
 
   return false;
 }
-  
-nsDisplayItemGeometry* 
+
+nsDisplayItemGeometry*
 nsDisplayBorder::AllocateGeometry(nsDisplayListBuilder* aBuilder)
 {
   return new nsDisplayBorderGeometry(this, aBuilder);
@@ -3636,7 +3636,7 @@ nsDisplayBorder::ComputeInvalidationRegion(nsDisplayListBuilder* aBuilder,
     aInvalidRegion->Or(*aInvalidRegion, GetBounds(aBuilder, &snap));
   }
 }
-  
+
 void
 nsDisplayBorder::Paint(nsDisplayListBuilder* aBuilder,
                        nsRenderingContext* aCtx) {
@@ -3923,7 +3923,7 @@ nsDisplayWrapList::nsDisplayWrapList(nsDisplayListBuilder* aBuilder,
 
   mList.AppendToTop(aItem);
   UpdateBounds(aBuilder);
-  
+
   if (!aFrame || !aFrame->IsTransformed()) {
     return;
   }
@@ -4459,7 +4459,7 @@ nsDisplayBlendContainer::BuildLayer(nsDisplayListBuilder* aBuilder,
   if (!container) {
     return nullptr;
   }
-  
+
   container->SetForceIsolatedGroup(true);
   return container.forget();
 }
@@ -4560,7 +4560,7 @@ nsDisplaySubDocument::BuildLayer(nsDisplayListBuilder* aBuilder,
   if ((mFlags & GENERATE_SCROLLABLE_LAYER) &&
       rootScrollFrame->GetContent() &&
       nsLayoutUtils::HasCriticalDisplayPort(rootScrollFrame->GetContent())) {
-    params.mInLowPrecisionDisplayPort = true; 
+    params.mInLowPrecisionDisplayPort = true;
   }
 
   RefPtr<Layer> layer = nsDisplayOwnLayer::BuildLayer(aBuilder, aManager, params);
@@ -4986,7 +4986,7 @@ nsDisplayScrollInfoLayer::ComputeFrameMetrics(Layer* aLayer,
   ContainerLayerParameters params = aContainerParameters;
   if (mScrolledFrame->GetContent() &&
       nsLayoutUtils::HasCriticalDisplayPort(mScrolledFrame->GetContent())) {
-    params.mInLowPrecisionDisplayPort = true; 
+    params.mInLowPrecisionDisplayPort = true;
   }
 
   nsRect viewport = mScrollFrame->GetRect() -
@@ -5443,7 +5443,7 @@ nsDisplayTransform::GetResultingTransformMatrix(const FrameTransformProperties&
   return GetResultingTransformMatrixInternal(aProperties, aOrigin, aAppUnitsPerPixel,
                                              aFlags, aBoundsOverride, aOutAncestor);
 }
- 
+
 Matrix4x4
 nsDisplayTransform::GetResultingTransformMatrix(const nsIFrame* aFrame,
                                                 const nsPoint& aOrigin,

layout/style/nsAnimationManager.cpp

@@ -579,7 +579,8 @@ private:
   {
     TimingParams timing;
 
-    timing.mDuration.SetAsUnrestrictedDouble() = aStyleAnimation.GetDuration();
+    timing.mDuration.emplace(StickyTimeDuration::FromMilliseconds(
+			       aStyleAnimation.GetDuration()));
     timing.mDelay = TimeDuration::FromMilliseconds(aStyleAnimation.GetDelay());
     timing.mIterations = aStyleAnimation.GetIterationCount();
     timing.mDirection = aStyleAnimation.GetDirection();

layout/style/nsTransitionManager.cpp

@@ -656,7 +656,7 @@ nsTransitionManager::ConsiderStartingTransition(
   }
 
   TimingParams timing;
-  timing.mDuration.SetAsUnrestrictedDouble() = duration;
+  timing.mDuration.emplace(StickyTimeDuration::FromMilliseconds(duration));
   timing.mDelay = TimeDuration::FromMilliseconds(delay);
   timing.mIterations = 1.0;
   timing.mDirection = dom::PlaybackDirection::Normal;

testing/web-platform/tests/web-animations/animatable/animate.html

@@ -140,6 +140,5 @@ test(function(t) {
                 'The returned Animation targets to the correct object');
 }, 'CSSPseudoElement.animate() creates an Animation object targeting ' +
    'to the correct CSSPseudoElement object');
-
 </script>
 </body>

testing/web-platform/tests/web-animations/keyframe-effect/constructor.html

@@ -501,7 +501,7 @@ var gKeyframeEffectOptionTests = [
     expected: { duration: NaN } },
   { desc:     "a negative value",
     input:    -1,
-    expected: { duration: -1 } },
+    expected: { duration: "auto" } },
   { desc:     "an Infinity duration",
     input:    { duration: Infinity },
     expected: { duration: Infinity } },
@@ -513,7 +513,7 @@ var gKeyframeEffectOptionTests = [
     expected: { duration: NaN } },
   { desc:     "a negative duration",
     input:    { duration: -1 },
-    expected: { duration: -1 } },
+    expected: { duration: "auto" } },
   { desc:     "a string duration",
     input:    { duration: "merrychristmas" },
     expected: { duration: "merrychristmas" } },