[Chromecast] Refactor SerializeToJson to return base::Optional.

This avoids some heap allocations and allows many callsites to
be more concise.

Bug: 646113
Bug: Internal b/117125296
Test: all the unittests I can run on desktop, CQ

Merge-With: eureka-internal/206552
Merge-With: eureka-internal/206533

Change-Id: I48505d8c2a693c7e36616d47b6aad2348db115a6
Reviewed-on: https://chromium-review.googlesource.com/c/1256214Reviewed-by: Luke Halliwell <halliwell@chromium.org>
Commit-Queue: Simeon Anfinrud <sanfin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596055}

chromecast/base/device_capabilities.h

@@ -120,7 +120,7 @@ class DeviceCapabilities {
     }
 
     // Accessor for complete capabilities string in JSON format.
-    const std::string& json_string() const { return *json_string_.get(); }
+    const std::string& json_string() const { return json_string_; }
 
    private:
     friend class base::RefCountedThreadSafe<Data>;
@@ -135,7 +135,7 @@ class DeviceCapabilities {
     ~Data();
 
     const std::unique_ptr<const base::DictionaryValue> dictionary_;
-    const std::unique_ptr<const std::string> json_string_;
+    const std::string json_string_;
 
     DISALLOW_COPY_AND_ASSIGN(Data);
   };

chromecast/base/device_capabilities_impl.cc

@@ -103,16 +103,13 @@ void DeviceCapabilities::Validator::SetPrivateValidatedValue(
 
 DeviceCapabilities::Data::Data()
     : dictionary_(new base::DictionaryValue),
-      json_string_(SerializeToJson(*dictionary_)) {
-  DCHECK(json_string_.get());
-}
+      json_string_(*SerializeToJson(*dictionary_)) {}
 
 DeviceCapabilities::Data::Data(
     std::unique_ptr<const base::DictionaryValue> dictionary)
     : dictionary_(std::move(dictionary)),
-      json_string_(SerializeToJson(*dictionary_)) {
+      json_string_(*SerializeToJson(*dictionary_)) {
   DCHECK(dictionary_.get());
-  DCHECK(json_string_.get());
 }
 
 DeviceCapabilitiesImpl::Data::~Data() {}

chromecast/base/device_capabilities_impl_unittest.cc

@@ -187,8 +187,8 @@ bool JsonStringEquals(const std::string& json,
                       const base::Value& value) {
   base::DictionaryValue dict_value;
   dict_value.Set(key, value.CreateDeepCopy());
-  std::unique_ptr<const std::string> dict_json(SerializeToJson(dict_value));
-  return dict_json.get() && *dict_json == json;
+  base::Optional<std::string> dict_json = SerializeToJson(dict_value);
+  return dict_json && *dict_json == json;
 }
 
 // The function runs through the set of basic operations of DeviceCapabilities.
@@ -273,9 +273,8 @@ class DeviceCapabilitiesImplTest : public ::testing::Test {
 
 // Tests that class is in correct state after Create().
 TEST_F(DeviceCapabilitiesImplTest, Create) {
-  std::unique_ptr<const std::string> empty_dict_string(
-      SerializeToJson(base::DictionaryValue()));
-  EXPECT_EQ(capabilities()->GetAllData()->json_string(), *empty_dict_string);
+  std::string empty_dict_string = *SerializeToJson(base::DictionaryValue());
+  EXPECT_EQ(capabilities()->GetAllData()->json_string(), empty_dict_string);
   EXPECT_TRUE(capabilities()->GetAllData()->dictionary().empty());
 }
 
@@ -290,9 +289,8 @@ TEST_F(DeviceCapabilitiesImplTest, Register) {
                                       false);
 
   EXPECT_EQ(capabilities()->GetValidator(key), &manager);
-  std::unique_ptr<const std::string> empty_dict_string(
-      SerializeToJson(base::DictionaryValue()));
-  EXPECT_EQ(capabilities()->GetAllData()->json_string(), *empty_dict_string);
+  std::string empty_dict_string = *SerializeToJson(base::DictionaryValue());
+  EXPECT_EQ(capabilities()->GetAllData()->json_string(), empty_dict_string);
   EXPECT_FALSE(capabilities()->GetCapability(key));
 }
 
@@ -309,9 +307,8 @@ TEST_F(DeviceCapabilitiesImplTest, Unregister) {
   delete manager;
 
   EXPECT_FALSE(capabilities()->GetValidator(key));
-  std::unique_ptr<const std::string> empty_dict_string(
-      SerializeToJson(base::DictionaryValue()));
-  EXPECT_EQ(capabilities()->GetAllData()->json_string(), *empty_dict_string);
+  std::string empty_dict_string = *SerializeToJson(base::DictionaryValue());
+  EXPECT_EQ(capabilities()->GetAllData()->json_string(), empty_dict_string);
   EXPECT_FALSE(capabilities()->GetCapability(key));
 }
 

chromecast/base/serializers.cc

@@ -23,12 +23,12 @@ std::unique_ptr<base::Value> DeserializeFromJson(const std::string& text) {
   return value;
 }
 
-std::unique_ptr<std::string> SerializeToJson(const base::Value& value) {
-  std::unique_ptr<std::string> json_str(new std::string());
-  JSONStringValueSerializer serializer(json_str.get());
-  if (!serializer.Serialize(value))
-    json_str.reset(nullptr);
+base::Optional<std::string> SerializeToJson(const base::Value& value) {
+  std::string json_str;
+  JSONStringValueSerializer serializer(&json_str);
+  if (serializer.Serialize(value))
     return json_str;
+  return base::nullopt;
 }
 
 std::unique_ptr<base::Value> DeserializeJsonFromFile(

chromecast/base/serializers.h

@@ -8,6 +8,7 @@
 #include <memory>
 #include <string>
 
+#include "base/optional.h"
 
 namespace base {
 class Value;
@@ -22,8 +23,10 @@ namespace chromecast {
 std::unique_ptr<base::Value> DeserializeFromJson(const std::string& text);
 
 // Helper function which serializes |value| into a JSON string. If a
-// serialization error occurs,the return value will hold the NULL pointer.
-std::unique_ptr<std::string> SerializeToJson(const base::Value& value);
+// serialization error occurs,the return value will be base::nullopt.
+// Dereferencing the result is equivalent to DCHECK()-ing that serialization
+// succeeded and retrieving the serialized string.
+base::Optional<std::string> SerializeToJson(const base::Value& value);
 
 // Helper function which deserializes JSON file at |path| into a base::Value.
 // If file in |path| is empty, is not valid JSON, or if some other

chromecast/base/serializers_unittest.cc

@@ -56,22 +56,22 @@ TEST(DeserializeFromJson, PoorlyFormedJsonObject) {
 
 TEST(SerializeToJson, BadValue) {
   base::Value value(std::vector<char>(12));
-  std::unique_ptr<std::string> str = SerializeToJson(value);
-  EXPECT_EQ(nullptr, str.get());
+  base::Optional<std::string> str = SerializeToJson(value);
+  EXPECT_EQ(base::nullopt, str);
 }
 
 TEST(SerializeToJson, EmptyValue) {
   base::DictionaryValue value;
-  std::unique_ptr<std::string> str = SerializeToJson(value);
-  ASSERT_NE(nullptr, str.get());
+  base::Optional<std::string> str = SerializeToJson(value);
+  ASSERT_NE(base::nullopt, str);
   EXPECT_EQ(kEmptyJsonString, *str);
 }
 
 TEST(SerializeToJson, PopulatedValue) {
   base::DictionaryValue orig_value;
   orig_value.SetString(kTestKey, kTestValue);
-  std::unique_ptr<std::string> str = SerializeToJson(orig_value);
-  ASSERT_NE(nullptr, str.get());
+  base::Optional<std::string> str = SerializeToJson(orig_value);
+  ASSERT_NE(nullptr, str);
 
   std::unique_ptr<base::Value> new_value = DeserializeFromJson(*str);
   ASSERT_NE(nullptr, new_value.get());

chromecast/crash/linux/crash_testing_utils.cc

@@ -62,7 +62,7 @@ int WriteLockFile(const std::string& path, base::ListValue* contents) {
   std::string lockfile;
 
   for (const auto& elem : *contents) {
-    std::unique_ptr<std::string> dump_info = SerializeToJson(elem);
+    base::Optional<std::string> dump_info = SerializeToJson(elem);
     RCHECK(dump_info, -1, "Failed to serialize DumpInfo");
     lockfile += *dump_info;
     lockfile += "\n";  // Add line seperatators

chromecast/crash/linux/synchronized_minidump_manager.cc

@@ -302,7 +302,7 @@ bool SynchronizedMinidumpManager::WriteFiles(const base::ListValue* dumps,
   std::string lockfile;
 
   for (const auto& elem : *dumps) {
-    std::unique_ptr<std::string> dump_info = SerializeToJson(elem);
+    base::Optional<std::string> dump_info = SerializeToJson(elem);
     RCHECK(dump_info, false);
     lockfile += *dump_info;
     lockfile += "\n";  // Add line seperatators