Move Storage for ListValue and DictValue in Union

base/values.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/values.h"
 
 #include <string.h>
 
 #include <algorithm>
 #include <cmath>
@@ skipping 55 matching lines @@
 
     case Value::Type::DICTIONARY:
       return CopyDictionaryWithoutEmptyChildren(
           static_cast<const DictionaryValue&>(node));
 
     default:
       return node.CreateDeepCopy();
   }
 }
 
-// TODO(crbug.com/646113): Remove this once all types are implemented.
-bool IsAssignmentSafe(Value::Type lhs, Value::Type rhs) {
-  auto IsImplemented = [](Value::Type type) {
-    return type == Value::Type::NONE || type == Value::Type::BOOLEAN ||
-           type == Value::Type::INTEGER || type == Value::Type::DOUBLE ||
-           type == Value::Type::STRING || type == Value::Type::BINARY;
-  };
-
-  return lhs == rhs || (IsImplemented(lhs) && IsImplemented(rhs));
-}
-
 }  // namespace
 
 // static
 std::unique_ptr<Value> Value::CreateNullValue() {
   return WrapUnique(new Value(Type::NONE));
 }
 
 // static
 std::unique_ptr<BinaryValue> BinaryValue::CreateWithCopiedBuffer(
     const char* buffer,
@@ skipping 25 matching lines @@
       return;
     case Type::DOUBLE:
       double_value_ = 0.0;
       return;
     case Type::STRING:
       string_value_.Init();
       return;
     case Type::BINARY:
       binary_value_.Init();
       return;
-
-    // TODO(crbug.com/646113): Implement these once the corresponding derived
-    // classes are removed.
+    case Type::DICTIONARY:
+      dict_ptr_.Init(MakeUnique<DictStorage>());
+      return;
     case Type::LIST:
-    case Type::DICTIONARY:
+      list_.Init();
       return;
   }
 }
 
 Value::Value(bool in_bool) : type_(Type::BOOLEAN), bool_value_(in_bool) {}
 
 Value::Value(int in_int) : type_(Type::INTEGER), int_value_(in_int) {}
 
 Value::Value(double in_double) : type_(Type::DOUBLE), double_value_(in_double) {
   if (!std::isfinite(double_value_)) {
@@ skipping 31 matching lines @@
 Value::Value(const std::vector<char>& in_blob) : type_(Type::BINARY) {
   binary_value_.Init(in_blob);
 }
 
 Value::Value(std::vector<char>&& in_blob) : type_(Type::BINARY) {
   binary_value_.Init(std::move(in_blob));
 }
 
 Value& Value::operator=(const Value& that) {
   if (this != &that) {
-    DCHECK(IsAssignmentSafe(type_, that.type_));
     if (type_ == that.type_) {
       InternalCopyAssignFrom(that);
     } else {
       InternalCleanup();
       InternalCopyConstructFrom(that);
     }
   }
 
   return *this;
 }
 
 Value& Value::operator=(Value&& that) {
   if (this != &that) {
-    DCHECK(IsAssignmentSafe(type_, that.type_));
     if (type_ == that.type_) {
       InternalMoveAssignFrom(std::move(that));
     } else {
       InternalCleanup();
       InternalMoveConstructFrom(std::move(that));
     }
   }
 
   return *this;
 }
@@ skipping 108 matching lines @@
 
 bool Value::GetAsBinary(const BinaryValue** out_value) const {
   if (out_value && is_blob()) {
     *out_value = this;
     return true;
   }
   return is_blob();
 }
 
 bool Value::GetAsList(ListValue** out_value) {
-  return false;
+  if (out_value && is_list()) {
+    *out_value = static_cast<ListValue*>(this);
+    return true;
+  }
+  return is_list();
 }
 
 bool Value::GetAsList(const ListValue** out_value) const {
-  return false;
+  if (out_value && is_list()) {
+    *out_value = static_cast<const ListValue*>(this);
+    return true;
+  }
+  return is_list();
 }
 
 bool Value::GetAsDictionary(DictionaryValue** out_value) {
-  return false;
+  if (out_value && is_dict()) {
+    *out_value = static_cast<DictionaryValue*>(this);
+    return true;
+  }
+  return is_dict();
 }
 
 bool Value::GetAsDictionary(const DictionaryValue** out_value) const {
-  return false;
+  if (out_value && is_dict()) {
+    *out_value = static_cast<const DictionaryValue*>(this);
+    return true;
+  }
+  return is_dict();
 }
 
 Value* Value::DeepCopy() const {
   // This method should only be getting called for null Values--all subclasses
   // need to provide their own implementation;.
   switch (type()) {
     case Type::NONE:
       return CreateNullValue().release();
 
     // For now, make FundamentalValues for backward-compatibility. Convert to
     // Value when that code is deleted.
     case Type::BOOLEAN:
       return new FundamentalValue(bool_value_);
     case Type::INTEGER:
       return new FundamentalValue(int_value_);
     case Type::DOUBLE:
       return new FundamentalValue(double_value_);
     // For now, make StringValues for backward-compatibility. Convert to
     // Value when that code is deleted.
     case Type::STRING:
       return new StringValue(*string_value_);
     // For now, make BinaryValues for backward-compatibility. Convert to
     // Value when that code is deleted.
     case Type::BINARY:
       return new BinaryValue(*binary_value_);
 
+    // TODO(crbug.com/646113): Clean this up when DictionaryValue and ListValue
+    // are completely inlined.
+    case Type::DICTIONARY: {
+      DictionaryValue* result = new DictionaryValue;
+
+      for (const auto& current_entry : **dict_ptr_) {
+        result->SetWithoutPathExpansion(current_entry.first,
+                                        current_entry.second->CreateDeepCopy());
+      }
+
+      return result;
+    }
+
+    case Type::LIST: {
+      ListValue* result = new ListValue;
+
+      for (const auto& entry : *list_)
+        result->Append(entry->CreateDeepCopy());
+
+      return result;
+    }
+
     default:
-      // All other types should be handled by subclasses.
       NOTREACHED();
       return nullptr;
   }
 }
 
 std::unique_ptr<Value> Value::CreateDeepCopy() const {
   return WrapUnique(DeepCopy());
 }
 
 bool Value::Equals(const Value* other) const {
   if (other->type() != type())
     return false;
 
   switch (type()) {
     case Type::NONE:
       return true;
     case Type::BOOLEAN:
       return bool_value_ == other->bool_value_;
     case Type::INTEGER:
       return int_value_ == other->int_value_;
     case Type::DOUBLE:
       return double_value_ == other->double_value_;
     case Type::STRING:
       return *string_value_ == *(other->string_value_);
     case Type::BINARY:
       return *binary_value_ == *(other->binary_value_);
-    default:
-      // This method should only be getting called for the above types -- all
-      // subclasses need to provide their own implementation;.
-      NOTREACHED();
-      return false;
+    // TODO(crbug.com/646113): Clean this up when DictionaryValue and ListValue
+    // are completely inlined.
+    case Type::DICTIONARY: {
+      if ((*dict_ptr_)->size() != (*other->dict_ptr_)->size())
+        return false;
+
+      return std::equal(std::begin(**dict_ptr_), std::end(**dict_ptr_),
+                        std::begin(**(other->dict_ptr_)),
+                        [](const DictStorage::value_type& lhs,
+                           const DictStorage::value_type& rhs) {
+                          if (lhs.first != rhs.first)
+                            return false;
+
+                          return lhs.second->Equals(rhs.second.get());
+                        });
+    }
+    case Type::LIST: {
+      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());
+                        });
+    }
   }
+
+  NOTREACHED();
+  return false;
 }
 
 // static
 bool Value::Equals(const Value* a, const Value* b) {
   if ((a == NULL) && (b == NULL)) return true;
   if ((a == NULL) ^  (b == NULL)) return false;
   return a->Equals(b);
 }
 
 void Value::InternalCopyFundamentalValue(const Value& that) {
@@ skipping 27 matching lines @@
     case Type::DOUBLE:
       InternalCopyFundamentalValue(that);
       return;
 
     case Type::STRING:
       string_value_.Init(*that.string_value_);
       return;
     case Type::BINARY:
       binary_value_.Init(*that.binary_value_);
       return;
-
-    // TODO(crbug.com/646113): Implement these once the corresponding derived
-    // classes are removed.
+    // DictStorage and ListStorage are move-only types due to the presence of

[brettw, 2017/02/15 21:28:29]

I think this comment is out-of-date now.

[jdoerrie, 2017/02/16 09:19:35]

I think it's still relevant. DictStorage and ListStorage are defined like this:

using DictStorage = std::map<std::string, std::unique_ptr<Value>>;
using ListStorage = std::vector<std::unique_ptr<Value>>;

The presence of std::unique_ptr<Value> makes both of them move-only, since they
don't have a copy constructor. This is why we currently have to call
|list_.Init(std::move(*that.CreateDeepCopy()->list_));| instead of
|list_.Init(*that.list_)|. This is different for StringValue and BinaryValue,
which is why I added the comment here. 

However, you are right that it is the proposal's goal to arrive at

using DictStorage = std::map<std::string, Value>;
using ListStorage = std::vector<Value>;

at which point we can simply use the copy constructors and this comment will be
obsolete.

+    // 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_));
+      return;
     case Type::LIST:
-    case Type::DICTIONARY:
+      list_.Init(std::move(*that.CreateDeepCopy()->list_));
       return;
   }
 }
 
 void Value::InternalMoveConstructFrom(Value&& that) {
   type_ = that.type_;
 
   switch (type_) {
     case Type::NONE:
     case Type::BOOLEAN:
     case Type::INTEGER:
     case Type::DOUBLE:
       InternalCopyFundamentalValue(that);
       return;
 
     case Type::STRING:
       string_value_.InitFromMove(std::move(that.string_value_));
       return;
     case Type::BINARY:
       binary_value_.InitFromMove(std::move(that.binary_value_));
       return;
-
-    // TODO(crbug.com/646113): Implement these once the corresponding derived
-    // classes are removed.
+    case Type::DICTIONARY:
+      dict_ptr_.InitFromMove(std::move(that.dict_ptr_));
+      return;
     case Type::LIST:
-    case Type::DICTIONARY:
+      list_.InitFromMove(std::move(that.list_));
       return;
   }
 }
 
 void Value::InternalCopyAssignFrom(const Value& that) {
   type_ = that.type_;
 
   switch (type_) {
     case Type::NONE:
     case Type::BOOLEAN:
     case Type::INTEGER:
     case Type::DOUBLE:
       InternalCopyFundamentalValue(that);
       return;
 
     case Type::STRING:
       *string_value_ = *that.string_value_;
       return;
     case Type::BINARY:
       *binary_value_ = *that.binary_value_;
       return;
-
-    // TODO(crbug.com/646113): Implement these once the corresponding derived
-    // classes are removed.
+    // DictStorage and ListStorage are move-only types due to the presence of

[brettw, 2017/02/15 21:28:29]

Ditto

[jdoerrie, 2017/02/16 09:19:35]

Same as above (s/copy constructor/copy assignment operator)

+    // 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_);
+      return;
     case Type::LIST:
-    case Type::DICTIONARY:
+      *list_ = std::move(*that.CreateDeepCopy()->list_);
       return;
   }
 }
 
 void Value::InternalMoveAssignFrom(Value&& that) {
   type_ = that.type_;
 
   switch (type_) {
     case Type::NONE:
     case Type::BOOLEAN:
     case Type::INTEGER:
     case Type::DOUBLE:
       InternalCopyFundamentalValue(that);
       return;
 
     case Type::STRING:
       *string_value_ = std::move(*that.string_value_);
       return;
     case Type::BINARY:
       *binary_value_ = std::move(*that.binary_value_);
       return;
-
-    // TODO(crbug.com/646113): Implement these once the corresponding derived
-    // classes are removed.
+    case Type::DICTIONARY:
+      *dict_ptr_ = std::move(*that.dict_ptr_);
+      return;
     case Type::LIST:
-    case Type::DICTIONARY:
+      *list_ = std::move(*that.list_);
       return;
   }
 }
 
 void Value::InternalCleanup() {
   switch (type_) {
     case Type::NONE:
     case Type::BOOLEAN:
     case Type::INTEGER:
     case Type::DOUBLE:
       // Nothing to do
       return;
 
     case Type::STRING:
       string_value_.Destroy();
       return;
     case Type::BINARY:
       binary_value_.Destroy();
       return;
-
-    // TODO(crbug.com/646113): Implement these once the corresponding derived
-    // classes are removed.
+    case Type::DICTIONARY:
+      dict_ptr_.Destroy();
+      return;
     case Type::LIST:
-    case Type::DICTIONARY:
+      list_.Destroy();
       return;
   }
 }
 
 ///////////////////// DictionaryValue ////////////////////
 
 // static
 std::unique_ptr<DictionaryValue> DictionaryValue::From(
     std::unique_ptr<Value> value) {
   DictionaryValue* out;
   if (value && value->GetAsDictionary(&out)) {
     ignore_result(value.release());
     return WrapUnique(out);
   }
   return nullptr;
 }
 
 DictionaryValue::DictionaryValue() : Value(Type::DICTIONARY) {}
 
-DictionaryValue::~DictionaryValue() {
-  Clear();
-}
-
-bool DictionaryValue::GetAsDictionary(DictionaryValue** out_value) {
-  if (out_value)
-    *out_value = this;
-  return true;
-}
-
-bool DictionaryValue::GetAsDictionary(const DictionaryValue** out_value) const {
-  if (out_value)
-    *out_value = this;
-  return true;
-}
-
 bool DictionaryValue::HasKey(StringPiece key) const {
   DCHECK(IsStringUTF8(key));
-  auto current_entry = dictionary_.find(key.as_string());
-  DCHECK((current_entry == dictionary_.end()) || current_entry->second);
-  return current_entry != dictionary_.end();
+  auto current_entry = (*dict_ptr_)->find(key.as_string());
+  DCHECK((current_entry == (*dict_ptr_)->end()) || current_entry->second);
+  return current_entry != (*dict_ptr_)->end();
 }
 
 void DictionaryValue::Clear() {
-  dictionary_.clear();
+  (*dict_ptr_)->clear();
 }
 
 void DictionaryValue::Set(StringPiece path, std::unique_ptr<Value> in_value) {
   DCHECK(IsStringUTF8(path));
   DCHECK(in_value);
 
   StringPiece current_path(path);
   DictionaryValue* current_dictionary = this;
   for (size_t delimiter_position = current_path.find('.');
        delimiter_position != StringPiece::npos;
@@ skipping 34 matching lines @@
 void DictionaryValue::SetString(StringPiece path, StringPiece in_value) {
   Set(path, new StringValue(in_value));
 }
 
 void DictionaryValue::SetString(StringPiece path, const string16& in_value) {
   Set(path, new StringValue(in_value));
 }
 
 void DictionaryValue::SetWithoutPathExpansion(StringPiece key,
                                               std::unique_ptr<Value> in_value) {
-  dictionary_[key.as_string()] = std::move(in_value);
+  (**dict_ptr_)[key.as_string()] = std::move(in_value);
 }
 
 void DictionaryValue::SetWithoutPathExpansion(StringPiece key,
                                               Value* in_value) {
   SetWithoutPathExpansion(key, WrapUnique(in_value));
 }
 
 void DictionaryValue::SetBooleanWithoutPathExpansion(StringPiece path,
                                                      bool in_value) {
   SetWithoutPathExpansion(path,
@@ skipping 159 matching lines @@
 
 bool DictionaryValue::GetList(StringPiece path, ListValue** out_value) {
   return static_cast<const DictionaryValue&>(*this).GetList(
       path,
       const_cast<const ListValue**>(out_value));
 }
 
 bool DictionaryValue::GetWithoutPathExpansion(StringPiece key,
                                               const Value** out_value) const {
   DCHECK(IsStringUTF8(key));
-  auto entry_iterator = dictionary_.find(key.as_string());
-  if (entry_iterator == dictionary_.end())
+  auto entry_iterator = (*dict_ptr_)->find(key.as_string());
+  if (entry_iterator == (*dict_ptr_)->end())
     return false;
 
   if (out_value)
     *out_value = entry_iterator->second.get();
   return true;
 }
 
 bool DictionaryValue::GetWithoutPathExpansion(StringPiece key,
                                               Value** out_value) {
   return static_cast<const DictionaryValue&>(*this).GetWithoutPathExpansion(
@@ skipping 107 matching lines @@
   }
 
   return current_dictionary->RemoveWithoutPathExpansion(current_path,
                                                         out_value);
 }
 
 bool DictionaryValue::RemoveWithoutPathExpansion(
     StringPiece key,
     std::unique_ptr<Value>* out_value) {
   DCHECK(IsStringUTF8(key));
-  auto entry_iterator = dictionary_.find(key.as_string());
-  if (entry_iterator == dictionary_.end())
+  auto entry_iterator = (*dict_ptr_)->find(key.as_string());
+  if (entry_iterator == (*dict_ptr_)->end())
     return false;
 
   if (out_value)
     *out_value = std::move(entry_iterator->second);
-  dictionary_.erase(entry_iterator);
+  (*dict_ptr_)->erase(entry_iterator);
   return true;
 }
 
 bool DictionaryValue::RemovePath(StringPiece path,
                                  std::unique_ptr<Value>* out_value) {
   bool result = false;
   size_t delimiter_position = path.find('.');
 
   if (delimiter_position == std::string::npos)
     return RemoveWithoutPathExpansion(path, out_value);
@@ skipping 31 matching lines @@
         continue;
       }
     }
     // All other cases: Make a copy and hook it up.
     SetWithoutPathExpansion(it.key(),
                             base::WrapUnique(merge_value->DeepCopy()));
   }
 }
 
 void DictionaryValue::Swap(DictionaryValue* other) {
-  dictionary_.swap(other->dictionary_);
+  dict_ptr_->swap(*(other->dict_ptr_));
 }
 
 DictionaryValue::Iterator::Iterator(const DictionaryValue& target)
-    : target_(target),
-      it_(target.dictionary_.begin()) {}
+    : target_(target), it_((*target.dict_ptr_)->begin()) {}
 
 DictionaryValue::Iterator::Iterator(const Iterator& other) = default;
 
 DictionaryValue::Iterator::~Iterator() {}
 
 DictionaryValue* DictionaryValue::DeepCopy() const {
-  DictionaryValue* result = new DictionaryValue;
-
-  for (const auto& current_entry : dictionary_) {
-    result->SetWithoutPathExpansion(current_entry.first,
-                                    current_entry.second->CreateDeepCopy());
-  }
-
-  return result;
+  return static_cast<DictionaryValue*>(Value::DeepCopy());
 }
 
 std::unique_ptr<DictionaryValue> DictionaryValue::CreateDeepCopy() const {
   return WrapUnique(DeepCopy());
 }
 
-bool DictionaryValue::Equals(const Value* other) const {
-  if (other->GetType() != GetType())
-    return false;
-
-  const DictionaryValue* other_dict =
-      static_cast<const DictionaryValue*>(other);
-  Iterator lhs_it(*this);
-  Iterator rhs_it(*other_dict);
-  while (!lhs_it.IsAtEnd() && !rhs_it.IsAtEnd()) {
-    if (lhs_it.key() != rhs_it.key() ||
-        !lhs_it.value().Equals(&rhs_it.value())) {
-      return false;
-    }
-    lhs_it.Advance();
-    rhs_it.Advance();
-  }
-  if (!lhs_it.IsAtEnd() || !rhs_it.IsAtEnd())
-    return false;
-
-  return true;
-}
-
 ///////////////////// ListValue ////////////////////
 
 // static
 std::unique_ptr<ListValue> ListValue::From(std::unique_ptr<Value> value) {
   ListValue* out;
   if (value && value->GetAsList(&out)) {
     ignore_result(value.release());
     return WrapUnique(out);
   }
   return nullptr;
 }
 
 ListValue::ListValue() : Value(Type::LIST) {}
 
-ListValue::~ListValue() {
-  Clear();
-}
-
 void ListValue::Clear() {
-  list_.clear();
+  list_->clear();
 }
 
 bool ListValue::Set(size_t index, Value* in_value) {
   return Set(index, WrapUnique(in_value));
 }
 
 bool ListValue::Set(size_t index, std::unique_ptr<Value> in_value) {
   if (!in_value)
     return false;
 
-  if (index >= list_.size()) {
+  if (index >= list_->size()) {
     // Pad out any intermediate indexes with null settings
-    while (index > list_.size())
+    while (index > list_->size())
       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);
+    DCHECK((*list_)[index] != in_value);
+    (*list_)[index] = std::move(in_value);
   }
   return true;
 }
 
 bool ListValue::Get(size_t index, const Value** out_value) const {
-  if (index >= list_.size())
+  if (index >= list_->size())
     return false;
 
   if (out_value)
-    *out_value = list_[index].get();
+    *out_value = (*list_)[index].get();
 
   return true;
 }
 
 bool ListValue::Get(size_t index, Value** out_value) {
   return static_cast<const ListValue&>(*this).Get(
       index,
       const_cast<const Value**>(out_value));
 }
 
@@ skipping 86 matching lines @@
   return true;
 }
 
 bool ListValue::GetList(size_t index, ListValue** out_value) {
   return static_cast<const ListValue&>(*this).GetList(
       index,
       const_cast<const ListValue**>(out_value));
 }
 
 bool ListValue::Remove(size_t index, std::unique_ptr<Value>* out_value) {
-  if (index >= list_.size())
+  if (index >= list_->size())
     return false;
 
   if (out_value)
-    *out_value = std::move(list_[index]);
+    *out_value = std::move((*list_)[index]);
 
-  list_.erase(list_.begin() + 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) {
+  for (auto it = list_->begin(); it != list_->end(); ++it) {
     if ((*it)->Equals(&value)) {
-      size_t previous_index = it - list_.begin();
-      list_.erase(it);
+      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 = std::move(*Storage::iterator(iter));
+    *out_value = std::move(*ListStorage::iterator(iter));
 
-  return list_.erase(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)
 void ListValue::Append(Value* in_value) {
   DCHECK(in_value);
   Append(WrapUnique(in_value));
 }
 #endif
 
 void ListValue::AppendBoolean(bool in_value) {
@@ skipping 25 matching lines @@
 
 void ListValue::AppendStrings(const std::vector<string16>& in_values) {
   for (std::vector<string16>::const_iterator it = in_values.begin();
        it != in_values.end(); ++it) {
     AppendString(*it);
   }
 }
 
 bool ListValue::AppendIfNotPresent(std::unique_ptr<Value> in_value) {
   DCHECK(in_value);
-  for (const auto& entry : list_) {
+  for (const auto& entry : *list_) {
     if (entry->Equals(in_value.get())) {
       return false;
     }
   }
-  list_.push_back(std::move(in_value));
+  list_->push_back(std::move(in_value));
   return true;
 }
 
 bool ListValue::Insert(size_t index, std::unique_ptr<Value> in_value) {
   DCHECK(in_value);
-  if (index > list_.size())
+  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(),
+  return std::find_if(list_->begin(), list_->end(),
                       [&value](const std::unique_ptr<Value>& entry) {
                         return entry->Equals(&value);
                       });
 }
 
 void ListValue::Swap(ListValue* other) {
-  list_.swap(other->list_);
-}
-
-bool ListValue::GetAsList(ListValue** out_value) {
-  if (out_value)
-    *out_value = this;
-  return true;
-}
-
-bool ListValue::GetAsList(const ListValue** out_value) const {
-  if (out_value)
-    *out_value = this;
-  return true;
+  list_->swap(*(other->list_));
 }
 
 ListValue* ListValue::DeepCopy() const {
-  ListValue* result = new ListValue;
-
-  for (const auto& entry : list_)
-    result->Append(entry->CreateDeepCopy());
-
-  return result;
+  return static_cast<ListValue*>(Value::DeepCopy());
 }
 
 std::unique_ptr<ListValue> ListValue::CreateDeepCopy() const {
   return WrapUnique(DeepCopy());
 }
 
-bool ListValue::Equals(const Value* other) const {
-  if (other->GetType() != GetType())
-    return false;
-
-  const ListValue* other_list =
-      static_cast<const ListValue*>(other);
-  Storage::const_iterator lhs_it, rhs_it;
-  for (lhs_it = begin(), rhs_it = other_list->begin();
-       lhs_it != end() && rhs_it != other_list->end();
-       ++lhs_it, ++rhs_it) {
-    if (!(*lhs_it)->Equals(rhs_it->get()))
-      return false;
-  }
-  if (lhs_it != end() || rhs_it != other_list->end())
-    return false;
-
-  return true;
-}
-
 ValueSerializer::~ValueSerializer() {
 }
 
 ValueDeserializer::~ValueDeserializer() {
 }
 
 std::ostream& operator<<(std::ostream& out, const Value& value) {
   std::string json;
   JSONWriter::WriteWithOptions(value, JSONWriter::OPTIONS_PRETTY_PRINT, &json);
   return out << json;
 }
 
 std::ostream& operator<<(std::ostream& out, const Value::Type& type) {
   if (static_cast<int>(type) < 0 ||
       static_cast<size_t>(type) >= arraysize(kTypeNames))
     return out << "Invalid Type (index = " << static_cast<int>(type) << ")";
   return out << Value::GetTypeName(type);
 }
 
 }  // namespace base