Integrate registry_hash_store_contents with the rest of tracked prefs.

components/user_prefs/tracked/pref_hash_store_impl.cc

Index: components/user_prefs/tracked/pref_hash_store_impl.cc
diff --git a/components/user_prefs/tracked/pref_hash_store_impl.cc b/components/user_prefs/tracked/pref_hash_store_impl.cc
index 3a25bbc6742fcdb39300bca0e025eed5c5570213..3282d92022cc550bf70db1303b8135f5581e2dda 100644
--- a/components/user_prefs/tracked/pref_hash_store_impl.cc
+++ b/components/user_prefs/tracked/pref_hash_store_impl.cc
@@ -9,6 +9,7 @@
 
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
 #include "base/metrics/histogram.h"
 #include "base/values.h"
 #include "components/user_prefs/tracked/hash_store_contents.h"
@@ -24,6 +25,7 @@ class PrefHashStoreImpl::PrefHashStoreTransactionImpl
   ~PrefHashStoreTransactionImpl() override;
 
   // PrefHashStoreTransaction implementation.
+  HashStoreContentsType GetStoreType() const override;
   ValueState CheckValue(const std::string& path,
                         const base::Value* value) const override;
   void StoreHash(const std::string& path, const base::Value* value) override;
@@ -64,6 +66,35 @@ std::unique_ptr<PrefHashStoreTransaction> PrefHashStoreImpl::BeginTransaction(
       new PrefHashStoreTransactionImpl(this, std::move(storage)));
 }
 
+std::unique_ptr<base::DictionaryValue> PrefHashStoreImpl::ComputeSplitMacs(
+    const std::string& path,
+    const base::DictionaryValue* split_values) {
+  if (!split_values)
+    return nullptr;
+
+  std::string keyed_path(path);
+  keyed_path.push_back('.');
+  const size_t common_part_length = keyed_path.length();
+
+  base::DictionaryValue* split_macs = new base::DictionaryValue;

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

std::unique_ptr here instead of only wrapping it at the end

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

Done.

+
+  for (base::DictionaryValue::Iterator it(*split_values); !it.IsAtEnd();
+       it.Advance()) {
+    // Keep the common part from the old |keyed_path| and replace the key to
+    // get the new |keyed_path|.
+    keyed_path.replace(common_part_length, std::string::npos, it.key());
+
+    split_macs->SetString(it.key(), ComputeMac(keyed_path, &it.value()));

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

SetStringWithoutPathExpansion

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

Done.

+  }
+
+  return base::WrapUnique(split_macs);
+}
+
+std::string PrefHashStoreImpl::ComputeMac(const std::string& path,

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

Same order as in header (this method before ComputeSplitMacs())

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

Done.

+                                          const base::Value* new_value) {
+  return pref_hash_calculator_.Calculate(path, new_value);
+}
+
 PrefHashStoreImpl::PrefHashStoreTransactionImpl::PrefHashStoreTransactionImpl(
     PrefHashStoreImpl* outer,
     std::unique_ptr<HashStoreContents> storage)
@@ -89,11 +120,15 @@ PrefHashStoreImpl::PrefHashStoreTransactionImpl::
   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("", hashes_dict));
+    contents_->SetSuperMac(outer_->ComputeMac("", hashes_dict));
   }
 }
 
+HashStoreContentsType
+PrefHashStoreImpl::PrefHashStoreTransactionImpl::GetStoreType() const {
+  return contents_->GetType();
+}
+
 PrefHashStoreTransaction::ValueState
 PrefHashStoreImpl::PrefHashStoreTransactionImpl::CheckValue(
     const std::string& path,
@@ -130,8 +165,7 @@ PrefHashStoreImpl::PrefHashStoreTransactionImpl::CheckValue(
 void PrefHashStoreImpl::PrefHashStoreTransactionImpl::StoreHash(
     const std::string& path,
     const base::Value* new_value) {
-  const std::string mac =
-      outer_->pref_hash_calculator_.Calculate(path, new_value);
+  const std::string mac = outer_->ComputeMac(path, new_value);
   contents_->SetMac(path, mac);
   super_mac_dirty_ = true;
 }
@@ -207,17 +241,16 @@ void PrefHashStoreImpl::PrefHashStoreTransactionImpl::StoreSplitHash(
   contents_->RemoveEntry(path);
 
   if (split_value) {
-    std::string keyed_path(path);
-    keyed_path.push_back('.');
-    const size_t common_part_length = keyed_path.length();
-    for (base::DictionaryValue::Iterator it(*split_value); !it.IsAtEnd();
+    std::unique_ptr<base::DictionaryValue> split_macs =
+        outer_->ComputeSplitMacs(path, split_value);
+
+    for (base::DictionaryValue::Iterator it(*split_macs); !it.IsAtEnd();
          it.Advance()) {
-      // Keep the common part from the old |keyed_path| and replace the key to
-      // get the new |keyed_path|.
-      keyed_path.replace(common_part_length, std::string::npos, it.key());
-      contents_->SetSplitMac(
-          path, it.key(),
-          outer_->pref_hash_calculator_.Calculate(keyed_path, &it.value()));
+      std::string mac;
+      bool is_string = it.value().GetAsString(&mac);

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

Too bad that we now have an extra string copy, I guess this is the fault of
values.h for not exposing something like GetAsStringPiece() for users not caring
for an actual copy... oh well, fine for now (assuming we don't see a regressions
in timing stats).

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

Tried some StringValue shenanigans. From my reading this ends up not copying the
string, but I'm not 100% sure. 

[gab, 2016/08/08 04:37:44]

Yes, your method of getting the underlying StringValue* and then using its
GetString method to get a const& out of it is the way to avoid the copy, yay :-)

+      DCHECK(is_string);
+
+      contents_->SetSplitMac(path, it.key(), mac);
     }
   }
   super_mac_dirty_ = true;