diff --git a/dom/animation/Animation.cpp b/dom/animation/Animation.cpp index 08edeaf9a153..90df6d7704a2 100644 --- a/dom/animation/Animation.cpp +++ b/dom/animation/Animation.cpp @@ -846,7 +846,8 @@ bool Animation::ShouldBeSynchronizedWithMainThread( void Animation::UpdateRelevance() { bool wasRelevant = mIsRelevant; - mIsRelevant = HasCurrentEffect() || IsInEffect(); + mIsRelevant = mReplaceState != AnimationReplaceState::Removed && + (HasCurrentEffect() || IsInEffect()); // Notify animation observers. if (wasRelevant && !mIsRelevant) { @@ -961,6 +962,9 @@ void Animation::Remove() { mReplaceState = AnimationReplaceState::Removed; + UpdateEffect(PostRestyleMode::IfNeeded); + PostUpdate(); + QueuePlaybackEvent(NS_LITERAL_STRING("remove"), GetTimelineCurrentTimeAsTimeStamp()); } diff --git a/dom/animation/KeyframeEffect.cpp b/dom/animation/KeyframeEffect.cpp index 4900d75e3338..f1d57e6725a9 100644 --- a/dom/animation/KeyframeEffect.cpp +++ b/dom/animation/KeyframeEffect.cpp @@ -317,7 +317,7 @@ nsCSSPropertyIDSet KeyframeEffect::GetPropertiesForCompositor( nsCSSPropertyIDSet properties; - if (!IsInEffect() && !IsCurrent()) { + if (!mAnimation || !mAnimation->IsRelevant()) { return properties; } @@ -796,7 +796,9 @@ void KeyframeEffect::UpdateTargetRegistration() { // something calls Animation::UpdateRelevance. Whenever our timing changes, // we should be notifying our Animation before calling this, so // Animation::IsRelevant() should be up-to-date by the time we get here. - MOZ_ASSERT(isRelevant == IsCurrent() || IsInEffect(), + MOZ_ASSERT(isRelevant == + ((IsCurrent() || IsInEffect()) && mAnimation && + mAnimation->ReplaceState() != AnimationReplaceState::Removed), "Out of date Animation::IsRelevant value"); if (isRelevant && !mInEffectSet) { @@ -1736,6 +1738,11 @@ bool KeyframeEffect::ContainsAnimatedScale(const nsIFrame* aFrame) const { return false; } + if (!mAnimation || + mAnimation->ReplaceState() == AnimationReplaceState::Removed) { + return false; + } + for (const AnimationProperty& prop : mProperties) { if (prop.mProperty != eCSSProperty_transform && prop.mProperty != eCSSProperty_scale && diff --git a/dom/animation/test/chrome.ini b/dom/animation/test/chrome.ini index ea5385db2662..9e7cf404a953 100644 --- a/dom/animation/test/chrome.ini +++ b/dom/animation/test/chrome.ini @@ -1,5 +1,6 @@ [DEFAULT] prefs = + dom.animations-api.autoremove.enabled=true dom.animations-api.compositing.enabled=true gfx.omta.background-color=true layout.css.individual-transform.enabled=true diff --git a/dom/animation/test/chrome/test_animation_observers_async.html b/dom/animation/test/chrome/test_animation_observers_async.html index bb1fe2833acd..baa310485bde 100644 --- a/dom/animation/test/chrome/test_animation_observers_async.html +++ b/dom/animation/test/chrome/test_animation_observers_async.html @@ -578,5 +578,60 @@ promise_test(t => { }); }, "tree_ordering: subtree"); +// Test that animations removed by auto-removal trigger an event +promise_test(async t => { + setupAsynchronousObserver(t, { observe: div, subtree: false }); + + // Start two animations such that one will be auto-removed + const animA = div.animate( + { opacity: 1 }, + { duration: 100 * MS_PER_SEC, fill: 'forwards' } + ); + const animB = div.animate( + { opacity: 1 }, + { duration: 100 * MS_PER_SEC, fill: 'forwards' } + ); + + // Wait for the MutationRecords corresponding to each addition. + await waitForNextFrame(); + + assert_records( + [ + { added: [animA], changed: [], removed: [] }, + { added: [animB], changed: [], removed: [] }, + ], + 'records after animation start' + ); + + // Finish the animations -- this should cause animA to be replaced, and + // automatically removed. + animA.finish(); + animB.finish(); + + // Wait for the MutationRecords corresponding to the timing changes and the + // subsequent removal to be delivered. + await waitForNextFrame(); + + assert_records( + [ + { added: [], changed: [animA], removed: [] }, + { added: [], changed: [animB], removed: [] }, + { added: [], changed: [], removed: [animA] }, + ], + 'records after finishing' + ); + + // Tidy up + animA.cancel(); + animB.cancel(); + + await waitForNextFrame(); + + assert_records( + [{ added: [], changed: [], removed: [animB] }], + 'records after tidying up end' + ); +}, 'Animations automatically removed are reported'); + runTest(); diff --git a/dom/animation/test/mozilla/file_disable_animations_api_autoremove.html b/dom/animation/test/mozilla/file_disable_animations_api_autoremove.html index 61605c609c1f..79cb50846722 100644 --- a/dom/animation/test/mozilla/file_disable_animations_api_autoremove.html +++ b/dom/animation/test/mozilla/file_disable_animations_api_autoremove.html @@ -32,6 +32,38 @@ promise_test(async t => { await waitForNextFrame(); }, 'Remove events should not be fired if the pref is not set'); +promise_test(async t => { + const div = addDiv(t); + div.style.opacity = '0.1'; + + const animA = div.animate( + { opacity: 0.2 }, + { duration: 1, fill: 'forwards' } + ); + const animB = div.animate( + { opacity: 0.3, composite: 'add' }, + { duration: 1, fill: 'forwards' } + ); + + await animA.finished; + + assert_approx_equals( + parseFloat(getComputedStyle(div).opacity), + 0.5, + 0.0001, + 'Covered animation should still contribute to effect stack when adding' + ); + + animB.cancel(); + + assert_approx_equals( + parseFloat(getComputedStyle(div).opacity), + 0.2, + 0.0001, + 'Covered animation should still contribute to animated style when replacing' + ); +}, 'Covered animations should still affect style if the pref is not set'); + done(); diff --git a/layout/base/nsLayoutUtils.cpp b/layout/base/nsLayoutUtils.cpp index 66fcc25e9518..ba6ab24856c3 100644 --- a/layout/base/nsLayoutUtils.cpp +++ b/layout/base/nsLayoutUtils.cpp @@ -225,6 +225,10 @@ static ContentMap& GetContentMap() { template static bool HasMatchingAnimations(EffectSet& aEffects, TestType&& aTest) { for (KeyframeEffect* effect : aEffects) { + if (!effect->GetAnimation() || !effect->GetAnimation()->IsRelevant()) { + continue; + } + if (aTest(*effect, aEffects)) { return true; } @@ -263,8 +267,7 @@ bool nsLayoutUtils::HasAnimationOfPropertySet( return HasMatchingAnimations( aFrame, aPropertySet, [&aPropertySet](KeyframeEffect& aEffect, const EffectSet&) { - return (aEffect.IsInEffect() || aEffect.IsCurrent()) && - aEffect.HasAnimationOfPropertySet(aPropertySet); + return aEffect.HasAnimationOfPropertySet(aPropertySet); }); } @@ -294,8 +297,7 @@ bool nsLayoutUtils::HasAnimationOfPropertySet( return HasMatchingAnimations( *aEffectSet, [&aPropertySet](KeyframeEffect& aEffect, const EffectSet& aEffectSet) { - return (aEffect.IsInEffect() || aEffect.IsCurrent()) && - aEffect.HasAnimationOfPropertySet(aPropertySet); + return aEffect.HasAnimationOfPropertySet(aPropertySet); }); } @@ -305,8 +307,7 @@ bool nsLayoutUtils::HasEffectiveAnimation( return HasMatchingAnimations( aFrame, aPropertySet, [&aPropertySet](KeyframeEffect& aEffect, const EffectSet& aEffectSet) { - return (aEffect.IsInEffect() || aEffect.IsCurrent()) && - aEffect.HasEffectiveAnimationOfPropertySet(aPropertySet, + return aEffect.HasEffectiveAnimationOfPropertySet(aPropertySet, aEffectSet); }); } diff --git a/testing/web-platform/tests/web-animations/animation-model/keyframe-effects/effect-value-replaced-animations.html b/testing/web-platform/tests/web-animations/animation-model/keyframe-effects/effect-value-replaced-animations.html new file mode 100644 index 000000000000..0defd8f19a21 --- /dev/null +++ b/testing/web-platform/tests/web-animations/animation-model/keyframe-effects/effect-value-replaced-animations.html @@ -0,0 +1,78 @@ + + +The effect value of a keyframe effect: Overlapping keyframes + + + + + +
+ + diff --git a/testing/web-platform/tests/web-animations/interfaces/Animatable/getAnimations.html b/testing/web-platform/tests/web-animations/interfaces/Animatable/getAnimations.html index 8b563848561b..b52b14856f1f 100644 --- a/testing/web-platform/tests/web-animations/interfaces/Animatable/getAnimations.html +++ b/testing/web-platform/tests/web-animations/interfaces/Animatable/getAnimations.html @@ -209,6 +209,16 @@ test(t => { }, 'Returns animations based on dynamic changes to individual' + ' animations\' current time'); +promise_test(async t => { + const div = createDiv(t); + + const animA = div.animate({ opacity: 1 }, { duration: 1, fill: 'forwards' }); + const animB = div.animate({ opacity: 1 }, { duration: 1, fill: 'forwards' }); + await animA.finished; + + assert_array_equals(div.getAnimations(), [animB]); +}, 'Does not return an animation that has been removed'); + promise_test(async t => { const div = createDiv(t); const watcher = EventWatcher(t, div, 'transitionrun');