Add scoped_ptr-safe base::Value to Dictionary/List conversion functions.

This change adds two static From() functions to the DictionaryValue and
ListValue classes which take a scoped_ptr to a Value and either convert
it to a scoped_ptr to a DictionaryValue or ListValue, or return nullptr.
These are intended to replace the existing pattern,

  make_scoped_ptr(static_cast<base::DictionaryValue*>(value.release()))

with the shorter and safer alternative,

  base::DictionaryValue::From(value.Pass())

Instances of this pattern in //extensions have been converted as
examples.

Review URL: https://codereview.chromium.org/1308013005

Cr-Commit-Position: refs/heads/master@{#348294}

14 files changed, 63 insertions, 50 deletions

diff --git a/base/values.cc b/base/values.cc
index c829a7f..9b2483e 100644
--- a/base/values.cc
+++ b/base/values.cc
@@ -351,6 +351,16 @@ bool BinaryValue::Equals(const Value* other) const {
 
 ///////////////////// DictionaryValue ////////////////////
 
+// static
+scoped_ptr<DictionaryValue> DictionaryValue::From(scoped_ptr<Value> value) {
+  DictionaryValue* out;
+  if (value && value->GetAsDictionary(&out)) {
+    ignore_result(value.release());
+    return make_scoped_ptr(out);
+  }
+  return nullptr;
+}
+
 DictionaryValue::DictionaryValue()
     : Value(TYPE_DICTIONARY) {
 }
@@ -870,6 +880,16 @@ bool DictionaryValue::Equals(const Value* other) const {
 
 ///////////////////// ListValue ////////////////////
 
+// static
+scoped_ptr<ListValue> ListValue::From(scoped_ptr<Value> value) {
+  ListValue* out;
+  if (value && value->GetAsList(&out)) {
+    ignore_result(value.release());
+    return make_scoped_ptr(out);
+  }
+  return nullptr;
+}
+
 ListValue::ListValue() : Value(TYPE_LIST) {
 }
 
diff --git a/base/values.h b/base/values.h
index 8756deb..56be542 100644
--- a/base/values.h
+++ b/base/values.h
@@ -209,6 +209,9 @@ class BASE_EXPORT BinaryValue: public Value {
 // are |std::string|s and should be UTF-8 encoded.
 class BASE_EXPORT DictionaryValue : public Value {
  public:
+  // Returns |value| if it is a dictionary, nullptr otherwise.
+  static scoped_ptr<DictionaryValue> From(scoped_ptr<Value> value);
+
   DictionaryValue();
   ~DictionaryValue() override;
 
@@ -387,6 +390,9 @@ class BASE_EXPORT ListValue : public Value {
   typedef ValueVector::iterator iterator;
   typedef ValueVector::const_iterator const_iterator;
 
+  // Returns |value| if it is a list, nullptr otherwise.
+  static scoped_ptr<ListValue> From(scoped_ptr<Value> value);
+
   ListValue();
   ~ListValue() override;
 
diff --git a/extensions/browser/api/sockets_tcp/sockets_tcp_apitest.cc b/extensions/browser/api/sockets_tcp/sockets_tcp_apitest.cc
index d6db162..b5bbd0c 100644
--- a/extensions/browser/api/sockets_tcp/sockets_tcp_apitest.cc
+++ b/extensions/browser/api/sockets_tcp/sockets_tcp_apitest.cc
@@ -63,8 +63,8 @@ IN_PROC_BROWSER_TEST_F(SocketsTcpApiTest, SocketsTcpCreateGood) {
           socket_create_function.get(), "[]", browser_context()));
 
   ASSERT_EQ(base::Value::TYPE_DICTIONARY, result->GetType());
-  base::DictionaryValue* value =
-      static_cast<base::DictionaryValue*>(result.get());
+  scoped_ptr<base::DictionaryValue> value =
+      base::DictionaryValue::From(result.Pass());
   int socketId = -1;
   EXPECT_TRUE(value->GetInteger("socketId", &socketId));
   ASSERT_TRUE(socketId > 0);
diff --git a/extensions/browser/api/sockets_tcp_server/sockets_tcp_server_apitest.cc b/extensions/browser/api/sockets_tcp_server/sockets_tcp_server_apitest.cc
index 3414f36..8811c2e 100644
--- a/extensions/browser/api/sockets_tcp_server/sockets_tcp_server_apitest.cc
+++ b/extensions/browser/api/sockets_tcp_server/sockets_tcp_server_apitest.cc
@@ -62,8 +62,8 @@ IN_PROC_BROWSER_TEST_F(SocketsTcpServerApiTest, SocketTCPCreateGood) {
       api_test_utils::RunFunctionAndReturnSingleResult(
           socket_create_function.get(), "[]", browser_context()));
   ASSERT_EQ(base::Value::TYPE_DICTIONARY, result->GetType());
-  base::DictionaryValue* value =
-      static_cast<base::DictionaryValue*>(result.get());
+  scoped_ptr<base::DictionaryValue> value =
+      base::DictionaryValue::From(result.Pass());
   int socketId = -1;
   EXPECT_TRUE(value->GetInteger("socketId", &socketId));
   ASSERT_TRUE(socketId > 0);
diff --git a/extensions/browser/api_test_utils.cc b/extensions/browser/api_test_utils.cc
index 587f8e2..47bcb0e 100644
--- a/extensions/browser/api_test_utils.cc
+++ b/extensions/browser/api_test_utils.cc
@@ -24,11 +24,7 @@ scoped_ptr<base::Value> ParseJSON(const std::string& data) {
 }
 
 scoped_ptr<base::ListValue> ParseList(const std::string& data) {
-  scoped_ptr<base::Value> result = ParseJSON(data);
-  scoped_ptr<base::ListValue> list_result;
-  if (result->GetAsList(nullptr))
-    list_result.reset(static_cast<base::ListValue*>(result.release()));
-  return list_result;
+  return base::ListValue::From(ParseJSON(data));
 }
 
 // This helps us be able to wait until an UIThreadExtensionFunction calls
@@ -75,11 +71,7 @@ namespace extensions {
 namespace api_test_utils {
 
 scoped_ptr<base::DictionaryValue> ParseDictionary(const std::string& data) {
-  scoped_ptr<base::Value> result = ParseJSON(data);
-  scoped_ptr<base::DictionaryValue> dict_result;
-  if (result->GetAsDictionary(nullptr))
-    dict_result.reset(static_cast<base::DictionaryValue*>(result.release()));
-  return dict_result;
+  return base::DictionaryValue::From(ParseJSON(data));
 }
 
 bool GetBoolean(const base::DictionaryValue* val, const std::string& key) {
diff --git a/extensions/common/extension_api.cc b/extensions/common/extension_api.cc
index 81ca72ae..ec6fa18 100644
--- a/extensions/common/extension_api.cc
+++ b/extensions/common/extension_api.cc
@@ -60,8 +60,7 @@ scoped_ptr<base::ListValue> LoadSchemaList(const std::string& name,
 
   CHECK(result.get()) << error_message << " for schema " << schema;
   CHECK(result->IsType(base::Value::TYPE_LIST)) << " for schema " << schema;
-  return scoped_ptr<base::ListValue>(static_cast<base::ListValue*>(
-      result.release()));
+  return base::ListValue::From(result.Pass());
 }
 
 const base::DictionaryValue* FindListItem(const base::ListValue* list,
diff --git a/extensions/common/features/json_feature_provider_source.cc b/extensions/common/features/json_feature_provider_source.cc
index 5b9ee16..5f9dcb5 100644
--- a/extensions/common/features/json_feature_provider_source.cc
+++ b/extensions/common/features/json_feature_provider_source.cc
@@ -30,7 +30,7 @@ void JSONFeatureProviderSource::LoadJSON(int resource_id) {
   scoped_ptr<base::DictionaryValue> value_as_dict;
   if (value) {
     CHECK(value->IsType(base::Value::TYPE_DICTIONARY)) << name_;
-    value_as_dict.reset(static_cast<base::DictionaryValue*>(value.release()));
+    value_as_dict = base::DictionaryValue::From(value.Pass());
   } else {
     // There was some error loading the features file.
     // http://crbug.com/176381
diff --git a/extensions/common/file_util.cc b/extensions/common/file_util.cc
index 53f2715..b615629 100644
--- a/extensions/common/file_util.cc
+++ b/extensions/common/file_util.cc
@@ -194,8 +194,8 @@ scoped_refptr<Extension> LoadExtension(const base::FilePath& extension_path,
                                        Manifest::Location location,
                                        int flags,
                                        std::string* error) {
-  scoped_ptr<base::DictionaryValue> manifest(
-      LoadManifest(extension_path, error));
+  scoped_ptr<base::DictionaryValue> manifest =
+      LoadManifest(extension_path, error);
   if (!manifest.get())
     return NULL;
   if (!extension_l10n_util::LocalizeExtension(
@@ -216,12 +216,13 @@ scoped_refptr<Extension> LoadExtension(const base::FilePath& extension_path,
   return extension;
 }
 
-base::DictionaryValue* LoadManifest(const base::FilePath& extension_path,
-                                    std::string* error) {
+scoped_ptr<base::DictionaryValue> LoadManifest(
+    const base::FilePath& extension_path,
+    std::string* error) {
   return LoadManifest(extension_path, kManifestFilename, error);
 }
 
-base::DictionaryValue* LoadManifest(
+scoped_ptr<base::DictionaryValue> LoadManifest(
     const base::FilePath& extension_path,
     const base::FilePath::CharType* manifest_filename,
     std::string* error) {
@@ -252,7 +253,7 @@ base::DictionaryValue* LoadManifest(
     return NULL;
   }
 
-  return static_cast<base::DictionaryValue*>(root.release());
+  return base::DictionaryValue::From(root.Pass());
 }
 
 bool ValidateExtension(const Extension* extension,
diff --git a/extensions/common/file_util.h b/extensions/common/file_util.h
index 015039e..5bb1287 100644
--- a/extensions/common/file_util.h
+++ b/extensions/common/file_util.h
@@ -59,11 +59,12 @@ scoped_refptr<Extension> LoadExtension(const base::FilePath& extension_root,
 
 // Loads an extension manifest from the specified directory. Returns NULL
 // on failure, with a description of the error in |error|.
-base::DictionaryValue* LoadManifest(const base::FilePath& extension_root,
-                                    std::string* error);
+scoped_ptr<base::DictionaryValue> LoadManifest(
+    const base::FilePath& extension_root,
+    std::string* error);
 
 // Convenience overload for specifying a manifest filename.
-base::DictionaryValue* LoadManifest(
+scoped_ptr<base::DictionaryValue> LoadManifest(
     const base::FilePath& extension_root,
     const base::FilePath::CharType* manifest_filename,
     std::string* error);
diff --git a/extensions/common/file_util_unittest.cc b/extensions/common/file_util_unittest.cc
index 586dbe2..df82b82 100644
--- a/extensions/common/file_util_unittest.cc
+++ b/extensions/common/file_util_unittest.cc
@@ -27,13 +27,13 @@ namespace extensions {
 namespace {
 
 scoped_refptr<Extension> LoadExtensionManifest(
-    base::DictionaryValue* manifest,
+    const base::DictionaryValue& manifest,
     const base::FilePath& manifest_dir,
     Manifest::Location location,
     int extra_flags,
     std::string* error) {
   scoped_refptr<Extension> extension =
-      Extension::Create(manifest_dir, location, *manifest, extra_flags, error);
+      Extension::Create(manifest_dir, location, manifest, extra_flags, error);
   return extension;
 }
 
@@ -49,11 +49,8 @@ scoped_refptr<Extension> LoadExtensionManifest(
     return NULL;
   CHECK_EQ(base::Value::TYPE_DICTIONARY, result->GetType());
   return LoadExtensionManifest(
-      static_cast<base::DictionaryValue*>(result.get()),
-      manifest_dir,
-      location,
-      extra_flags,
-      error);
+      *base::DictionaryValue::From(result.Pass()).get(), manifest_dir, location,
+      extra_flags, error);
 }
 
 }  // namespace
@@ -294,7 +291,7 @@ TEST_F(FileUtilTest, BackgroundScriptsMustExist) {
   std::string error;
   std::vector<extensions::InstallWarning> warnings;
   scoped_refptr<Extension> extension = LoadExtensionManifest(
-      value.get(), temp.path(), Manifest::UNPACKED, 0, &error);
+      *value.get(), temp.path(), Manifest::UNPACKED, 0, &error);
   ASSERT_TRUE(extension.get()) << error;
 
   EXPECT_FALSE(
@@ -308,8 +305,8 @@ TEST_F(FileUtilTest, BackgroundScriptsMustExist) {
   scripts->Clear();
   scripts->Append(new base::StringValue("http://google.com/foo.js"));
 
-  extension = LoadExtensionManifest(
-      value.get(), temp.path(), Manifest::UNPACKED, 0, &error);
+  extension = LoadExtensionManifest(*value.get(), temp.path(),
+                                    Manifest::UNPACKED, 0, &error);
   ASSERT_TRUE(extension.get()) << error;
 
   warnings.clear();
diff --git a/extensions/renderer/event_bindings.cc b/extensions/renderer/event_bindings.cc
index 229a1c8..82e63471 100644
--- a/extensions/renderer/event_bindings.cc
+++ b/extensions/renderer/event_bindings.cc
@@ -265,7 +265,7 @@ void EventBindings::AttachFilteredEvent(
       args.GetReturnValue().Set(static_cast<int32_t>(-1));
       return;
     }
-    filter.reset(static_cast<base::DictionaryValue*>(filter_value.release()));
+    filter = base::DictionaryValue::From(filter_value.Pass());
   }
 
   // Hold onto a weak reference to |filter| so that it can be used after passing
diff --git a/extensions/renderer/guest_view/guest_view_internal_custom_bindings.cc b/extensions/renderer/guest_view/guest_view_internal_custom_bindings.cc
index 3b5ce79..7eb1292 100644
--- a/extensions/renderer/guest_view/guest_view_internal_custom_bindings.cc
+++ b/extensions/renderer/guest_view/guest_view_internal_custom_bindings.cc
@@ -151,9 +151,8 @@ void GuestViewInternalCustomBindings::AttachGuest(
     scoped_ptr<V8ValueConverter> converter(V8ValueConverter::create());
     scoped_ptr<base::Value> params_as_value(
         converter->FromV8Value(args[2], context()->v8_context()));
-    CHECK(params_as_value->IsType(base::Value::TYPE_DICTIONARY));
-    params.reset(
-        static_cast<base::DictionaryValue*>(params_as_value.release()));
+    params = base::DictionaryValue::From(params_as_value.Pass());
+    CHECK(params);
   }
 
   // Add flag to |params| to indicate that the element size is specified in
@@ -226,9 +225,8 @@ void GuestViewInternalCustomBindings::AttachIframeGuest(
     scoped_ptr<V8ValueConverter> converter(V8ValueConverter::create());
     scoped_ptr<base::Value> params_as_value(
         converter->FromV8Value(args[2], context()->v8_context()));
-    CHECK(params_as_value->IsType(base::Value::TYPE_DICTIONARY));
-    params.reset(
-        static_cast<base::DictionaryValue*>(params_as_value.release()));
+    params = base::DictionaryValue::From(params_as_value.Pass());
+    CHECK(params);
   }
 
   // Add flag to |params| to indicate that the element size is specified in
diff --git a/extensions/utility/unpacker.cc b/extensions/utility/unpacker.cc
index 2cd8f01..6e28a49b 100644
--- a/extensions/utility/unpacker.cc
+++ b/extensions/utility/unpacker.cc
@@ -112,7 +112,7 @@ Unpacker::Unpacker(const base::FilePath& working_dir,
 Unpacker::~Unpacker() {
 }
 
-base::DictionaryValue* Unpacker::ReadManifest() {
+scoped_ptr<base::DictionaryValue> Unpacker::ReadManifest() {
   base::FilePath manifest_path = extension_dir_.Append(kManifestFilename);
   if (!base::PathExists(manifest_path)) {
     SetError(errors::kInvalidManifest);
@@ -132,7 +132,7 @@ base::DictionaryValue* Unpacker::ReadManifest() {
     return NULL;
   }
 
-  return static_cast<base::DictionaryValue*>(root.release());
+  return base::DictionaryValue::From(root.Pass());
 }
 
 bool Unpacker::ReadAllMessageCatalogs(const std::string& default_locale) {
@@ -161,7 +161,7 @@ bool Unpacker::ReadAllMessageCatalogs(const std::string& default_locale) {
 
 bool Unpacker::Run() {
   // Parse the manifest.
-  parsed_manifest_.reset(ReadManifest());
+  parsed_manifest_ = ReadManifest();
   if (!parsed_manifest_.get())
     return false;  // Error was already reported.
 
@@ -253,8 +253,8 @@ bool Unpacker::AddDecodedImage(const base::FilePath& path) {
 bool Unpacker::ReadMessageCatalog(const base::FilePath& message_path) {
   std::string error;
   JSONFileValueDeserializer deserializer(message_path);
-  scoped_ptr<base::DictionaryValue> root(static_cast<base::DictionaryValue*>(
-      deserializer.Deserialize(NULL, &error)));
+  scoped_ptr<base::DictionaryValue> root = base::DictionaryValue::From(
+      make_scoped_ptr(deserializer.Deserialize(NULL, &error)));
   if (!root.get()) {
     base::string16 messages_file = message_path.LossyDisplayName();
     if (error.empty()) {
diff --git a/extensions/utility/unpacker.h b/extensions/utility/unpacker.h
index 7e5a7e4..eaffc7b 100644
--- a/extensions/utility/unpacker.h
+++ b/extensions/utility/unpacker.h
@@ -55,8 +55,7 @@ class Unpacker {
   bool DumpMessageCatalogsToFile();
 
   // Parse the manifest.json file inside the extension (not in the header).
-  // Caller takes ownership of return value.
-  base::DictionaryValue* ReadManifest();
+  scoped_ptr<base::DictionaryValue> ReadManifest();
 
   // Parse all _locales/*/messages.json files inside the extension.
   bool ReadAllMessageCatalogs(const std::string& default_locale);