Bug 1372317 part 3 - Remove nsBaseHashtable::LookupRemoveIf() since it's not used anymore. r=froydnj
authorMats Palmgren <mats@mozilla.com>
Sun, 18 Jun 2017 17:07:54 +0200
changeset 364621 4e2d838994772117314888a99bc71fc4ee822f30
parent 364620 ab565909b1d0e52c55a272251cb91d98ae8a36a8
child 364622 3b544d62b76798ecdd553cba7a87be92012dd939
push id32050
push usercbook@mozilla.com
push dateMon, 19 Jun 2017 11:37:33 +0000
treeherdermozilla-central@d39cd452b52b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1372317
milestone56.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 1372317 part 3 - Remove nsBaseHashtable::LookupRemoveIf() since it's not used anymore. r=froydnj Also, change the gtest to test Lookup() and Lookup().Remove() in various forms. MozReview-Commit-ID: 6AguNDhcR5W
xpcom/ds/nsBaseHashtable.h
xpcom/tests/gtest/TestHashtables.cpp
--- a/xpcom/ds/nsBaseHashtable.h
+++ b/xpcom/ds/nsBaseHashtable.h
@@ -154,51 +154,16 @@ public:
   }
 
   /**
    * remove the data for the associated key
    * @param aKey the key to remove from the hashtable
    */
   void Remove(KeyType aKey) { this->RemoveEntry(aKey); }
 
-  /**
-   * Looks up aKey in the hashtable and if found calls the given callback
-   * aFunction with the value.  If the callback returns true then the entry
-   * is removed.  If aKey doesn't exist nothing happens.
-   * The hashtable must not be modified in the callback function.
-   *
-   * A typical usage of this API looks like this:
-   *
-   *   table.LookupRemoveIf(key, [](T* aValue) {
-   *     // ... do stuff using aValue ...
-   *     return aValue->IsEmpty(); // or some other condition to remove it
-   *   });
-   *
-   * This is useful for cases where you want to lookup and possibly modify
-   * the value and then maybe remove the entry but would like to avoid two
-   * hashtable lookups.
-   */
-  template<class F>
-  void LookupRemoveIf(KeyType aKey, F aFunction)
-  {
-#ifdef DEBUG
-    auto tableGeneration = GetGeneration();
-#endif
-    EntryType* ent = this->GetEntry(aKey);
-    if (!ent) {
-      return;
-    }
-    bool shouldRemove = aFunction(ent->mData);
-    MOZ_ASSERT(tableGeneration == GetGeneration(),
-               "hashtable was modified by the LookupRemoveIf callback!");
-    if (shouldRemove) {
-      this->RemoveEntry(ent);
-    }
-  }
-
   struct LookupResult {
   private:
     EntryType* mEntry;
     nsBaseHashtable& mTable;
 #ifdef DEBUG
     uint32_t mTableGeneration;
 #endif
 
--- a/xpcom/tests/gtest/TestHashtables.cpp
+++ b/xpcom/tests/gtest/TestHashtables.cpp
@@ -446,36 +446,37 @@ TEST(Hashtables, DataHashtable_LookupFor
   }
 
   for (auto& entity : gEntities) {
     ASSERT_TRUE(UniToEntity.LookupForAdd(entity.mUnicode));
   }
 
   // 0 should not be found
   size_t count = UniToEntity.Count();
-  UniToEntity.LookupRemoveIf(0U,
-    [] (const char*) { return true; });
+  UniToEntity.Lookup(0U).Remove();
   ASSERT_TRUE(count == UniToEntity.Count());
 
-  // LookupRemoveIf should find all but remove none.
+  // Lookup should find all entries
   count = 0;
   for (auto& entity : gEntities) {
-    UniToEntity.LookupRemoveIf(entity.mUnicode,
-      [&count] (const char*) { count++; return false; });
+    if (UniToEntity.Lookup(entity.mUnicode)) {
+      count++;
+    }
   }
   ASSERT_TRUE(count == UniToEntity.Count());
 
   for (auto& entity : gEntities) {
     ASSERT_TRUE(UniToEntity.LookupForAdd(entity.mUnicode));
   }
 
-  // LookupRemoveIf should remove all entries.
+  // Lookup().Remove() should remove all entries.
   for (auto& entity : gEntities) {
-    UniToEntity.LookupRemoveIf(entity.mUnicode,
-      [] (const char*) { return true; });
+    if (auto entry = UniToEntity.Lookup(entity.mUnicode)) {
+      entry.Remove();
+    }
   }
   ASSERT_TRUE(0 == UniToEntity.Count());
 }
 
 TEST(Hashtables, ClassHashtable_LookupForAdd)
 {
   // check a class-hashtable LookupForAdd with null values
   nsClassHashtable<nsCStringHashKey,TestUniChar> EntToUniClass(ENTITY_COUNT);
@@ -490,31 +491,32 @@ TEST(Hashtables, ClassHashtable_LookupFo
 
   for (auto& entity : gEntities) {
     ASSERT_TRUE(EntToUniClass.LookupForAdd(nsDependentCString(entity.mStr)));
     ASSERT_TRUE(EntToUniClass.LookupForAdd(nsDependentCString(entity.mStr)).Data() == nullptr);
   }
 
   // "" should not be found
   size_t count = EntToUniClass.Count();
-  EntToUniClass.LookupRemoveIf(nsDependentCString(""),
-    [] (const TestUniChar*) { return true; });
+  EntToUniClass.Lookup(nsDependentCString("")).Remove();
   ASSERT_TRUE(count == EntToUniClass.Count());
 
-  // LookupRemoveIf should find all but remove none.
+  // Lookup should find all entries.
   count = 0;
   for (auto& entity : gEntities) {
-    EntToUniClass.LookupRemoveIf(nsDependentCString(entity.mStr),
-      [&count] (const TestUniChar*) { count++; return false; });
+    if (EntToUniClass.Lookup(nsDependentCString(entity.mStr))) {
+      count++;
+    }
   }
   ASSERT_TRUE(count == EntToUniClass.Count());
 
   for (auto& entity : gEntities) {
     ASSERT_TRUE(EntToUniClass.LookupForAdd(nsDependentCString(entity.mStr)));
   }
 
-  // LookupRemoveIf should remove all entries.
+  // Lookup().Remove() should remove all entries.
   for (auto& entity : gEntities) {
-    EntToUniClass.LookupRemoveIf(nsDependentCString(entity.mStr),
-      [] (const TestUniChar*) { return true; });
+    if (auto entry = EntToUniClass.Lookup(nsDependentCString(entity.mStr))) {
+      entry.Remove();
+    }
   }
   ASSERT_TRUE(0 == EntToUniClass.Count());
 }