Unit test for NULL values in preference validation delegate.

chrome/browser/safe_browsing/preference_validation_delegate_unittest.cc

Index: chrome/browser/safe_browsing/preference_validation_delegate_unittest.cc
diff --git a/chrome/browser/safe_browsing/preference_validation_delegate_unittest.cc b/chrome/browser/safe_browsing/preference_validation_delegate_unittest.cc
index 0e66dc00ed80e9e9806b607197dcf4a5344ea45b..7bbeae49a00ce84134cd3fee37439b37772c508d 100644
--- a/chrome/browser/safe_browsing/preference_validation_delegate_unittest.cc
+++ b/chrome/browser/safe_browsing/preference_validation_delegate_unittest.cc
@@ -123,9 +123,9 @@ class PreferenceValidationDelegateValues
         return value.PassAs<Value>();
       }
       default:
-        ADD_FAILURE() << "unsupported value type " << value_type;
+        // An unknown type means a NULL value.

[gab, 2014/06/16 15:51:41]

Specifically put -1 in a constant and handle it above; e.g. kNullValueForTest,
as this is essentially a hack for your test and it feels fairly obscure right
now.

+        return scoped_ptr<Value>();
     }
-    return scoped_ptr<Value>();
   }
 
   base::Value::Type value_type_;
@@ -139,14 +139,19 @@ TEST_P(PreferenceValidationDelegateValues, Value) {
                                           TrackedPreferenceHelper::DONT_RESET);
   safe_browsing::ClientIncidentReport_IncidentData* incident =
       incidents_.back();
-  EXPECT_EQ(std::string(expected_value_),
-            incident->tracked_preference().atomic_value());
+  if (!*expected_value_) {

[gab, 2014/06/16 15:51:41]

This is a fairly obscure way to do "check that the string is empty which is a
trick in this test to indicate a NULL value"; can you make this clearer to a the
next reader who will see it as a whole (and not in a diff like I am viewing it
now which clearly labels all the pieces of the hack)

+    EXPECT_FALSE(incident->tracked_preference().has_atomic_value());
+  } else {
+    EXPECT_EQ(std::string(expected_value_),
+              incident->tracked_preference().atomic_value());
+  }
 }
 
 INSTANTIATE_TEST_CASE_P(
     Values,
     PreferenceValidationDelegateValues,
     testing::Values(
+        std::tr1::make_tuple(static_cast<base::Value::Type>(-1), ""),

[gab, 2014/06/16 15:51:41]

"" and NULL value is not the same; I guess this is fine here, but please add
extra comments to explain this case.

         std::tr1::make_tuple(base::Value::TYPE_NULL, "null"),
         std::tr1::make_tuple(base::Value::TYPE_BOOLEAN, "false"),
         std::tr1::make_tuple(base::Value::TYPE_INTEGER, "47"),