Expand the JsonPrefStore's interface to allow for an alternate filename to be specified.

base/prefs/json_pref_store.h

Index: base/prefs/json_pref_store.h
diff --git a/base/prefs/json_pref_store.h b/base/prefs/json_pref_store.h
index 49e74ee23a5c5273999354634c2d99b2a1c7ae8b..f0933caf4529276597c05c9fc20eeffdb33e275f 100644
--- a/base/prefs/json_pref_store.h
+++ b/base/prefs/json_pref_store.h
@@ -43,12 +43,23 @@ class BASE_PREFS_EXPORT JsonPrefStore
       const base::FilePath& pref_filename,
       base::SequencedWorkerPool* worker_pool);
 
-  // |sequenced_task_runner| is must be a shutdown-blocking task runner, ideally
-  // created by GetTaskRunnerForFile() method above.
+  // Same as the constructor below with no alternate filename.
   JsonPrefStore(const base::FilePath& pref_filename,
                 base::SequencedTaskRunner* sequenced_task_runner,
                 scoped_ptr<PrefFilter> pref_filter);
 
+  // |sequenced_task_runner| must be a shutdown-blocking task runner, ideally
+  // created by the GetTaskRunnerForFile() method above.
+  // |pref_filename| is the path to the file to read prefs from.
+  // |pref_alternate_filename| is the path to an alternate file which prefs
+  // could be read from. If |pref_filename| exists, |pref_alternate_filename|

[Bernhard Bauer, 2014/06/19 15:38:09]

Why do we delete the alternate pref file if the primary one exists?

[gab, 2014/06/19 16:03:51]

The alternate pref file is the old name of the file, if we already have the
primary then its safe to delete the old one.

I guess that since the new name was introduced in this CL it's not really
possible to have both (as the new one will always first be created by moving the
old one), but I thought it was nice extra cleanup to avoid having the same prefs
lying around in various deprecated files.

[Bernhard Bauer, 2014/06/19 17:20:20]

Yeah, I'm just a bit wary of deleting a file that shouldn't exist anyway if
everything is going normal. I think I would be a little happier if we DCHECK
that the file doesn't exist, to document our assumptions and to be notified (at
least in Debug builds) if they are incorrect.

[gab, 2014/06/19 17:50:10]

I'm not really worried of this assumption being incorrect on developer machines,
I'm more worried that some users end up in weird states (e.g., tampered users)
where they have both yet one doesn't matter.

[Bernhard Bauer, 2014/06/20 14:16:28]

See, my immediate reaction to that would be to touch as little on disk as
possible, if we can simply use the new file :)

I don't feel strongly about though, so LGTM.

[gab, 2014/06/20 14:48:28]

Thanks, as discussed this shouldn't happen in normal checkouts and it will work
just fine in unexpected checkouts if we don't clean up the old file. Therefore I
removed the DeleteFile as you suggested.

+  // will be deleted; otherwise, |pref_alternate_filename| will be moved to
+  // |pref_filename| before the read occurs.
+  JsonPrefStore(const base::FilePath& pref_filename,
+                const base::FilePath& pref_alternate_filename,
+                base::SequencedTaskRunner* sequenced_task_runner,
+                scoped_ptr<PrefFilter> pref_filter);
+
   // PrefStore overrides:
   virtual bool GetValue(const std::string& key,
                         const base::Value** result) const OVERRIDE;
@@ -115,7 +126,8 @@ class BASE_PREFS_EXPORT JsonPrefStore
                         scoped_ptr<base::DictionaryValue> prefs,
                         bool schedule_write);
 
-  base::FilePath path_;
+  const base::FilePath path_;
+  const base::FilePath alternate_path_;
   const scoped_refptr<base::SequencedTaskRunner> sequenced_task_runner_;
 
   scoped_ptr<base::DictionaryValue> prefs_;