Bug 1695162 - Make nsTHashtable::ConstIter return a real ConstIterator. r=xpcom-reviewers,nika

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

dom/animation/EffectSet.h

@@ -112,38 +112,29 @@ class EffectSet {
   // 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().
+  //
+  // XXX Except for the active iterator checks, this could be replaced by the
+  // STL-style iterators of nsTHashtable now.
   class Iterator {
    public:
-    explicit Iterator(EffectSet& aEffectSet)
-        : mEffectSet(aEffectSet),
-          mHashIterator(aEffectSet.mEffects.Iter()),
-          mIsEndIterator(false) {
-#ifdef DEBUG
-      mEffectSet.mActiveIterators++;
-#endif
-    }
+    explicit Iterator(EffectSet& aEffectSet) : Iterator(aEffectSet, false) {}
 
-    Iterator(Iterator&& aOther)
-        : mEffectSet(aOther.mEffectSet),
-          mHashIterator(std::move(aOther.mHashIterator)),
-          mIsEndIterator(aOther.mIsEndIterator) {
-#ifdef DEBUG
-      mEffectSet.mActiveIterators++;
-#endif
-    }
+    Iterator() = delete;
+    Iterator(const Iterator&) = delete;
+    Iterator(Iterator&&) = delete;
+    Iterator& operator=(const Iterator&) = delete;
+    Iterator& operator=(Iterator&&) = delete;
 
     static Iterator EndIterator(EffectSet& aEffectSet) {
-      Iterator result(aEffectSet);
-      result.mIsEndIterator = true;
-      return result;
+      return {aEffectSet, true};
     }
 
-    ~Iterator() {
 #ifdef DEBUG
+    ~Iterator() {
       MOZ_ASSERT(mEffectSet.mActiveIterators > 0);
       mEffectSet.mActiveIterators--;
-#endif
     }
+#endif
 
     bool operator!=(const Iterator& aOther) const {
       if (Done() || aOther.Done()) {
@@ -164,15 +155,24 @@ class EffectSet {
     }
 
    private:
-    Iterator() = delete;
-    Iterator(const Iterator&) = delete;
-    Iterator& operator=(const Iterator&) = delete;
-    Iterator& operator=(const Iterator&&) = delete;
+    Iterator(EffectSet& aEffectSet, bool aIsEndIterator)
+        :
+#ifdef DEBUG
+          mEffectSet(aEffectSet),
+#endif
+          mHashIterator(aEffectSet.mEffects.ConstIter()),
+          mIsEndIterator(aIsEndIterator) {
+#ifdef DEBUG
+      mEffectSet.mActiveIterators++;
+#endif
+    }
 
     bool Done() const { return mIsEndIterator || mHashIterator.Done(); }
 
+#ifdef DEBUG
     EffectSet& mEffectSet;
-    OwningEffectSet::Iterator mHashIterator;
+#endif
+    OwningEffectSet::ConstIterator mHashIterator;
     bool mIsEndIterator;
   };
 

dom/base/Document.cpp

@@ -528,7 +528,7 @@ void IdentifierMapEntry::FireChangeCallbacks(Element* aOldElement,
                                              bool aImageOnly) {
   if (!mChangeCallbacks) return;
 
-  for (auto iter = mChangeCallbacks->ConstIter(); !iter.Done(); iter.Next()) {
+  for (auto iter = mChangeCallbacks->Iter(); !iter.Done(); iter.Next()) {
     IdentifierMapEntry::ChangeCallbackEntry* entry = iter.Get();
     // Don't fire image changes for non-image observers, and don't fire element
     // changes for image observers when an image override is active.

dom/base/DocumentOrShadowRoot.cpp

@@ -808,7 +808,7 @@ void DocumentOrShadowRoot::Traverse(DocumentOrShadowRoot* tmp,
   });
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mAdoptedStyleSheets);
 
-  for (auto iter = tmp->mIdentifierMap.ConstIter(); !iter.Done(); iter.Next()) {
+  for (auto iter = tmp->mIdentifierMap.Iter(); !iter.Done(); iter.Next()) {
     iter.Get()->Traverse(&cb);
   }
 

dom/html/HTMLMediaElement.cpp

@@ -3964,7 +3964,7 @@ static unsigned MediaElementTableCount(HTMLMediaElement* aElement,
   uint32_t uriCount = 0;
   uint32_t otherCount = 0;
   for (auto it = gElementTable->ConstIter(); !it.Done(); it.Next()) {
-    MediaElementSetForURI* entry = it.Get();
+    const MediaElementSetForURI* entry = it.Get();
     uint32_t count = 0;
     for (const auto& elem : entry->mElements) {
       if (elem == aElement) {

netwerk/cookie/CookieStorage.h

@@ -41,6 +41,7 @@ class CookieEntry : public CookieKey {
   ~CookieEntry() = default;
 
   inline ArrayType& GetCookies() { return mCookies; }
+  inline const ArrayType& GetCookies() const { return mCookies; }
 
   size_t SizeOfExcludingThis(MallocSizeOf aMallocSizeOf) const;
 

xpcom/ds/nsTHashtable.h

@@ -377,6 +377,31 @@ class MOZ_NEEDS_NO_VTABLE_TYPE nsTHashtable {
   }
 
  public:
+  class ConstIterator {
+   public:
+    explicit ConstIterator(nsTHashtable* aTable)
+        : mBaseIterator(&aTable->mTable) {}
+    ~ConstIterator() = default;
+
+    KeyType Key() const { return Get()->GetKey(); }
+
+    const EntryType* Get() const {
+      return static_cast<const EntryType*>(mBaseIterator.Get());
+    }
+
+    bool Done() const { return mBaseIterator.Done(); }
+    void Next() { mBaseIterator.Next(); }
+
+    ConstIterator() = delete;
+    ConstIterator(const ConstIterator&) = delete;
+    ConstIterator(ConstIterator&& aOther) = delete;
+    ConstIterator& operator=(const ConstIterator&) = delete;
+    ConstIterator& operator=(ConstIterator&&) = delete;
+
+   protected:
+    PLDHashTable::Iterator mBaseIterator;
+  };
+
   // This is an iterator that also allows entry removal. Example usage:
   //
   //   for (auto iter = table.Iter(); !iter.Done(); iter.Next()) {
@@ -385,27 +410,23 @@ class MOZ_NEEDS_NO_VTABLE_TYPE nsTHashtable {
   //     // ... possibly call iter.Remove() once ...
   //   }
   //
-  class Iterator : public PLDHashTable::Iterator {
+  class Iterator final : public ConstIterator {
    public:
-    typedef PLDHashTable::Iterator Base;
+    using ConstIterator::ConstIterator;
 
-    explicit Iterator(nsTHashtable* aTable) : Base(&aTable->mTable) {}
-    Iterator(Iterator&& aOther) : Base(aOther.mTable) {}
-    ~Iterator() = default;
+    using ConstIterator::Get;
 
-    EntryType* Get() const { return static_cast<EntryType*>(Base::Get()); }
+    EntryType* Get() const {
+      return static_cast<EntryType*>(this->mBaseIterator.Get());
+    }
 
-   private:
-    Iterator() = delete;
-    Iterator(const Iterator&) = delete;
-    Iterator& operator=(const Iterator&) = delete;
-    Iterator& operator=(const Iterator&&) = delete;
+    void Remove() { this->mBaseIterator.Remove(); }
   };
 
   Iterator Iter() { return Iterator(this); }
 
-  Iterator ConstIter() const {
-    return Iterator(const_cast<nsTHashtable*>(this));
+  ConstIterator ConstIter() const {
+    return ConstIterator(const_cast<nsTHashtable*>(this));
   }
 
   using const_iterator = ::detail::nsTHashtable_base_iterator<const EntryType>;
@@ -772,27 +793,48 @@ class nsTHashtable<nsPtrHashKey<T>>
   }
 
  public:
-  class Iterator : public Base::Iterator {
+  class ConstIterator {
    public:
-    typedef nsTHashtable::Base::Iterator Base;
+    explicit ConstIterator(nsTHashtable* aTable)
+        : mBaseIterator(&aTable->mTable) {}
+    ~ConstIterator() = default;
 
-    explicit Iterator(nsTHashtable* aTable) : Base(aTable) {}
-    Iterator(Iterator&& aOther) : Base(std::move(aOther)) {}
-    ~Iterator() = default;
+    KeyType Key() const { return Get()->GetKey(); }
 
-    EntryType* Get() const { return reinterpret_cast<EntryType*>(Base::Get()); }
+    const EntryType* Get() const {
+      return static_cast<const EntryType*>(mBaseIterator.Get());
+    }
 
-   private:
-    Iterator() = delete;
-    Iterator(const Iterator&) = delete;
-    Iterator& operator=(const Iterator&) = delete;
-    Iterator& operator=(Iterator&&) = delete;
+    bool Done() const { return mBaseIterator.Done(); }
+    void Next() { mBaseIterator.Next(); }
+
+    ConstIterator() = delete;
+    ConstIterator(const ConstIterator&) = delete;
+    ConstIterator(ConstIterator&& aOther) = delete;
+    ConstIterator& operator=(const ConstIterator&) = delete;
+    ConstIterator& operator=(ConstIterator&&) = delete;
+
+   protected:
+    PLDHashTable::Iterator mBaseIterator;
+  };
+
+  class Iterator final : public ConstIterator {
+   public:
+    using ConstIterator::ConstIterator;
+
+    using ConstIterator::Get;
+
+    EntryType* Get() const {
+      return static_cast<EntryType*>(this->mBaseIterator.Get());
+    }
+
+    void Remove() { this->mBaseIterator.Remove(); }
   };
 
   Iterator Iter() { return Iterator(this); }
 
-  Iterator ConstIter() const {
-    return Iterator(const_cast<nsTHashtable*>(this));
+  ConstIterator ConstIter() const {
+    return ConstIterator(const_cast<nsTHashtable*>(this));
   }
 
   using const_iterator = ::detail::nsTHashtable_base_iterator<const EntryType>;