Remove raw base::DictionaryValue::Set in //extensions

extensions/common/message_bundle_unittest.cc

 // Copyright 2014 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 "extensions/common/message_bundle.h"
 
 #include <stddef.h>
 
 #include <memory>
 #include <string>
+#include <utility>
 #include <vector>
 
 #include "base/i18n/rtl.h"
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "extensions/common/error_utils.h"
 #include "extensions/common/extension_l10n_util.h"
 #include "extensions/common/manifest_constants.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace extensions {
 
 namespace errors = manifest_errors;
 
 class MessageBundleTest : public testing::Test {
  protected:
   enum BadDictionary {
     INVALID_NAME,
     NAME_NOT_A_TREE,
     EMPTY_NAME_TREE,
     MISSING_MESSAGE,
     PLACEHOLDER_NOT_A_TREE,
     EMPTY_PLACEHOLDER_TREE,
     CONTENT_MISSING,
     MESSAGE_PLACEHOLDER_DOESNT_MATCH,
   };
 
   // Helper method for dictionary building.
   void SetDictionary(const std::string& name,

[Devlin, 2017/06/02 15:38:49]

This method seems a little unnecessary now.  It'd be cleaner and more concise to
just inline dict->Set(name, std::move(value)) everywhere.

[jdoerrie, 2017/06/06 12:40:23]

Done.

-                     base::DictionaryValue* subtree,
+                     std::unique_ptr<base::DictionaryValue> subtree,
                      base::DictionaryValue* target) {
-    target->Set(name, static_cast<base::Value*>(subtree));
+    target->Set(name, std::move(subtree));
   }
 
   void CreateContentTree(const std::string& name,
                          const std::string& content,
                          base::DictionaryValue* dict) {
-    base::DictionaryValue* content_tree = new base::DictionaryValue;
+    auto content_tree = base::MakeUnique<base::DictionaryValue>();
     content_tree->SetString(MessageBundle::kContentKey, content);
-    SetDictionary(name, content_tree, dict);
+    SetDictionary(name, std::move(content_tree), dict);
   }
 
   void CreatePlaceholdersTree(base::DictionaryValue* dict) {
-    base::DictionaryValue* placeholders_tree = new base::DictionaryValue;
-    CreateContentTree("a", "A", placeholders_tree);
-    CreateContentTree("b", "B", placeholders_tree);
-    CreateContentTree("c", "C", placeholders_tree);
-    SetDictionary(MessageBundle::kPlaceholdersKey,
-                  placeholders_tree,
+    auto placeholders_tree = base::MakeUnique<base::DictionaryValue>();
+    CreateContentTree("a", "A", placeholders_tree.get());
+    CreateContentTree("b", "B", placeholders_tree.get());
+    CreateContentTree("c", "C", placeholders_tree.get());
+    SetDictionary(MessageBundle::kPlaceholdersKey, std::move(placeholders_tree),
                   dict);
   }
 
   void CreateMessageTree(const std::string& name,
                          const std::string& message,
                          bool create_placeholder_subtree,
                          base::DictionaryValue* dict) {
-    base::DictionaryValue* message_tree = new base::DictionaryValue;
+    auto message_tree = base::MakeUnique<base::DictionaryValue>();
     if (create_placeholder_subtree)
-      CreatePlaceholdersTree(message_tree);
+      CreatePlaceholdersTree(message_tree.get());
     message_tree->SetString(MessageBundle::kMessageKey, message);
-    SetDictionary(name, message_tree, dict);
+    SetDictionary(name, std::move(message_tree), dict);
   }
 
   std::unique_ptr<base::DictionaryValue> CreateGoodDictionary() {
-    std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
+    auto dict = base::MakeUnique<base::DictionaryValue>();
     CreateMessageTree("n1", "message1 $a$ $b$", true, dict.get());
     CreateMessageTree("n2", "message2 $c$", true, dict.get());
     CreateMessageTree("n3", "message3", false, dict.get());
     return dict;
   }
 
   std::unique_ptr<base::DictionaryValue> CreateBadDictionary(
       enum BadDictionary what_is_bad) {
     std::unique_ptr<base::DictionaryValue> dict = CreateGoodDictionary();
     // Now remove/break things.
     switch (what_is_bad) {
       case INVALID_NAME:
         CreateMessageTree("n 5", "nevermind", false, dict.get());
         break;
       case NAME_NOT_A_TREE:
         dict->SetString("n4", "whatever");
         break;
       case EMPTY_NAME_TREE: {
-          base::DictionaryValue* empty_tree = new base::DictionaryValue;
-          SetDictionary("n4", empty_tree, dict.get());
+        auto empty_tree = base::MakeUnique<base::DictionaryValue>();
+        SetDictionary("n4", std::move(empty_tree), dict.get());
         }

[Devlin, 2017/06/02 15:38:48]

broken before, but can we fix this brace?

[jdoerrie, 2017/06/06 12:40:23]

Done, inlined empty_tree so braces are not necessary anymore.

         break;
       case MISSING_MESSAGE:
         dict->Remove("n1.message", NULL);
         break;
       case PLACEHOLDER_NOT_A_TREE:
         dict->SetString("n1.placeholders", "whatever");
         break;
       case EMPTY_PLACEHOLDER_TREE: {
-          base::DictionaryValue* empty_tree = new base::DictionaryValue;
-          SetDictionary("n1.placeholders", empty_tree, dict.get());
+        auto empty_tree = base::MakeUnique<base::DictionaryValue>();
+        SetDictionary("n1.placeholders", std::move(empty_tree), dict.get());
         }
         break;
       case CONTENT_MISSING:
          dict->Remove("n1.placeholders.a.content", NULL);
         break;
       case MESSAGE_PLACEHOLDER_DOESNT_MATCH:
         base::DictionaryValue* value;
         dict->Remove("n1.placeholders.a", NULL);
         dict->GetDictionary("n1.placeholders", &value);
         CreateContentTree("x", "X", value);
@@ skipping 301 matching lines @@
   messages.insert(std::make_pair("message_name", "message_value"));
   (*GetExtensionToL10nMessagesMap())[extension_id] = messages;
 
   L10nMessagesMap* map = GetL10nMessagesMap(extension_id);
   ASSERT_TRUE(NULL != map);
   EXPECT_EQ(1U, map->size());
   EXPECT_EQ("message_value", (*map)["message_name"]);
 }
 
 }  // namespace extensions