Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(216)

Unified Diff: base/id_map.h

Issue 2340813003: Switch base::IDMap to use unique_ptr. (Closed)
Patch Set: Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: base/id_map.h
diff --git a/base/id_map.h b/base/id_map.h
index c0976b0557d9d15a5e51e8eae5b31afc7fd0f733..a252440c8c1f79ecbabea10642b9e02104d40cd7 100644
--- a/base/id_map.h
+++ b/base/id_map.h
@@ -7,7 +7,10 @@
#include <stddef.h>
#include <stdint.h>
+#include <memory>
#include <set>
+#include <type_traits>
+#include <utility>
#include "base/containers/hash_tables.h"
#include "base/logging.h"
@@ -40,7 +43,10 @@ class IDMap {
using KeyType = K;
private:
- typedef base::hash_map<KeyType, T*> HashTable;
+ typedef typename std::conditional<OS == IDMapExternalPointer,
danakj 2016/09/15 20:08:40 can you change these both to "using" instead of ty
aelias_OOO_until_Jul13 2016/09/15 21:09:35 Done.
+ T*,
+ std::unique_ptr<T>>::type V;
+ typedef base::hash_map<KeyType, V> HashTable;
public:
IDMap() : iteration_depth_(0), next_id_(1), check_on_null_data_(false) {
@@ -56,7 +62,6 @@ class IDMap {
// thread. However, all the accesses may take place on another thread (or
// sequence), such as the IO thread. Detaching again to clean this up.
sequence_checker_.DetachFromSequence();
- Releaser<OS, 0>::release_all(&data_);
}
// Sets whether Add and Replace should DCHECK if passed in NULL data.
@@ -69,7 +74,7 @@ class IDMap {
DCHECK(!check_on_null_data_ || data);
KeyType this_id = next_id_;
DCHECK(data_.find(this_id) == data_.end()) << "Inserting duplicate item";
- data_[this_id] = data;
+ data_[this_id] = V(data);
next_id_++;
return this_id;
}
@@ -82,7 +87,7 @@ class IDMap {
DCHECK(sequence_checker_.CalledOnValidSequence());
DCHECK(!check_on_null_data_ || data);
DCHECK(data_.find(id) == data_.end()) << "Inserting duplicate item";
- data_[id] = data;
+ data_[id] = V(data);
}
void Remove(KeyType id) {
@@ -94,36 +99,29 @@ class IDMap {
}
if (iteration_depth_ == 0) {
- Releaser<OS, 0>::release(i->second);
data_.erase(i);
} else {
removed_ids_.insert(id);
}
}
- // Replaces the value for |id| with |new_data| and returns a pointer to the
- // existing value. If there is no entry for |id|, the map is not altered and
- // nullptr is returned. The OwnershipSemantics of the map have no effect on
- // how the existing value is treated, the IDMap does not delete the existing
- // value being replaced.
- T* Replace(KeyType id, T* new_data) {
+ // Replaces the value for |id| with |new_data| and returns the existing value
+ // (as a unique_ptr<> if in IDMapOwnPointer mode). Should only be called with
+ // an already added id.
+ V Replace(KeyType id, V new_data) {
DCHECK(sequence_checker_.CalledOnValidSequence());
DCHECK(!check_on_null_data_ || new_data);
typename HashTable::iterator i = data_.find(id);
- if (i == data_.end()) {
- NOTREACHED() << "Attempting to replace an item not in the list";
- return nullptr;
- }
+ DCHECK(i != data_.end());
- T* temp = i->second;
- i->second = new_data;
- return temp;
+ std::swap(i->second, new_data);
+ return new_data;
}
void Clear() {
DCHECK(sequence_checker_.CalledOnValidSequence());
if (iteration_depth_ == 0) {
- Releaser<OS, 0>::release_all(&data_);
+ data_.clear();
} else {
for (typename HashTable::iterator i = data_.begin();
i != data_.end(); ++i)
@@ -140,8 +138,8 @@ class IDMap {
DCHECK(sequence_checker_.CalledOnValidSequence());
typename HashTable::const_iterator i = data_.find(id);
if (i == data_.end())
- return NULL;
- return i->second;
+ return nullptr;
+ return &*i->second;
}
size_t size() const {
@@ -202,7 +200,7 @@ class IDMap {
ReturnType* GetCurrentValue() const {
DCHECK(map_->sequence_checker_.CalledOnValidSequence());
- return iter_->second;
+ return &*iter_->second;
}
void Advance() {
@@ -234,25 +232,6 @@ class IDMap {
typedef Iterator<const T> const_iterator;
private:
-
- // The dummy parameter is there because C++ standard does not allow
- // explicitly specialized templates inside classes
- template<IDMapOwnershipSemantics OI, int dummy> struct Releaser {
- static inline void release(T* ptr) {}
- static inline void release_all(HashTable* table) {}
- };
-
- template<int dummy> struct Releaser<IDMapOwnPointer, dummy> {
- static inline void release(T* ptr) { delete ptr;}
- static inline void release_all(HashTable* table) {
- for (typename HashTable::iterator i = table->begin();
- i != table->end(); ++i) {
- delete i->second;
- }
- table->clear();
- }
- };
-
void Compact() {
DCHECK_EQ(0, iteration_depth_);
for (const auto& i : removed_ids_)

Powered by Google App Engine
This is Rietveld 408576698