Integrate registry_hash_store_contents with the rest of tracked prefs.

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 "components/user_prefs/tracked/tracked_preference_histogram_names.h"
 
@@ skipping 33 matching lines @@
   NOTREACHED() << "Unexpected PrefHashStoreTransaction::ValueState: "
                << value_state;
   return DONT_RESET;
 }
 
 bool TrackedPreferenceHelper::IsPersonal() const {
   return personal_;
 }
 
 void TrackedPreferenceHelper::ReportValidationResult(
-    PrefHashStoreTransaction::ValueState value_state) const {
+    PrefHashStoreTransaction::ValueState value_state,
+    HashStoreContentsType validation_type) const {
+  std::string histogram_name_prefix;
+  switch (validation_type) {
+    case HashStoreContentsType::DICTIONARY_HASH_STORE_CONTENTS:
+      // Don't add a prefix for consistency with existing metrics.
+      histogram_name_prefix = "";
+      break;
+    case HashStoreContentsType::REGISTRY_HASH_STORE_CONTENTS:
+      histogram_name_prefix = "RegistryStore";
+      break;
+    default:
+      NOTREACHED() << "Unexpected HashStoreContentsType";
+  }
+
+  std::string histogram_name;

[gab, 2016/08/03 18:19:36]

const char* histogram_name;

to avoid an unnecessary string copy of the constants.

[proberge, 2016/08/04 00:13:47]

Done.

   switch (value_state) {
     case PrefHashStoreTransaction::UNCHANGED:
-      UMA_HISTOGRAM_ENUMERATION(
-          user_prefs::tracked::kTrackedPrefHistogramUnchanged, reporting_id_,
-          reporting_ids_count_);
-      return;
+      histogram_name = user_prefs::tracked::kTrackedPrefHistogramUnchanged;
+      break;
     case PrefHashStoreTransaction::CLEARED:
-      UMA_HISTOGRAM_ENUMERATION(
-          user_prefs::tracked::kTrackedPrefHistogramCleared, reporting_id_,
-          reporting_ids_count_);
-      return;
+      histogram_name = user_prefs::tracked::kTrackedPrefHistogramCleared;
+      break;
     case PrefHashStoreTransaction::SECURE_LEGACY:
-      UMA_HISTOGRAM_ENUMERATION(
-          user_prefs::tracked::kTrackedPrefHistogramMigratedLegacyDeviceId,
-          reporting_id_, reporting_ids_count_);
-      return;
+      histogram_name =
+          user_prefs::tracked::kTrackedPrefHistogramMigratedLegacyDeviceId;
+      break;
     case PrefHashStoreTransaction::CHANGED:
-      UMA_HISTOGRAM_ENUMERATION(
-          user_prefs::tracked::kTrackedPrefHistogramChanged, reporting_id_,
-          reporting_ids_count_);
-      return;
+      histogram_name = user_prefs::tracked::kTrackedPrefHistogramChanged;
+      break;
     case PrefHashStoreTransaction::UNTRUSTED_UNKNOWN_VALUE:
-      UMA_HISTOGRAM_ENUMERATION(
-          user_prefs::tracked::kTrackedPrefHistogramInitialized, reporting_id_,
-          reporting_ids_count_);
-      return;
+      histogram_name = user_prefs::tracked::kTrackedPrefHistogramInitialized;
+      break;
     case PrefHashStoreTransaction::TRUSTED_UNKNOWN_VALUE:
-      UMA_HISTOGRAM_ENUMERATION(
-          user_prefs::tracked::kTrackedPrefHistogramTrustedInitialized,
-          reporting_id_, reporting_ids_count_);
-      return;
+      histogram_name =
+          user_prefs::tracked::kTrackedPrefHistogramTrustedInitialized;
+      break;
     case PrefHashStoreTransaction::TRUSTED_NULL_VALUE:
-      UMA_HISTOGRAM_ENUMERATION(
-          user_prefs::tracked::kTrackedPrefHistogramNullInitialized,
-          reporting_id_, reporting_ids_count_);
-      return;
+      histogram_name =
+          user_prefs::tracked::kTrackedPrefHistogramNullInitialized;
+      break;
+    default:
+      NOTREACHED() << "Unexpected PrefHashStoreTransaction::ValueState: "
+                   << value_state;
   }
-  NOTREACHED() << "Unexpected PrefHashStoreTransaction::ValueState: "
-               << value_state;
+
+  // Using FactoryGet to allow dynamic histogram names. Should be equivalent to
+  // UMA_HISTOGRAM_ENUMERATION(name, reporting_id_, reporting_ids_count_);
+  // TODO: RegistryStoreSettings.TrackedPreferenceInitialized does not match
+  // format from histograms.xml.
+  base::HistogramBase* histogram = base::LinearHistogram::FactoryGet(
+      histogram_name_prefix.append(histogram_name), 1, reporting_ids_count_,

[gab, 2016/08/03 18:19:36]

Add a '.' between prefix and base histogram name

[proberge, 2016/08/04 00:13:46]

Done. Any more efficient way than doing two appends?

[gab, 2016/08/05 19:59:25]

In C++ strings are mutable so two appends is efficient (unlike Java where each
string modifications results in a new string)

+      reporting_ids_count_ + 1, base::HistogramBase::kUmaTargetedHistogramFlag);
+  histogram->Add(reporting_id_);
 }
 
 void TrackedPreferenceHelper::ReportAction(ResetAction reset_action) const {
   switch (reset_action) {
     case DONT_RESET:
       // No report for DONT_RESET.
       break;
     case WANTED_RESET:
       UMA_HISTOGRAM_ENUMERATION(
           user_prefs::tracked::kTrackedPrefHistogramWantedReset, reporting_id_,
@@ skipping 10 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);
 }