Integrate registry_hash_store_contents with the rest of tracked prefs.

components/user_prefs/tracked/tracked_atomic_preference.cc

Index: components/user_prefs/tracked/tracked_atomic_preference.cc
diff --git a/components/user_prefs/tracked/tracked_atomic_preference.cc b/components/user_prefs/tracked/tracked_atomic_preference.cc
index 4eb4b6129d2b86e411928032141ac769387e7c42..a3a2fcd4067e22518ada0d6910bf8f92e98aed3b 100644
--- a/components/user_prefs/tracked/tracked_atomic_preference.cc
+++ b/components/user_prefs/tracked/tracked_atomic_preference.cc
@@ -32,20 +32,30 @@ void TrackedAtomicPreference::OnNewValue(
 
 bool TrackedAtomicPreference::EnforceAndReport(
     base::DictionaryValue* pref_store_contents,
-    PrefHashStoreTransaction* transaction) const {
+    PrefHashStoreTransaction* transaction,
+    PrefHashStoreTransaction* registry_transaction) const {
   const base::Value* value = NULL;
   pref_store_contents->Get(pref_path_, &value);
   PrefHashStoreTransaction::ValueState value_state =
       transaction->CheckValue(pref_path_, value);
+  helper_.ReportValidationResult(value_state, transaction->GetStoreType());
 
-  helper_.ReportValidationResult(value_state);
+  PrefHashStoreTransaction::ValueState registry_value_state =

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

Unused outside of if scope below, move it in there.

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

Used on line 76 to decide whether to update the registry.

+      PrefHashStoreTransaction::UNCHANGED;
+  if (registry_transaction) {
+    registry_value_state = registry_transaction->CheckValue(pref_path_, value);
+    helper_.ReportValidationResult(registry_value_state,
+                                   registry_transaction->GetStoreType());

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

Instead of adding "StoreType", I'd prefer to make this more generic, i.e.
GetUMAPrefix() -- then TrackedPreferenceHelper doesn't need to all of a sudden
depend on HashStoreContents (and worse, depend on its impl types.

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

The issue with GetUMAPrefix is that it's odd for DictionaryHashStoreContents to
return an empty string (to avoid changing the existing metrics). 
The dependency is on an enum, not the impl type. 

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

I think that's fine, there can be a comment on it explaining why.
But the enum has to be aligned with the impl types, exposing implementation
details in the interface

+
+    // TODO(proberge): Call delegate_->OnAtomicPreferenceValidation.
+  }
 
-  TrackedPreferenceHelper::ResetAction reset_action =
-      helper_.GetAction(value_state);
   if (delegate_) {
     delegate_->OnAtomicPreferenceValidation(pref_path_, value, value_state,
                                             helper_.IsPersonal());
   }
+  TrackedPreferenceHelper::ResetAction reset_action =
+      helper_.GetAction(value_state);
   helper_.ReportAction(reset_action);
 
   bool was_reset = false;
@@ -61,5 +71,13 @@ bool TrackedAtomicPreference::EnforceAndReport(
     transaction->StoreHash(pref_path_, new_value);
   }
 
+  if (registry_transaction &&
+      (value_state != PrefHashStoreTransaction::UNCHANGED ||

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

Here I think what you want to check is |was_reset|.

i.e. if you have an external_validation_transaction and either the pref was
reset or your external MAC was invalid, you want to store a new MAC?

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

Done.

+       registry_value_state != PrefHashStoreTransaction::UNCHANGED)) {
+    const base::Value* new_value = NULL;
+    pref_store_contents->Get(pref_path_, &new_value);
+    registry_transaction->StoreHash(pref_path_, new_value);
+  }
+
   return was_reset;
 }