Bug 1117603 part 2 - Don't unregister from the refresh driver unless we are also queueing events; r=dbaron
☠☠ backed out by 5329cda711c8 ☠ ☠
authorBrian Birtles <birtles@gmail.com>
Tue, 24 Mar 2015 09:06:06 +0900
changeset 265485 67fece2790496af172de79b63ffe9e334e05d279
parent 265484 fe8ba59678a2f1c53e9b2af0f9e8e557429f3e8b
child 265486 33ab56c02e6ab589bd48f9f37eb42e44973ac520
push id830
push userraliiev@mozilla.com
push dateFri, 19 Jun 2015 19:24:37 +0000
treeherdermozilla-release@932614382a68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs1117603
milestone39.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 1117603 part 2 - Don't unregister from the refresh driver unless we are also queueing events; r=dbaron
layout/style/AnimationCommon.cpp
layout/style/AnimationCommon.h
--- a/layout/style/AnimationCommon.cpp
+++ b/layout/style/AnimationCommon.cpp
@@ -94,33 +94,49 @@ CommonAnimationManager::RemoveAllElement
     AnimationPlayerCollection* head =
       static_cast<AnimationPlayerCollection*>(
         PR_LIST_HEAD(&mElementCollections));
     head->Destroy();
   }
 }
 
 void
-CommonAnimationManager::CheckNeedsRefresh()
+CommonAnimationManager::MaybeStartObservingRefreshDriver()
+{
+  if (mIsObservingRefreshDriver || !NeedsRefresh()) {
+    return;
+  }
+
+  mPresContext->RefreshDriver()->AddRefreshObserver(this, Flush_Style);
+  mIsObservingRefreshDriver = true;
+}
+
+void
+CommonAnimationManager::MaybeStartOrStopObservingRefreshDriver()
+{
+  bool needsRefresh = NeedsRefresh();
+  if (needsRefresh && !mIsObservingRefreshDriver) {
+    mPresContext->RefreshDriver()->AddRefreshObserver(this, Flush_Style);
+  } else if (!needsRefresh && mIsObservingRefreshDriver) {
+    mPresContext->RefreshDriver()->RemoveRefreshObserver(this, Flush_Style);
+  }
+  mIsObservingRefreshDriver = needsRefresh;
+}
+
+bool
+CommonAnimationManager::NeedsRefresh() const
 {
   for (PRCList *l = PR_LIST_HEAD(&mElementCollections);
        l != &mElementCollections;
        l = PR_NEXT_LINK(l)) {
     if (static_cast<AnimationPlayerCollection*>(l)->mNeedsRefreshes) {
-      if (!mIsObservingRefreshDriver) {
-        mPresContext->RefreshDriver()->AddRefreshObserver(this, Flush_Style);
-        mIsObservingRefreshDriver = true;
-      }
-      return;
+      return true;
     }
   }
-  if (mIsObservingRefreshDriver) {
-    mIsObservingRefreshDriver = false;
-    mPresContext->RefreshDriver()->RemoveRefreshObserver(this, Flush_Style);
-  }
+  return false;
 }
 
 AnimationPlayerCollection*
 CommonAnimationManager::GetAnimationsForCompositor(nsIContent* aContent,
                                                    nsIAtom* aElementProperty,
                                                    nsCSSProperty aProperty)
 {
   if (!aContent->MayHaveAnimations())
@@ -271,17 +287,17 @@ CommonAnimationManager::AddStyleUpdatesT
     }
   }
 }
 
 void
 CommonAnimationManager::NotifyCollectionUpdated(AnimationPlayerCollection&
                                                   aCollection)
 {
-  CheckNeedsRefresh();
+  MaybeStartObservingRefreshDriver();
   mPresContext->ClearLastStyleUpdateForAllAnimations();
   mPresContext->RestyleManager()->IncrementAnimationGeneration();
   aCollection.UpdateAnimationGeneration(mPresContext);
   aCollection.PostRestyleForAnimation(mPresContext);
 }
 
 /* static */ bool
 CommonAnimationManager::ExtractComputedValueForTransition(
@@ -727,17 +743,17 @@ AnimationPlayerCollection::EnsureStyleRu
     nsCSSPropertySet properties;
 
     for (size_t playerIdx = mPlayers.Length(); playerIdx-- != 0; ) {
       mPlayers[playerIdx]->ComposeStyle(mStyleRule, properties,
                                         mNeedsRefreshes);
     }
   }
 
-  mManager->CheckNeedsRefresh();
+  mManager->MaybeStartObservingRefreshDriver();
 
   // If one of our animations just started or stopped filling, we need
   // to notify the transition manager.  This does the notification a bit
   // more than necessary, but it's easier than doing it exactly.
   if (mManager->IsAnimationManager()) {
     mManager->mPresContext->TransitionManager()->
       UpdateCascadeResultsWithAnimations(this);
   }
--- a/layout/style/AnimationCommon.h
+++ b/layout/style/AnimationCommon.h
@@ -126,21 +126,25 @@ public:
 
 protected:
   virtual ~CommonAnimationManager();
 
   // For ElementCollectionRemoved
   friend struct mozilla::AnimationPlayerCollection;
 
   void AddElementCollection(AnimationPlayerCollection* aCollection);
-  void ElementCollectionRemoved() { CheckNeedsRefresh(); }
+  void ElementCollectionRemoved() { MaybeStartOrStopObservingRefreshDriver(); }
   void RemoveAllElementCollections();
 
-  // Check to see if we should stop or start observing the refresh driver
-  void CheckNeedsRefresh();
+  // We should normally only call MaybeStartOrStopObservingRefreshDriver in
+  // situations where we will also queue events since otherwise we may stop
+  // getting refresh driver ticks before we queue the necessary events.
+  void MaybeStartObservingRefreshDriver();
+  void MaybeStartOrStopObservingRefreshDriver();
+  bool NeedsRefresh() const;
 
   virtual nsIAtom* GetAnimationsAtom() = 0;
   virtual nsIAtom* GetAnimationsBeforeAtom() = 0;
   virtual nsIAtom* GetAnimationsAfterAtom() = 0;
 
   virtual bool IsAnimationManager() {
     return false;
   }