Move preference MACs to the protected preference stores.

chrome/browser/prefs/pref_hash_store_impl.cc

Index: chrome/browser/prefs/pref_hash_store_impl.cc
diff --git a/chrome/browser/prefs/pref_hash_store_impl.cc b/chrome/browser/prefs/pref_hash_store_impl.cc
index 270c6d0fa93c7342a80cfd6b2979e3c805f544c2..b0164521513d12803362dabf27eba812f4256c94 100644
--- a/chrome/browser/prefs/pref_hash_store_impl.cc
+++ b/chrome/browser/prefs/pref_hash_store_impl.cc
@@ -8,31 +8,15 @@
 #include "base/metrics/histogram.h"
 #include "base/values.h"
 #include "chrome/browser/prefs/pref_hash_store_transaction.h"
-#include "chrome/browser/prefs/tracked/hash_store_contents.h"
-
-namespace {
-
-// Returns true if the dictionary of hashes stored in |contents| is trusted
-// (which implies unknown values can be trusted as newly tracked values).
-bool IsHashDictionaryTrusted(const PrefHashCalculator& calculator,
-                             const HashStoreContents& contents) {
-  const base::DictionaryValue* store_contents = contents.GetContents();
-  std::string super_mac = contents.GetSuperMac();
-  // The store must be initialized and have a valid super MAC to be trusted.
-  return store_contents && !super_mac.empty() &&
-         calculator.Validate(contents.hash_store_id(),
-                             store_contents,
-                             super_mac) == PrefHashCalculator::VALID;
-}
-
-}  // namespace
+#include "chrome/browser/prefs/tracked/dictionary_hash_store_contents.h"
 
 class PrefHashStoreImpl::PrefHashStoreTransactionImpl
     : public PrefHashStoreTransaction {
  public:
   // Constructs a PrefHashStoreTransactionImpl which can use the private
   // members of its |outer| PrefHashStoreImpl.
-  explicit PrefHashStoreTransactionImpl(PrefHashStoreImpl* outer);
+  PrefHashStoreTransactionImpl(PrefHashStoreImpl* outer,
+                               base::DictionaryValue* storage);
   virtual ~PrefHashStoreTransactionImpl();
 
   // PrefHashStoreTransaction implementation.
@@ -48,76 +32,76 @@ class PrefHashStoreImpl::PrefHashStoreTransactionImpl
   virtual void StoreSplitHash(
       const std::string& path,
       const base::DictionaryValue* split_value) OVERRIDE;
+  virtual bool HasHash(const std::string& path) const OVERRIDE;
+  virtual void ImportHash(const std::string& path,
+                          const base::Value* hash) OVERRIDE;
+  virtual void ClearHash(const std::string& path) OVERRIDE;
 
  private:
   bool GetSplitMacs(const std::string& path,
                     std::map<std::string, std::string>* split_macs) const;
+
   PrefHashStoreImpl* outer_;
-  bool has_changed_;
+  DictionaryHashStoreContents contents_;
+
+  bool super_mac_valid_;
+  bool super_mac_dirty_;
 
   DISALLOW_COPY_AND_ASSIGN(PrefHashStoreTransactionImpl);
 };
 
 PrefHashStoreImpl::PrefHashStoreImpl(const std::string& seed,
                                      const std::string& device_id,
-                                     scoped_ptr<HashStoreContents> contents,
                                      bool use_super_mac)
     : pref_hash_calculator_(seed, device_id),
-      contents_(contents.Pass()),
-      initial_hashes_dictionary_trusted_(
-          use_super_mac
-              ? IsHashDictionaryTrusted(pref_hash_calculator_, *contents_)
-              : false),
-      use_super_mac_(use_super_mac),
-      has_pending_write_(false) {
-  DCHECK(contents_);
-  UMA_HISTOGRAM_BOOLEAN("Settings.HashesDictionaryTrusted",

[erikwright (departed), 2014/06/12 20:49:41]

We need to rethink where this goes. May need an explicit check in FilterOnLoad
or something.

-                        initial_hashes_dictionary_trusted_);
+      use_super_mac_(use_super_mac) {
 }
 
-PrefHashStoreImpl::~PrefHashStoreImpl() {}
-
-void PrefHashStoreImpl::Reset() {
-  contents_->Reset();
+PrefHashStoreImpl::~PrefHashStoreImpl() {
 }
 
-scoped_ptr<PrefHashStoreTransaction> PrefHashStoreImpl::BeginTransaction() {
+scoped_ptr<PrefHashStoreTransaction> PrefHashStoreImpl::BeginTransaction(
+    base::DictionaryValue* storage) {
   return scoped_ptr<PrefHashStoreTransaction>(
-      new PrefHashStoreTransactionImpl(this));
-}
-
-void PrefHashStoreImpl::CommitPendingWrite() {
-  if (has_pending_write_) {
-    contents_->CommitPendingWrite();
-    has_pending_write_ = false;
-  }
+      new PrefHashStoreTransactionImpl(this, storage));
 }
 
 PrefHashStoreImpl::PrefHashStoreTransactionImpl::PrefHashStoreTransactionImpl(
-    PrefHashStoreImpl* outer) : outer_(outer), has_changed_(false) {
+    PrefHashStoreImpl* outer,
+    base::DictionaryValue* storage)
+    : outer_(outer),
+      contents_(storage),
+      super_mac_valid_(false),
+      super_mac_dirty_(false) {
+  if (outer_->use_super_mac_) {
+    const base::DictionaryValue* store_contents = contents_.GetContents();
+    std::string super_mac = contents_.GetSuperMac();
+    // The store must be initialized and have a valid super MAC to be trusted.
+    super_mac_valid_ =
+        store_contents && !super_mac.empty() &&
+        outer_->pref_hash_calculator_.Validate(
+            contents_.hash_store_id(), store_contents, super_mac) ==
+            PrefHashCalculator::VALID;
+  }
 }
 
 PrefHashStoreImpl::PrefHashStoreTransactionImpl::
     ~PrefHashStoreTransactionImpl() {
   // Update the super MAC if and only if the hashes dictionary has been
   // modified in this transaction.
-  if (has_changed_) {
-    if (outer_->use_super_mac_) {
-      // Get the dictionary of hashes (or NULL if it doesn't exist).
-      const base::DictionaryValue* hashes_dict =
-          outer_->contents_->GetContents();
-      outer_->contents_->SetSuperMac(outer_->pref_hash_calculator_.Calculate(
-          outer_->contents_->hash_store_id(), hashes_dict));
-    }
-    outer_->has_pending_write_ = true;
+  if (super_mac_dirty_ && outer_->use_super_mac_) {
+    // Get the dictionary of hashes (or NULL if it doesn't exist).
+    const base::DictionaryValue* hashes_dict = contents_.GetContents();
+    contents_.SetSuperMac(outer_->pref_hash_calculator_.Calculate(
+        contents_.hash_store_id(), hashes_dict));
   }
-
 }
 
 PrefHashStoreTransaction::ValueState
 PrefHashStoreImpl::PrefHashStoreTransactionImpl::CheckValue(
-    const std::string& path, const base::Value* initial_value) const {
-  const base::DictionaryValue* hashed_prefs = outer_->contents_->GetContents();
+    const std::string& path,
+    const base::Value* initial_value) const {
+  const base::DictionaryValue* hashed_prefs = contents_.GetContents();
 
   std::string last_hash;
   if (hashed_prefs)
@@ -126,8 +110,8 @@ PrefHashStoreImpl::PrefHashStoreTransactionImpl::CheckValue(
   if (last_hash.empty()) {
     // In the absence of a hash for this pref, always trust a NULL value, but
     // only trust an existing value if the initial hashes dictionary is trusted.
-    return (!initial_value || outer_->initial_hashes_dictionary_trusted_) ?
-               TRUSTED_UNKNOWN_VALUE : UNTRUSTED_UNKNOWN_VALUE;
+    return (!initial_value || super_mac_valid_) ? TRUSTED_UNKNOWN_VALUE
+                                                : UNTRUSTED_UNKNOWN_VALUE;
   }
 
   PrefHashCalculator::ValidationResult validation_result =
@@ -148,17 +132,42 @@ PrefHashStoreImpl::PrefHashStoreTransactionImpl::CheckValue(
 }
 
 void PrefHashStoreImpl::PrefHashStoreTransactionImpl::StoreHash(
-    const std::string& path, const base::Value* new_value) {
+    const std::string& path,
+    const base::Value* new_value) {
   const std::string mac =
       outer_->pref_hash_calculator_.Calculate(path, new_value);
-  (*outer_->contents_->GetMutableContents())->SetString(path, mac);
-  has_changed_ = true;
+  (*contents_.GetMutableContents())->SetString(path, mac);
+  super_mac_dirty_ = true;
+}
+
+bool PrefHashStoreImpl::PrefHashStoreTransactionImpl::HasHash(
+    const std::string& path) const {
+  const base::DictionaryValue* hashed_prefs = contents_.GetContents();
+  return hashed_prefs && hashed_prefs->Get(path, NULL);
+}
+
+void PrefHashStoreImpl::PrefHashStoreTransactionImpl::ImportHash(
+    const std::string& path,
+    const base::Value* hash) {
+  if (!hash)
+    (*contents_.GetMutableContents())->RemovePath(path, NULL);
+  else
+    (*contents_.GetMutableContents())->Set(path, hash->DeepCopy());
+  super_mac_dirty_ = super_mac_dirty_ || super_mac_valid_;
+}
+
+void PrefHashStoreImpl::PrefHashStoreTransactionImpl::ClearHash(
+    const std::string& path) {
+  if ((*contents_.GetMutableContents())->RemovePath(path, NULL) &&
+      super_mac_valid_) {
+    super_mac_dirty_ = true;
+  }
 }
 
 bool PrefHashStoreImpl::PrefHashStoreTransactionImpl::StampSuperMac() {
-  if (!outer_->use_super_mac_ || outer_->initial_hashes_dictionary_trusted_)
+  if (!outer_->use_super_mac_ || super_mac_valid_)
     return false;
-  has_changed_ = true;
+  super_mac_dirty_ = true;
   return true;
 }
 
@@ -179,10 +188,8 @@ PrefHashStoreImpl::PrefHashStoreTransactionImpl::CheckSplitValue(
   if (!initial_split_value || initial_split_value->empty())
     return has_hashes ? CLEARED : UNCHANGED;
 
-  if (!has_hashes) {
-    return outer_->initial_hashes_dictionary_trusted_ ?
-        TRUSTED_UNKNOWN_VALUE : UNTRUSTED_UNKNOWN_VALUE;
-  }
+  if (!has_hashes)
+    return super_mac_valid_ ? TRUSTED_UNKNOWN_VALUE : UNTRUSTED_UNKNOWN_VALUE;
 
   bool has_secure_legacy_id_hashes = false;
   std::string keyed_path(path);
@@ -234,15 +241,15 @@ PrefHashStoreImpl::PrefHashStoreTransactionImpl::CheckSplitValue(
   }
 
   return invalid_keys->empty()
-      ? (has_secure_legacy_id_hashes ? SECURE_LEGACY : UNCHANGED)
-      : CHANGED;
+             ? (has_secure_legacy_id_hashes ? SECURE_LEGACY : UNCHANGED)
+             : CHANGED;
 }
 
 void PrefHashStoreImpl::PrefHashStoreTransactionImpl::StoreSplitHash(
     const std::string& path,
     const base::DictionaryValue* split_value) {
   scoped_ptr<HashStoreContents::MutableDictionary> mutable_dictionary =
-      outer_->contents_->GetMutableContents();
+      contents_.GetMutableContents();
   (*mutable_dictionary)->Remove(path, NULL);
 
   if (split_value) {
@@ -259,7 +266,7 @@ void PrefHashStoreImpl::PrefHashStoreTransactionImpl::StoreSplitHash(
           outer_->pref_hash_calculator_.Calculate(keyed_path, &it.value()));
     }
   }
-  has_changed_ = true;
+  super_mac_dirty_ = true;
 }
 
 bool PrefHashStoreImpl::PrefHashStoreTransactionImpl::GetSplitMacs(
@@ -268,7 +275,7 @@ bool PrefHashStoreImpl::PrefHashStoreTransactionImpl::GetSplitMacs(
   DCHECK(split_macs);
   DCHECK(split_macs->empty());
 
-  const base::DictionaryValue* hashed_prefs = outer_->contents_->GetContents();
+  const base::DictionaryValue* hashed_prefs = contents_.GetContents();
   const base::DictionaryValue* split_mac_dictionary = NULL;
   if (!hashed_prefs || !hashed_prefs->GetDictionary(key, &split_mac_dictionary))
     return false;