Bug 1430884 - Throttle transform animations without %0 or 100% keyframe. r?birtles draft
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Fri, 22 Jun 2018 14:39:16 +0900
changeset 809463 0bd3d1928c6a4dc4d2d14be5211fc7daaf7c96e4
parent 809462 f3188463cecce67b004b24192b8794c13ff528b4
push id113685
push userhikezoe@mozilla.com
push dateFri, 22 Jun 2018 05:39:34 +0000
reviewersbirtles
bugs1430884
milestone62.0a1
Bug 1430884 - Throttle transform animations without %0 or 100% keyframe. r?birtles MozReview-Commit-ID: 3vLAlSkLz97
dom/animation/KeyframeEffect.cpp
dom/animation/test/mozilla/file_restyles.html
layout/style/nsStyleStruct.cpp
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -1612,19 +1612,33 @@ KeyframeEffect::CalculateCumulativeChang
     }
 
     for (const AnimationPropertySegment& segment : property.mSegments) {
       // In case composite operation is not 'replace' or value is null,
       // we can't throttle animations which will not cause any layout changes
       // on invisible elements because we can't calculate the change hint for
       // such properties until we compose it.
       if (!segment.HasReplaceableValues()) {
-        mCumulativeChangeHint = ~nsChangeHint_Hints_CanIgnoreIfNotVisible;
-        return;
+        if (property.mProperty != eCSSProperty_transform) {
+          mCumulativeChangeHint = ~nsChangeHint_Hints_CanIgnoreIfNotVisible;
+          return;
+        }
+        // In the case the property is transform, we want to forcibly optimize
+        // it even if we can't precompute the cumulative change hints since it's
+        // the second common property for animations, so we suppose the worst
+        // case here, i.e. all possible change hints happen.
+        mCumulativeChangeHint |= nsChangeHint_AddOrRemoveTransform |
+                                 nsChangeHint_RepaintFrame |
+                                 nsChangeHint_UpdateContainingBlock |
+                                 nsChangeHint_UpdateOverflow |
+                                 nsChangeHint_UpdatePostTransformOverflow |
+                                 nsChangeHint_UpdateTransformLayer;
+        continue;
       }
+
       RefPtr<ComputedStyle> fromContext =
         CreateComputedStyleForAnimationValue(property.mProperty,
                                              segment.mFromValue,
                                              presContext,
                                              aComputedStyle);
       if (!fromContext) {
         mCumulativeChangeHint = ~nsChangeHint_Hints_CanIgnoreIfNotVisible;
         return;
--- a/dom/animation/test/mozilla/file_restyles.html
+++ b/dom/animation/test/mozilla/file_restyles.html
@@ -1764,12 +1764,31 @@ waitForAllPaints(() => {
 
     const markers = await observeStyling(5);
 
     is(markers.length, 0,
        'Transform animations on visibility hidden element should be throttled');
     await ensureElementRemoval(div);
   });
 
+  add_task(async function restyling_transform_aimations_on_invisible_element() {
+    // 'opacity: 0' prevents transform animations to be sent to the compositor.
+    const div = addDiv(null, { style: 'visibility: hidden; opacity: 0' });
+
+    const animation =
+      div.animate([ { transform: 'rotate(360deg)' } ],
+                  { duration: 100 * MS_PER_SEC,
+                    iterations: Infinity });
+
+    await waitForAnimationReadyToRestyle(animation);
+
+    const markers = await observeStyling(5);
+
+    is(markers.length, 0,
+       'Transform animations without 100% keyframe on visibility hidden ' +
+       'element should be throttled');
+    await ensureElementRemoval(div);
+  });
+
 });
 
 </script>
 </body>
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -3813,16 +3813,20 @@ nsStyleDisplay::CalcDifference(const nsS
 
   /* If we've added or removed the transform property, we need to reconstruct the frame to add
    * or remove the view object, and also to handle abs-pos and fixed-pos containers.
    */
   if (HasTransformStyle() != aNewData.HasTransformStyle()) {
     // We do not need to apply nsChangeHint_UpdateTransformLayer since
     // nsChangeHint_RepaintFrame will forcibly invalidate the frame area and
     // ensure layers are rebuilt (or removed).
+    //
+    // XXX: If we add a new change hint for transform changes here or in
+    // CompareTransformValues(), we have to modify
+    // KeyframeEffect::CalculateCumulativeChangeHint too!
     hint |= nsChangeHint_UpdateContainingBlock |
             nsChangeHint_AddOrRemoveTransform |
             nsChangeHint_UpdateOverflow |
             nsChangeHint_RepaintFrame;
   } else {
     /* Otherwise, if we've kept the property lying around and we already had a
      * transform, we need to see whether or not we've changed the transform.
      * If so, we need to recompute its overflow rect (which probably changed