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

base/prefs/json_pref_store.h

 // Copyright (c) 2012 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 BASE_PREFS_JSON_PREF_STORE_H_
 #define BASE_PREFS_JSON_PREF_STORE_H_
 
 #include <set>
 #include <string>
 
@@ skipping 25 matching lines @@
     : public PersistentPrefStore,
       public base::ImportantFileWriter::DataSerializer,
       public base::SupportsWeakPtr<JsonPrefStore> {
  public:
   // Returns instance of SequencedTaskRunner which guarantees that file
   // operations on the same file will be executed in sequenced order.
   static scoped_refptr<base::SequencedTaskRunner> GetTaskRunnerForFile(
       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;
   virtual void AddObserver(PrefStore::Observer* observer) OVERRIDE;
   virtual void RemoveObserver(PrefStore::Observer* observer) OVERRIDE;
   virtual bool HasObservers() const OVERRIDE;
   virtual bool IsInitializationComplete() const OVERRIDE;
 
   // PersistentPrefStore overrides:
   virtual bool GetMutableValue(const std::string& key,
@@ skipping 46 matching lines @@
   // potentially been intercepted and modified by |pref_filter_|.
   // |initialization_successful| is pre-determined by OnFileRead() and should
   // be used when reporting OnInitializationCompleted().
   // |schedule_write| indicates whether a write should be immediately scheduled
   // (typically because the |pref_filter_| has already altered the |prefs|) --
   // this will be ignored if this store is read-only.
   void FinalizeFileRead(bool initialization_successful,
                         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_;
 
   bool read_only_;
 
   // Helper for safely writing pref data.
   base::ImportantFileWriter writer_;
 
   scoped_ptr<PrefFilter> pref_filter_;
   ObserverList<PrefStore::Observer, true> observers_;
 
   scoped_ptr<ReadErrorDelegate> error_delegate_;
 
   bool initialized_;
   bool filtering_in_progress_;
   PrefReadError read_error_;
 
   std::set<std::string> keys_need_empty_value_;
 
   DISALLOW_COPY_AND_ASSIGN(JsonPrefStore);
 };
 
 #endif  // BASE_PREFS_JSON_PREF_STORE_H_