Bug 1419079 - Don't bail out from CalculateCumulativeChangeHint() in the case of opacity property even if there is a missing keyframe or composite operation is not 'replace'. r?birtles draft
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Wed, 17 Jan 2018 11:48:20 +0900
changeset 721323 137a3d7d0b08587d3b8c5df2c8c12db91b96383c
parent 721314 4d2384cbcd11c4f998d1853c451cc9845825da71
child 746297 9996b618e109e20f1653f0909f63d510264797e4
push id95798
push userhikezoe@mozilla.com
push dateWed, 17 Jan 2018 02:49:43 +0000
reviewersbirtles
bugs1419079
milestone59.0a1
Bug 1419079 - Don't bail out from CalculateCumulativeChangeHint() in the case of opacity property even if there is a missing keyframe or composite operation is not 'replace'. r?birtles For opacity property, we only generate nsChangeHint_UpdateUsesOpacity, nsChangeHint_UpdateOpacityLayer and nsChangeHint_RepaintFrame. All of them are included in nsChangeHint_Hints_CanIgnoreIfNotVisible. So we can throttle opacity animations on out-of-view elements whatever underlying opacity value is. MozReview-Commit-ID: FdQJbItAndG
dom/animation/KeyframeEffectReadOnly.cpp
dom/animation/test/mozilla/file_restyles.html
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -1759,16 +1759,25 @@ KeyframeEffectReadOnly::CreateStyleConte
 
 template<typename StyleType>
 void
 KeyframeEffectReadOnly::CalculateCumulativeChangeHint(StyleType* aStyleContext)
 {
   mCumulativeChangeHint = nsChangeHint(0);
 
   for (const AnimationProperty& property : mProperties) {
+    // For opacity property we don't produce any change hints that are not
+    // included in nsChangeHint_Hints_CanIgnoreIfNotVisible so we can throttle
+    // opacity animations regardless of the change they produce.  This
+    // optimization is particularly important since it allows us to throttle
+    // opacity animations with missing 0%/100% keyframes.
+    if (property.mProperty == eCSSProperty_opacity) {
+      continue;
+    }
+
     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;
--- a/dom/animation/test/mozilla/file_restyles.html
+++ b/dom/animation/test/mozilla/file_restyles.html
@@ -20,16 +20,19 @@ SimpleTest.finish = function finish() {
 <script src="/tests/SimpleTest/paint_listener.js"></script>
 <script src="../testcommon.js"></script>
 <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css">
 <style>
 @keyframes opacity {
   from { opacity: 1; }
   to { opacity: 0; }
 }
+@keyframes opacity-without-end-value {
+  from { opacity: 0; }
+}
 @keyframes background-color {
   from { background-color: red; }
   to { background-color: blue; }
 }
 @keyframes rotate {
   from { transform: rotate(0deg); }
   to { transform: rotate(360deg); }
 }
@@ -363,16 +366,37 @@ waitForAllPaints(() => {
     is(markers.length, 0,
        'Animations running on the compositor for elements ' +
        'which are scrolled out should never cause restyles');
 
     await ensureElementRemoval(parentElement);
   });
 
   add_task(
+    async function no_restyling_missing_keyframe_opacity_animations_on_scrolled_out_element() {
+      var parentElement = addDiv(null,
+        { style: 'overflow-y: scroll; height: 20px;' });
+      var div = addDiv(null,
+        { style: 'animation: opacity-without-end-value 100s; ' +
+                 'position: relative; top: 100px;' });
+      parentElement.appendChild(div);
+      var animation = div.getAnimations()[0];
+      await animation.ready;
+
+      var markers = await observeStyling(5);
+
+      is(markers.length, 0,
+         'Opacity animations on scrolled out elements should never cause ' +
+         'restyles even if the animation has missing keyframes');
+
+      await ensureElementRemoval(parentElement);
+    }
+  );
+
+  add_task(
     async function restyling_transform_animations_in_scrolled_out_element() {
       if (hasConformantPromiseHandling) {
         return;
       }
 
       // Skip this test on Android since this test have been failing
       // intermittently.
       // Bug 1413817: We should audit this test still fails once we have the
@@ -986,16 +1010,37 @@ waitForAllPaints(() => {
     var markers = await observeStyling(5);
 
     is(markers.length, expectedRestyleCount,
        'Discrete animation has has no keyframe whose offset is 0 or 1 in an ' +
        'out-of-view element should not be throttled');
     await ensureElementRemoval(div);
   });
 
+  // Tests that missing keyframes animation on scrolled out element that the
+  // animation is not able to be throttled.
+  add_task(
+    async function no_throttling_missing_keyframe_animations_out_of_view_element() {
+      var div =
+        addDiv(null, { style: 'transform: translateY(-400px);' +
+                              'visibility: collapse;' });
+      var animation =
+        div.animate([{ visibility: 'visible' }], 100 * MS_PER_SEC);
+      await animation.ready;
+
+      const expectedRestyleCount = tweakExpectedRestyleCount(animation, 5);
+      var markers = await observeStyling(5);
+      is(markers.length, expectedRestyleCount,
+         'visibility animation has no keyframe whose offset is 0 or 1 in an ' +
+         'out-of-view element and produces change hint other than paint-only ' +
+         'change hint should not be throttled');
+      await ensureElementRemoval(div);
+    }
+  );
+
   // Counter part of the above test.
   add_task(async function no_restyling_discrete_animations_out_of_view_element() {
     var div = addDiv(null, { style: 'transform: translateY(-400px);' });
     var animation =
       div.animate({ visibility: ['visible', 'hidden'] }, 100 * MS_PER_SEC);
 
     await animation.ready;