Bug 1179071 - Merge RemovingIterator into Iterator. r=froydnj.
authorNicholas Nethercote <nnethercote@mozilla.com>
Mon, 06 Jul 2015 22:02:26 -0700
changeset 251848 1b82ea8d56bcb96e576ae56e4c1b4c5f9f2b4fac
parent 251847 694521589eb069ddc72fb1cc46c7e1bd1f39bd92
child 251849 cec27384d57d5f9bc5c25f177547c0b11b3e43b3
push id29013
push usercbook@mozilla.com
push dateWed, 08 Jul 2015 09:47:46 +0000
treeherdermozilla-central@9b902b7669ae [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1179071, 1131308
milestone42.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 1179071 - Merge RemovingIterator into Iterator. r=froydnj. The original motivation for the Iterator/RemovingIterator split was that PLDHashTable Checker class would treat them differently. But that didn't end up happening (see bug 1131308). So this patch merges them. This is a small code size win now but it will become bigger when I add iterators to nsTHashTable and nsBaseHashtable. The only complication is that PLDHashTable::Iter() is now non-const, which is a problem if you use it in a const method. So I added PLDHashTable::ConstIter() which is used in just two places. It's a bit of a hack -- effectively a const_cast -- but I don't think it's too bad.
dom/plugins/base/nsJSNPRuntime.cpp
js/xpconnect/src/XPCJSRuntime.cpp
js/xpconnect/src/XPCMaps.h
js/xpconnect/src/XPCWrappedNativeScope.cpp
layout/style/nsRuleNode.cpp
modules/libpref/prefapi.cpp
netwerk/cache/nsCacheEntry.cpp
netwerk/cache/nsCacheEntry.h
netwerk/cache/nsCacheService.cpp
netwerk/dns/nsHostResolver.cpp
rdf/base/nsInMemoryDataSource.cpp
security/manager/ssl/nsNSSShutDown.cpp
xpcom/glue/nsBaseHashtable.h
xpcom/glue/nsTHashtable.h
xpcom/glue/pldhash.cpp
xpcom/glue/pldhash.h
xpcom/tests/gtest/TestPLDHash.cpp
--- a/dom/plugins/base/nsJSNPRuntime.cpp
+++ b/dom/plugins/base/nsJSNPRuntime.cpp
@@ -1964,17 +1964,17 @@ nsJSNPRuntime::OnPluginDestroy(NPP npp)
         e.removeFront();
       }
     }
 
     sJSObjWrappersAccessible = true;
   }
 
   if (sNPObjWrappers) {
-    for (auto i = sNPObjWrappers->RemovingIter(); !i.Done(); i.Next()) {
+    for (auto i = sNPObjWrappers->Iter(); !i.Done(); i.Next()) {
       auto entry = static_cast<NPObjWrapperHashEntry*>(i.Get());
 
       if (entry->mNpp == npp) {
         // HACK: temporarily hide the table we're enumerating so that
         // invalidate() and deallocate() don't touch it.
         PLDHashTable *tmp = sNPObjWrappers;
         sNPObjWrappers = nullptr;
 
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -803,48 +803,48 @@ XPCJSRuntime::FinalizeCallback(JSFreeOp*
             // compartments being collected. Currently, though, NativeInterfaces
             // are shared between compartments. This ought to be fixed.
             bool doSweep = !isCompartmentGC;
 
             // We don't want to sweep the JSClasses at shutdown time.
             // At this point there may be JSObjects using them that have
             // been removed from the other maps.
             if (!nsXPConnect::XPConnect()->IsShuttingDown()) {
-                for (auto i = self->mNativeScriptableSharedMap->RemovingIter(); !i.Done(); i.Next()) {
+                for (auto i = self->mNativeScriptableSharedMap->Iter(); !i.Done(); i.Next()) {
                     auto entry = static_cast<XPCNativeScriptableSharedMap::Entry*>(i.Get());
                     XPCNativeScriptableShared* shared = entry->key;
                     if (shared->IsMarked()) {
                         shared->Unmark();
                     } else if (doSweep) {
                         delete shared;
                         i.Remove();
                     }
                 }
             }
 
             if (!isCompartmentGC) {
-                for (auto i = self->mClassInfo2NativeSetMap->RemovingIter(); !i.Done(); i.Next()) {
+                for (auto i = self->mClassInfo2NativeSetMap->Iter(); !i.Done(); i.Next()) {
                     auto entry = static_cast<ClassInfo2NativeSetMap::Entry*>(i.Get());
                     if (!entry->value->IsMarked())
                         i.Remove();
                 }
             }
 
-            for (auto i = self->mNativeSetMap->RemovingIter(); !i.Done(); i.Next()) {
+            for (auto i = self->mNativeSetMap->Iter(); !i.Done(); i.Next()) {
                 auto entry = static_cast<NativeSetMap::Entry*>(i.Get());
                 XPCNativeSet* set = entry->key_value;
                 if (set->IsMarked()) {
                     set->Unmark();
                 } else if (doSweep) {
                     XPCNativeSet::DestroyInstance(set);
                     i.Remove();
                 }
             }
 
-            for (auto i = self->mIID2NativeInterfaceMap->RemovingIter(); !i.Done(); i.Next()) {
+            for (auto i = self->mIID2NativeInterfaceMap->Iter(); !i.Done(); i.Next()) {
                 auto entry = static_cast<IID2NativeInterfaceMap::Entry*>(i.Get());
                 XPCNativeInterface* iface = entry->value;
                 if (iface->IsMarked()) {
                     iface->Unmark();
                 } else if (doSweep) {
                     XPCNativeInterface::DestroyInstance(iface);
                     i.Remove();
                 }
@@ -899,17 +899,17 @@ XPCJSRuntime::FinalizeCallback(JSFreeOp*
             // finalized and destroyed. We *do* know that the protos'
             // JSObjects would not have been finalized if there were any
             // wrappers that referenced the proto but where not themselves
             // slated for finalization in this gc cycle. So... at this point
             // we know that any and all wrappers that might have been
             // referencing the protos in the dying list are themselves dead.
             // So, we can safely delete all the protos in the list.
 
-            for (auto i = self->mDyingWrappedNativeProtoMap->RemovingIter(); !i.Done(); i.Next()) {
+            for (auto i = self->mDyingWrappedNativeProtoMap->Iter(); !i.Done(); i.Next()) {
                 auto entry = static_cast<XPCWrappedNativeProtoMap::Entry*>(i.Get());
                 delete static_cast<const XPCWrappedNativeProto*>(entry->key);
                 i.Remove();
             }
 
             MOZ_ASSERT(self->mGCIsRunning, "bad state");
             self->mGCIsRunning = false;
 
@@ -3653,17 +3653,17 @@ XPCJSRuntime::DebugDump(int16_t depth)
                         mThisTranslatorMap, mThisTranslatorMap->Count()));
 
         XPC_LOG_ALWAYS(("mNativeSetMap @ %x with %d sets(s)",
                         mNativeSetMap, mNativeSetMap->Count()));
 
         // iterate sets...
         if (depth && mNativeSetMap->Count()) {
             XPC_LOG_INDENT();
-            for (auto i = mNativeSetMap->RemovingIter(); !i.Done(); i.Next()) {
+            for (auto i = mNativeSetMap->Iter(); !i.Done(); i.Next()) {
                 auto entry = static_cast<NativeSetMap::Entry*>(i.Get());
                 entry->key_value->DebugDump(depth);
             }
             XPC_LOG_OUTDENT();
         }
 
         XPC_LOG_OUTDENT();
 #endif
--- a/js/xpconnect/src/XPCMaps.h
+++ b/js/xpconnect/src/XPCMaps.h
@@ -145,18 +145,16 @@ public:
                    "problems!");
 #endif
         PL_DHashTableRemove(mTable, wrapper->GetIdentityObject());
     }
 
     inline uint32_t Count() { return mTable->EntryCount(); }
 
     PLDHashTable::Iterator Iter() const { return PLDHashTable::Iterator(mTable); }
-    PLDHashTable::RemovingIterator RemovingIter() { return PLDHashTable::RemovingIterator(mTable); }
-
     size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf);
 
     ~Native2WrappedNativeMap();
 private:
     Native2WrappedNativeMap();    // no implementation
     explicit Native2WrappedNativeMap(int size);
 
     static size_t SizeOfEntryExcludingThis(PLDHashEntryHdr* hdr, mozilla::MallocSizeOf mallocSizeOf, void*);
@@ -258,17 +256,17 @@ public:
     inline void Remove(XPCNativeInterface* iface)
     {
         NS_PRECONDITION(iface,"bad param");
         PL_DHashTableRemove(mTable, iface->GetIID());
     }
 
     inline uint32_t Count() { return mTable->EntryCount(); }
 
-    PLDHashTable::RemovingIterator RemovingIter() { return PLDHashTable::RemovingIterator(mTable); }
+    PLDHashTable::Iterator Iter() { return PLDHashTable::Iterator(mTable); }
 
     size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf);
 
     ~IID2NativeInterfaceMap();
 private:
     IID2NativeInterfaceMap();    // no implementation
     explicit IID2NativeInterfaceMap(int size);
 
@@ -314,17 +312,17 @@ public:
     inline void Remove(nsIClassInfo* info)
     {
         NS_PRECONDITION(info,"bad param");
         PL_DHashTableRemove(mTable, info);
     }
 
     inline uint32_t Count() { return mTable->EntryCount(); }
 
-    PLDHashTable::RemovingIterator RemovingIter() { return PLDHashTable::RemovingIterator(mTable); }
+    PLDHashTable::Iterator Iter() { return PLDHashTable::Iterator(mTable); }
 
     // ClassInfo2NativeSetMap holds pointers to *some* XPCNativeSets.
     // So we don't want to count those XPCNativeSets, because they are better
     // counted elsewhere (i.e. in XPCJSRuntime::mNativeSetMap, which holds
     // pointers to *all* XPCNativeSets).  Hence the "Shallow".
     size_t ShallowSizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf);
 
     ~ClassInfo2NativeSetMap();
@@ -372,17 +370,17 @@ public:
     {
         NS_PRECONDITION(info,"bad param");
         PL_DHashTableRemove(mTable, info);
     }
 
     inline uint32_t Count() { return mTable->EntryCount(); }
 
     PLDHashTable::Iterator Iter() const { return PLDHashTable::Iterator(mTable); }
-    PLDHashTable::RemovingIterator RemovingIter() { return PLDHashTable::RemovingIterator(mTable); }
+    PLDHashTable::Iterator Iter() { return PLDHashTable::Iterator(mTable); }
 
     size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf);
 
     ~ClassInfo2WrappedNativeProtoMap();
 private:
     ClassInfo2WrappedNativeProtoMap();    // no implementation
     explicit ClassInfo2WrappedNativeProtoMap(int size);
 
@@ -443,17 +441,17 @@ public:
 
         XPCNativeSetKey key(set, nullptr, 0);
         PL_DHashTableRemove(mTable, &key);
     }
 
     inline uint32_t Count() { return mTable->EntryCount(); }
 
     PLDHashTable::Iterator Iter() const { return PLDHashTable::Iterator(mTable); }
-    PLDHashTable::RemovingIterator RemovingIter() { return PLDHashTable::RemovingIterator(mTable); }
+    PLDHashTable::Iterator Iter() { return PLDHashTable::Iterator(mTable); }
 
     size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf);
 
     ~NativeSetMap();
 private:
     NativeSetMap();    // no implementation
     explicit NativeSetMap(int size);
 
@@ -540,17 +538,17 @@ public:
     };
 
     static XPCNativeScriptableSharedMap* newMap(int length);
 
     bool GetNewOrUsed(uint32_t flags, char* name, XPCNativeScriptableInfo* si);
 
     inline uint32_t Count() { return mTable->EntryCount(); }
 
-    PLDHashTable::RemovingIterator RemovingIter() { return PLDHashTable::RemovingIterator(mTable); }
+    PLDHashTable::Iterator Iter() { return PLDHashTable::Iterator(mTable); }
 
     ~XPCNativeScriptableSharedMap();
 private:
     XPCNativeScriptableSharedMap();    // no implementation
     explicit XPCNativeScriptableSharedMap(int size);
 private:
     PLDHashTable* mTable;
 };
@@ -581,17 +579,17 @@ public:
     {
         NS_PRECONDITION(proto,"bad param");
         PL_DHashTableRemove(mTable, proto);
     }
 
     inline uint32_t Count() { return mTable->EntryCount(); }
 
     PLDHashTable::Iterator Iter() const { return PLDHashTable::Iterator(mTable); }
-    PLDHashTable::RemovingIterator RemovingIter() { return PLDHashTable::RemovingIterator(mTable); }
+    PLDHashTable::Iterator Iter() { return PLDHashTable::Iterator(mTable); }
 
     ~XPCWrappedNativeProtoMap();
 private:
     XPCWrappedNativeProtoMap();    // no implementation
     explicit XPCWrappedNativeProtoMap(int size);
 private:
     PLDHashTable* mTable;
 };
--- a/js/xpconnect/src/XPCWrappedNativeScope.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeScope.cpp
@@ -646,22 +646,22 @@ XPCWrappedNativeScope::SystemIsBeingShut
 
     for (cur = gDyingScopes; cur; cur = cur->mNext) {
         // Give the Components object a chance to try to clean up.
         if (cur->mComponents)
             cur->mComponents->SystemIsBeingShutDown();
 
         // Walk the protos first. Wrapper shutdown can leave dangling
         // proto pointers in the proto map.
-        for (auto i = cur->mWrappedNativeProtoMap->RemovingIter(); !i.Done(); i.Next()) {
+        for (auto i = cur->mWrappedNativeProtoMap->Iter(); !i.Done(); i.Next()) {
             auto entry = static_cast<ClassInfo2WrappedNativeProtoMap::Entry*>(i.Get());
             entry->value->SystemIsBeingShutDown();
             i.Remove();
         }
-        for (auto i = cur->mWrappedNativeMap->RemovingIter(); !i.Done(); i.Next()) {
+        for (auto i = cur->mWrappedNativeMap->Iter(); !i.Done(); i.Next()) {
             auto entry = static_cast<Native2WrappedNativeMap::Entry*>(i.Get());
             XPCWrappedNative* wrapper = entry->value;
             if (wrapper->IsValid()) {
                 wrapper->SystemIsBeingShutDown();
             }
             i.Remove();
         }
     }
--- a/layout/style/nsRuleNode.cpp
+++ b/layout/style/nsRuleNode.cpp
@@ -9370,17 +9370,17 @@ nsRuleNode::SweepChildren(nsTArray<nsRul
                "missing DestroyIfNotMarked() call");
   NS_ASSERTION(HaveChildren(),
                "why call SweepChildren with no children?");
   uint32_t childrenDestroyed = 0;
   nsRuleNode* survivorsWithChildren = nullptr;
   if (ChildrenAreHashed()) {
     PLDHashTable* children = ChildrenHash();
     uint32_t oldChildCount = children->EntryCount();
-    for (auto iter = children->RemovingIter(); !iter.Done(); iter.Next()) {
+    for (auto iter = children->Iter(); !iter.Done(); iter.Next()) {
       auto entry = static_cast<ChildrenHashEntry*>(iter.Get());
       nsRuleNode* node = entry->mRuleNode;
       if (node->DestroyIfNotMarked()) {
         iter.Remove();
       } else if (node->HaveChildren()) {
         // When children are hashed mNextSibling is not normally used but we
         // use it here to build a list of children that needs to be swept.
         nsRuleNode** headQ = &survivorsWithChildren;
--- a/modules/libpref/prefapi.cpp
+++ b/modules/libpref/prefapi.cpp
@@ -550,17 +550,17 @@ PREF_DeleteBranch(const char *branch_nam
     nsAutoCString branch_dot(branch_name);
     if ((len > 1) && branch_name[len - 1] != '.')
         branch_dot += '.';
 
     /* Delete a branch. Used for deleting mime types */
     const char *to_delete = branch_dot.get();
     MOZ_ASSERT(to_delete);
     len = strlen(to_delete);
-    for (auto iter = gHashTable->RemovingIter(); !iter.Done(); iter.Next()) {
+    for (auto iter = gHashTable->Iter(); !iter.Done(); iter.Next()) {
         auto entry = static_cast<PrefHashEntry*>(iter.Get());
 
         /* note if we're deleting "ldap" then we want to delete "ldap.xxx"
             and "ldap" (if such a leaf node exists) but not "ldap_1.xxx" */
         if (PL_strncmp(entry->key, to_delete, (uint32_t) len) == 0 ||
             (len-1 == (int)strlen(entry->key) &&
              PL_strncmp(entry->key, to_delete, (uint32_t)(len-1)) == 0)) {
             iter.Remove();
@@ -599,17 +599,17 @@ PREF_ClearAllUserPrefs()
 #ifndef MOZ_B2G
     MOZ_ASSERT(NS_IsMainThread());
 #endif
 
     if (!gHashTable)
         return NS_ERROR_NOT_INITIALIZED;
 
     std::vector<std::string> prefStrings;
-    for (auto iter = gHashTable->RemovingIter(); !iter.Done(); iter.Next()) {
+    for (auto iter = gHashTable->Iter(); !iter.Done(); iter.Next()) {
         auto pref = static_cast<PrefHashEntry*>(iter.Get());
 
         if (PREF_HAS_USER_VALUE(pref)) {
             prefStrings.push_back(std::string(pref->key));
 
             pref->flags &= ~PREF_USERSET;
             if (!(pref->flags & PREF_HAS_DEFAULT)) {
                 iter.Remove();
--- a/netwerk/cache/nsCacheEntry.cpp
+++ b/netwerk/cache/nsCacheEntry.cpp
@@ -458,27 +458,21 @@ nsCacheEntryHashTable::RemoveEntry( nsCa
     // XXX debug code to make sure we have the entry we're trying to remove
     nsCacheEntry *check = GetEntry(&(cacheEntry->mKey));
     NS_ASSERTION(check == cacheEntry, "### Attempting to remove unknown cache entry!!!");
 #endif
     PL_DHashTableRemove(&table, &(cacheEntry->mKey));
 }
 
 PLDHashTable::Iterator
-nsCacheEntryHashTable::Iter() const
+nsCacheEntryHashTable::Iter()
 {
     return PLDHashTable::Iterator(&table);
 }
 
-PLDHashTable::RemovingIterator
-nsCacheEntryHashTable::RemovingIter()
-{
-    return PLDHashTable::RemovingIterator(&table);
-}
-
 /**
  *  hash table operation callback functions
  */
 
 PLDHashNumber
 nsCacheEntryHashTable::HashKey( PLDHashTable *table, const void *key)
 {
     return HashString(*static_cast<const nsCString *>(key));
--- a/netwerk/cache/nsCacheEntry.h
+++ b/netwerk/cache/nsCacheEntry.h
@@ -269,18 +269,17 @@ public:
 
     void          Init();
     void          Shutdown();
 
     nsCacheEntry *GetEntry( const nsCString * key);
     nsresult      AddEntry( nsCacheEntry *entry);
     void          RemoveEntry( nsCacheEntry *entry);
 
-    PLDHashTable::Iterator Iter() const;
-    PLDHashTable::RemovingIterator RemovingIter();
+    PLDHashTable::Iterator Iter();
 
 private:
     // PLDHashTable operation callbacks
     static PLDHashNumber  HashKey( PLDHashTable *table, const void *key);
 
     static bool           MatchEntry( PLDHashTable *           table,
                                       const PLDHashEntryHdr *  entry,
                                       const void *             key);
--- a/netwerk/cache/nsCacheService.cpp
+++ b/netwerk/cache/nsCacheService.cpp
@@ -2914,17 +2914,17 @@ nsCacheService::ClearDoomList()
     }
 }
 
 void
 nsCacheService::DoomActiveEntries(DoomCheckFn check)
 {
     nsAutoTArray<nsCacheEntry*, 8> array;
 
-    for (auto iter = mActiveEntries.RemovingIter(); !iter.Done(); iter.Next()) {
+    for (auto iter = mActiveEntries.Iter(); !iter.Done(); iter.Next()) {
         nsCacheEntry* entry =
             static_cast<nsCacheEntryHashTableEntry*>(iter.Get())->cacheEntry;
 
         if (check && !check(entry)) {
             continue;
         }
 
         array.AppendElement(entry);
--- a/netwerk/dns/nsHostResolver.cpp
+++ b/netwerk/dns/nsHostResolver.cpp
@@ -620,17 +620,17 @@ nsHostResolver::FlushCache()
             node = node->next;
             PR_REMOVE_AND_INIT_LINK(rec);
             PL_DHashTableRemove(&mDB, (nsHostKey *) rec);
             NS_RELEASE(rec);
         }
     }
 
     // Refresh the cache entries that are resolving RIGHT now, remove the rest.
-    for (auto iter = mDB.RemovingIter(); !iter.Done(); iter.Next()) {
+    for (auto iter = mDB.Iter(); !iter.Done(); iter.Next()) {
         auto entry = static_cast<nsHostDBEnt *>(iter.Get());
         // Try to remove the record, or mark it for refresh.
         if (entry->rec->RemoveOrRefresh()) {
             PR_REMOVE_LINK(entry->rec);
             iter.Remove();
         }
     }
 }
--- a/rdf/base/nsInMemoryDataSource.cpp
+++ b/rdf/base/nsInMemoryDataSource.cpp
@@ -1803,17 +1803,17 @@ InMemoryDataSource::Sweep()
     return NS_OK;
 }
 
 
 void
 InMemoryDataSource::SweepForwardArcsEntries(PLDHashTable* aTable,
                                             SweepInfo* aInfo)
 {
-    for (auto iter = aTable->RemovingIter(); !iter.Done(); iter.Next()) {
+    for (auto iter = aTable->Iter(); !iter.Done(); iter.Next()) {
         auto entry = static_cast<Entry*>(iter.Get());
 
         Assertion* as = entry->mAssertions;
         if (as && (as->mHashEntry)) {
             // Stuff in sub-hashes must be swept recursively (max depth: 1)
             SweepForwardArcsEntries(as->u.hash.mPropertyHash, aInfo);
 
             // If the sub-hash is now empty, clean it up.
--- a/security/manager/ssl/nsNSSShutDown.cpp
+++ b/security/manager/ssl/nsNSSShutDown.cpp
@@ -172,17 +172,17 @@ nsresult nsNSSShutDownList::evaporateAll
 
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("now evaporating NSS resources\n"));
 
   // Never free more than one entry, because other threads might be calling
   // us and remove themselves while we are iterating over the list,
   // and the behaviour of changing the list while iterating is undefined.
   while (true) {
     MutexAutoLock lock(mListLock);
-    auto iter = mObjects.RemovingIter();
+    auto iter = mObjects.Iter();
     if (iter.Done()) {
       break;
     }
     auto entry = static_cast<ObjectHashEntry*>(iter.Get());
     {
       MutexAutoUnlock unlock(singleton->mListLock);
       entry->obj->shutdown(nsNSSShutDownObject::calledFromList);
     }
--- a/xpcom/glue/nsBaseHashtable.h
+++ b/xpcom/glue/nsBaseHashtable.h
@@ -163,17 +163,17 @@ public:
   /**
    * enumerate entries in the hashtable, without allowing changes
    * @param aEnumFunc enumeration callback
    * @param aUserArg passed unchanged to the EnumReadFunction
    */
   uint32_t EnumerateRead(EnumReadFunction aEnumFunc, void* aUserArg) const
   {
     uint32_t n = 0;
-    for (auto iter = this->mTable.Iter(); !iter.Done(); iter.Next()) {
+    for (auto iter = this->mTable.ConstIter(); !iter.Done(); iter.Next()) {
       auto entry = static_cast<EntryType*>(iter.Get());
       PLDHashOperator op = aEnumFunc(entry->GetKey(), entry->mData, aUserArg);
       n++;
       MOZ_ASSERT(!(op & PL_DHASH_REMOVE));
       if (op & PL_DHASH_STOP) {
         break;
       }
     }
@@ -199,17 +199,17 @@ public:
    * enumerate entries in the hashtable, allowing changes. This
    * functions write-locks the hashtable.
    * @param aEnumFunc enumeration callback
    * @param aUserArg passed unchanged to the EnumFunction
    */
   uint32_t Enumerate(EnumFunction aEnumFunc, void* aUserArg)
   {
     uint32_t n = 0;
-    for (auto iter = this->mTable.RemovingIter(); !iter.Done(); iter.Next()) {
+    for (auto iter = this->mTable.Iter(); !iter.Done(); iter.Next()) {
       auto entry = static_cast<EntryType*>(iter.Get());
       PLDHashOperator op = aEnumFunc(entry->GetKey(), entry->mData, aUserArg);
       n++;
       if (op & PL_DHASH_REMOVE) {
         iter.Remove();
       }
       if (op & PL_DHASH_STOP) {
         break;
--- a/xpcom/glue/nsTHashtable.h
+++ b/xpcom/glue/nsTHashtable.h
@@ -208,17 +208,17 @@ public:
    * @param     enumFunc the <code>Enumerator</code> function to call
    * @param     userArg a pointer to pass to the
    *            <code>Enumerator</code> function
    * @return    the number of entries actually enumerated
    */
   uint32_t EnumerateEntries(Enumerator aEnumFunc, void* aUserArg)
   {
     uint32_t n = 0;
-    for (auto iter = mTable.RemovingIter(); !iter.Done(); iter.Next()) {
+    for (auto iter = mTable.Iter(); !iter.Done(); iter.Next()) {
       auto entry = static_cast<EntryType*>(iter.Get());
       PLDHashOperator op = aEnumFunc(entry, aUserArg);
       n++;
       if (op & PL_DHASH_REMOVE) {
         iter.Remove();
       }
       if (op & PL_DHASH_STOP) {
         break;
--- a/xpcom/glue/pldhash.cpp
+++ b/xpcom/glue/pldhash.cpp
@@ -748,17 +748,17 @@ PLDHashTable::SizeOfExcludingThis(
 #endif
 
   if (!mEntryStore) {
     return 0;
   }
 
   size_t n = aMallocSizeOf(mEntryStore);
   if (aSizeOfEntryExcludingThis) {
-    for (auto iter = Iter(); !iter.Done(); iter.Next()) {
+    for (auto iter = ConstIter(); !iter.Done(); iter.Next()) {
       n += aSizeOfEntryExcludingThis(iter.Get(), aMallocSizeOf, aArg);
     }
   }
 
   return n;
 }
 
 MOZ_ALWAYS_INLINE size_t
@@ -789,45 +789,51 @@ PL_DHashTableSizeOfIncludingThis(
   return aTable->SizeOfIncludingThis(aSizeOfEntryExcludingThis,
                                      aMallocSizeOf, aArg);
 }
 
 PLDHashTable::Iterator::Iterator(Iterator&& aOther)
   : mTable(aOther.mTable)
   , mCurrent(aOther.mCurrent)
   , mLimit(aOther.mLimit)
+  , mHaveRemoved(aOther.mHaveRemoved)
 {
   // No need to change |mChecker| here.
   aOther.mTable = nullptr;
   aOther.mCurrent = nullptr;
   aOther.mLimit = nullptr;
+  aOther.mHaveRemoved = false;
 }
 
-PLDHashTable::Iterator::Iterator(const PLDHashTable* aTable)
+PLDHashTable::Iterator::Iterator(PLDHashTable* aTable)
   : mTable(aTable)
   , mCurrent(mTable->mEntryStore)
   , mLimit(mTable->mEntryStore + mTable->Capacity() * mTable->mEntrySize)
+  , mHaveRemoved(false)
 {
 #ifdef DEBUG
   mTable->mChecker.StartReadOp();
 #endif
 
   // Advance to the first live entry, or to the end if there are none.
   while (IsOnNonLiveEntry()) {
     mCurrent += mTable->mEntrySize;
   }
 }
 
 PLDHashTable::Iterator::~Iterator()
 {
+  if (mTable) {
+    if (mHaveRemoved) {
+      mTable->ShrinkIfAppropriate();
+    }
 #ifdef DEBUG
-  if (mTable) {
     mTable->mChecker.EndReadOp();
+#endif
   }
-#endif
 }
 
 bool
 PLDHashTable::Iterator::Done() const
 {
   return mCurrent == mLimit;
 }
 
@@ -852,51 +858,21 @@ PLDHashTable::Iterator::Next()
 {
   MOZ_ASSERT(!Done());
 
   do {
     mCurrent += mTable->mEntrySize;
   } while (IsOnNonLiveEntry());
 }
 
-PLDHashTable::RemovingIterator::RemovingIterator(RemovingIterator&& aOther)
-  : Iterator(mozilla::Move(aOther))
-  , mHaveRemoved(aOther.mHaveRemoved)
-{
-  // Do nothing with mChecker here. We treat RemovingIterator like Iterator --
-  // i.e. as a read operation -- until the very end. Then, if any elements have
-  // been removed, we temporarily treat it as a write operation.
-}
-
-PLDHashTable::RemovingIterator::RemovingIterator(PLDHashTable* aTable)
-  : Iterator(aTable)
-  , mHaveRemoved(false)
-{
-}
-
-PLDHashTable::RemovingIterator::~RemovingIterator()
-{
-  if (mHaveRemoved) {
-#ifdef DEBUG
-    AutoIteratorRemovalOp op(mTable->mChecker);
-#endif
-
-    // Why is this cast needed? In Iterator, |mTable| is const. In
-    // RemovingIterator it should be non-const, but it inherits from Iterator
-    // so that's not possible. But it's ok because RemovingIterator's
-    // constructor takes a pointer to a non-const table in the first place.
-    const_cast<PLDHashTable*>(mTable)->ShrinkIfAppropriate();
-  }
-}
-
 void
-PLDHashTable::RemovingIterator::Remove()
+PLDHashTable::Iterator::Remove()
 {
   // This cast is needed for the same reason as the one in the destructor.
-  const_cast<PLDHashTable*>(mTable)->RawRemove(Get());
+  mTable->RawRemove(Get());
   mHaveRemoved = true;
 }
 
 #ifdef DEBUG
 MOZ_ALWAYS_INLINE void
 PLDHashTable::MarkImmutable()
 {
   mChecker.SetNonWritable();
--- a/xpcom/glue/pldhash.h
+++ b/xpcom/glue/pldhash.h
@@ -343,91 +343,81 @@ public:
 #ifdef DEBUG
   void MarkImmutable();
 #endif
 
   void MoveEntryStub(const PLDHashEntryHdr* aFrom, PLDHashEntryHdr* aTo);
 
   void ClearEntryStub(PLDHashEntryHdr* aEntry);
 
-  // This is an iterator for PLDHashtable. It is not safe to modify the
-  // table while it is being iterated over; on debug builds, attempting to do
-  // so will result in an assertion failure.
+  // This is an iterator for PLDHashtable. Assertions will detect some, but not
+  // all, mid-iteration table modifications that might invalidate (e.g.
+  // reallocate) the entry storage.
+  //
+  // Any element can be removed during iteration using Remove(). If any
+  // elements are removed, the table may be resized once iteration ends.
   //
   // Example usage:
   //
   //   for (auto iter = table.Iter(); !iter.Done(); iter.Next()) {
   //     auto entry = static_cast<FooEntry*>(iter.Get());
   //     // ... do stuff with |entry| ...
+  //     // ... possibly call iter.Remove() once ...
   //   }
   //
   // or:
   //
   //   for (PLDHashTable::Iterator iter(&table); !iter.Done(); iter.Next()) {
   //     auto entry = static_cast<FooEntry*>(iter.Get());
   //     // ... do stuff with |entry| ...
+  //     // ... possibly call iter.Remove() once ...
   //   }
   //
   // The latter form is more verbose but is easier to work with when
   // making subclasses of Iterator.
   //
   class Iterator
   {
   public:
-    explicit Iterator(const PLDHashTable* aTable);
+    explicit Iterator(PLDHashTable* aTable);
     Iterator(Iterator&& aOther);
     ~Iterator();
+
     bool Done() const;                // Have we finished?
     PLDHashEntryHdr* Get() const;     // Get the current entry.
     void Next();                      // Advance to the next entry.
 
+    // Remove the current entry. Must only be called once per entry, and Get()
+    // must not be called on that entry afterwards.
+    void Remove();
+
   protected:
-    const PLDHashTable* mTable;       // Main table pointer.
+    PLDHashTable* mTable;             // Main table pointer.
 
   private:
     char* mCurrent;                   // Pointer to the current entry.
     char* mLimit;                     // One past the last entry.
 
+    bool mHaveRemoved;                // Have any elements been removed?
+
     bool IsOnNonLiveEntry() const;
 
     Iterator() = delete;
     Iterator(const Iterator&) = delete;
     Iterator& operator=(const Iterator&) = delete;
     Iterator& operator=(const Iterator&&) = delete;
   };
 
-  Iterator Iter() const { return Iterator(this); }
-
-  // This is an iterator that allows elements to be removed during iteration.
-  // If any elements are removed, the table may be resized once iteration ends.
-  // Its usage is similar to that of Iterator, with the addition that Remove()
-  // can be called once per element.
-  class RemovingIterator : public Iterator
-  {
-  public:
-    explicit RemovingIterator(PLDHashTable* aTable);
-    RemovingIterator(RemovingIterator&& aOther);
-    ~RemovingIterator();
+  Iterator Iter() { return Iterator(this); }
 
-    // Remove the current entry. Must only be called once per entry, and Get()
-    // must not be called on that entry afterwards.
-    void Remove();
-
-  private:
-    bool mHaveRemoved;      // Have any elements been removed?
-
-    RemovingIterator() = delete;
-    RemovingIterator(const RemovingIterator&) = delete;
-    RemovingIterator& operator=(const RemovingIterator&) = delete;
-    RemovingIterator& operator=(const RemovingIterator&&) = delete;
-  };
-
-  RemovingIterator RemovingIter() const
+  // Use this if you need to initialize an Iterator in a const method. If you
+  // use this case, you should not call Remove() on the iterator.
+  Iterator ConstIter() const
   {
-    return RemovingIterator(const_cast<PLDHashTable*>(this));
+    return Iterator(const_cast<PLDHashTable*>(this));
   }
 
 private:
   static bool EntryIsFree(PLDHashEntryHdr* aEntry);
 
   // We store mHashShift rather than sizeLog2 to optimize the collision-free
   // case in SearchTable.
   uint32_t CapacityFromHashShift() const
--- a/xpcom/tests/gtest/TestPLDHash.cpp
+++ b/xpcom/tests/gtest/TestPLDHash.cpp
@@ -46,20 +46,16 @@ TEST(PLDHashTableTest, LazyStorage)
 
   // No result to check here, but call it to make sure it doesn't crash.
   PL_DHashTableRemove(&t, (const void*)2);
 
   for (auto iter = t.Iter(); !iter.Done(); iter.Next()) {
     ASSERT_TRUE(false); // shouldn't hit this on an empty table
   }
 
-  for (auto iter = t.RemovingIter(); !iter.Done(); iter.Next()) {
-    ASSERT_TRUE(false); // shouldn't hit this on an empty table
-  }
-
   // Using a null |mallocSizeOf| should be fine because it shouldn't be called
   // for an empty table.
   mozilla::MallocSizeOf mallocSizeOf = nullptr;
   ASSERT_EQ(PL_DHashTableSizeOfExcludingThis(&t, nullptr, mallocSizeOf), 0u);
 }
 
 // A trivial hash function is good enough here. It's also super-fast for
 // test_pldhash_grow_to_max_capacity() because we insert the integers 0..,
@@ -180,71 +176,60 @@ TEST(PLDHashTableIterator, Iterator)
       saw88 = true;
     }
     if (entry->key == (const void*)99) {
       saw99 = true;
     }
     n++;
   }
   ASSERT_TRUE(saw77 && saw88 && saw99 && n == 3);
-}
 
-TEST(PLDHashTableTest, RemovingIterator)
-{
-  PLDHashTable t(&trivialOps, sizeof(PLDHashEntryStub));
-
-  // Explicitly test the move constructor. We do this because, due to copy
-  // elision, compilers might optimize away move constructor calls for normal
-  // iterator use.
-  {
-    PLDHashTable::RemovingIterator iter1(&t);
-    PLDHashTable::RemovingIterator iter2(mozilla::Move(iter1));
-  }
+  t.Clear();
 
   // First, we insert 64 items, which results in a capacity of 128, and a load
   // factor of 50%.
   for (intptr_t i = 0; i < 64; i++) {
     PL_DHashTableAdd(&t, (const void*)i);
   }
   ASSERT_EQ(t.EntryCount(), 64u);
   ASSERT_EQ(t.Capacity(), 128u);
 
   // The first removing iterator does no removing; capacity and entry count are
   // unchanged.
-  for (PLDHashTable::RemovingIterator iter(&t); !iter.Done(); iter.Next()) {
+  for (PLDHashTable::Iterator iter(&t); !iter.Done(); iter.Next()) {
     (void) iter.Get();
   }
   ASSERT_EQ(t.EntryCount(), 64u);
   ASSERT_EQ(t.Capacity(), 128u);
 
   // The second removing iterator removes 16 items. This reduces the load
   // factor to 37.5% (48 / 128), which isn't low enough to shrink the table.
-  for (auto iter = t.RemovingIter(); !iter.Done(); iter.Next()) {
+  for (auto iter = t.Iter(); !iter.Done(); iter.Next()) {
     auto entry = static_cast<PLDHashEntryStub*>(iter.Get());
     if ((intptr_t)(entry->key) % 4 == 0) {
       iter.Remove();
     }
   }
   ASSERT_EQ(t.EntryCount(), 48u);
   ASSERT_EQ(t.Capacity(), 128u);
 
   // The third removing iterator removes another 16 items. This reduces
   // the load factor to 25% (32 / 128), so the table is shrunk.
-  for (auto iter = t.RemovingIter(); !iter.Done(); iter.Next()) {
+  for (auto iter = t.Iter(); !iter.Done(); iter.Next()) {
     auto entry = static_cast<PLDHashEntryStub*>(iter.Get());
     if ((intptr_t)(entry->key) % 2 == 0) {
       iter.Remove();
     }
   }
   ASSERT_EQ(t.EntryCount(), 32u);
   ASSERT_EQ(t.Capacity(), 64u);
 
   // The fourth removing iterator removes all remaining items. This reduces
   // the capacity to the minimum.
-  for (auto iter = t.RemovingIter(); !iter.Done(); iter.Next()) {
+  for (auto iter = t.Iter(); !iter.Done(); iter.Next()) {
     iter.Remove();
   }
   ASSERT_EQ(t.EntryCount(), 0u);
   ASSERT_EQ(t.Capacity(), unsigned(PL_DHASH_MIN_CAPACITY));
 }
 
 // See bug 931062, we skip this test on Android due to OOM. Also, it's slow,
 // and so should always be last.