Bug 1244638 - Part 2: Throw TypeError if iterationStart is NaN, negative value or Infinity. r=birtles, r=smaug
authorDaisuke Akatsuka <daisuke@mozilla-japan.org>
Fri, 11 Mar 2016 18:04:39 +0900
changeset 288497 729fd971cfa66054610a55acef9d8101c33f0b67
parent 288496 a25e46c8d9c598f3232ea676a0deb78ac6411f2d
child 288498 0a78c483152b5fbe3514261880eaa25893013f6d
push id73429
push userbbirtles@mozilla.com
push dateSun, 13 Mar 2016 13:44:35 +0000
treeherdermozilla-inbound@7a82b7529ca5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles, smaug
bugs1244638
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1244638 - Part 2: Throw TypeError if iterationStart is NaN, negative value or Infinity. r=birtles, r=smaug MozReview-Commit-ID: A8zSm6TgwOP
dom/animation/AnimationEffectTiming.cpp
dom/animation/AnimationEffectTiming.h
dom/animation/TimingParams.cpp
dom/animation/TimingParams.h
dom/webidl/AnimationEffectTiming.webidl
testing/web-platform/tests/web-animations/animation-effect-timing/iterationStart.html
--- a/dom/animation/AnimationEffectTiming.cpp
+++ b/dom/animation/AnimationEffectTiming.cpp
@@ -35,25 +35,31 @@ AnimationEffectTiming::SetEndDelay(doubl
     return;
   }
   mTiming.mEndDelay = endDelay;
 
   NotifyTimingUpdate();
 }
 
 void
-AnimationEffectTiming::SetIterationStart(double aIterationStart)
+AnimationEffectTiming::SetIterationStart(double aIterationStart,
+                                         ErrorResult& aRv)
 {
   if (mTiming.mIterationStart == aIterationStart) {
     return;
   }
+
+  TimingParams::ValidateIterationStart(aIterationStart, aRv);
+  if (aRv.Failed()) {
+    return;
+  }
+
   mTiming.mIterationStart = aIterationStart;
-  if (mEffect) {
-    mEffect->NotifySpecifiedTimingUpdated();
-  }
+
+  NotifyTimingUpdate();
 }
 
 void
 AnimationEffectTiming::SetDuration(const UnrestrictedDoubleOrString& aDuration,
                                    ErrorResult& aRv)
 {
   Maybe<StickyTimeDuration> newDuration =
     TimingParams::ParseDuration(aDuration, aRv);
--- a/dom/animation/AnimationEffectTiming.h
+++ b/dom/animation/AnimationEffectTiming.h
@@ -20,17 +20,17 @@ public:
     : AnimationEffectTimingReadOnly(aTiming)
     , mEffect(aEffect) { }
 
   JSObject* WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
 
   void Unlink() override { mEffect = nullptr; }
 
   void SetEndDelay(double aEndDelay);
-  void SetIterationStart(double aIterationStart);
+  void SetIterationStart(double aIterationStart, ErrorResult& aRv);
   void SetDuration(const UnrestrictedDoubleOrString& aDuration,
                    ErrorResult& aRv);
 
 private:
   void NotifyTimingUpdate();
   KeyframeEffect* MOZ_NON_OWNING_REF mEffect;
 };
 
--- a/dom/animation/TimingParams.cpp
+++ b/dom/animation/TimingParams.cpp
@@ -59,21 +59,27 @@ TimingParamsFromOptionsUnion(
       if (target.IsElement()) {
         targetElement = &target.GetAsElement();
       } else {
         targetElement = target.GetAsCSSPseudoElement().ParentElement();
       }
     }
     const dom::AnimationEffectTimingProperties& timing =
       GetTimingProperties(aOptions);
+
     Maybe<StickyTimeDuration> duration =
       TimingParams::ParseDuration(timing.mDuration, aRv);
     if (aRv.Failed()) {
       return result;
     }
+    TimingParams::ValidateIterationStart(timing.mIterationStart, aRv);
+    if (aRv.Failed()) {
+      return result;
+    }
+
     result.mDuration = duration;
     result.mDelay = TimeDuration::FromMilliseconds(timing.mDelay);
     result.mEndDelay = TimeDuration::FromMilliseconds(timing.mEndDelay);
     result.mIterations = timing.mIterations;
     result.mIterationStart = timing.mIterationStart;
     result.mDirection = timing.mDirection;
     result.mFill = timing.mFill;
     result.mFunction =
--- a/dom/animation/TimingParams.h
+++ b/dom/animation/TimingParams.h
@@ -44,31 +44,40 @@ struct TimingParams
     ErrorResult& aRv);
 
   // Range-checks and validates an UnrestrictedDoubleOrString or
   // OwningUnrestrictedDoubleOrString object and converts to a
   // StickyTimeDuration value or Nothing() if aDuration is "auto".
   // Caller must check aRv.Failed().
   template <class DoubleOrString>
   static Maybe<StickyTimeDuration> ParseDuration(DoubleOrString& aDuration,
-                                                 ErrorResult& aRv) {
+                                                 ErrorResult& aRv)
+  {
     Maybe<StickyTimeDuration> result;
     if (aDuration.IsUnrestrictedDouble()) {
       double durationInMs = aDuration.GetAsUnrestrictedDouble();
       if (durationInMs >= 0) {
         result.emplace(StickyTimeDuration::FromMilliseconds(durationInMs));
         return result;
       }
     } else if (aDuration.GetAsString().EqualsLiteral("auto")) {
       return result;
     }
     aRv.Throw(NS_ERROR_DOM_TYPE_ERR);
     return result;
   }
 
+  static void ValidateIterationStart(double aIterationStart,
+                                     ErrorResult& aRv)
+  {
+    if (aIterationStart < 0) {
+      aRv.Throw(NS_ERROR_DOM_TYPE_ERR);
+    }
+  }
+
   // 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
   double mIterationStart = 0.0;
   dom::PlaybackDirection mDirection = dom::PlaybackDirection::Normal;
   dom::FillMode mFill = dom::FillMode::Auto;
--- a/dom/webidl/AnimationEffectTiming.webidl
+++ b/dom/webidl/AnimationEffectTiming.webidl
@@ -12,16 +12,17 @@
 
 [Func="nsDocument::IsWebAnimationsEnabled"]
 interface AnimationEffectTiming : AnimationEffectTimingReadOnly {
   //Bug 1244633 - implement AnimationEffectTiming delay
   //inherit attribute double                             delay;
   inherit attribute double                             endDelay;
   //Bug 1244637 - implement AnimationEffectTiming fill
   //inherit attribute FillMode                           fill;
+  [SetterThrows]
   inherit attribute double                             iterationStart;
   //Bug 1244640 - implement AnimationEffectTiming iterations
   //inherit attribute unrestricted double                iterations;
   [SetterThrows]
   inherit attribute (unrestricted double or DOMString) duration;
   //Bug 1244642 - implement AnimationEffectTiming direction
   //inherit attribute PlaybackDirection                  direction;
   //Bug 1244643 - implement AnimationEffectTiming easing
--- a/testing/web-platform/tests/web-animations/animation-effect-timing/iterationStart.html
+++ b/testing/web-platform/tests/web-animations/animation-effect-timing/iterationStart.html
@@ -49,10 +49,25 @@ test(function(t) {
                            duration: 100,
                            delay: 0 });
   anim.finish();
   anim.effect.timing.iterationStart = 2.5;
   assert_equals(anim.effect.getComputedTiming().progress, 0.5);
   assert_equals(anim.effect.getComputedTiming().currentIteration, 3);
 }, 'Test that changing the iterationStart affects computed timing ' +
    'when forwards-filling');
+
+test(function(t) {
+  var div = createDiv(t);
+  var anim = div.animate({ opacity: [ 0, 1 ] }, 100);
+  assert_throws({ name: 'TypeError' },
+                function() {
+                  anim.effect.timing.iterationStart = -1;
+                });
+  assert_throws({ name: 'TypeError' },
+                function() {
+                  div.animate({ opacity: [ 0, 1 ] },
+                              { iterationStart: -1 });
+                });
+}, 'Test invalid iterationStart value');
+
 </script>
 </body>