Bug 1442817 - Don't flush throttled animations on getAnimations(). r?birtles draft
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Fri, 09 Mar 2018 09:38:40 +0900
changeset 765095 3c969da320aa9a04900d56f9882cd8b1c6f3fd49
parent 765094 23719dafbf57297c21c5f691b227678558b8e735
push id101970
push userhikezoe@mozilla.com
push dateFri, 09 Mar 2018 00:58:32 +0000
reviewersbirtles
bugs1442817
milestone60.0a1
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
dom/animation/CSSPseudoElement.cpp
dom/animation/test/mozilla/file_restyles.html
dom/base/Element.cpp
--- 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();