Bug 1188251 part 3 - Add AnimationCollection::RequestRestyle; r=dholbert
authorBrian Birtles <birtles@gmail.com>
Mon, 17 Aug 2015 13:59:44 +0900
changeset 257997 eb67f47a146dc79c39cbffab9e522288901be911
parent 257996 6bb23d47f4694dc552469db17a3db180e266d3e9
child 257998 ae2c4fad97a0b41f71364091cfe1cfc9dbd9eae3
push id29238
push userryanvm@gmail.com
push dateMon, 17 Aug 2015 13:06:57 +0000
treeherdermozilla-central@a6eeb28458fd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1188251
milestone43.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 1188251 part 3 - Add AnimationCollection::RequestRestyle; r=dholbert Ultimately we want to move throttling logic to AnimationCollection and Animation::Tick (and later to KeyframeEffect::SetParentTime). This is so that we can support script-generated animations without having to introduce yet another manager. To that end this patch introduces a method on AnimationCollection that can be called from Animation::Tick to perform the necessary notifications needed to update style. Later in this patch series we will extend RequestRestyle to incorporate more of the throttling logic and further extend it to cover some of the other notifications such as updating layers. This patch tracks whether or not we have already posted a restyle for animation to avoid making redundant calls. Calls to nsIDocument::SetNeedStyleFlush are cheap and more difficult to detect when they have completed so we don't filter redundant calls in the Restyle::Throttled case. If mHasPendingAnimationRestyle is set and AnimationCommon::EnsureStyleRuleFor is *never* called then we could arrive at situation where we fail to make post further restyles for animation. I have verified that if we fail to reset mHasPendingAnimationRestyle at the appropriate point (e.g. resetting it at the end of EnsureStyleRuleFor *after* the early-returns) then a number of existing tests fail. Furthermore, I have observed that it is reset by the beginning of each tick in almost every case except for a few instances of browser mochitests such as browser/components/customizableui/test/browser_1007336_lwthemes_in_customize_mode.js. In this case, during the async cleanup of the test, we have an opacity transition on a vbox element that becomes display:none and appears to be skipped during restyling. However, even in this case, EnsureStyleRuleFor is called within one or at most two ticks and mHasPendingAnimationRestyle flag is cleared (i.e. it does not get stuck).
layout/style/AnimationCommon.cpp
layout/style/AnimationCommon.h
layout/style/nsAnimationManager.cpp
layout/style/nsTransitionManager.cpp
--- a/layout/style/AnimationCommon.cpp
+++ b/layout/style/AnimationCommon.cpp
@@ -23,17 +23,16 @@
 #include "nsDisplayList.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/dom/KeyframeEffect.h"
 #include "RestyleManager.h"
 #include "nsRuleProcessorData.h"
 #include "nsStyleSet.h"
 #include "nsStyleChangeList.h"
 
-
 using mozilla::layers::Layer;
 using mozilla::dom::Animation;
 using mozilla::dom::KeyframeEffectReadOnly;
 
 namespace mozilla {
 
 /* static */ bool
 IsGeometricProperty(nsCSSProperty aProperty)
@@ -819,16 +818,18 @@ AnimationCollection::Tick()
     mAnimations[animIdx]->Tick();
   }
 }
 
 void
 AnimationCollection::EnsureStyleRuleFor(TimeStamp aRefreshTime,
                                         EnsureStyleRuleFlags aFlags)
 {
+  mHasPendingAnimationRestyle = false;
+
   if (!mNeedsRefreshes) {
     mStyleRuleRefreshTime = aRefreshTime;
     return;
   }
 
   if (!mStyleRuleRefreshTime.IsNull() &&
       mStyleRuleRefreshTime == aRefreshTime) {
     // mStyleRule may be null and valid, if we have no style to apply.
@@ -960,16 +961,39 @@ AnimationCollection::CanThrottleAnimatio
       return false;
     }
   }
 
   return true;
 }
 
 void
+AnimationCollection::RequestRestyle(RestyleType aRestyleType)
+{
+  nsPresContext* presContext = mManager->PresContext();
+  if (!presContext) {
+    // Pres context will be null after the manager is disconnected.
+    return;
+  }
+
+  switch (aRestyleType) {
+    case RestyleType::Throttled:
+      presContext->Document()->SetNeedStyleFlush();
+      break;
+
+    case RestyleType::Standard:
+      if (!mHasPendingAnimationRestyle) {
+        mHasPendingAnimationRestyle = true;
+        PostRestyleForAnimation(presContext);
+      }
+      break;
+  }
+}
+
+void
 AnimationCollection::UpdateAnimationGeneration(nsPresContext* aPresContext)
 {
   mAnimationGeneration =
     aPresContext->RestyleManager()->GetAnimationGeneration();
 }
 
 void
 AnimationCollection::UpdateCheckGeneration(
--- a/layout/style/AnimationCommon.h
+++ b/layout/style/AnimationCommon.h
@@ -244,16 +244,17 @@ struct AnimationCollection : public PRCL
   AnimationCollection(dom::Element *aElement, nsIAtom *aElementProperty,
                       CommonAnimationManager *aManager)
     : mElement(aElement)
     , mElementProperty(aElementProperty)
     , mManager(aManager)
     , mAnimationGeneration(0)
     , mCheckGeneration(0)
     , mNeedsRefreshes(true)
+    , mHasPendingAnimationRestyle(false)
 #ifdef DEBUG
     , mCalledPropertyDtor(false)
 #endif
   {
     MOZ_COUNT_CTOR(AnimationCollection);
     PR_INIT_CLIST(this);
   }
   ~AnimationCollection()
@@ -285,16 +286,26 @@ struct AnimationCollection : public PRCL
   enum CanAnimateFlags {
     // Testing for width, height, top, right, bottom, or left.
     CanAnimate_HasGeometricProperty = 1,
     // Allow the case where OMTA is allowed in general, but not for the
     // specified property.
     CanAnimate_AllowPartial = 2
   };
 
+  enum class RestyleType {
+    // Animation style has changed but the compositor is applying the same
+    // change so we might be able to defer updating the main thread until it
+    // becomes necessary.
+    Throttled,
+    // Animation style has changed and needs to be updated on the main thread.
+    Standard
+  };
+  void RequestRestyle(RestyleType aRestyleType);
+
 private:
   static bool
   CanAnimatePropertyOnCompositor(const dom::Element *aElement,
                                  nsCSSProperty aProperty,
                                  CanAnimateFlags aFlags);
 
 public:
   static bool IsCompositorAnimationDisabledForFrame(nsIFrame* aFrame);
@@ -429,16 +440,22 @@ public:
   // The refresh time associated with mStyleRule.
   TimeStamp mStyleRuleRefreshTime;
 
   // False when we know that our current style rule is valid
   // indefinitely into the future (because all of our animations are
   // either completed or paused).  May be invalidated by a style change.
   bool mNeedsRefreshes;
 
+private:
+  // Whether or not we have already posted for animation restyle.
+  // This is used to avoid making redundant requests and is reset each time
+  // the animation restyle is performed.
+  bool mHasPendingAnimationRestyle;
+
 #ifdef DEBUG
   bool mCalledPropertyDtor;
 #endif
 };
 
 /**
  * Utility class for referencing the element that created a CSS animation or
  * transition. It is non-owning (i.e. it uses a raw pointer) since it is only
--- a/layout/style/nsAnimationManager.cpp
+++ b/layout/style/nsAnimationManager.cpp
@@ -957,17 +957,16 @@ nsAnimationManager::WillRefresh(mozilla:
 
   FlushAnimations(Can_Throttle);
 }
 
 void
 nsAnimationManager::FlushAnimations(FlushFlags aFlags)
 {
   TimeStamp now = mPresContext->RefreshDriver()->MostRecentRefresh();
-  bool didThrottle = false;
   for (PRCList *l = PR_LIST_HEAD(&mElementCollections);
        l != &mElementCollections;
        l = PR_NEXT_LINK(l)) {
     AnimationCollection* collection = static_cast<AnimationCollection*>(l);
 
     if (collection->mStyleRuleRefreshTime == now) {
       continue;
     }
@@ -982,21 +981,15 @@ nsAnimationManager::FlushAnimations(Flus
       collection->CanThrottleAnimation(now);
 
     for (auto iter = collection->mAnimations.cbegin();
          canThrottleTick && iter != collection->mAnimations.cend();
          ++iter) {
       canThrottleTick &= (*iter)->CanThrottle();
     }
 
-    if (!canThrottleTick) {
-      collection->PostRestyleForAnimation(mPresContext);
-    } else {
-      didThrottle = true;
-    }
-  }
-
-  if (didThrottle) {
-    mPresContext->Document()->SetNeedStyleFlush();
+    collection->RequestRestyle(canThrottleTick ?
+                               AnimationCollection::RestyleType::Throttled :
+                               AnimationCollection::RestyleType::Standard);
   }
 
   MaybeStartOrStopObservingRefreshDriver();
 }
--- a/layout/style/nsTransitionManager.cpp
+++ b/layout/style/nsTransitionManager.cpp
@@ -910,17 +910,16 @@ void
 nsTransitionManager::FlushTransitions(FlushFlags aFlags)
 {
   if (PR_CLIST_IS_EMPTY(&mElementCollections)) {
     // no transitions, leave early
     return;
   }
 
   TimeStamp now = mPresContext->RefreshDriver()->MostRecentRefresh();
-  bool didThrottle = false;
   // Post restyle events for frames that are transitioning.
   {
     PRCList *next = PR_LIST_HEAD(&mElementCollections);
     while (next != &mElementCollections) {
       AnimationCollection* collection = static_cast<AnimationCollection*>(next);
       next = PR_NEXT_LINK(next);
 
       if (collection->mStyleRuleRefreshTime == now) {
@@ -953,22 +952,17 @@ nsTransitionManager::FlushTransitions(Fl
       // applies (in which case we just made it not apply).
       MOZ_ASSERT(collection->mElementProperty ==
                    nsGkAtoms::transitionsProperty ||
                  collection->mElementProperty ==
                    nsGkAtoms::transitionsOfBeforeProperty ||
                  collection->mElementProperty ==
                    nsGkAtoms::transitionsOfAfterProperty,
                  "Unexpected element property; might restyle too much");
-      if (!canThrottleTick) {
-        collection->PostRestyleForAnimation(mPresContext);
-      } else {
-        didThrottle = true;
-      }
+
+      collection->RequestRestyle(canThrottleTick ?
+                                 AnimationCollection::RestyleType::Throttled :
+                                 AnimationCollection::RestyleType::Standard);
     }
   }
 
-  if (didThrottle) {
-    mPresContext->Document()->SetNeedStyleFlush();
-  }
-
   MaybeStartOrStopObservingRefreshDriver();
 }