Multi-strategy based tracking.

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 144351c2af0567ca39838f2babc3699042236d2b..7dfb018a1c68f431ebd1af16d9a602fc43b2cad1 100644
--- a/chrome/browser/prefs/pref_hash_store_impl.cc
+++ b/chrome/browser/prefs/pref_hash_store_impl.cc
@@ -8,7 +8,6 @@
 #include "base/metrics/histogram.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
-#include "base/prefs/scoped_user_pref_update.h"
 #include "base/values.h"
 #include "chrome/common/pref_names.h"
 
@@ -65,26 +64,127 @@ PrefHashStore::ValueState PrefHashStoreImpl::CheckValue(
 void PrefHashStoreImpl::StoreHash(
     const std::string& path, const base::Value* new_value) {
   DictionaryPrefUpdate update(local_state_, prefs::kProfilePreferenceHashes);
+  StoreHashInternal(path, new_value, &update);
+}
+
+PrefHashStore::ValueState PrefHashStoreImpl::CheckSplitValue(
+    const std::string& path,
+    const base::DictionaryValue* initial_split_value,
+    std::vector<std::string>* invalid_keys) const {
+  DCHECK(invalid_keys && invalid_keys->empty());
+
+  const bool has_hashes = HasSplitHashesAtPath(path);
+
+  // Treat NULL and empty the same; otherwise we would need to store a hash
+  // for the entire dictionary (or some other special beacon) to
+  // differentiate these two cases which are really the same for
+  // dictionaries.
+  if (!initial_split_value || initial_split_value->empty())
+    return has_hashes ? CLEARED : UNCHANGED;
 
-  // Get the dictionary corresponding to the profile name, which may have a
-  // '.'
+  if (!has_hashes) {
+    return initial_hashes_dictionary_trusted_ ?
+        TRUSTED_UNKNOWN_VALUE : UNTRUSTED_UNKNOWN_VALUE;
+  }
+
+  for (base::DictionaryValue::Iterator it(*initial_split_value); !it.IsAtEnd();
+       it.Advance()) {
+    ValueState value_state = CheckValue(path + "." + it.key(), &it.value());

[robertshield, 2014/01/16 03:59:30]

the +'s yield a fair amount of object construction in this loop. Consider using
string.append() instead (for bonus points using string::reserve if you can
reasonably estimate the final length of the string. 

[gab, 2014/01/16 18:58:06]

How about this? Out-of-scope string with "|path|." only replacing the key
inplace on every pass.

+    switch (value_state) {
+      case CLEARED:
+        // CLEARED doesn't make sense as a NULL value would never be sampled
+        // by the DictionaryValue::Iterator; in fact it is a known weakness of
+        // this current algorithm to not detect the case where a single key is
+        // cleared entirely from the dictionary pref.
+        NOTREACHED();
+        break;
+      case MIGRATED:
+        // Split tracked preferences were introduced after the migration started
+        // so no migration is expected, but declare it invalid in Release builds
+        // anyways.
+        NOTREACHED();
+        invalid_keys->push_back(it.key());
+        break;
+      case UNCHANGED:
+        break;
+      case CHANGED:  // Falls through.
+      case UNTRUSTED_UNKNOWN_VALUE:  // Falls through.
+      case TRUSTED_UNKNOWN_VALUE:
+        // Declare this value invalid, whether it was changed or never seen
+        // before (note that the initial hashes being trusted is irrelevant for
+        // individual entries in this scenario).
+        invalid_keys->push_back(it.key());
+        break;
+    }
+  }
+  return invalid_keys->empty() ? UNCHANGED : CHANGED;
+}
+
+void PrefHashStoreImpl::StoreSplitHash(
+    const std::string& path,
+    const base::DictionaryValue* split_value) {
+  // TODO(gab): This method should be a transaction on the underlying dictionary
+  // to avoid the case where a scheduled write kicks in on the blocking pool
+  // right after ClearPath() and chrome subsequently crashes; resulting in no
+  // hashes being stored on disk in the next startup...

[robertshield, 2014/01/16 03:59:30]

Can you file / reference a bug for the transactional support?

[gab, 2014/01/16 18:58:06]

Done.

+  DictionaryPrefUpdate update(local_state_, prefs::kProfilePreferenceHashes);
+  ClearPath(path, &update);
+
+  if (split_value) {
+    for (base::DictionaryValue::Iterator it(*split_value); !it.IsAtEnd();
+         it.Advance()) {
+      StoreHashInternal(path + "." + it.key(), &it.value(), &update);

[robertshield, 2014/01/16 03:59:30]

Same comment on object construction here.

[gab, 2014/01/16 18:58:06]

Done.

+    }
+  }
+}
+
+void PrefHashStoreImpl::ClearPath(const std::string& path,
+                                  DictionaryPrefUpdate* update) {
   base::DictionaryValue* hashes_dict = NULL;
-  if (!update->GetDictionaryWithoutPathExpansion(hash_store_id_,
-                                                 &hashes_dict)) {
-    hashes_dict = new base::DictionaryValue;
-    update->SetWithoutPathExpansion(hash_store_id_, hashes_dict);
+  if (update->Get()->GetDictionaryWithoutPathExpansion(hash_store_id_,
+                                                       &hashes_dict)) {
+    hashes_dict->Remove(path, NULL);
   }
+  UpdateHashOfHashes(hashes_dict, update);
+}
+
+bool PrefHashStoreImpl::HasSplitHashesAtPath(const std::string& path) const {
+  const base::DictionaryValue* pref_hash_dicts =
+      local_state_->GetDictionary(prefs::kProfilePreferenceHashes);
+  const base::DictionaryValue* hashed_prefs = NULL;
+  pref_hash_dicts->GetDictionaryWithoutPathExpansion(hash_store_id_,
+                                                     &hashed_prefs);
+  return hashed_prefs && hashed_prefs->GetDictionary(path, NULL);
+}
 
+void PrefHashStoreImpl::StoreHashInternal(const std::string& path,
+                                          const base::Value* new_value,
+                                          DictionaryPrefUpdate* update) {
+  base::DictionaryValue* hashes_dict = NULL;
+
+  // Get the dictionary corresponding to the profile name, which may have a '.'
+  if (!update->Get()->GetDictionaryWithoutPathExpansion(hash_store_id_,
+                                                        &hashes_dict)) {
+    hashes_dict = new base::DictionaryValue;
+    update->Get()->SetWithoutPathExpansion(hash_store_id_, hashes_dict);
+  }
   hashes_dict->SetString(
       path, pref_hash_calculator_.Calculate(path, new_value));
 
+  UpdateHashOfHashes(hashes_dict, update);
+}
+
+void PrefHashStoreImpl::UpdateHashOfHashes(
+    const base::DictionaryValue* hashes_dict,
+    DictionaryPrefUpdate* update) {
+
   // Get the dictionary where the hash of hashes are stored.
   base::DictionaryValue* hash_of_hashes_dict = NULL;
-  if (!update->GetDictionaryWithoutPathExpansion(internals::kHashOfHashesPref,
-                                                 &hash_of_hashes_dict)) {
+  if (!update->Get()->GetDictionaryWithoutPathExpansion(
+          internals::kHashOfHashesPref, &hash_of_hashes_dict)) {
     hash_of_hashes_dict = new base::DictionaryValue;
-    update->SetWithoutPathExpansion(internals::kHashOfHashesPref,
-                                    hash_of_hashes_dict);
+    update->Get()->SetWithoutPathExpansion(internals::kHashOfHashesPref,
+                                           hash_of_hashes_dict);
   }
   // Use the |hash_store_id_| as the hashed path to avoid having the hash
   // depend on kProfilePreferenceHashes.