Bug 1439485 - Don't try to unthrottle transform animations that don't affect overflow region. r=birtles
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Tue, 27 Feb 2018 14:46:32 +0900
changeset 405462 c4425fcdfb5b3631f6eb465df9a15c61a8bd2786
parent 405461 4e1c789da3fc3018311a09bdf34e08a743f8905e
child 405495 bd6e200b5a6b2f9e5a9b31fcc92285aa7fc9afcc
child 405498 60f738c47df60adb6c01df2ef404f74c4b0b8344
push id33519
push useraiakab@mozilla.com
push dateTue, 27 Feb 2018 09:56:16 +0000
treeherdermozilla-central@c4425fcdfb5b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles
bugs1439485
milestone60.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1439485 - Don't try to unthrottle transform animations that don't affect overflow region. r=birtles MozReview-Commit-ID: AtQPiPnsC3z
dom/animation/KeyframeEffectReadOnly.cpp
dom/animation/KeyframeEffectReadOnly.h
dom/animation/test/mozilla/file_restyles.html
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -765,22 +765,21 @@ KeyframeEffectReadOnly::ComposeStyle(
                "out of array bounds");
 
     ComposeStyleRule(Forward<ComposeAnimationResult>(aComposeResult),
                      prop,
                      *segment,
                      computedTiming);
   }
 
-  // If the animation produces any transform change hint, we need to record the
-  // current time to unthrottle the animation periodically when the animation is
-  // being throttled because it's scrolled out of view.
-  if (mCumulativeChangeHint & (nsChangeHint_UpdatePostTransformOverflow |
-                               nsChangeHint_AddOrRemoveTransform |
-                               nsChangeHint_UpdateTransformLayer)) {
+  // If the animation produces a transform change hint that affects the overflow
+  // region, we need to record the current time to unthrottle the animation
+  // periodically when the animation is being throttled because it's scrolled
+  // out of view.
+  if (HasTransformThatMightAffectOverflow()) {
     nsPresContext* presContext =
       nsContentUtils::GetContextForContent(mTarget->mElement);
     if (presContext) {
       TimeStamp now = presContext->RefreshDriver()->MostRecentRefresh();
       EffectSet* effectSet =
         EffectSet::GetEffectSet(mTarget->mElement, mTarget->mPseudoType);
       MOZ_ASSERT(effectSet, "ComposeStyle should only be called on an effect "
                             "that is part of an effect set");
@@ -1468,19 +1467,17 @@ KeyframeEffectReadOnly::CanThrottle() co
     }
 
     const bool isVisibilityHidden =
       !frame->IsVisibleOrMayHaveVisibleDescendants();
     if (isVisibilityHidden ||
         frame->IsScrolledOutOfView()) {
       // If there are transform change hints, unthrottle the animation
       // periodically since it might affect the overflow region.
-      if (mCumulativeChangeHint & (nsChangeHint_UpdatePostTransformOverflow |
-                                   nsChangeHint_AddOrRemoveTransform |
-                                   nsChangeHint_UpdateTransformLayer)) {
+      if (HasTransformThatMightAffectOverflow()) {
         return isVisibilityHidden
           ? CanThrottleTransformChangesInScrollable(*frame)
           : CanThrottleTransformChanges(*frame);
       }
       return true;
     }
   }
 
@@ -1509,17 +1506,17 @@ KeyframeEffectReadOnly::CanThrottle() co
     if (!layer ||
         effectSet->GetAnimationGeneration() !=
           layer->GetAnimationGeneration()) {
       return false;
     }
 
     // If this is a transform animation that affects the overflow region,
     // we should unthrottle the animation periodically.
-    if (record.mProperty == eCSSProperty_transform &&
+    if (HasTransformThatMightAffectOverflow() &&
         !CanThrottleTransformChangesInScrollable(*frame)) {
       return false;
     }
   }
 
   for (const AnimationProperty& property : mProperties) {
     if (!property.mIsRunningOnCompositor) {
       return false;
--- a/dom/animation/KeyframeEffectReadOnly.h
+++ b/dom/animation/KeyframeEffectReadOnly.h
@@ -483,16 +483,27 @@ private:
     const nsIFrame* aFrame,
     AnimationPerformanceWarning::Type& aPerformanceWarning);
   static bool IsGeometricProperty(const nsCSSPropertyID aProperty);
 
   static const TimeDuration OverflowRegionRefreshInterval();
 
   void UpdateEffectSet(mozilla::EffectSet* aEffectSet = nullptr) const;
 
+  // Returns true if this effect has transform and the transform might affect
+  // the overflow region.
+  // This function is used for updating scroll bars or notifying intersection
+  // observers reflected by the transform.
+  bool HasTransformThatMightAffectOverflow() const
+  {
+    return mCumulativeChangeHint & (nsChangeHint_UpdatePostTransformOverflow |
+                                    nsChangeHint_AddOrRemoveTransform |
+                                    nsChangeHint_UpdateTransformLayer);
+  }
+
   // FIXME: This flag will be removed in bug 1324966.
   bool mIsComposingStyle = false;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_KeyframeEffectReadOnly_h
--- a/dom/animation/test/mozilla/file_restyles.html
+++ b/dom/animation/test/mozilla/file_restyles.html
@@ -1591,12 +1591,37 @@ waitForAllPaints(() => {
       } else {
         todo_is(markers.length, 0,
                 'Bug 1307341 The opacity animation keeps running on the ' +
                 'compositor when color style is changed');
       }
       await ensureElementRemoval(div);
     }
   );
+
+  add_task(
+    async function no_overflow_transform_animations_in_scrollable_element() {
+      await SpecialPowers.pushPrefEnv({ set: [["ui.showHideScrollbars", 1]] });
+
+      var parentElement = addDiv(null,
+        { style: 'overflow-y: scroll; height: 100px;' });
+      var div = addDiv(null);
+      var animation =
+        div.animate({ transform: [ 'translateY(10px)', 'translateY(10px)' ] },
+                    100 * MS_PER_SEC);
+      parentElement.appendChild(div);
+
+      await animation.ready;
+      ok(SpecialPowers.wrap(animation).isRunningOnCompositor);
+
+      var markers = await observeStyling(20);
+      is(markers.length, 0,
+         'No-overflow transform animations running on the compositor should ' +
+         'never update style on the main thread');
+
+      await ensureElementRemoval(parentElement);
+    }
+  );
+
 });
 
 </script>
 </body>