Bug 1188251 part 10 - Remove throttling from EnsureStyleRuleFor; r=dholbert
authorBrian Birtles <birtles@gmail.com>
Tue, 18 Aug 2015 16:11:55 +0900
changeset 258136 3e2e93d1bf5baee2b3469098bce0cc8bab49b8e3
parent 258135 01bca969ceda5e6ded11cef25f9c8d5f4794f9f7
child 258137 058fb2d079be64c57c129090ac6347123f61d4a9
push id63831
push userbbirtles@mozilla.com
push dateTue, 18 Aug 2015 07:12:26 +0000
treeherdermozilla-inbound@74f24cabb959 [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 10 - Remove throttling from EnsureStyleRuleFor; r=dholbert EnsureStyleRuleFor contains logic for performing throttled updates to the style rule but it is only used in one case: inside nsTransitionManager::UpdateCascadeResults to determine what properties are being animated by CSS animations. We would like to remove throttling logic from EnsureStyleRuleFor altogether but if that one case where it is currently used is run on every tick then removing this logic could effectively mean we end up updating the style rule on every tick. Fortunately nsTransitionManager::UpdateCascadeResults is only called in the following cases: 1. From nsTransitionManager::StyleContextChanged (via TransitionManager::UpdateCascadeResultsWithTransitions), when we are processing style changes for transitions. 2. From AnimationCollection::EnsureStyleRuleFor (via nsAnimationManager::MaybeUpdateCascadeResults and nsTransitionManager::UpdateCascadeResultsWithAnimations), when we are updating the animation style rule from CSS animations. 3. From nsAnimationManager::CheckAnimationRule (via TransitionManager::UpdateCascadeResultsWithAnimationsToBeDestroyed), when we are processing style changes for CSS animations. None of these things should be happenning on a regular throttle-able tick so by removing this logic we shouldn't be causing any additional work. I have verified, using a test case that combines transitions and animations on the same property, that we have the same behavior with regard to calling EnsureStyleRuleFor both before and after this patch (specifically we avoid calling it altogether while running only the transition but when the animation starts and clobbers the transition we end up calling EnsureStyleRuleFor once on each tick).
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
@@ -284,17 +284,17 @@ CommonAnimationManager::AddStyleUpdatesT
 {
   TimeStamp now = mPresContext->RefreshDriver()->MostRecentRefresh();
 
   PRCList* next = PR_LIST_HEAD(&mElementCollections);
   while (next != &mElementCollections) {
     AnimationCollection* collection = static_cast<AnimationCollection*>(next);
     next = PR_NEXT_LINK(next);
 
-    collection->EnsureStyleRuleFor(now, EnsureStyleRule_IsNotThrottled);
+    collection->EnsureStyleRuleFor(now);
 
     dom::Element* elementToRestyle = collection->GetElementToRestyle();
     if (elementToRestyle) {
       nsRestyleHint rshint = collection->IsForTransitions()
         ? eRestyle_CSSTransitions : eRestyle_CSSAnimations;
       aTracker.AddPendingRestyle(elementToRestyle, rshint, nsChangeHint(0));
     }
   }
@@ -424,18 +424,17 @@ CommonAnimationManager::GetAnimationRule
   }
 
   RestyleManager* restyleManager = mPresContext->RestyleManager();
   if (restyleManager->SkipAnimationRules()) {
     return nullptr;
   }
 
   collection->EnsureStyleRuleFor(
-    mPresContext->RefreshDriver()->MostRecentRefresh(),
-    EnsureStyleRule_IsNotThrottled);
+    mPresContext->RefreshDriver()->MostRecentRefresh());
 
   return collection->mStyleRule;
 }
 
 /* static */ const CommonAnimationManager::LayerAnimationRecord
   CommonAnimationManager::sLayerAnimationInfo[] =
     { { eCSSProperty_transform,
         nsDisplayItem::TYPE_TRANSFORM,
@@ -857,51 +856,31 @@ AnimationCollection::Tick()
 {
   for (size_t animIdx = 0, animEnd = mAnimations.Length();
        animIdx != animEnd; animIdx++) {
     mAnimations[animIdx]->Tick();
   }
 }
 
 void
-AnimationCollection::EnsureStyleRuleFor(TimeStamp aRefreshTime,
-                                        EnsureStyleRuleFlags aFlags)
+AnimationCollection::EnsureStyleRuleFor(TimeStamp aRefreshTime)
 {
   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.
     return;
   }
 
-  // If we're performing animations on the compositor thread, then we can skip
-  // most of the work in this method. But even if we are throttled, then we
-  // have to do the work if an animation is ending in order to get correct end
-  // of animation behavior (the styles of the animation disappear, or the fill
-  // mode behavior). CanThrottle returns false for any finishing animations
-  // so we can force style recalculation in that case.
-  if (aFlags == EnsureStyleRule_IsThrottled) {
-    for (size_t animIdx = mAnimations.Length(); animIdx-- != 0; ) {
-      if (!mAnimations[animIdx]->CanThrottle()) {
-        aFlags = EnsureStyleRule_IsNotThrottled;
-        break;
-      }
-    }
-  }
-
-  if (aFlags == EnsureStyleRule_IsThrottled) {
-    return;
-  }
-
   if (mManager->IsAnimationManager()) {
     // Update cascade results before updating the style rule, since the
     // cascade results can influence the style rule.
     static_cast<nsAnimationManager*>(mManager)->MaybeUpdateCascadeResults(this);
   }
 
   mStyleRuleRefreshTime = aRefreshTime;
   mStyleRule = nullptr;
--- a/layout/style/AnimationCommon.h
+++ b/layout/style/AnimationCommon.h
@@ -233,21 +233,16 @@ public:
 private:
   ~AnimValuesStyleRule() {}
 
   InfallibleTArray<PropertyValuePair> mPropertyValuePairs;
 };
 
 typedef InfallibleTArray<nsRefPtr<dom::Animation>> AnimationPtrArray;
 
-enum EnsureStyleRuleFlags {
-  EnsureStyleRule_IsThrottled,
-  EnsureStyleRule_IsNotThrottled
-};
-
 struct AnimationCollection : public PRCList
 {
   AnimationCollection(dom::Element *aElement, nsIAtom *aElementProperty,
                       CommonAnimationManager *aManager)
     : mElement(aElement)
     , mElementProperty(aElementProperty)
     , mManager(aManager)
     , mAnimationGeneration(0)
@@ -276,17 +271,17 @@ struct AnimationCollection : public PRCL
     mElement->DeleteProperty(mElementProperty);
   }
 
   static void PropertyDtor(void *aObject, nsIAtom *aPropertyName,
                            void *aPropertyValue, void *aData);
 
   void Tick();
 
-  void EnsureStyleRuleFor(TimeStamp aRefreshTime, EnsureStyleRuleFlags aFlags);
+  void EnsureStyleRuleFor(TimeStamp aRefreshTime);
 
   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
   };
--- a/layout/style/nsAnimationManager.cpp
+++ b/layout/style/nsAnimationManager.cpp
@@ -529,17 +529,17 @@ nsAnimationManager::CheckAnimationRule(n
   // Cancel removed animations
   for (size_t newAnimIdx = newAnimations.Length(); newAnimIdx-- != 0; ) {
     newAnimations[newAnimIdx]->CancelFromStyle();
   }
 
   UpdateCascadeResults(aStyleContext, collection);
 
   TimeStamp refreshTime = mPresContext->RefreshDriver()->MostRecentRefresh();
-  collection->EnsureStyleRuleFor(refreshTime, EnsureStyleRule_IsNotThrottled);
+  collection->EnsureStyleRuleFor(refreshTime);
   // We don't actually dispatch the pending events now.  We'll either
   // dispatch them the next time we get a refresh driver notification
   // or the next time somebody calls
   // nsPresShell::FlushPendingNotifications.
   if (mEventDispatcher.HasQueuedEvents()) {
     mPresContext->Document()->SetNeedStyleFlush();
   }
 
--- a/layout/style/nsTransitionManager.cpp
+++ b/layout/style/nsTransitionManager.cpp
@@ -462,17 +462,17 @@ nsTransitionManager::StyleContextChanged
     UpdateCascadeResultsWithTransitions(collection);
 
     // Set the style rule refresh time to null so that EnsureStyleRuleFor
     // creates a new style rule if we started *or* stopped transitions.
     collection->mStyleRuleRefreshTime = TimeStamp();
     collection->UpdateCheckGeneration(mPresContext);
     collection->mNeedsRefreshes = true;
     TimeStamp now = mPresContext->RefreshDriver()->MostRecentRefresh();
-    collection->EnsureStyleRuleFor(now, EnsureStyleRule_IsNotThrottled);
+    collection->EnsureStyleRuleFor(now);
   }
 
   // We want to replace the new style context with the after-change style.
   *aNewStyleContext = afterChangeStyle;
   if (collection) {
     // Since we have transition styles, we have to undo this replacement.
     // The check of collection->mCheckGeneration against the restyle
     // manager's GetAnimationGeneration() will ensure that we don't go
@@ -819,19 +819,17 @@ nsTransitionManager::UpdateCascadeResult
   nsCSSPropertySet propertiesWithTransitions;
 #endif
 
   // http://dev.w3.org/csswg/css-transitions/#application says that
   // transitions do not apply when the same property has a CSS Animation
   // on that element (even though animations are lower in the cascade).
   if (aAnimations) {
     TimeStamp now = mPresContext->RefreshDriver()->MostRecentRefresh();
-    // Passing EnsureStyleRule_IsThrottled is OK since we will
-    // unthrottle when animations are finishing.
-    aAnimations->EnsureStyleRuleFor(now, EnsureStyleRule_IsThrottled);
+    aAnimations->EnsureStyleRuleFor(now);
 
     if (aAnimations->mStyleRule) {
       aAnimations->mStyleRule->AddPropertiesToSet(propertiesUsed);
     }
   }
 
   // Since we should never have more than one transition for the same
   // property, it doesn't matter what order we iterate the transitions.