Bug 708901 - Migrate to nsTHashSet in dom/animation. r=birtles

Depends on D108591

Differential Revision: https://phabricator.services.mozilla.com/D108592

dom/animation/AnimationTimeline.cpp

@@ -48,7 +48,7 @@ void AnimationTimeline::RemoveAnimation(Animation* aAnimation) {
   if (static_cast<LinkedListElement<Animation>*>(aAnimation)->isInList()) {
     static_cast<LinkedListElement<Animation>*>(aAnimation)->remove();
   }
-  mAnimations.RemoveEntry(aAnimation);
+  mAnimations.Remove(aAnimation);
 }
 
 }  // namespace mozilla::dom

dom/animation/AnimationTimeline.h

@@ -15,7 +15,7 @@
 #include "mozilla/Attributes.h"
 #include "nsHashKeys.h"
 #include "nsIGlobalObject.h"
-#include "nsTHashtable.h"
+#include "nsTHashSet.h"
 
 namespace mozilla {
 namespace dom {
@@ -107,7 +107,7 @@ class AnimationTimeline : public nsISupports, public nsWrapperCache {
   //
   // The hashset keeps a strong reference to each animation since
   // dealing with addref/release with LinkedList is difficult.
-  typedef nsTHashtable<nsRefPtrHashKey<dom::Animation>> AnimationSet;
+  typedef nsTHashSet<nsRefPtrHashKey<dom::Animation>> AnimationSet;
   AnimationSet mAnimations;
   LinkedList<dom::Animation> mAnimationOrder;
 };

dom/animation/EffectSet.cpp

@@ -29,9 +29,9 @@ void EffectSet::PropertyDtor(void* aObject, nsAtom* aPropertyName,
 }
 
 void EffectSet::Traverse(nsCycleCollectionTraversalCallback& aCallback) {
-  for (auto iter = mEffects.Iter(); !iter.Done(); iter.Next()) {
-    CycleCollectionNoteChild(aCallback, iter.Get()->GetKey(),
-                             "EffectSet::mEffects[]", aCallback.Flags());
+  for (const auto& key : mEffects) {
+    CycleCollectionNoteChild(aCallback, key, "EffectSet::mEffects[]",
+                             aCallback.Flags());
   }
 }
 

dom/animation/EffectSet.h

@@ -12,8 +12,8 @@
 #include "mozilla/EnumeratedArray.h"
 #include "mozilla/TimeStamp.h"
 #include "mozilla/dom/KeyframeEffect.h"
-#include "nsHashKeys.h"    // For nsPtrHashKey
-#include "nsTHashtable.h"  // For nsTHashtable
+#include "nsHashKeys.h"  // For nsPtrHashKey
+#include "nsTHashSet.h"
 
 class nsPresContext;
 enum class DisplayItemType : uint8_t;
@@ -103,7 +103,7 @@ class EffectSet {
   bool MayHaveTransformAnimation() const { return mMayHaveTransformAnim; }
 
  private:
-  typedef nsTHashtable<nsRefPtrHashKey<dom::KeyframeEffect>> OwningEffectSet;
+  typedef nsTHashSet<nsRefPtrHashKey<dom::KeyframeEffect>> OwningEffectSet;
 
  public:
   // A simple iterator to support iterating over the effects in this object in
@@ -114,10 +114,11 @@ class EffectSet {
   // the rather complicated: iter.Get()->GetKey().
   //
   // XXX Except for the active iterator checks, this could be replaced by the
-  // STL-style iterators of nsTHashtable now.
+  // STL-style iterators of nsTHashSet directly now.
   class Iterator {
    public:
-    explicit Iterator(EffectSet& aEffectSet) : Iterator(aEffectSet, false) {}
+    explicit Iterator(EffectSet& aEffectSet)
+        : Iterator(aEffectSet, aEffectSet.mEffects.begin()) {}
 
     Iterator() = delete;
     Iterator(const Iterator&) = delete;
@@ -126,7 +127,7 @@ class EffectSet {
     Iterator& operator=(Iterator&&) = delete;
 
     static Iterator EndIterator(EffectSet& aEffectSet) {
-      return {aEffectSet, true};
+      return {aEffectSet, aEffectSet.mEffects.end()};
     }
 
 #ifdef DEBUG
@@ -137,43 +138,33 @@ class EffectSet {
 #endif
 
     bool operator!=(const Iterator& aOther) const {
-      if (Done() || aOther.Done()) {
-        return Done() != aOther.Done();
-      }
-      return mHashIterator.Get() != aOther.mHashIterator.Get();
+      return mHashIterator != aOther.mHashIterator;
     }
 
     Iterator& operator++() {
-      MOZ_ASSERT(!Done());
-      mHashIterator.Next();
+      ++mHashIterator;
       return *this;
     }
 
-    dom::KeyframeEffect* operator*() {
-      MOZ_ASSERT(!Done());
-      return mHashIterator.Get()->GetKey();
-    }
+    dom::KeyframeEffect* operator*() { return *mHashIterator; }
 
    private:
-    Iterator(EffectSet& aEffectSet, bool aIsEndIterator)
+    Iterator(EffectSet& aEffectSet,
+             OwningEffectSet::const_iterator aHashIterator)
         :
 #ifdef DEBUG
           mEffectSet(aEffectSet),
 #endif
-          mHashIterator(aEffectSet.mEffects.ConstIter()),
-          mIsEndIterator(aIsEndIterator) {
+          mHashIterator(std::move(aHashIterator)) {
 #ifdef DEBUG
       mEffectSet.mActiveIterators++;
 #endif
     }
 
-    bool Done() const { return mIsEndIterator || mHashIterator.Done(); }
-
 #ifdef DEBUG
     EffectSet& mEffectSet;
 #endif
-    OwningEffectSet::ConstIterator mHashIterator;
-    bool mIsEndIterator;
+    OwningEffectSet::const_iterator mHashIterator;
   };
 
   friend class Iterator;

dom/animation/PendingAnimationTracker.cpp

@@ -28,7 +28,7 @@ PendingAnimationTracker::PendingAnimationTracker(dom::Document* aDocument)
 
 void PendingAnimationTracker::AddPending(dom::Animation& aAnimation,
                                          AnimationSet& aSet) {
-  aSet.PutEntry(&aAnimation);
+  aSet.Insert(&aAnimation);
 
   // Schedule a paint. Otherwise animations that don't trigger a paint by
   // themselves (e.g. CSS animations with an empty keyframes rule) won't
@@ -38,7 +38,7 @@ void PendingAnimationTracker::AddPending(dom::Animation& aAnimation,
 
 void PendingAnimationTracker::RemovePending(dom::Animation& aAnimation,
                                             AnimationSet& aSet) {
-  aSet.RemoveEntry(&aAnimation);
+  aSet.Remove(&aAnimation);
 }
 
 bool PendingAnimationTracker::IsWaiting(const dom::Animation& aAnimation,
@@ -50,15 +50,16 @@ void PendingAnimationTracker::TriggerPendingAnimationsOnNextTick(
     const TimeStamp& aReadyTime) {
   auto triggerAnimationsAtReadyTime = [aReadyTime](
                                           AnimationSet& aAnimationSet) {
-    for (auto iter = aAnimationSet.Iter(); !iter.Done(); iter.Next()) {
-      dom::Animation* animation = iter.Get()->GetKey();
+    for (auto iter = aAnimationSet.begin(), end = aAnimationSet.end();
+         iter != end; ++iter) {
+      dom::Animation* animation = *iter;
       dom::AnimationTimeline* timeline = animation->GetTimeline();
 
       // If the animation does not have a timeline, just drop it from the map.
       // The animation will detect that it is not being tracked and will trigger
       // itself on the next tick where it has a timeline.
       if (!timeline) {
-        iter.Remove();
+        aAnimationSet.Remove(iter);
         continue;
       }
 
@@ -75,7 +76,7 @@ void PendingAnimationTracker::TriggerPendingAnimationsOnNextTick(
       Nullable<TimeDuration> readyTime = timeline->ToTimelineTime(aReadyTime);
       animation->TriggerOnNextTick(readyTime);
 
-      iter.Remove();
+      aAnimationSet.Remove(iter);
     }
   };
 
@@ -88,8 +89,8 @@ void PendingAnimationTracker::TriggerPendingAnimationsOnNextTick(
 
 void PendingAnimationTracker::TriggerPendingAnimationsNow() {
   auto triggerAndClearAnimations = [](AnimationSet& aAnimationSet) {
-    for (auto iter = aAnimationSet.Iter(); !iter.Done(); iter.Next()) {
-      iter.Get()->GetKey()->TriggerNow();
+    for (const auto& animation : aAnimationSet) {
+      animation->TriggerNow();
     }
     aAnimationSet.Clear();
   };
@@ -139,8 +140,7 @@ void PendingAnimationTracker::MarkAnimationsThatMightNeedSynchronization() {
   // on the main thread that really don't need to.
 
   mHasPlayPendingGeometricAnimations = CheckState::Absent;
-  for (auto iter = mPlayPendingSet.ConstIter(); !iter.Done(); iter.Next()) {
-    auto animation = iter.Get()->GetKey();
+  for (const auto& animation : mPlayPendingSet) {
     if (animation->GetEffect() && animation->GetEffect()->AffectsGeometry()) {
       mHasPlayPendingGeometricAnimations &= ~CheckState::Absent;
       mHasPlayPendingGeometricAnimations |= IsTransition(*animation)
@@ -160,8 +160,7 @@ void PendingAnimationTracker::MarkAnimationsThatMightNeedSynchronization() {
     return;
   }
 
-  for (auto iter = mPlayPendingSet.Iter(); !iter.Done(); iter.Next()) {
-    auto animation = iter.Get()->GetKey();
+  for (const auto& animation : mPlayPendingSet) {
     bool isTransition = IsTransition(*animation);
     if ((isTransition &&
          mHasPlayPendingGeometricAnimations & CheckState::TransitionsPresent) ||

dom/animation/PendingAnimationTracker.h

@@ -10,7 +10,7 @@
 #include "mozilla/dom/Animation.h"
 #include "mozilla/TypedEnumBits.h"
 #include "nsCycleCollectionParticipant.h"
-#include "nsTHashtable.h"
+#include "nsTHashSet.h"
 
 class nsIFrame;
 
@@ -79,7 +79,7 @@ class PendingAnimationTracker final {
 
   void EnsurePaintIsScheduled();
 
-  typedef nsTHashtable<nsRefPtrHashKey<dom::Animation>> AnimationSet;
+  typedef nsTHashSet<nsRefPtrHashKey<dom::Animation>> AnimationSet;
 
   void AddPending(dom::Animation& aAnimation, AnimationSet& aSet);
   void RemovePending(dom::Animation& aAnimation, AnimationSet& aSet);