Bug 1442817 - Don't flush throttled animations on getAnimations(). r?birtles
If the element has throttled animations that will be affected by pending style
changes, the throttled animations will be updated though sequential tasks after
normal styling process. E.g. UpdateEffectProperties, UpdateCascadeResults etc.
So we don't need to flush them in advance of the sequential tasks.
The first test case in this patch fails without this fix.
MozReview-Commit-ID: GystAqK2bQE
--- a/dom/animation/CSSPseudoElement.cpp
+++ b/dom/animation/CSSPseudoElement.cpp
@@ -51,17 +51,22 @@ CSSPseudoElement::WrapObject(JSContext*
}
void
CSSPseudoElement::GetAnimations(const AnimationFilter& filter,
nsTArray<RefPtr<Animation>>& aRetVal)
{
nsIDocument* doc = mParentElement->GetComposedDoc();
if (doc) {
- doc->FlushPendingNotifications(FlushType::Style);
+ // We don't need to explicitly flush throttled animations here, since
+ // updating the animation style of (pseudo-)elements will never affect the
+ // set of running animations and it's only the set of running animations
+ // that is important here.
+ doc->FlushPendingNotifications(
+ ChangesToFlush(FlushType::Style, false /* flush animations */));
}
Element::GetAnimationsUnsorted(mParentElement, mPseudoType, aRetVal);
aRetVal.Sort(AnimationPtrComparator<RefPtr<Animation>>());
}
already_AddRefed<Animation>
CSSPseudoElement::Animate(
--- a/dom/animation/test/mozilla/file_restyles.html
+++ b/dom/animation/test/mozilla/file_restyles.html
@@ -1680,12 +1680,54 @@ waitForAllPaints(() => {
is(markers.length, 0,
'No-overflow transform animations running on the compositor should ' +
'never update style on the main thread');
await ensureElementRemoval(parentElement);
}
);
+ add_task(async function no_flush_on_getAnimations() {
+ var div = addDiv(null);
+ var animation =
+ div.animate({ opacity: [ '0', '1' ] }, 100 * MS_PER_SEC);
+ await animation.ready;
+
+ ok(SpecialPowers.wrap(animation).isRunningOnCompositor);
+
+ var markers = observeAnimSyncStyling(() => {
+ is(div.getAnimations().length, 1, 'There should be one animation');
+ });
+ is(markers.length, 0,
+ 'Element.getAnimations() should not flush throttled animation style');
+
+ await ensureElementRemoval(div);
+ });
+
+ add_task(async function restyling_for_throttled_animation_on_getAnimations() {
+ var div = addDiv(null, { style: 'animation: opacity 100s' });
+ var animation = div.getAnimations()[0];
+
+ await animation.ready;
+ ok(SpecialPowers.wrap(animation).isRunningOnCompositor);
+
+ var markers = observeAnimSyncStyling(() => {
+ div.style.animationDuration = '0s';
+ is(div.getAnimations().length, 0, 'There should be no animation');
+ });
+
+ if (isServo) {
+ is(markers.length, 1, // For discarding the throttled animation.
+ 'Element.getAnimations() should flush throttled animation style so ' +
+ 'that the throttled animation is discarded');
+ } else {
+ is(markers.length, 2, // One is done through UpdateOnlyAnimationStyles(),
+ // the other is for discarding the animation.
+ 'Element.getAnimations() should flush throttled animation style that ' +
+ 'the throttled animation is discarded');
+ }
+
+ await ensureElementRemoval(div);
+ });
});
</script>
</body>
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -3817,17 +3817,22 @@ Element::Animate(const Nullable<ElementO
}
void
Element::GetAnimations(const AnimationFilter& filter,
nsTArray<RefPtr<Animation>>& aAnimations)
{
nsIDocument* doc = GetComposedDoc();
if (doc) {
- doc->FlushPendingNotifications(FlushType::Style);
+ // We don't need to explicitly flush throttled animations here, since
+ // updating the animation style of elements will never affect the set of
+ // running animations and it's only the set of running animations that is
+ // important here.
+ doc->FlushPendingNotifications(
+ ChangesToFlush(FlushType::Style, false /* flush animations */));
}
Element* elem = this;
CSSPseudoElementType pseudoType = CSSPseudoElementType::NotPseudo;
// For animations on generated-content elements, the animations are stored
// on the parent element.
if (IsGeneratedContentContainerForBefore()) {
elem = GetParentElement();