From 905f2e1beb7db232619bf7f7bff006f5b4b38e03 Mon Sep 17 00:00:00 2001 From: Brian Birtles Date: Wed, 4 Mar 2020 19:02:47 +0000 Subject: [PATCH] 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 --- dom/animation/AnimationEffect.h | 4 +-- layout/style/nsAnimationManager.cpp | 31 ++++++++++++++++--- layout/style/nsAnimationManager.h | 14 +++++++++ ...nimationEffect-updateTiming.tentative.html | 5 --- .../CSSAnimation-effect.tentative.html | 1 - 5 files changed, 42 insertions(+), 13 deletions(-) diff --git a/dom/animation/AnimationEffect.h b/dom/animation/AnimationEffect.h index ec0ba3d7a4521..67d5140c2b037 100644 --- 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 index 1846c08cd786b..68c63047d1a31 100644 --- 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 index 05e334b58f4aa..5c59f17a219c2 100644 --- 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 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 index de6953c761fac..e6556dac4bde9 100644 --- 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 index 95a9041872042..5e2d18b5bf965 100644 --- 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);