Bug 1619516 - Flush styles for getTiming and getComputedTiming for an…

… effect associated with a CSS animation; r=boris As per [1] we should return the fully-updated result here. [1] https://drafts.csswg.org/css-animations-2/#requirements-on-pending-style-changes Differential Revision: https://phabricator.services.mozilla.com/D65317 --HG-- extra : moz-landing-system : lando
dothq · Mar 4, 2020 · 905f2e1 · 905f2e1
1 parent e50e057
commit 905f2e1
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 13 deletions.
diff --git a/dom/animation/AnimationEffect.h b/dom/animation/AnimationEffect.h
@@ -43,8 +43,8 @@ class AnimationEffect : public nsISupports, public nsWrapperCache {
   }
 
   // AnimationEffect interface
-  void GetTiming(EffectTiming& aRetVal) const;
-  void GetComputedTimingAsDict(ComputedEffectTiming& aRetVal) const;
+  virtual void GetTiming(EffectTiming& aRetVal) const;
+  virtual void GetComputedTimingAsDict(ComputedEffectTiming& aRetVal) const;
   virtual void UpdateTiming(const OptionalEffectTiming& aTiming,
                             ErrorResult& aRv);
 

diff --git a/layout/style/nsAnimationManager.cpp b/layout/style/nsAnimationManager.cpp
@@ -323,6 +323,17 @@ void CSSAnimation::UpdateTiming(SeekFlag aSeekFlag,
 
 /////////////////////// CSSAnimationKeyframeEffect ////////////////////////
 
+void CSSAnimationKeyframeEffect::GetTiming(EffectTiming& aRetVal) const {
+  MaybeFlushUnanimatedStyle();
+  KeyframeEffect::GetTiming(aRetVal);
+}
+
+void CSSAnimationKeyframeEffect::GetComputedTimingAsDict(
+    ComputedEffectTiming& aRetVal) const {
+  MaybeFlushUnanimatedStyle();
+  KeyframeEffect::GetComputedTimingAsDict(aRetVal);
+}
+
 void CSSAnimationKeyframeEffect::UpdateTiming(
     const OptionalEffectTiming& aTiming, ErrorResult& aRv) {
   KeyframeEffect::UpdateTiming(aTiming, aRv);
@@ -331,7 +342,7 @@ void CSSAnimationKeyframeEffect::UpdateTiming(
     return;
   }
 
-  if (mAnimation && mAnimation->AsCSSAnimation()) {
+  if (CSSAnimation* cssAnimation = GetOwningCSSAnimation()) {
     CSSAnimationProperties updatedProperties = CSSAnimationProperties::None;
     if (aTiming.mDuration.WasPassed()) {
       updatedProperties |= CSSAnimationProperties::Duration;
@@ -349,7 +360,7 @@ void CSSAnimationKeyframeEffect::UpdateTiming(
       updatedProperties |= CSSAnimationProperties::FillMode;
     }
 
-    mAnimation->AsCSSAnimation()->AddOverriddenProperties(updatedProperties);
+    cssAnimation->AddOverriddenProperties(updatedProperties);
   }
 }
 
@@ -362,9 +373,19 @@ void CSSAnimationKeyframeEffect::SetKeyframes(JSContext* aContext,
     return;
   }
 
-  if (mAnimation && mAnimation->AsCSSAnimation()) {
-    mAnimation->AsCSSAnimation()->AddOverriddenProperties(
-        CSSAnimationProperties::Keyframes);
+  if (CSSAnimation* cssAnimation = GetOwningCSSAnimation()) {
+    cssAnimation->AddOverriddenProperties(CSSAnimationProperties::Keyframes);
+  }
+}
+
+void CSSAnimationKeyframeEffect::MaybeFlushUnanimatedStyle() const {
+  if (!GetOwningCSSAnimation()) {
+    return;
+  }
+
+  if (dom::Document* doc = GetRenderedDocument()) {
+    doc->FlushPendingNotifications(
+        ChangesToFlush(FlushType::Style, false /* flush animations */));
   }
 }
 

diff --git a/layout/style/nsAnimationManager.h b/layout/style/nsAnimationManager.h
@@ -232,10 +232,24 @@ class CSSAnimationKeyframeEffect : public dom::KeyframeEffect {
       : KeyframeEffect(aDocument, std::move(aTarget), std::move(aTiming),
                        aOptions) {}
 
+  void GetTiming(dom::EffectTiming& aRetVal) const override;
+  void GetComputedTimingAsDict(
+      dom::ComputedEffectTiming& aRetVal) const override;
   void UpdateTiming(const dom::OptionalEffectTiming& aTiming,
                     ErrorResult& aRv) override;
   void SetKeyframes(JSContext* aContext, JS::Handle<JSObject*> aKeyframes,
                     ErrorResult& aRv) override;
+
+ private:
+  dom::CSSAnimation* GetOwningCSSAnimation() {
+    return mAnimation ? mAnimation->AsCSSAnimation() : nullptr;
+  }
+  const dom::CSSAnimation* GetOwningCSSAnimation() const {
+    return mAnimation ? mAnimation->AsCSSAnimation() : nullptr;
+  }
+
+  // Flushes styles if our owning animation is a CSSAnimation
+  void MaybeFlushUnanimatedStyle() const;
 };
 
 template <>

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
@@ -23,7 +23,6 @@
 
   div.style.animationDuration = '4s';
   div.style.animationDelay = '6s';
-  getComputedStyle(div).animationDuration;
 
   assert_equals(
     animation.effect.getTiming().duration,
@@ -61,7 +60,6 @@
   div.style.animationDirection = 'alternate';
   div.style.animationDelay = '6s';
   div.style.animationFillMode = 'both';
-  getComputedStyle(div).animationIterationCount;
 
   assert_equals(
     animation.effect.getTiming().iterations,
@@ -97,7 +95,6 @@
   div.style.animationFillMode = 'forwards';
   div.style.animationIterationCount = '6';
   div.style.animationDirection = 'reverse';
-  getComputedStyle(div).animationDelay;
 
   assert_equals(
     animation.effect.getTiming().delay,
@@ -132,7 +129,6 @@
   }, 'Negative iteration count should cause an error to be thrown');
 
   div.style.animationDuration = '4s';
-  getComputedStyle(div).animationDuration;
 
   assert_equals(
     animation.effect.getTiming().duration,
@@ -155,7 +151,6 @@
 
   div.style.animationDuration = '6s';
   div.style.animationTimingFunction = 'ease-in';
-  getComputedStyle(div).animationDuration;
 
   assert_equals(
     animation.effect.getTiming().easing,

diff --git a/testing/web-platform/tests/css/css-animations/CSSAnimation-effect.tentative.html b/testing/web-platform/tests/css/css-animations/CSSAnimation-effect.tentative.html
@@ -158,7 +158,6 @@
   div.style.animationDelay = '8s';
   div.style.animationFillMode = 'both';
   div.style.animationPlayState = 'paused';
-  getComputedStyle(div).animationDuration;
 
   // Update the keyframes
   keyframesRule.deleteRule(0);