Make super MAC optional.

chrome/browser/prefs/pref_hash_store_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/prefs/pref_hash_store_impl.h"
 
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/values.h"
 #include "chrome/browser/prefs/pref_hash_store_transaction.h"
@@ skipping 42 matching lines @@
   bool GetSplitMacs(const std::string& path,
                     std::map<std::string, std::string>* split_macs) const;
   PrefHashStoreImpl* outer_;
   bool has_changed_;
 
   DISALLOW_COPY_AND_ASSIGN(PrefHashStoreTransactionImpl);
 };
 
 PrefHashStoreImpl::PrefHashStoreImpl(const std::string& seed,
                                      const std::string& device_id,
-                                     scoped_ptr<HashStoreContents> contents)
+                                     scoped_ptr<HashStoreContents> contents,
+                                     bool use_super_mac)
     : pref_hash_calculator_(seed, device_id),
       contents_(contents.Pass()),
       initial_hashes_dictionary_trusted_(
-          IsHashDictionaryTrusted(pref_hash_calculator_, *contents_)),
+          use_super_mac
+              ? IsHashDictionaryTrusted(pref_hash_calculator_, *contents_)
+              : false),

[gab, 2014/06/11 18:57:54]

What do you think of making the default "true" here so that we get
TrustedInitialized reports instead of Initialized noise for tracked-only prefs?

[gab, 2014/06/11 21:54:32]

Actually, never mind.

Setting this to false will keep it such that for tracked-only pref we get:
1) Prefs initialized from NULL value => TrustedInitialized
2) Prefs initialized from any other value => Initialized

The only thing we lose is that we will get an Initialization spike rather than a
TrustedInitialized spike if we ever add a non-protected tracked pref -- which I
don't think we care about.

+      use_super_mac_(use_super_mac),
       has_pending_write_(false) {
   DCHECK(contents_);
   UMA_HISTOGRAM_BOOLEAN("Settings.HashesDictionaryTrusted",
                         initial_hashes_dictionary_trusted_);
 }
 
 PrefHashStoreImpl::~PrefHashStoreImpl() {}
 
 void PrefHashStoreImpl::Reset() {
   contents_->Reset();
@@ skipping 13 matching lines @@
 
 PrefHashStoreImpl::PrefHashStoreTransactionImpl::PrefHashStoreTransactionImpl(
     PrefHashStoreImpl* outer) : outer_(outer), has_changed_(false) {
 }
 
 PrefHashStoreImpl::PrefHashStoreTransactionImpl::
     ~PrefHashStoreTransactionImpl() {
   // Update the super MAC if and only if the hashes dictionary has been
   // modified in this transaction.
   if (has_changed_) {
-    // 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));
-
+    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;
   }
 
 }
 
 PrefHashStoreTransaction::ValueState
 PrefHashStoreImpl::PrefHashStoreTransactionImpl::CheckValue(
     const std::string& path, const base::Value* initial_value) const {
   const base::DictionaryValue* hashed_prefs = outer_->contents_->GetContents();
 
@@ skipping 27 matching lines @@
 
 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);
   (*outer_->contents_->GetMutableContents())->SetString(path, mac);
   has_changed_ = true;
 }
 
 bool PrefHashStoreImpl::PrefHashStoreTransactionImpl::StampSuperMac() {
-  if (outer_->initial_hashes_dictionary_trusted_)
+  if (!outer_->use_super_mac_ || outer_->initial_hashes_dictionary_trusted_)

[gab, 2014/06/11 21:54:32]

I don't think this is even required since line 104 already takes care of this.

[erikwright (departed), 2014/06/13 13:25:24]

This saves us thinking that we have altered the store. During filter on load
that will save us from telling the JsonPrefStore that it needs to commit to
disk.

[gab, 2014/06/13 14:48:57]

I see, good point :-)!

     return false;
   has_changed_ = true;
   return true;
 }
 
 PrefHashStoreTransaction::ValueState
 PrefHashStoreImpl::PrefHashStoreTransactionImpl::CheckSplitValue(
     const std::string& path,
     const base::DictionaryValue* initial_split_value,
     std::vector<std::string>* invalid_keys) const {
@@ skipping 106 matching lines @@
        it.Advance()) {
     std::string mac_string;
     if (!it.value().GetAsString(&mac_string)) {
       NOTREACHED();
       continue;
     }
     split_macs->insert(make_pair(it.key(), mac_string));
   }
   return true;
 }