Cleaning up base::Value assignment.

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 155 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) {
-    if (type_ == that.type_) {
-      InternalCopyAssignFromSameType(that);
-    } else {
-      InternalCleanup();
-      InternalCopyConstructFrom(that);
-    }
+  if (type_ == that.type_) {
+    InternalCopyAssignFromSameType(that);
+  } else {
+    // This is not a self assignment because the type_ doesn't match.
+    InternalCleanup();
+    InternalCopyConstructFrom(that);
   }
 
   return *this;
 }
 
 Value& Value::operator=(Value&& that) {
-  if (this != &that) {
-    if (type_ == that.type_) {
-      InternalMoveAssignFromSameType(std::move(that));
-    } else {
-      InternalCleanup();
-      InternalMoveConstructFrom(std::move(that));
-    }
-  }
+  DCHECK(this != &that) << "attempt to self move assign.";
+  InternalCleanup();
+  InternalMoveConstructFrom(std::move(that));
 
   return *this;
 }
 
 Value::~Value() {
   InternalCleanup();
   alive_ = false;
 }
 
 // static
@@ skipping 320 matching lines @@
     case Type::DICTIONARY:
       dict_ptr_.InitFromMove(std::move(that.dict_ptr_));
       return;
     case Type::LIST:
       list_.InitFromMove(std::move(that.list_));
       return;
   }
 }
 
 void Value::InternalCopyAssignFromSameType(const Value& that) {
+  // TODO(crbug.com/646113): make this a DCHECK once base::Value does not have
+  // subclasses.
   CHECK_EQ(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;
     // 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_);
       return;
     case Type::LIST:
       *list_ = std::move(*that.CreateDeepCopy()->list_);
       return;
   }
 }
 
-void Value::InternalMoveAssignFromSameType(Value&& that) {
-  CHECK_EQ(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;
-    case Type::DICTIONARY:
-      *dict_ptr_ = std::move(*that.dict_ptr_);
-      return;
-    case Type::LIST:
-      *list_ = std::move(*that.list_);
-      return;
-  }
-}
-
 void Value::InternalCleanup() {
   CHECK(alive_);
 
   switch (type_) {
     case Type::NONE:
     case Type::BOOLEAN:
     case Type::INTEGER:
     case Type::DOUBLE:
       // Nothing to do
       return;
@@ skipping 756 matching lines @@
 }
 
 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