From ef42e66fc8b4773502023b53128544ba21f6ad8f Mon Sep 17 00:00:00 2001
From: Brian Birtles <birtles@gmail.com>
Date: Wed, 4 Mar 2020 00:36:46 +0000
Subject: [PATCH] Bug 1459536 - Allow CSS animation timing properties to be
 overridden using the Web Animations API; r=boris

Differential Revision: https://phabricator.services.mozilla.com/D65098

--HG--
extra : moz-landing-system : lando
---
 dom/animation/AnimationEffect.h               |   3 +-
 dom/animation/TimingParams.h                  |   4 +
 layout/style/nsAnimationManager.cpp           |  53 +++++-
 layout/style/nsAnimationManager.h             |   2 +
 ...nimationEffect-updateTiming.tentative.html | 179 ++++++++++++++++++
 5 files changed, 238 insertions(+), 3 deletions(-)
 create mode 100644 testing/web-platform/tests/css/css-animations/AnimationEffect-updateTiming.tentative.html

diff --git a/dom/animation/AnimationEffect.h b/dom/animation/AnimationEffect.h
index 2c8b18abec71..ec0ba3d7a452 100644
--- a/dom/animation/AnimationEffect.h
+++ b/dom/animation/AnimationEffect.h
@@ -45,7 +45,8 @@ class AnimationEffect : public nsISupports, public nsWrapperCache {
   // AnimationEffect interface
   void GetTiming(EffectTiming& aRetVal) const;
   void GetComputedTimingAsDict(ComputedEffectTiming& aRetVal) const;
-  void UpdateTiming(const OptionalEffectTiming& aTiming, ErrorResult& aRv);
+  virtual void UpdateTiming(const OptionalEffectTiming& aTiming,
+                            ErrorResult& aRv);
 
   const TimingParams& SpecifiedTiming() const { return mTiming; }
   void SetSpecifiedTiming(TimingParams&& aTiming);
diff --git a/dom/animation/TimingParams.h b/dom/animation/TimingParams.h
index 89ff42a1b212..ff9e4f204161 100644
--- a/dom/animation/TimingParams.h
+++ b/dom/animation/TimingParams.h
@@ -154,6 +154,10 @@ struct TimingParams {
     mDuration = std::move(aDuration);
     Update();
   }
+  void SetDuration(const Maybe<StickyTimeDuration>& aDuration) {
+    mDuration = aDuration;
+    Update();
+  }
   const Maybe<StickyTimeDuration>& Duration() const { return mDuration; }
 
   void SetDelay(const TimeDuration& aDelay) {
diff --git a/layout/style/nsAnimationManager.cpp b/layout/style/nsAnimationManager.cpp
index d4a6d9934448..c758d84f238e 100644
--- a/layout/style/nsAnimationManager.cpp
+++ b/layout/style/nsAnimationManager.cpp
@@ -290,6 +290,36 @@ void CSSAnimation::UpdateTiming(SeekFlag aSeekFlag,
 
 /////////////////////// CSSAnimationKeyframeEffect ////////////////////////
 
+void CSSAnimationKeyframeEffect::UpdateTiming(
+    const OptionalEffectTiming& aTiming, ErrorResult& aRv) {
+  KeyframeEffect::UpdateTiming(aTiming, aRv);
+
+  if (aRv.Failed()) {
+    return;
+  }
+
+  if (mAnimation && mAnimation->AsCSSAnimation()) {
+    CSSAnimationProperties updatedProperties = CSSAnimationProperties::None;
+    if (aTiming.mDuration.WasPassed()) {
+      updatedProperties |= CSSAnimationProperties::Duration;
+    }
+    if (aTiming.mIterations.WasPassed()) {
+      updatedProperties |= CSSAnimationProperties::IterationCount;
+    }
+    if (aTiming.mDirection.WasPassed()) {
+      updatedProperties |= CSSAnimationProperties::Direction;
+    }
+    if (aTiming.mDelay.WasPassed()) {
+      updatedProperties |= CSSAnimationProperties::Delay;
+    }
+    if (aTiming.mFill.WasPassed()) {
+      updatedProperties |= CSSAnimationProperties::FillMode;
+    }
+
+    mAnimation->AsCSSAnimation()->AddOverriddenProperties(updatedProperties);
+  }
+}
+
 void CSSAnimationKeyframeEffect::SetKeyframes(JSContext* aContext,
                                               JS::Handle<JSObject*> aKeyframes,
                                               ErrorResult& aRv) {
@@ -403,8 +433,27 @@ static void UpdateOldAnimationPropertiesWithNew(
   // identity (and any expando properties attached to it).
   if (aOld.GetEffect()) {
     dom::AnimationEffect* oldEffect = aOld.GetEffect();
-    animationChanged = oldEffect->SpecifiedTiming() != aNewTiming;
-    oldEffect->SetSpecifiedTiming(std::move(aNewTiming));
+
+    // Copy across the changes that are not overridden
+    TimingParams updatedTiming = oldEffect->SpecifiedTiming();
+    if (~aOverriddenProperties & CSSAnimationProperties::Duration) {
+      updatedTiming.SetDuration(aNewTiming.Duration());
+    }
+    if (~aOverriddenProperties & CSSAnimationProperties::IterationCount) {
+      updatedTiming.SetIterations(aNewTiming.Iterations());
+    }
+    if (~aOverriddenProperties & CSSAnimationProperties::Direction) {
+      updatedTiming.SetDirection(aNewTiming.Direction());
+    }
+    if (~aOverriddenProperties & CSSAnimationProperties::Delay) {
+      updatedTiming.SetDelay(aNewTiming.Delay());
+    }
+    if (~aOverriddenProperties & CSSAnimationProperties::FillMode) {
+      updatedTiming.SetFill(aNewTiming.Fill());
+    }
+
+    animationChanged = oldEffect->SpecifiedTiming() != updatedTiming;
+    oldEffect->SetSpecifiedTiming(std::move(updatedTiming));
 
     KeyframeEffect* oldKeyframeEffect = oldEffect->AsKeyframeEffect();
     if (~aOverriddenProperties & CSSAnimationProperties::Keyframes &&
diff --git a/layout/style/nsAnimationManager.h b/layout/style/nsAnimationManager.h
index dfd7016f709d..9ed5c37c3adc 100644
--- a/layout/style/nsAnimationManager.h
+++ b/layout/style/nsAnimationManager.h
@@ -286,6 +286,8 @@ class CSSAnimationKeyframeEffect : public dom::KeyframeEffect {
       : KeyframeEffect(aDocument, std::move(aTarget), std::move(aTiming),
                        aOptions) {}
 
+  void UpdateTiming(const dom::OptionalEffectTiming& aTiming,
+                    ErrorResult& aRv) override;
   void SetKeyframes(JSContext* aContext, JS::Handle<JSObject*> aKeyframes,
                     ErrorResult& aRv) override;
 };
diff --git a/testing/web-platform/tests/css/css-animations/AnimationEffect-updateTiming.tentative.html b/testing/web-platform/tests/css/css-animations/AnimationEffect-updateTiming.tentative.html
new file mode 100644
index 000000000000..de6953c761fa
--- /dev/null
+++ b/testing/web-platform/tests/css/css-animations/AnimationEffect-updateTiming.tentative.html
@@ -0,0 +1,179 @@
+<!doctype html>
+<meta charset=utf-8>
+<title>AnimationEffect.updateTiming() for CSS animations</title>
+<!-- TODO: Add a more specific link for this once it is specified. -->
+<link rel="help" href="https://drafts.csswg.org/css-animations-2/">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="support/testcommon.js"></script>
+<style>
+@keyframes anim-empty { }
+</style>
+<body>
+<div id="log"></div>
+<script>
+"use strict";
+
+test(t => {
+  const div = addDiv(t);
+  div.style.animation = 'anim-empty 100s';
+
+  const animation = div.getAnimations()[0];
+  animation.effect.updateTiming({ duration: 2 * MS_PER_SEC });
+
+  div.style.animationDuration = '4s';
+  div.style.animationDelay = '6s';
+  getComputedStyle(div).animationDuration;
+
+  assert_equals(
+    animation.effect.getTiming().duration,
+    2 * MS_PER_SEC,
+    'Duration should be the value set by the API'
+  );
+  assert_equals(
+    animation.effect.getTiming().delay,
+    6 * MS_PER_SEC,
+    'Delay should be the value set by style'
+  );
+}, 'AnimationEffect.updateTiming({ duration }) causes changes to the'
+   + ' animation-duration to be ignored');
+
+// The above test covers duration (with delay as a control). The remaining
+// properties are:
+//
+//   iterations - animation-iteration-count
+//   direction - animation-direction
+//   delay - animation-delay
+//   fill - animation-fill-mode
+//
+// Which we test in two batches, overriding two properties each time and using
+// the remaining two properties as control values to check they are NOT
+// overridden.
+
+test(t => {
+  const div = addDiv(t);
+  div.style.animation = 'anim-empty 100s';
+
+  const animation = div.getAnimations()[0];
+  animation.effect.updateTiming({ iterations: 2, direction: 'reverse' });
+
+  div.style.animationIterationCount = '4';
+  div.style.animationDirection = 'alternate';
+  div.style.animationDelay = '6s';
+  div.style.animationFillMode = 'both';
+  getComputedStyle(div).animationIterationCount;
+
+  assert_equals(
+    animation.effect.getTiming().iterations,
+    2,
+    'Iterations should be the value set by the API'
+  );
+  assert_equals(
+    animation.effect.getTiming().direction,
+    'reverse',
+    'Direction should be the value set by the API'
+  );
+  assert_equals(
+    animation.effect.getTiming().delay,
+    6 * MS_PER_SEC,
+    'Delay should be the value set by style'
+  );
+  assert_equals(
+    animation.effect.getTiming().fill,
+    'both',
+    'Fill should be the value set by style'
+  );
+}, 'AnimationEffect.updateTiming({ iterations, direction }) causes changes to'
+   + ' the animation-iteration-count and animation-direction to be ignored');
+
+test(t => {
+  const div = addDiv(t);
+  div.style.animation = 'anim-empty 100s';
+
+  const animation = div.getAnimations()[0];
+  animation.effect.updateTiming({ delay: 2 * MS_PER_SEC, fill: 'both' });
+
+  div.style.animationDelay = '4s';
+  div.style.animationFillMode = 'forwards';
+  div.style.animationIterationCount = '6';
+  div.style.animationDirection = 'reverse';
+  getComputedStyle(div).animationDelay;
+
+  assert_equals(
+    animation.effect.getTiming().delay,
+    2 * MS_PER_SEC,
+    'Delay should be the value set by the API'
+  );
+  assert_equals(
+    animation.effect.getTiming().fill,
+    'both',
+    'Fill should be the value set by the API'
+  );
+  assert_equals(
+    animation.effect.getTiming().iterations,
+    6,
+    'Iterations should be the value set by style'
+  );
+  assert_equals(
+    animation.effect.getTiming().direction,
+    'reverse',
+    'Direction should be the value set by style'
+  );
+}, 'AnimationEffect.updateTiming({ delay, fill }) causes changes to'
+   + ' the animation-delay and animation-fill-mode to be ignored');
+
+test(t => {
+  const div = addDiv(t);
+  div.style.animation = 'anim-empty 100s';
+
+  const animation = div.getAnimations()[0];
+  assert_throws_js(TypeError, () => {
+    animation.effect.updateTiming({ duration: 2 * MS_PER_SEC, iterations: -1 });
+  }, 'Negative iteration count should cause an error to be thrown');
+
+  div.style.animationDuration = '4s';
+  getComputedStyle(div).animationDuration;
+
+  assert_equals(
+    animation.effect.getTiming().duration,
+    4 * MS_PER_SEC,
+    'Duration should be the value set by style'
+  );
+}, 'AnimationEffect.updateTiming() does override to changes from animation-*'
+   + ' properties if there is an error');
+
+test(t => {
+  const div = addDiv(t);
+  div.style.animation = 'anim-empty 100s';
+
+  const animation = div.getAnimations()[0];
+  animation.effect.updateTiming({
+    easing: 'steps(4)',
+    endDelay: 2 * MS_PER_SEC,
+    iterationStart: 4,
+  });
+
+  div.style.animationDuration = '6s';
+  div.style.animationTimingFunction = 'ease-in';
+  getComputedStyle(div).animationDuration;
+
+  assert_equals(
+    animation.effect.getTiming().easing,
+    'steps(4)',
+    'endDelay should be the value set by the API'
+  );
+  assert_equals(
+    animation.effect.getTiming().endDelay,
+    2 * MS_PER_SEC,
+    'endDelay should be the value set by the API'
+  );
+  assert_equals(
+    animation.effect.getTiming().iterationStart,
+    4,
+    'iterationStart should be the value set by style'
+  );
+}, 'AnimationEffect properties that do not map to animation-* properties'
+   + ' should not be changed when animation-* style is updated');
+
+</script>
+</body>