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

Unified Diff: base/values.cc

Issue 2816513002: Revert of Change base::Value::ListStorage to std::vector<base::Value> (Closed)
Patch Set: Created 3 years, 8 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
« no previous file with comments | « base/values.h ('k') | base/values_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/values.cc
diff --git a/base/values.cc b/base/values.cc
index 74e83b9900f56008af6b7edbe5dbc50a90ac7d13..ca12edcb00cd611d2199611640c8b09f9711c481 100644
--- a/base/values.cc
+++ b/base/values.cc
@@ -35,7 +35,7 @@
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,7 +375,12 @@
std::tie(v.first, *v.second);
});
case Value::Type::LIST:
- return *lhs.list_ == *rhs.list_;
+ if (lhs.list_->size() != rhs.list_->size())
+ return false;
+ return std::equal(
+ std::begin(*lhs.list_), std::end(*lhs.list_), std::begin(*rhs.list_),
+ [](const Value::ListStorage::value_type& u,
+ const Value::ListStorage::value_type& v) { return *u == *v; });
}
NOTREACHED();
@@ -414,7 +419,11 @@
return std::tie(u.first, *u.second) < std::tie(v.first, *v.second);
});
case Value::Type::LIST:
- return *lhs.list_ < *rhs.list_;
+ return std::lexicographical_compare(
+ std::begin(*lhs.list_), std::end(*lhs.list_), std::begin(*rhs.list_),
+ std::end(*rhs.list_),
+ [](const Value::ListStorage::value_type& u,
+ const Value::ListStorage::value_type& v) { return *u < *v; });
}
NOTREACHED();
@@ -485,10 +494,11 @@
case Type::BINARY:
binary_value_.Init(*that.binary_value_);
return;
- // DictStorage is a move-only type due to the presence of unique_ptrs. This
- // is why the explicit copy of every element is necessary here.
- // TODO(crbug.com/646113): Clean this up when DictStorage can be copied
- // directly.
+ // DictStorage and ListStorage are move-only types due to the presence of
+ // unique_ptrs. This is why the explicit copy of every element is necessary
+ // here.
+ // TODO(crbug.com/646113): Clean this up when DictStorage and ListStorage
+ // can be copied directly.
case Type::DICTIONARY:
dict_ptr_.Init(MakeUnique<DictStorage>());
for (const auto& it : **that.dict_ptr_) {
@@ -498,7 +508,10 @@
}
return;
case Type::LIST:
- list_.Init(*that.list_);
+ list_.Init();
+ list_->reserve(that.list_->size());
+ for (const auto& it : *that.list_)
+ list_->push_back(MakeUnique<Value>(*it));
return;
}
}
@@ -548,17 +561,16 @@
case Type::BINARY:
*binary_value_ = *that.binary_value_;
return;
- // DictStorage is a move-only type due to the presence of unique_ptrs. This
- // is why the explicit call to the copy constructor is necessary here.
- // TODO(crbug.com/646113): Clean this up when DictStorage can be copied
- // directly.
- case Type::DICTIONARY: {
- Value copy = that;
- *dict_ptr_ = std::move(*copy.dict_ptr_);
- return;
- }
+ // DictStorage and ListStorage are move-only types due to the presence of
+ // unique_ptrs. This is why the explicit call to the copy constructor 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(*Value(that).dict_ptr_);
+ return;
case Type::LIST:
- *list_ = *that.list_;
+ *list_ = std::move(*Value(that).list_);
return;
}
}
@@ -1065,10 +1077,6 @@
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));
}
@@ -1083,7 +1091,9 @@
Append(MakeUnique<Value>());
Append(std::move(in_value));
} else {
- (*list_)[index] = std::move(*in_value);
+ // TODO(dcheng): remove this DCHECK once the raw pointer version is removed?
+ DCHECK((*list_)[index] != in_value);
+ (*list_)[index] = std::move(in_value);
}
return true;
}
@@ -1093,7 +1103,7 @@
return false;
if (out_value)
- *out_value = &(*list_)[index];
+ *out_value = (*list_)[index].get();
return true;
}
@@ -1203,35 +1213,36 @@
return false;
if (out_value)
- *out_value = MakeUnique<Value>(std::move((*list_)[index]));
+ *out_value = std::move((*list_)[index]);
list_->erase(list_->begin() + index);
return true;
}
bool ListValue::Remove(const Value& value, size_t* index) {
- auto it = std::find(list_->begin(), list_->end(), value);
-
- if (it == list_->end())
- return false;
-
- if (index)
- *index = std::distance(list_->begin(), it);
-
- list_->erase(it);
- return true;
+ for (auto it = list_->begin(); it != list_->end(); ++it) {
+ if (**it == value) {
+ size_t previous_index = it - list_->begin();
+ list_->erase(it);
+
+ if (index)
+ *index = previous_index;
+ return true;
+ }
+ }
+ return false;
}
ListValue::iterator ListValue::Erase(iterator iter,
std::unique_ptr<Value>* out_value) {
if (out_value)
- *out_value = MakeUnique<Value>(std::move(*iter));
+ *out_value = std::move(*ListStorage::iterator(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)
@@ -1277,10 +1288,11 @@
bool ListValue::AppendIfNotPresent(std::unique_ptr<Value> in_value) {
DCHECK(in_value);
- if (std::find(list_->begin(), list_->end(), *in_value) != list_->end())
- return false;
-
- list_->push_back(std::move(*in_value));
+ for (const auto& entry : *list_) {
+ if (*entry == *in_value)
+ return false;
+ }
+ list_->push_back(std::move(in_value));
return true;
}
@@ -1289,12 +1301,15 @@
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(list_->begin(), list_->end(), value);
+ return std::find_if(list_->begin(), list_->end(),
+ [&value](const std::unique_ptr<Value>& entry) {
+ return *entry == value;
+ });
}
void ListValue::Swap(ListValue* other) {
« no previous file with comments | « base/values.h ('k') | base/values_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698