Make DictionaryValue::DeepCopyWithoutEmptyChildren return a scoped_ptr
BUG=none
Review URL: https://codereview.chromium.org/1150863002
Cr-Commit-Position: refs/heads/master@{#331223}
CrOS-Libchrome-Original-Commit: 6e04d5002ba4cff956df7f246c8bf3bfe0c8a6f2
diff --git a/base/values.cc b/base/values.cc
index ebd7157..4534d27 100644
--- a/base/values.cc
+++ b/base/values.cc
@@ -20,46 +20,49 @@
namespace {
+scoped_ptr<Value> CopyWithoutEmptyChildren(const Value& node);
+
// Make a deep copy of |node|, but don't include empty lists or dictionaries
// in the copy. It's possible for this function to return NULL and it
// expects |node| to always be non-NULL.
-Value* CopyWithoutEmptyChildren(const Value* node) {
- DCHECK(node);
- switch (node->GetType()) {
- case Value::TYPE_LIST: {
- const ListValue* list = static_cast<const ListValue*>(node);
- ListValue* copy = new ListValue;
- for (ListValue::const_iterator it = list->begin(); it != list->end();
- ++it) {
- Value* child_copy = CopyWithoutEmptyChildren(*it);
- if (child_copy)
- copy->Append(child_copy);
- }
- if (!copy->empty())
- return copy;
-
- delete copy;
- return NULL;
+scoped_ptr<ListValue> CopyListWithoutEmptyChildren(const ListValue& list) {
+ scoped_ptr<ListValue> copy;
+ for (ListValue::const_iterator it = list.begin(); it != list.end(); ++it) {
+ scoped_ptr<Value> child_copy = CopyWithoutEmptyChildren(**it);
+ if (child_copy) {
+ if (!copy)
+ copy.reset(new ListValue);
+ copy->Append(child_copy.Pass());
}
+ }
+ return copy;
+}
- case Value::TYPE_DICTIONARY: {
- const DictionaryValue* dict = static_cast<const DictionaryValue*>(node);
- DictionaryValue* copy = new DictionaryValue;
- for (DictionaryValue::Iterator it(*dict); !it.IsAtEnd(); it.Advance()) {
- Value* child_copy = CopyWithoutEmptyChildren(&it.value());
- if (child_copy)
- copy->SetWithoutPathExpansion(it.key(), child_copy);
- }
- if (!copy->empty())
- return copy;
-
- delete copy;
- return NULL;
+scoped_ptr<DictionaryValue> CopyDictionaryWithoutEmptyChildren(
+ const DictionaryValue& dict) {
+ scoped_ptr<DictionaryValue> copy;
+ for (DictionaryValue::Iterator it(dict); !it.IsAtEnd(); it.Advance()) {
+ scoped_ptr<Value> child_copy = CopyWithoutEmptyChildren(it.value());
+ if (child_copy) {
+ if (!copy)
+ copy.reset(new DictionaryValue);
+ copy->SetWithoutPathExpansion(it.key(), child_copy.Pass());
}
+ }
+ return copy;
+}
+
+scoped_ptr<Value> CopyWithoutEmptyChildren(const Value& node) {
+ switch (node.GetType()) {
+ case Value::TYPE_LIST:
+ return CopyListWithoutEmptyChildren(static_cast<const ListValue&>(node));
+
+ case Value::TYPE_DICTIONARY:
+ return CopyDictionaryWithoutEmptyChildren(
+ static_cast<const DictionaryValue&>(node));
default:
- // For everything else, just make a copy.
- return node->DeepCopy();
+ return node.CreateDeepCopy();
}
}
@@ -789,9 +792,12 @@
return result;
}
-DictionaryValue* DictionaryValue::DeepCopyWithoutEmptyChildren() const {
- Value* copy = CopyWithoutEmptyChildren(this);
- return copy ? static_cast<DictionaryValue*>(copy) : new DictionaryValue;
+scoped_ptr<DictionaryValue> DictionaryValue::DeepCopyWithoutEmptyChildren()
+ const {
+ scoped_ptr<DictionaryValue> copy = CopyDictionaryWithoutEmptyChildren(*this);
+ if (!copy)
+ copy.reset(new DictionaryValue);
+ return copy;
}
void DictionaryValue::MergeDictionary(const DictionaryValue* dictionary) {
diff --git a/base/values.h b/base/values.h
index e32edec..7feef9d 100644
--- a/base/values.h
+++ b/base/values.h
@@ -338,7 +338,7 @@
// Makes a copy of |this| but doesn't include empty dictionaries and lists in
// the copy. This never returns NULL, even if |this| itself is empty.
- DictionaryValue* DeepCopyWithoutEmptyChildren() const;
+ scoped_ptr<DictionaryValue> DeepCopyWithoutEmptyChildren() const;
// Merge |dictionary| into this dictionary. This is done recursively, i.e. any
// sub-dictionaries will be merged as well. In case of key collisions, the
diff --git a/base/values_unittest.cc b/base/values_unittest.cc
index 6466a96..37ed7ce 100644
--- a/base/values_unittest.cc
+++ b/base/values_unittest.cc
@@ -672,25 +672,25 @@
root->Set("empty_list", make_scoped_ptr(new ListValue));
root->SetWithoutPathExpansion("a.b.c.d.e",
make_scoped_ptr(new DictionaryValue));
- root.reset(root->DeepCopyWithoutEmptyChildren());
+ root = root->DeepCopyWithoutEmptyChildren();
EXPECT_TRUE(root->empty());
// Make sure we don't prune too much.
root->SetBoolean("bool", true);
root->Set("empty_dict", make_scoped_ptr(new DictionaryValue));
root->SetString("empty_string", std::string());
- root.reset(root->DeepCopyWithoutEmptyChildren());
+ root = root->DeepCopyWithoutEmptyChildren();
EXPECT_EQ(2U, root->size());
// Should do nothing.
- root.reset(root->DeepCopyWithoutEmptyChildren());
+ root = root->DeepCopyWithoutEmptyChildren();
EXPECT_EQ(2U, root->size());
// Nested test cases. These should all reduce back to the bool and string
// set above.
{
root->Set("a.b.c.d.e", make_scoped_ptr(new DictionaryValue));
- root.reset(root->DeepCopyWithoutEmptyChildren());
+ root = root->DeepCopyWithoutEmptyChildren();
EXPECT_EQ(2U, root->size());
}
{
@@ -698,7 +698,7 @@
inner->Set("empty_dict", make_scoped_ptr(new DictionaryValue));
inner->Set("empty_list", make_scoped_ptr(new ListValue));
root->Set("dict_with_empty_children", inner.Pass());
- root.reset(root->DeepCopyWithoutEmptyChildren());
+ root = root->DeepCopyWithoutEmptyChildren();
EXPECT_EQ(2U, root->size());
}
{
@@ -706,7 +706,7 @@
inner->Append(make_scoped_ptr(new DictionaryValue));
inner->Append(make_scoped_ptr(new ListValue));
root->Set("list_with_empty_children", inner.Pass());
- root.reset(root->DeepCopyWithoutEmptyChildren());
+ root = root->DeepCopyWithoutEmptyChildren();
EXPECT_EQ(2U, root->size());
}
@@ -720,7 +720,7 @@
inner2->Set("empty_dict", make_scoped_ptr(new DictionaryValue));
inner2->Set("empty_list", make_scoped_ptr(new ListValue));
root->Set("dict_with_empty_children", inner2.Pass());
- root.reset(root->DeepCopyWithoutEmptyChildren());
+ root = root->DeepCopyWithoutEmptyChildren();
EXPECT_EQ(2U, root->size());
}
@@ -732,7 +732,7 @@
inner->Append(make_scoped_ptr(new DictionaryValue));
inner->Append(inner2.Pass());
root->Set("list_with_empty_children", inner.Pass());
- root.reset(root->DeepCopyWithoutEmptyChildren());
+ root = root->DeepCopyWithoutEmptyChildren();
EXPECT_EQ(3U, root->size());
ListValue* inner_value, *inner_value2;