Bug 1237467 part 3 - Add debug methods to determine if an EffectSet is currently being enumerated; r=heycam
authorBrian Birtles <birtles@gmail.com>
Fri, 15 Jan 2016 15:15:47 +0900
changeset 317175 9f5396096fd990fa4ca05a2001476c98a20d5a6b
parent 317174 7fdf99692385affe2fa3170d918c7a6ea008c0e7
child 317176 d51451c41cabf2117e0de495d68d437bec0b37ad
push id1079
push userjlund@mozilla.com
push dateFri, 15 Apr 2016 21:02:33 +0000
treeherdermozilla-release@575fbf6786d5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1237467
milestone46.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 1237467 part 3 - Add debug methods to determine if an EffectSet is currently being enumerated; r=heycam
dom/animation/EffectSet.h
--- a/dom/animation/EffectSet.h
+++ b/dom/animation/EffectSet.h
@@ -3,16 +3,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef mozilla_EffectSet_h
 #define mozilla_EffectSet_h
 
 #include "mozilla/AnimValuesStyleRule.h"
+#include "mozilla/DebugOnly.h"
 #include "mozilla/EffectCompositor.h"
 #include "mozilla/EnumeratedArray.h"
 #include "mozilla/TimeStamp.h"
 #include "nsCSSPseudoElements.h" // For nsCSSPseudoElements::Type
 #include "nsHashKeys.h" // For nsPtrHashKey
 #include "nsTHashtable.h" // For nsTHashtable
 
 class nsPresContext;
@@ -27,27 +28,31 @@ class KeyframeEffectReadOnly;
 // A wrapper around a hashset of AnimationEffect objects to handle
 // storing the set as a property of an element.
 class EffectSet
 {
 public:
   EffectSet()
     : mCascadeNeedsUpdate(false)
     , mAnimationGeneration(0)
+    , mActiveIterators(0)
 #ifdef DEBUG
     , mCalledPropertyDtor(false)
 #endif
   {
     MOZ_COUNT_CTOR(EffectSet);
   }
 
   ~EffectSet()
   {
     MOZ_ASSERT(mCalledPropertyDtor,
                "must call destructor through element property dtor");
+    MOZ_ASSERT(mActiveIterators == 0,
+               "Effect set should not be destroyed while it is being "
+               "enumerated");
     MOZ_COUNT_DTOR(EffectSet);
   }
   static void PropertyDtor(void* aObject, nsIAtom* aPropertyName,
                            void* aPropertyValue, void* aData);
 
   // Methods for supporting cycle-collection
   void Traverse(nsCycleCollectionTraversalCallback& aCallback);
 
@@ -69,30 +74,45 @@ public:
   // range-based for loops.
   //
   // This allows us to avoid exposing mEffects directly and saves the
   // caller from having to dereference hashtable iterators using
   // the rather complicated: iter.Get()->GetKey().
   class Iterator
   {
   public:
-    explicit Iterator(OwningEffectSet::Iterator&& aHashIterator)
-      : mHashIterator(mozilla::Move(aHashIterator))
-      , mIsEndIterator(false) { }
+    explicit Iterator(EffectSet& aEffectSet)
+      : mEffectSet(aEffectSet)
+      , mHashIterator(mozilla::Move(aEffectSet.mEffects.Iter()))
+      , mIsEndIterator(false)
+    {
+      mEffectSet.mActiveIterators++;
+    }
+
     Iterator(Iterator&& aOther)
-      : mHashIterator(mozilla::Move(aOther.mHashIterator))
-      , mIsEndIterator(aOther.mIsEndIterator) { }
+      : mEffectSet(aOther.mEffectSet)
+      , mHashIterator(mozilla::Move(aOther.mHashIterator))
+      , mIsEndIterator(aOther.mIsEndIterator)
+    {
+      mEffectSet.mActiveIterators++;
+    }
 
-    static Iterator EndIterator(OwningEffectSet::Iterator&& aHashIterator)
+    static Iterator EndIterator(EffectSet& aEffectSet)
     {
-      Iterator result(mozilla::Move(aHashIterator));
+      Iterator result(aEffectSet);
       result.mIsEndIterator = true;
       return result;
     }
 
+    ~Iterator()
+    {
+      MOZ_ASSERT(mEffectSet.mActiveIterators > 0);
+      mEffectSet.mActiveIterators--;
+    }
+
     bool operator!=(const Iterator& aOther) const {
       if (Done() || aOther.Done()) {
         return Done() != aOther.Done();
       }
       return mHashIterator.Get() != aOther.mHashIterator.Get();
     }
 
     Iterator& operator++() {
@@ -112,25 +132,29 @@ public:
     Iterator(const Iterator&) = delete;
     Iterator& operator=(const Iterator&) = delete;
     Iterator& operator=(const Iterator&&) = delete;
 
     bool Done() const {
       return mIsEndIterator || mHashIterator.Done();
     }
 
+    EffectSet& mEffectSet;
     OwningEffectSet::Iterator mHashIterator;
     bool mIsEndIterator;
   };
 
-  Iterator begin() { return Iterator(mEffects.Iter()); }
-  Iterator end()
-  {
-    return Iterator::EndIterator(mEffects.Iter());
-  }
+  friend class Iterator;
+
+  Iterator begin() { return Iterator(*this); }
+  Iterator end() { return Iterator::EndIterator(*this); }
+#ifdef DEBUG
+  bool IsBeingEnumerated() const { return mActiveIterators != 0; }
+#endif
+
   bool IsEmpty() const { return mEffects.IsEmpty(); }
 
   RefPtr<AnimValuesStyleRule>& AnimationRule(EffectCompositor::CascadeLevel
                                              aCascadeLevel)
   {
     return mAnimationRule[aCascadeLevel];
   }
 
@@ -190,16 +214,20 @@ private:
   // RestyleManager keeps track of the number of animation restyles.
   // 'mini-flushes' (see nsTransitionManager::UpdateAllThrottledStyles()).
   // mAnimationGeneration is the sequence number of the last flush where a
   // transition/animation changed.  We keep a similar count on the
   // corresponding layer so we can check that the layer is up to date with
   // the animation manager.
   uint64_t mAnimationGeneration;
 
+  // Track how many iterators are referencing this effect set when we are
+  // destroyed, we can assert that nothing is still pointing to us.
+  DebugOnly<uint64_t> mActiveIterators;
+
 #ifdef DEBUG
   bool mCalledPropertyDtor;
 #endif
 };
 
 } // namespace mozilla
 
 #endif // mozilla_EffectSet_h