Made type conversions from string more resilient by providing a default value.

Bug: 960779
Change-Id: I21dedf74f9930732cb130ece2bfeee737aba6dc5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1603062
Commit-Queue: Allen Bauer <kylixrd@chromium.org>
Auto-Submit: Allen Bauer <kylixrd@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#658316}

ui/views/metadata/metadata_types.h

@@ -185,8 +185,8 @@ class ClassPropertyMetaData
   ~ClassPropertyMetaData() override = default;
 
   void SetValueAsString(void* obj, const base::string16& new_value) override {
-    (static_cast<TClass*>(obj)->*Set)(
-        Convert<base::string16, TValue>(new_value));
+    (static_cast<TClass*>(obj)->*Set)(Convert<base::string16, TValue>(
+        new_value, (static_cast<TClass*>(obj)->*Get)()));
   }
 
   PropertyFlags GetPropertyFlags() const override {

ui/views/metadata/type_conversion.cc

@@ -84,81 +84,97 @@ base::string16 ConvertToString<gfx::Size>(const gfx::Size& source_value) {
 }
 
 template <>
-int8_t ConvertFromString<int8_t>(const base::string16& source_value) {
+int8_t ConvertFromString<int8_t>(const base::string16& source_value,
+                                 int8_t default_value) {
   int32_t ret = 0;
   return base::StringToInt(source_value, &ret) &&
                  base::IsValueInRangeForNumericType<int8_t>(ret)
              ? static_cast<int8_t>(ret)
-             : 0;
+             : default_value;
 }
 
 template <>
-int16_t ConvertFromString<int16_t>(const base::string16& source_value) {
+int16_t ConvertFromString<int16_t>(const base::string16& source_value,
+                                   int16_t default_value) {
   int32_t ret = 0;
   return base::StringToInt(source_value, &ret) &&
                  base::IsValueInRangeForNumericType<int16_t>(ret)
              ? static_cast<int16_t>(ret)
-             : 0;
+             : default_value;
 }
 
 template <>
-int32_t ConvertFromString<int32_t>(const base::string16& source_value) {
+int32_t ConvertFromString<int32_t>(const base::string16& source_value,
+                                   int32_t default_value) {
   int32_t ret = 0;
-  return base::StringToInt(source_value, &ret) ? static_cast<int32_t>(ret) : 0;
+  return base::StringToInt(source_value, &ret) ? static_cast<int32_t>(ret)
+                                               : default_value;
 }
 
 template <>
-int64_t ConvertFromString<int64_t>(const base::string16& source_value) {
+int64_t ConvertFromString<int64_t>(const base::string16& source_value,
+                                   int64_t default_value) {
   int64_t ret = 0;
-  return base::StringToInt64(source_value, &ret) ? ret : 0;
+  return base::StringToInt64(source_value, &ret) ? ret : default_value;
 }
 
 template <>
-uint8_t ConvertFromString<uint8_t>(const base::string16& source_value) {
+uint8_t ConvertFromString<uint8_t>(const base::string16& source_value,
+                                   uint8_t default_value) {
   uint32_t ret = 0;
   return base::StringToUint(source_value, &ret) &&
                  base::IsValueInRangeForNumericType<uint8_t>(ret)
              ? static_cast<uint8_t>(ret)
-             : 0;
+             : default_value;
 }
 
 template <>
-uint16_t ConvertFromString<uint16_t>(const base::string16& source_value) {
+uint16_t ConvertFromString<uint16_t>(const base::string16& source_value,
+                                     uint16_t default_value) {
   uint32_t ret = 0;
   return base::StringToUint(source_value, &ret) &&
                  base::IsValueInRangeForNumericType<uint16_t>(ret)
              ? static_cast<uint16_t>(ret)
-             : 0;
+             : default_value;
 }
 
 template <>
-uint32_t ConvertFromString<uint32_t>(const base::string16& source_value) {
+uint32_t ConvertFromString<uint32_t>(const base::string16& source_value,
+                                     uint32_t default_value) {
   uint32_t ret = 0;
   return base::StringToUint(source_value, &ret) ? static_cast<uint32_t>(ret)
-                                                : 0;
+                                                : default_value;
 }
 
 template <>
-uint64_t ConvertFromString<uint64_t>(const base::string16& source_value) {
+uint64_t ConvertFromString<uint64_t>(const base::string16& source_value,
+                                     uint64_t default_value) {
   uint64_t ret = 0;
-  return base::StringToUint64(source_value, &ret) ? ret : 0;
+  return base::StringToUint64(source_value, &ret) ? ret : default_value;
 }
 
 template <>
-float ConvertFromString<float>(const base::string16& source_value) {
-  return static_cast<float>(ConvertFromString<double>(source_value));
+float ConvertFromString<float>(const base::string16& source_value,
+                               float default_value) {
+  return static_cast<float>(
+      ConvertFromString<double>(source_value, default_value));
 }
 
 template <>
-double ConvertFromString<double>(const base::string16& source_value) {
+double ConvertFromString<double>(const base::string16& source_value,
+                                 double default_value) {
   double ret = 0;
-  return base::StringToDouble(base::UTF16ToUTF8(source_value), &ret) ? ret
-                                                                     : 0.0;
+  return base::StringToDouble(base::UTF16ToUTF8(source_value), &ret)
+             ? ret
+             : default_value;
 }
 
 template <>
-bool ConvertFromString<bool>(const base::string16& source_value) {
-  return source_value == base::ASCIIToUTF16("true");
+bool ConvertFromString<bool>(const base::string16& source_value,
+                             bool default_value) {
+  if (source_value == base::ASCIIToUTF16("false"))
+    return false;
+  return source_value == base::ASCIIToUTF16("true") || default_value;
 }
 
 }  // namespace metadata

ui/views/metadata/type_conversion.h

@@ -33,17 +33,23 @@ template <typename TSource, typename TTarget>
 class TypeConverter {
  public:
   static TTarget Convert(ArgType<TSource>) = delete;
+  static TTarget Convert(ArgType<TSource>, ArgType<TTarget>) = delete;
 
  private:
   TypeConverter();
 };
 
-// Master Type Conversion Function --------------------------------------------
+// Master Type Conversion Functions --------------------------------------------
 template <typename TSource, typename TTarget>
 TTarget Convert(ArgType<TSource> source_value) {
   return TypeConverter<TSource, TTarget>::Convert(source_value);
 }
 
+template <typename TSource, typename TTarget>
+TTarget Convert(ArgType<TSource> source_value, ArgType<TTarget> default_value) {
+  return TypeConverter<TSource, TTarget>::Convert(source_value, default_value);
+}
+
 // String Conversions ---------------------------------------------------------
 
 template <typename TSource>
@@ -106,58 +112,74 @@ class TypeConverter<base::string16, base::string16> {
   static base::string16 Convert(const base::string16& source_val) {
     return ConvertToString<base::string16>(source_val);
   }
+  static base::string16 Convert(const base::string16& source_val,
+                                const base::string16& default_value) {
+    return ConvertToString<base::string16>(source_val);
+  }
 };
 
 template <typename TTarget>
-TTarget ConvertFromString(const base::string16& source_value) = delete;
+TTarget ConvertFromString(const base::string16&, ArgType<TTarget>) = delete;
 
 template <>
 VIEWS_EXPORT int8_t
-ConvertFromString<int8_t>(const base::string16& source_value);
+ConvertFromString<int8_t>(const base::string16& source_value,
+                          int8_t default_value);
 
 template <>
 VIEWS_EXPORT int16_t
-ConvertFromString<int16_t>(const base::string16& source_value);
+ConvertFromString<int16_t>(const base::string16& source_value,
+                           int16_t default_value);
 
 template <>
 VIEWS_EXPORT int32_t
-ConvertFromString<int32_t>(const base::string16& source_value);
+ConvertFromString<int32_t>(const base::string16& source_value,
+                           int32_t default_value);
 
 template <>
 VIEWS_EXPORT int64_t
-ConvertFromString<int64_t>(const base::string16& source_value);
+ConvertFromString<int64_t>(const base::string16& source_value,
+                           int64_t default_value);
 
 template <>
 VIEWS_EXPORT uint8_t
-ConvertFromString<uint8_t>(const base::string16& source_value);
+ConvertFromString<uint8_t>(const base::string16& source_value,
+                           uint8_t default_value);
 
 template <>
 VIEWS_EXPORT uint16_t
-ConvertFromString<uint16_t>(const base::string16& source_value);
+ConvertFromString<uint16_t>(const base::string16& source_value,
+                            uint16_t default_value);
 
 template <>
 VIEWS_EXPORT uint32_t
-ConvertFromString<uint32_t>(const base::string16& source_value);
+ConvertFromString<uint32_t>(const base::string16& source_value,
+                            uint32_t default_value);
 
 template <>
 VIEWS_EXPORT uint64_t
-ConvertFromString<uint64_t>(const base::string16& source_value);
+ConvertFromString<uint64_t>(const base::string16& source_value,
+                            uint64_t default_value);
 
 template <>
 VIEWS_EXPORT double ConvertFromString<double>(
-    const base::string16& source_value);
+    const base::string16& source_value,
+    double default_value);
 
 template <>
-VIEWS_EXPORT float ConvertFromString<float>(const base::string16& source_value);
+VIEWS_EXPORT float ConvertFromString<float>(const base::string16& source_value,
+                                            float default_value);
 
 template <>
-VIEWS_EXPORT bool ConvertFromString<bool>(const base::string16& source_value);
+VIEWS_EXPORT bool ConvertFromString<bool>(const base::string16& source_value,
+                                          bool default_value);
 
 template <typename TTarget>
 class TypeConverter<base::string16, TTarget> {
  public:
-  static TTarget Convert(const base::string16& source_value) {
-    return ConvertFromString<TTarget>(source_value);
+  static TTarget Convert(const base::string16& source_value,
+                         ArgType<TTarget> default_value) {
+    return ConvertFromString<TTarget>(source_value, default_value);
   }
 };
 

ui/views/metadata/type_conversion_unittest.cc

@@ -9,20 +9,30 @@
 #include "testing/platform_test.h"
 #include "ui/gfx/geometry/rect.h"
 
-namespace TC = views::metadata;
+namespace VM = views::metadata;
 
 using TypeConversionTest = PlatformTest;
 
 TEST_F(TypeConversionTest, TestConversion_IntToString) {
   int from_int = 5;
-  base::string16 to_string = TC::Convert<int, base::string16>(from_int);
+  base::string16 to_string = VM::Convert<int, base::string16>(from_int);
 
   EXPECT_EQ(to_string, base::ASCIIToUTF16("5"));
 }
 
 TEST_F(TypeConversionTest, TestConversion_StringToInt) {
   base::string16 from_string = base::ASCIIToUTF16("10");
-  int to_int = TC::Convert<base::string16, int>(from_string);
+  int to_int = VM::Convert<base::string16, int>(from_string, 0);
 
   EXPECT_EQ(to_int, 10);
 }
+
+// This tests whether the converter handles a bogus input string, in which case
+// the default value should be returned.
+TEST_F(TypeConversionTest, TestConversion_StringToIntDefault) {
+  const int default_value = 1000;
+  base::string16 from_string = base::ASCIIToUTF16("Foo");
+  int to_int = VM::Convert<base::string16, int>(from_string, default_value);
+
+  EXPECT_EQ(to_int, default_value);
+}