Move preference MACs to the protected preference stores.

chrome/browser/prefs/tracked/pref_store_hash_store_contents.h

+// Copyright 2014 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.
+
+#ifndef CHROME_BROWSER_PREFS_TRACKED_PREF_STORE_HASH_STORE_CONTENTS_H_
+#define CHROME_BROWSER_PREFS_TRACKED_PREF_STORE_HASH_STORE_CONTENTS_H_
+
+#include <string>
+#include "base/compiler_specific.h"
+#include "base/macros.h"
+#include "chrome/browser/prefs/tracked/hash_store_contents.h"
+
+class PersistentPrefStore;
+class WriteablePrefStore;
+
+class PrefStoreHashStoreContents : public HashStoreContents {

[gab, 2014/06/06 21:54:59]

To avoid a lot of new code (trying to keep things small), as part of my proposed
model, I suggest we re-use the same logic for both PrefServiceHashStoreContents
and DictionaryHashStoreContents; the only difference being how the hash store
dictionary is obtained.

So we could have a HashStoreContents with most of the logic which takes in a
delegate or has a single virtual method to be able to get the actual
DictionaryValue from either a PrefService or a DictionaryValue.

FWICT, the only other differences are that (1) the hash_store_id is empty (we
can explicitly pass in an empty string) and (2) we ideally want to name the pref
"super_mac" rather than "hash_of_hashes", we can either make that a parameter to
be removed later or just not care...

[erikwright (departed), 2014/06/10 20:27:48]

I think the other methods like "Initialized", "Reset", where the super MAC is
stored, also all differ due to either tracking a single hash store or multiple
hash stores in the underlying data store.
In the production code it may be sufficient just to have a class that is only
aware of how to read (export) MACs from local state.

You will probably still need something able to create a legacy store (for
testing purposes) but that could be something test-specific. At which point,
yes, there would be only one HashStoreContents implementation. But I think that
the thing you end up with will look like this (new) class so I would go ahead
and add the new class and plan to remove/pare down the old one.

[gab, 2014/06/11 18:23:55]

Okay, keeping them as two separate classes with slight code duplication SGTM
given the former will go away.

Please add a comment on PrefServiceHashStoreContents stating that it is
deprecated and a TODO to remove it in M30+ once this migration step becomes
irrelevant.

+ public:
+  // Does increment the reference count on |pref_store|. Assumes that
+  // |pref_store| indirectly owns this delegate.
+  explicit PrefStoreHashStoreContents(PersistentPrefStore* pref_store);
+
+  // Does increment the reference count on |pref_store|. Assumes that
+  // |pref_store| indirectly owns this delegate. Ignores calls to
+  // CommitPendingWrite() because this is not supported by WriteablePrefStore.
+  explicit PrefStoreHashStoreContents(WriteablePrefStore* pref_store);
+
+  // A super MAC used to validate the hash store contents when determining
+  // whether
+  // an unknown value is to be trusted is stored as a string under
+  // |kSuperMacPref|
+  static const char kSuperMacPref[];
+
+  // HashStoreContents implementation
+  virtual std::string hash_store_id() const OVERRIDE;
+  virtual void Reset() OVERRIDE;
+  virtual bool IsInitialized() const OVERRIDE;
+  virtual const base::DictionaryValue* GetContents() const OVERRIDE;
+  virtual scoped_ptr<MutableDictionary> GetMutableContents() OVERRIDE;
+  virtual std::string GetSuperMac() const OVERRIDE;
+  virtual void SetSuperMac(const std::string& super_mac) OVERRIDE;
+  virtual void CommitPendingWrite() OVERRIDE;
+
+ private:
+  WriteablePrefStore* const pref_store_;
+  PersistentPrefStore* const persistent_pref_store_;
+
+  DISALLOW_COPY_AND_ASSIGN(PrefStoreHashStoreContents);
+};
+
+#endif  // CHROME_BROWSER_PREFS_TRACKED_PREF_STORE_HASH_STORE_CONTENTS_H_