Change ScopedPtrHashMap's 2nd template parameter

base/containers/scoped_ptr_hash_map.h

Index: base/containers/scoped_ptr_hash_map.h
diff --git a/base/containers/scoped_ptr_hash_map.h b/base/containers/scoped_ptr_hash_map.h
index dedf21365bee5eea44d1a4522d777e05736df558..749e0c99270bfc8942e27df867f51746467e26dd 100644
--- a/base/containers/scoped_ptr_hash_map.h
+++ b/base/containers/scoped_ptr_hash_map.h
@@ -16,10 +16,12 @@
 
 namespace base {
 
-// This type acts like a hash_map<K, scoped_ptr<V> >, based on top of
+// This type acts like a hash_map<K, scoped_ptr<V, D> >, based on top of

[danakj, 2015/04/23 20:36:22]

I don't see this Deleter thing for hash_map, nor for unordered_map which we will
replace this class with in the future.

I don't think we should diverge from unordered_map here, we'll have to undo it
in the future.

[danakj, 2015/04/23 20:37:10]

Oh, never mind, I see this is an argument to scoped_ptr instead. 

[kcwu, 2015/04/24 06:30:28]

Acknowledged.

 // base::hash_map. The ScopedPtrHashMap has ownership of all values in the data
 // structure.
-template <typename Key, typename Value>
+template <typename Key,
+          typename Value,

[danakj, 2015/04/23 20:54:51]

Maybe we can take this opportunity to make this more like unordered_map though,
changing the Value to be a scoped_ptr<T,D> instead of a T.

So callers would change to:

ScopedPtrHashMap<key, scoped_ptr<value>> map;

We can pull out the type inside the scoped_ptr<value> with scoped_ptr::T for the
Value type inside this class. And we can use scoped_ptr::D for deleting.

[kcwu, 2015/04/24 06:30:28]

Done.
Is my current change what you expected?

+          typename Deleter = base::DefaultDeleter<Value>>
 class ScopedPtrHashMap {
   typedef base::hash_map<Key, Value*> Container;
 
@@ -34,15 +36,15 @@ class ScopedPtrHashMap {
 
   ~ScopedPtrHashMap() { clear(); }
 
-  void swap(ScopedPtrHashMap<Key, Value>& other) {
+  void swap(ScopedPtrHashMap<Key, Value, Deleter>& other) {
     data_.swap(other.data_);
   }
 
   // Replaces value but not key if key is already present.
-  iterator set(const Key& key, scoped_ptr<Value> data) {
+  iterator set(const Key& key, scoped_ptr<Value, Deleter> data) {
     iterator it = find(key);
     if (it != end()) {
-      delete it->second;
+      Deleter()(it->second);
       it->second = data.release();
       return it;
     }
@@ -51,7 +53,8 @@ class ScopedPtrHashMap {
   }
 
   // Does nothing if key is already present
-  std::pair<iterator, bool> add(const Key& key, scoped_ptr<Value> data) {
+  std::pair<iterator, bool> add(const Key& key,
+                                scoped_ptr<Value, Deleter> data) {
     std::pair<iterator, bool> result =
         data_.insert(std::make_pair(key, data.get()));
     if (result.second)
@@ -60,7 +63,7 @@ class ScopedPtrHashMap {
   }
 
   void erase(iterator it) {
-    delete it->second;
+    Deleter()(it->second);
     data_.erase(it);
   }
 
@@ -72,38 +75,38 @@ class ScopedPtrHashMap {
     return 1;
   }
 
-  scoped_ptr<Value> take(iterator it) {
+  scoped_ptr<Value, Deleter> take(iterator it) {
     DCHECK(it != data_.end());
     if (it == data_.end())
-      return scoped_ptr<Value>();
+      return scoped_ptr<Value, Deleter>();
 
-    scoped_ptr<Value> ret(it->second);
+    scoped_ptr<Value, Deleter> ret(it->second);
     it->second = NULL;
     return ret.Pass();
   }
 
-  scoped_ptr<Value> take(const Key& k) {
+  scoped_ptr<Value, Deleter> take(const Key& k) {
     iterator it = find(k);
     if (it == data_.end())
-      return scoped_ptr<Value>();
+      return scoped_ptr<Value, Deleter>();
 
     return take(it);
   }
 
-  scoped_ptr<Value> take_and_erase(iterator it) {
+  scoped_ptr<Value, Deleter> take_and_erase(iterator it) {
     DCHECK(it != data_.end());
     if (it == data_.end())
-      return scoped_ptr<Value>();
+      return scoped_ptr<Value, Deleter>();
 
-    scoped_ptr<Value> ret(it->second);
+    scoped_ptr<Value, Deleter> ret(it->second);
     data_.erase(it);
     return ret.Pass();
   }
 
-  scoped_ptr<Value> take_and_erase(const Key& k) {
+  scoped_ptr<Value, Deleter> take_and_erase(const Key& k) {
     iterator it = find(k);
     if (it == data_.end())
-      return scoped_ptr<Value>();
+      return scoped_ptr<Value, Deleter>();
 
     return take_and_erase(it);
   }
@@ -119,7 +122,18 @@ class ScopedPtrHashMap {
 
   inline bool contains(const Key& k) const { return data_.count(k) > 0; }
 
-  inline void clear() { STLDeleteValues(&data_); }
+  inline void clear() {
+    auto it = data_.begin();
+    while (it != data_.end()) {
+      // NOTE: Like STLDeleteContainerPointers, deleting behind the iterator.
+      // Deleting the value does not always invalidate the iterator, but it may
+      // do so if the key is a pointer into the value object.
+      auto temp = it;
+      ++it;
+      Deleter()(temp->second);
+    }
+    data_.clear();
+  }
 
   inline const_iterator find(const Key& k) const { return data_.find(k); }
   inline iterator find(const Key& k) { return data_.find(k); }