Bug 1376498 part 1 - Unify the ns[Base|Interface|RefPtr]Hashtable::Remove() signatures for consistency. Make it return true if an entry was removed, with an optional out param to move the value. r=froydnj
authorMats Palmgren <mats@mozilla.com>
Wed, 05 Jul 2017 02:01:44 +0200
changeset 367275 1ed4185c7b73685f8971e111cc5cdd02464cce9c
parent 367274 2cb8b8e9d1e978c4824c4daa5ca44d533e2eac4f
child 367276 d46d1d8506ad2e87db24da457da24722810a3e1b
push id92181
push usermpalmgren@mozilla.com
push dateWed, 05 Jul 2017 00:01:54 +0000
treeherdermozilla-inbound@4ccb89990d13 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1376498
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 1376498 part 1 - Unify the ns[Base|Interface|RefPtr]Hashtable::Remove() signatures for consistency. Make it return true if an entry was removed, with an optional out param to move the value. r=froydnj MozReview-Commit-ID: JVciibiSPL5
xpcom/ds/nsBaseHashtable.h
xpcom/ds/nsInterfaceHashtable.h
xpcom/ds/nsRefPtrHashtable.h
--- a/xpcom/ds/nsBaseHashtable.h
+++ b/xpcom/ds/nsBaseHashtable.h
@@ -149,20 +149,38 @@ public:
     }
 
     ent->mData = aData;
 
     return true;
   }
 
   /**
-   * remove the data for the associated key
+   * Remove the entry associated with aKey (if any), optionally _moving_ its
+   * current value into *aData.  Return true if found.
    * @param aKey the key to remove from the hashtable
+   * @param aData where to move the value (if non-null).  If an entry is not
+   *              found, *aData will be assigned a default-constructed value
+   *              (i.e. reset to zero or nullptr for primitive types).
+   * @return true if an entry for aKey was found (and removed)
    */
-  void Remove(KeyType aKey) { this->RemoveEntry(aKey); }
+  bool Remove(KeyType aKey, DataType* aData = nullptr)
+  {
+    if (auto* ent = this->GetEntry(aKey)) {
+      if (aData) {
+        *aData = mozilla::Move(ent->mData);
+      }
+      this->RemoveEntry(ent);
+      return true;
+    }
+    if (aData) {
+      *aData = mozilla::Move(DataType());
+    }
+    return false;
+  }
 
   struct LookupResult {
   private:
     EntryType* mEntry;
     nsBaseHashtable& mTable;
 #ifdef DEBUG
     uint32_t mTableGeneration;
 #endif
--- a/xpcom/ds/nsInterfaceHashtable.h
+++ b/xpcom/ds/nsInterfaceHashtable.h
@@ -48,16 +48,27 @@ public:
 
   /**
    * Gets a weak reference to the hashtable entry.
    * @param aFound If not nullptr, will be set to true if the entry is found,
    *               to false otherwise.
    * @return The entry, or nullptr if not found. Do not release this pointer!
    */
   Interface* GetWeak(KeyType aKey, bool* aFound = nullptr) const;
+
+  /**
+   * Remove the entry associated with aKey (if any), optionally _moving_ its
+   * current value into *aData, thereby avoiding calls to AddRef and Release.
+   * Return true if found.
+   * @param aKey the key to remove from the hashtable
+   * @param aData where to move the value (if non-null).  If an entry is not
+   *              found it will be set to nullptr.
+   * @return true if an entry for aKey was found (and removed)
+   */
+  inline bool Remove(KeyType aKey, Interface** aData = nullptr);
 };
 
 template<typename K, typename T>
 inline void
 ImplCycleCollectionUnlink(nsInterfaceHashtable<K, T>& aField)
 {
   aField.Clear();
 }
@@ -134,9 +145,30 @@ nsInterfaceHashtable<KeyClass, Interface
 
   // Key does not exist, return nullptr and set aFound to false
   if (aFound) {
     *aFound = false;
   }
   return nullptr;
 }
 
+template<class KeyClass, class Interface>
+bool
+nsInterfaceHashtable<KeyClass, Interface>::Remove(KeyType aKey,
+                                                  Interface** aData)
+{
+  typename base_type::EntryType* ent = this->GetEntry(aKey);
+
+  if (ent) {
+    if (aData) {
+      ent->mData.forget(aData);
+    }
+    this->RemoveEntry(ent);
+    return true;
+  }
+
+  if (aData) {
+    *aData = nullptr;
+  }
+  return false;
+}
+
 #endif // nsInterfaceHashtable_h__
--- a/xpcom/ds/nsRefPtrHashtable.h
+++ b/xpcom/ds/nsRefPtrHashtable.h
@@ -52,27 +52,26 @@ public:
   // Overload Put, rather than overriding it.
   using base_type::Put;
 
   void Put(KeyType aKey, already_AddRefed<PtrType> aData);
 
   MOZ_MUST_USE bool Put(KeyType aKey, already_AddRefed<PtrType> aData,
                         const mozilla::fallible_t&);
 
-  // Overload Remove, rather than overriding it.
-  using base_type::Remove;
-
   /**
-   * Remove the data for the associated key, swapping the current value into
-   * pData, thereby avoiding calls to AddRef and Release.
+   * Remove the entry associated with aKey (if any), optionally _moving_ its
+   * current value into *aData, thereby avoiding calls to AddRef and Release.
+   * Return true if found.
    * @param aKey the key to remove from the hashtable
-   * @param aData This is an XPCOM getter, so aData is already_addrefed.
-   *   If the key doesn't exist, aData will be set to nullptr. Must be non-null.
+   * @param aData where to move the value (if non-null).  If an entry is not
+   *              found it will be set to nullptr.
+   * @return true if an entry for aKey was found (and removed)
    */
-  bool Remove(KeyType aKey, UserDataType* aData);
+  inline bool Remove(KeyType aKey, UserDataType* aData = nullptr);
 };
 
 template<typename K, typename T>
 inline void
 ImplCycleCollectionUnlink(nsRefPtrHashtable<K, T>& aField)
 {
   aField.Clear();
 }
@@ -168,24 +167,25 @@ nsRefPtrHashtable<KeyClass, PtrType>::Pu
   return true;
 }
 
 template<class KeyClass, class PtrType>
 bool
 nsRefPtrHashtable<KeyClass, PtrType>::Remove(KeyType aKey,
                                              UserDataType* aRefPtr)
 {
-  MOZ_ASSERT(aRefPtr);
   typename base_type::EntryType* ent = this->GetEntry(aKey);
 
   if (ent) {
-    ent->mData.forget(aRefPtr);
+    if (aRefPtr) {
+      ent->mData.forget(aRefPtr);
+    }
     this->RemoveEntry(ent);
     return true;
   }
 
-  // If the key doesn't exist, set *aRefPtr to null
-  // so that it is a valid XPCOM getter.
-  *aRefPtr = nullptr;
+  if (aRefPtr) {
+    *aRefPtr = nullptr;
+  }
   return false;
 }
 
 #endif // nsRefPtrHashtable_h__