Send a TrackedPreference incident when registry pref validation fails.

components/user_prefs/tracked/tracked_preference_helper.cc

Index: components/user_prefs/tracked/tracked_preference_helper.cc
diff --git a/components/user_prefs/tracked/tracked_preference_helper.cc b/components/user_prefs/tracked/tracked_preference_helper.cc
index 2d39749cf53c27f70741bdbe308e5c0ecfb9a20b..cab44ad5642e3129702d6639c77c945ba9001f49 100644
--- a/components/user_prefs/tracked/tracked_preference_helper.cc
+++ b/components/user_prefs/tracked/tracked_preference_helper.cc
@@ -38,6 +38,10 @@ TrackedPreferenceHelper::ResetAction TrackedPreferenceHelper::GetAction(
     case PrefHashStoreTransaction::SECURE_LEGACY:
       // Accept secure legacy device ID based hashes.
       return DONT_RESET;
+    case PrefHashStoreTransaction::UNSUPPORTED:
+      NOTREACHED()
+          << "GetAction should not be called with an UNSUPPORTED value state";
+      return DONT_RESET;
     case PrefHashStoreTransaction::UNTRUSTED_UNKNOWN_VALUE:  // Falls through.
     case PrefHashStoreTransaction::CHANGED:
       return enforce_ ? DO_RESET : WANTED_RESET;
@@ -80,6 +84,8 @@ void TrackedPreferenceHelper::ReportValidationResult(
       histogram_name =
           user_prefs::tracked::kTrackedPrefHistogramNullInitialized;
       break;
+    case PrefHashStoreTransaction::UNSUPPORTED:
+      return;

[gab, 2016/10/03 21:03:58]

This should be NOTREACHED() as well

[proberge, 2016/10/05 15:36:40]

Done.

     default:

[gab, 2016/10/03 21:03:58]

And while you're here if you don't mind: remove default (i.e. move "NOTREACHED()
+ return" outside of switch).

Preferred style for switches on enums is no default since clang now detects
unhandled enum values :-).

[proberge, 2016/10/05 15:36:40]

I think default:foo is slightly cleaner than adding if(!histogram_name) { foo }
after the switch. Leaving as-is.

[gab, 2016/10/05 17:23:21]

No, the presence of the default prevents clang from generating a compile error
if a new enum value is added. This is bad and you should avoid default cases
when handling enums as much as possible.

You shouldn't handle not having |histogram_name| as it should never happen.
Instead you should document that as

DCHECK(!histogram_name.empty());

after the switch.

[proberge, 2016/10/05 17:53:26]

Done, but can't use empty() on a char*. Initialized it to nullptr and
DCHECK(histogram_name) instead.

       NOTREACHED() << "Unexpected PrefHashStoreTransaction::ValueState: "
                    << value_state;