Send a TrackedPreference incident when registry pref validation fails.

components/user_prefs/tracked/tracked_preference_helper.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/user_prefs/tracked/tracked_preference_helper.h"
 
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/histogram_macros.h"
 #include "components/user_prefs/tracked/tracked_preference_histogram_names.h"
@@ skipping 20 matching lines @@
     case PrefHashStoreTransaction::CLEARED:
       // Unfortunate case, but there is nothing we can do.
       return DONT_RESET;
     case PrefHashStoreTransaction::TRUSTED_NULL_VALUE:  // Falls through.
     case PrefHashStoreTransaction::TRUSTED_UNKNOWN_VALUE:
       // It is okay to seed the hash in this case.
       return DONT_RESET;
     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;
   }
   NOTREACHED() << "Unexpected PrefHashStoreTransaction::ValueState: "
                << value_state;
   return DONT_RESET;
 }
 
 bool TrackedPreferenceHelper::IsPersonal() const {
@@ skipping 22 matching lines @@
       histogram_name = user_prefs::tracked::kTrackedPrefHistogramInitialized;
       break;
     case PrefHashStoreTransaction::TRUSTED_UNKNOWN_VALUE:
       histogram_name =
           user_prefs::tracked::kTrackedPrefHistogramTrustedInitialized;
       break;
     case PrefHashStoreTransaction::TRUSTED_NULL_VALUE:
       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;
       return;
   }
 
   std::string full_histogram_name(histogram_name);
   if (!validation_type_suffix.empty()) {
     full_histogram_name.push_back('.');
     validation_type_suffix.AppendToString(&full_histogram_name);
   }
@@ skipping 27 matching lines @@
     size_t count) const {
   // The histogram below is an expansion of the UMA_HISTOGRAM_COUNTS_100 macro
   // adapted to allow for a dynamically suffixed histogram name.
   // Note: The factory creates and owns the histogram.
   base::HistogramBase* histogram = base::LinearHistogram::FactoryGet(
       user_prefs::tracked::kTrackedSplitPrefHistogramChanged + pref_path_, 1,
       100,  // Allow counts up to 100.
       101, base::HistogramBase::kUmaTargetedHistogramFlag);
   histogram->Add(count);
 }