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 fe49826dc45f0016ede4e32b495ddb4cc4584ce3..2415d7f844a909a3ebbdcbd70741b05c8dff52ae 100644
--- a/components/user_prefs/tracked/tracked_atomic_preference.cc
+++ b/components/user_prefs/tracked/tracked_atomic_preference.cc
@@ -24,6 +24,10 @@ TrackedAtomicPreference::TrackedAtomicPreference(
       delegate_(delegate) {
 }
 
+TrackedPreferenceType TrackedAtomicPreference::GetType() const {
+  return TrackedPreferenceType::ATOMIC;
+}
+
 void TrackedAtomicPreference::OnNewValue(
     const base::Value* value,
     PrefHashStoreTransaction* transaction) const {
@@ -32,20 +36,32 @@ void TrackedAtomicPreference::OnNewValue(
 
 bool TrackedAtomicPreference::EnforceAndReport(
     base::DictionaryValue* pref_store_contents,
-    PrefHashStoreTransaction* transaction) const {
+    PrefHashStoreTransaction* transaction,
+    PrefHashStoreTransaction* external_validation_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->GetStoreUMASuffix());
 
-  TrackedPreferenceHelper::ResetAction reset_action =
-      helper_.GetAction(value_state);
+  PrefHashStoreTransaction::ValueState external_validation_value_state =
+      PrefHashStoreTransaction::UNCHANGED;
+  if (external_validation_transaction) {
+    external_validation_value_state =
+        external_validation_transaction->CheckValue(pref_path_, value);
+    helper_.ReportValidationResult(
+        external_validation_value_state,
+        external_validation_transaction->GetStoreUMASuffix());
+
+    // TODO(proberge): Call delegate_->OnAtomicPreferenceValidation.
+  }
+
   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 +77,16 @@ bool TrackedAtomicPreference::EnforceAndReport(
     transaction->StoreHash(pref_path_, new_value);
   }
 
+  // Update MACs in the external store if there is one and there either was a
+  // reset or external validation failed.
+  if (external_validation_transaction &&
+      (was_reset ||
+       external_validation_value_state !=
+           PrefHashStoreTransaction::UNCHANGED)) {
+    const base::Value* new_value = nullptr;
+    pref_store_contents->Get(pref_path_, &new_value);
+    external_validation_transaction->StoreHash(pref_path_, new_value);

[gab, 2016/09/16 19:47:33]

This is fine in practice because it runs as part of FilterOnLoad() and can't
race with other registry writes, but architecturally it's so far from it that
it's hard to justify this with a comment here... WDYT?

[proberge, 2016/09/20 21:35:45]

This is similar logic to line 77. Also, in theory we shouldn't be writing any
prefs at this time since they're not loaded yet.

[gab, 2016/09/21 17:55:30]

It's different than 77 because 77 writes to |transaction| which is known to be
owned by this sequence (and therefore we have exclusive access to its state
right now). |external_validation_transaction| is different because it writes to
unowned state which other threads also write to.

[proberge, 2016/09/21 21:09:24]

Hmm, you're right. Your original comment said "it's so far from it that it's
hard to justify this with a comment here". Are you suggesting to simply add a
comment, or to move this to a separate method? or to move this logic out into
pref_hash_filter?

[gab, 2016/09/22 18:38:28]

I think just adding to TrackedPreference::EnforceAndReport's definition
something like "This call assumes exclusive access to this preference and its
associated state and as such should only be called before any other subsystem is
made aware of it." should do it.

[proberge, 2016/09/22 19:10:15]

Done.

+  }
+
   return was_reset;
 }