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

Unified Diff: base/values.cc

Issue 2740143002: Change base::Value::ListStorage to std::vector<base::Value> (Closed)
Patch Set: Fix Android Compilation Error Created 3 years, 9 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/values.cc
diff --git a/base/values.cc b/base/values.cc
index 7ae3b630346cc30b797dd4c7e40b6a2a61afb5f8..f47f68e594265b07b137fa8067094aeca91d4cc2 100644
--- a/base/values.cc
+++ b/base/values.cc
@@ -35,7 +35,7 @@ std::unique_ptr<Value> CopyWithoutEmptyChildren(const Value& node);
std::unique_ptr<ListValue> CopyListWithoutEmptyChildren(const ListValue& list) {
std::unique_ptr<ListValue> copy;
for (const auto& entry : list) {
- std::unique_ptr<Value> child_copy = CopyWithoutEmptyChildren(*entry);
+ std::unique_ptr<Value> child_copy = CopyWithoutEmptyChildren(entry);
if (child_copy) {
if (!copy)
copy.reset(new ListValue);
@@ -375,10 +375,7 @@ Value* Value::DeepCopy() const {
case Type::LIST: {
ListValue* result = new ListValue;
-
- for (const auto& entry : *list_)
- result->Append(entry->CreateDeepCopy());
-
+ *result->list_ = *list_;
return result;
}
@@ -429,12 +426,10 @@ bool Value::Equals(const Value* other) const {
if (list_->size() != other->list_->size())
return false;
- return std::equal(std::begin(*list_), std::end(*list_),
- std::begin(*(other->list_)),
- [](const ListStorage::value_type& lhs,
- const ListStorage::value_type& rhs) {
- return lhs->Equals(rhs.get());
- });
+ return std::equal(
+ std::begin(*list_), std::end(*list_), std::begin(*(other->list_)),
+ [](const ListStorage::value_type& lhs,
+ const ListStorage::value_type& rhs) { return lhs.Equals(&rhs); });
}
}
@@ -487,15 +482,17 @@ void Value::InternalCopyConstructFrom(const Value& that) {
case Type::BINARY:
binary_value_.Init(*that.binary_value_);
return;
- // DictStorage and ListStorage are move-only types due to the presence of
- // unique_ptrs. This is why the call to |CreateDeepCopy| is necessary here.
- // TODO(crbug.com/646113): Clean this up when DictStorage and ListStorage
- // can be copied directly.
- case Type::DICTIONARY:
- dict_ptr_.Init(std::move(*that.CreateDeepCopy()->dict_ptr_));
+ // DictStorage is a move-only type due to the presence of unique_ptrs. This
+ // is why the call to |CreateDeepCopy| is necessary here.
+ // TODO(crbug.com/646113): Clean this up when DictStorage can be copied
+ // directly.
+ case Type::DICTIONARY: {
+ auto deep_copy = that.CreateDeepCopy();
+ dict_ptr_.Init(std::move(*deep_copy->dict_ptr_));
return;
+ }
case Type::LIST:
- list_.Init(std::move(*that.CreateDeepCopy()->list_));
+ list_.Init(*that.list_);
return;
}
}
@@ -545,15 +542,17 @@ void Value::InternalCopyAssignFromSameType(const Value& that) {
case Type::BINARY:
*binary_value_ = *that.binary_value_;
return;
- // DictStorage and ListStorage are move-only types due to the presence of
- // unique_ptrs. This is why the call to |CreateDeepCopy| is necessary here.
- // TODO(crbug.com/646113): Clean this up when DictStorage and ListStorage
- // can be copied directly.
- case Type::DICTIONARY:
- *dict_ptr_ = std::move(*that.CreateDeepCopy()->dict_ptr_);
+ // DictStorage is a move-only type due to the presence of unique_ptrs. This
+ // is why the call to |CreateDeepCopy| is necessary here.
+ // TODO(crbug.com/646113): Clean this up when DictStorage can be copied
+ // directly.
+ case Type::DICTIONARY: {
+ auto deep_copy = that.CreateDeepCopy();
+ *dict_ptr_ = std::move(*deep_copy->dict_ptr_);
return;
+ }
case Type::LIST:
- *list_ = std::move(*that.CreateDeepCopy()->list_);
+ *list_ = *that.list_;
return;
}
}
@@ -1062,6 +1061,10 @@ void ListValue::Clear() {
list_->clear();
}
+void ListValue::Reserve(size_t n) {
+ list_->reserve(n);
+}
+
bool ListValue::Set(size_t index, Value* in_value) {
return Set(index, WrapUnique(in_value));
}
@@ -1076,9 +1079,7 @@ bool ListValue::Set(size_t index, std::unique_ptr<Value> in_value) {
Append(CreateNullValue());
Append(std::move(in_value));
} else {
- // TODO(dcheng): remove this DCHECK once the raw pointer version is removed?
- DCHECK((*list_)[index] != in_value);
- (*list_)[index] = std::move(in_value);
+ (*list_)[index] = std::move(*in_value);
}
return true;
}
@@ -1088,7 +1089,7 @@ bool ListValue::Get(size_t index, const Value** out_value) const {
return false;
if (out_value)
- *out_value = (*list_)[index].get();
+ *out_value = &(*list_)[index];
return true;
}
@@ -1199,36 +1200,37 @@ bool ListValue::Remove(size_t index, std::unique_ptr<Value>* out_value) {
return false;
if (out_value)
- *out_value = std::move((*list_)[index]);
+ *out_value = MakeUnique<Value>(std::move((*list_)[index]));
list_->erase(list_->begin() + index);
return true;
}
bool ListValue::Remove(const Value& value, size_t* index) {
- for (auto it = list_->begin(); it != list_->end(); ++it) {
- if ((*it)->Equals(&value)) {
- size_t previous_index = it - list_->begin();
- list_->erase(it);
+ auto it = std::find_if(
+ list_->begin(), list_->end(),
+ [&value](const Value& entry) { return entry.Equals(&value); });
- if (index)
- *index = previous_index;
- return true;
- }
- }
- return false;
+ if (it == list_->end())
+ return false;
+
+ if (index)
+ *index = std::distance(list_->begin(), it);
+
+ list_->erase(it);
+ return true;
}
ListValue::iterator ListValue::Erase(iterator iter,
std::unique_ptr<Value>* out_value) {
if (out_value)
- *out_value = std::move(*ListStorage::iterator(iter));
+ *out_value = MakeUnique<Value>(std::move(*iter));
return list_->erase(iter);
}
void ListValue::Append(std::unique_ptr<Value> in_value) {
- list_->push_back(std::move(in_value));
+ list_->push_back(std::move(*in_value));
}
#if !defined(OS_LINUX)
@@ -1275,11 +1277,11 @@ void ListValue::AppendStrings(const std::vector<string16>& in_values) {
bool ListValue::AppendIfNotPresent(std::unique_ptr<Value> in_value) {
DCHECK(in_value);
for (const auto& entry : *list_) {
- if (entry->Equals(in_value.get())) {
+ if (entry.Equals(in_value.get())) {
return false;
}
}
- list_->push_back(std::move(in_value));
+ list_->push_back(std::move(*in_value));
return true;
}
@@ -1288,15 +1290,14 @@ bool ListValue::Insert(size_t index, std::unique_ptr<Value> in_value) {
if (index > list_->size())
return false;
- list_->insert(list_->begin() + index, std::move(in_value));
+ list_->insert(list_->begin() + index, std::move(*in_value));
return true;
}
ListValue::const_iterator ListValue::Find(const Value& value) const {
- return std::find_if(list_->begin(), list_->end(),
- [&value](const std::unique_ptr<Value>& entry) {
- return entry->Equals(&value);
- });
+ return std::find_if(
+ list_->begin(), list_->end(),
+ [&value](const Value& entry) { return entry.Equals(&value); });
}
void ListValue::Swap(ListValue* other) {

Powered by Google App Engine
This is Rietveld 408576698