From ebabc765c0e00045d82b5707964d227ab3535355 Mon Sep 17 00:00:00 2001
From: Nicholas Nethercote <nnethercote@mozilla.com>
Date: Thu, 3 Dec 2015 14:00:02 -0800
Subject: [PATCH] Bug 1187134 (part 3) - Replace nsBaseHashtable::Enumerate()
 calls in netwerk/cache{,2}/ with iterators. r=valentin.

---
 netwerk/cache2/CacheFile.cpp  | 96 +++++++++++++++--------------------
 netwerk/cache2/CacheFile.h    |  9 ----
 netwerk/cache2/CacheIndex.cpp |  7 ++-
 3 files changed, 45 insertions(+), 67 deletions(-)

diff --git a/netwerk/cache2/CacheFile.cpp b/netwerk/cache2/CacheFile.cpp
index 31e124249ed..224523ab190 100644
--- a/netwerk/cache2/CacheFile.cpp
+++ b/netwerk/cache2/CacheFile.cpp
@@ -547,7 +547,25 @@ CacheFile::OnFileOpened(CacheFileHandle *aHandle, nsresult aResult)
         mMetadata->SetHandle(mHandle);
 
         // Write all cached chunks, otherwise they may stay unwritten.
-        mCachedChunks.Enumerate(&CacheFile::WriteAllCachedChunks, this);
+        for (auto iter = mCachedChunks.Iter(); !iter.Done(); iter.Next()) {
+          uint32_t idx = iter.Key();
+          const RefPtr<CacheFileChunk>& chunk = iter.Data();
+
+          LOG(("CacheFile::OnFileOpened() - write [this=%p, idx=%u, chunk=%p]",
+               this, idx, chunk.get()));
+
+          mChunks.Put(idx, chunk);
+          chunk->mFile = this;
+          chunk->mActiveChunk = true;
+
+          MOZ_ASSERT(chunk->IsReady());
+
+          // This would be cleaner if we had an nsRefPtr constructor that took
+          // a RefPtr<Derived>.
+          ReleaseOutsideLock(RefPtr<nsISupports>(chunk));
+
+          iter.Remove();
+        }
 
         return NS_OK;
       }
@@ -1714,8 +1732,29 @@ CacheFile::NotifyListenersAboutOutputRemoval()
   AssertOwnsLock();
 
   // First fail all chunk listeners that wait for non-existent chunk
-  mChunkListeners.Enumerate(&CacheFile::FailListenersIfNonExistentChunk,
-                            this);
+  for (auto iter = mChunkListeners.Iter(); !iter.Done(); iter.Next()) {
+    uint32_t idx = iter.Key();
+    nsAutoPtr<ChunkListeners>& listeners = iter.Data();
+
+    LOG(("CacheFile::NotifyListenersAboutOutputRemoval() - fail "
+         "[this=%p, idx=%u]", this, idx));
+
+    RefPtr<CacheFileChunk> chunk;
+    mChunks.Get(idx, getter_AddRefs(chunk));
+    if (chunk) {
+      MOZ_ASSERT(!chunk->IsReady());
+      continue;
+    }
+
+    for (uint32_t i = 0 ; i < listeners->mItems.Length() ; i++) {
+      ChunkListenerItem *item = listeners->mItems[i];
+      NotifyChunkListener(item->mCallback, item->mTarget,
+                          NS_ERROR_NOT_AVAILABLE, idx, nullptr);
+      delete item;
+    }
+
+    iter.Remove();
+  }
 
   // Fail all update listeners
   mChunks.Enumerate(&CacheFile::FailUpdateListeners, this);
@@ -1839,57 +1878,6 @@ CacheFile::PostWriteTimer()
   CacheFileIOManager::ScheduleMetadataWrite(this);
 }
 
-PLDHashOperator
-CacheFile::WriteAllCachedChunks(const uint32_t& aIdx,
-                                RefPtr<CacheFileChunk>& aChunk,
-                                void* aClosure)
-{
-  CacheFile *file = static_cast<CacheFile*>(aClosure);
-
-  LOG(("CacheFile::WriteAllCachedChunks() [this=%p, idx=%u, chunk=%p]",
-       file, aIdx, aChunk.get()));
-
-  file->mChunks.Put(aIdx, aChunk);
-  aChunk->mFile = file;
-  aChunk->mActiveChunk = true;
-
-  MOZ_ASSERT(aChunk->IsReady());
-
-  // this would be cleaner if we had an nsRefPtr constructor
-  // that took a RefPtr<Derived>
-  file->ReleaseOutsideLock(RefPtr<nsISupports>(aChunk));
-
-  return PL_DHASH_REMOVE;
-}
-
-PLDHashOperator
-CacheFile::FailListenersIfNonExistentChunk(
-  const uint32_t& aIdx,
-  nsAutoPtr<ChunkListeners>& aListeners,
-  void* aClosure)
-{
-  CacheFile *file = static_cast<CacheFile*>(aClosure);
-
-  LOG(("CacheFile::FailListenersIfNonExistentChunk() [this=%p, idx=%u]",
-       file, aIdx));
-
-  RefPtr<CacheFileChunk> chunk;
-  file->mChunks.Get(aIdx, getter_AddRefs(chunk));
-  if (chunk) {
-    MOZ_ASSERT(!chunk->IsReady());
-    return PL_DHASH_NEXT;
-  }
-
-  for (uint32_t i = 0 ; i < aListeners->mItems.Length() ; i++) {
-    ChunkListenerItem *item = aListeners->mItems[i];
-    file->NotifyChunkListener(item->mCallback, item->mTarget,
-                              NS_ERROR_NOT_AVAILABLE, aIdx, nullptr);
-    delete item;
-  }
-
-  return PL_DHASH_REMOVE;
-}
-
 PLDHashOperator
 CacheFile::FailUpdateListeners(
   const uint32_t& aIdx,
diff --git a/netwerk/cache2/CacheFile.h b/netwerk/cache2/CacheFile.h
index 878119b9b26..b2ded77dc6d 100644
--- a/netwerk/cache2/CacheFile.h
+++ b/netwerk/cache2/CacheFile.h
@@ -169,15 +169,6 @@ private:
   void WriteMetadataIfNeededLocked(bool aFireAndForget = false);
   void PostWriteTimer();
 
-  static PLDHashOperator WriteAllCachedChunks(const uint32_t& aIdx,
-                                              RefPtr<CacheFileChunk>& aChunk,
-                                              void* aClosure);
-
-  static PLDHashOperator FailListenersIfNonExistentChunk(
-                           const uint32_t& aIdx,
-                           nsAutoPtr<mozilla::net::ChunkListeners>& aListeners,
-                           void* aClosure);
-
   static PLDHashOperator FailUpdateListeners(const uint32_t& aIdx,
                                              RefPtr<CacheFileChunk>& aChunk,
                                              void* aClosure);
diff --git a/netwerk/cache2/CacheIndex.cpp b/netwerk/cache2/CacheIndex.cpp
index 6c93e005528..e88b9a1d3cd 100644
--- a/netwerk/cache2/CacheIndex.cpp
+++ b/netwerk/cache2/CacheIndex.cpp
@@ -110,10 +110,9 @@ public:
     }
   }
 
-  // We cannot rely on nsTHashtable::GetEntry() in case we are enumerating the
-  // entries and returning PL_DHASH_REMOVE. Destructor is called before the
-  // entry is removed. Caller must call one of following methods to skip
-  // lookup in the hashtable.
+  // We cannot rely on nsTHashtable::GetEntry() in case we are removing entries
+  // while iterating. Destructor is called before the entry is removed. Caller
+  // must call one of following methods to skip lookup in the hashtable.
   void DoNotSearchInIndex()   { mDoNotSearchInIndex = true; }
   void DoNotSearchInUpdates() { mDoNotSearchInUpdates = true; }