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

The alternate filename is intended to be the old name of the pref file being read, JsonPrefStore handles the migration if required. JsonPrefStore's FileThreadDeserializer is the best place to do this as it's the only code in the prefs stack running on a thread allowed to do IO (another solution would be to let JsonPrefStore take a PreReadOnFileThreadCallback to abstract the required work away, but the direct approach seemed easier here without breaking encapsulation).

BUG=372547

Review URL: https://codereview.chromium.org/347793002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@278776 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 249 insertions, 10 deletions

diff --git a/base/prefs/json_pref_store.cc b/base/prefs/json_pref_store.cc
index a6c2362..9180984 100644
--- a/base/prefs/json_pref_store.cc
+++ b/base/prefs/json_pref_store.cc
@@ -37,21 +37,23 @@ class FileThreadDeserializer
         origin_loop_proxy_(base::MessageLoopProxy::current()) {
   }
 
-  void Start(const base::FilePath& path) {
+  void Start(const base::FilePath& path,
+             const base::FilePath& alternate_path) {
     DCHECK(origin_loop_proxy_->BelongsToCurrentThread());
     // TODO(gab): This should use PostTaskAndReplyWithResult instead of using
     // the |error_| member to pass data across tasks.
     sequenced_task_runner_->PostTask(
         FROM_HERE,
         base::Bind(&FileThreadDeserializer::ReadFileAndReport,
-                   this, path));
+                   this, path, alternate_path));
   }
 
   // Deserializes JSON on the sequenced task runner.
-  void ReadFileAndReport(const base::FilePath& path) {
+  void ReadFileAndReport(const base::FilePath& path,
+                         const base::FilePath& alternate_path) {
     DCHECK(sequenced_task_runner_->RunsTasksOnCurrentThread());
 
-    value_.reset(DoReading(path, &error_, &no_dir_));
+    value_.reset(DoReading(path, alternate_path, &error_, &no_dir_));
 
     origin_loop_proxy_->PostTask(
         FROM_HERE,
@@ -65,8 +67,14 @@ class FileThreadDeserializer
   }
 
   static base::Value* DoReading(const base::FilePath& path,
+                                const base::FilePath& alternate_path,
                                 PersistentPrefStore::PrefReadError* error,
                                 bool* no_dir) {
+    if (!base::PathExists(path) && !alternate_path.empty() &&
+        base::PathExists(alternate_path)) {
+      base::Move(alternate_path, path);
+    }
+
     int error_code;
     std::string error_msg;
     JSONFileValueSerializer serializer(path);
@@ -167,6 +175,22 @@ JsonPrefStore::JsonPrefStore(const base::FilePath& filename,
       read_error_(PREF_READ_ERROR_NONE) {
 }
 
+JsonPrefStore::JsonPrefStore(const base::FilePath& filename,
+                             const base::FilePath& alternate_filename,
+                             base::SequencedTaskRunner* sequenced_task_runner,
+                             scoped_ptr<PrefFilter> pref_filter)
+    : path_(filename),
+      alternate_path_(alternate_filename),
+      sequenced_task_runner_(sequenced_task_runner),
+      prefs_(new base::DictionaryValue()),
+      read_only_(false),
+      writer_(filename, sequenced_task_runner),
+      pref_filter_(pref_filter.Pass()),
+      initialized_(false),
+      filtering_in_progress_(false),
+      read_error_(PREF_READ_ERROR_NONE) {
+}
+
 bool JsonPrefStore::GetValue(const std::string& key,
                              const base::Value** result) const {
   base::Value* tmp = NULL;
@@ -252,7 +276,8 @@ PersistentPrefStore::PrefReadError JsonPrefStore::ReadPrefs() {
   PrefReadError error;
   bool no_dir;
   scoped_ptr<base::Value> value(
-      FileThreadDeserializer::DoReading(path_, &error, &no_dir));
+      FileThreadDeserializer::DoReading(path_, alternate_path_, &error,
+                                        &no_dir));
   OnFileRead(value.Pass(), error, no_dir);
   return filtering_in_progress_ ? PREF_READ_ERROR_ASYNCHRONOUS_TASK_INCOMPLETE :
                                   error;
@@ -271,7 +296,7 @@ void JsonPrefStore::ReadPrefsAsync(ReadErrorDelegate* error_delegate) {
   // in the end.
   scoped_refptr<FileThreadDeserializer> deserializer(
       new FileThreadDeserializer(this, sequenced_task_runner_.get()));
-  deserializer->Start(path_);
+  deserializer->Start(path_, alternate_path_);
 }
 
 void JsonPrefStore::CommitPendingWrite() {
diff --git a/base/prefs/json_pref_store.h b/base/prefs/json_pref_store.h
index 49e74ee..6ceea68 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 the
+  // desired prefs may have previously been written to. If |pref_filename|
+  // doesn't exist and |pref_alternate_filename| does, |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_;
diff --git a/base/prefs/json_pref_store_unittest.cc b/base/prefs/json_pref_store_unittest.cc
index 2b4a9e8..441c229 100644
--- a/base/prefs/json_pref_store_unittest.cc
+++ b/base/prefs/json_pref_store_unittest.cc
@@ -44,7 +44,7 @@ class InterceptingPrefFilter : public PrefFilter {
 
   bool has_intercepted_prefs() const { return intercepted_prefs_ != NULL; }
 
-  // Finalize an intercepted read, handing |intercept_prefs_| back to its
+  // Finalize an intercepted read, handing |intercepted_prefs_| back to its
   // JsonPrefStore.
   void ReleasePrefs();
 
@@ -122,6 +122,23 @@ TEST_F(JsonPrefStoreTest, NonExistentFile) {
   EXPECT_FALSE(pref_store->ReadOnly());
 }
 
+// Test fallback behavior for a nonexistent file and alternate file.
+TEST_F(JsonPrefStoreTest, NonExistentFileAndAlternateFile) {
+  base::FilePath bogus_input_file = data_dir_.AppendASCII("read.txt");
+  base::FilePath bogus_alternate_input_file =
+      data_dir_.AppendASCII("read_alternate.txt");
+  ASSERT_FALSE(PathExists(bogus_input_file));
+  ASSERT_FALSE(PathExists(bogus_alternate_input_file));
+  scoped_refptr<JsonPrefStore> pref_store = new JsonPrefStore(
+      bogus_input_file,
+      bogus_alternate_input_file,
+      message_loop_.message_loop_proxy().get(),
+      scoped_ptr<PrefFilter>());
+  EXPECT_EQ(PersistentPrefStore::PREF_READ_ERROR_NO_FILE,
+            pref_store->ReadPrefs());
+  EXPECT_FALSE(pref_store->ReadOnly());
+}
+
 // Test fallback behavior for an invalid file.
 TEST_F(JsonPrefStoreTest, InvalidFile) {
   base::FilePath invalid_file_original = data_dir_.AppendASCII("invalid.json");
@@ -469,4 +486,189 @@ TEST_F(JsonPrefStoreTest, ReadAsyncWithInterceptor) {
       pref_store.get(), input_file, data_dir_.AppendASCII("write.golden.json"));
 }
 
+TEST_F(JsonPrefStoreTest, AlternateFile) {
+  ASSERT_TRUE(
+      base::CopyFile(data_dir_.AppendASCII("read.json"),
+                     temp_dir_.path().AppendASCII("alternate.json")));
+
+  // Test that the alternate file is moved to the main file and read as-is from
+  // there.
+  base::FilePath input_file = temp_dir_.path().AppendASCII("write.json");
+  base::FilePath alternate_input_file =
+      temp_dir_.path().AppendASCII("alternate.json");
+  ASSERT_FALSE(PathExists(input_file));
+  ASSERT_TRUE(PathExists(alternate_input_file));
+  scoped_refptr<JsonPrefStore> pref_store = new JsonPrefStore(
+      input_file,
+      alternate_input_file,
+      message_loop_.message_loop_proxy().get(),
+      scoped_ptr<PrefFilter>());
+
+  ASSERT_FALSE(PathExists(input_file));
+  ASSERT_TRUE(PathExists(alternate_input_file));
+  ASSERT_EQ(PersistentPrefStore::PREF_READ_ERROR_NONE, pref_store->ReadPrefs());
+
+  ASSERT_TRUE(PathExists(input_file));
+  ASSERT_FALSE(PathExists(alternate_input_file));
+
+  EXPECT_FALSE(pref_store->ReadOnly());
+  EXPECT_TRUE(pref_store->IsInitializationComplete());
+
+  // The JSON file looks like this:
+  // {
+  //   "homepage": "http://www.cnn.com",
+  //   "some_directory": "/usr/local/",
+  //   "tabs": {
+  //     "new_windows_in_tabs": true,
+  //     "max_tabs": 20
+  //   }
+  // }
+
+  RunBasicJsonPrefStoreTest(
+      pref_store.get(), input_file, data_dir_.AppendASCII("write.golden.json"));
+}
+
+TEST_F(JsonPrefStoreTest, AlternateFileIgnoredWhenMainFileExists) {
+  ASSERT_TRUE(
+      base::CopyFile(data_dir_.AppendASCII("read.json"),
+                     temp_dir_.path().AppendASCII("write.json")));
+  ASSERT_TRUE(
+      base::CopyFile(data_dir_.AppendASCII("invalid.json"),
+                     temp_dir_.path().AppendASCII("alternate.json")));
+
+  // Test that the alternate file is ignored and that the read occurs from the
+  // existing main file. There is no attempt at even deleting the alternate
+  // file as this scenario should never happen in normal user-data-dirs.
+  base::FilePath input_file = temp_dir_.path().AppendASCII("write.json");
+  base::FilePath alternate_input_file =
+      temp_dir_.path().AppendASCII("alternate.json");
+  ASSERT_TRUE(PathExists(input_file));
+  ASSERT_TRUE(PathExists(alternate_input_file));
+  scoped_refptr<JsonPrefStore> pref_store = new JsonPrefStore(
+      input_file,
+      alternate_input_file,
+      message_loop_.message_loop_proxy().get(),
+      scoped_ptr<PrefFilter>());
+
+  ASSERT_TRUE(PathExists(input_file));
+  ASSERT_TRUE(PathExists(alternate_input_file));
+  ASSERT_EQ(PersistentPrefStore::PREF_READ_ERROR_NONE, pref_store->ReadPrefs());
+
+  ASSERT_TRUE(PathExists(input_file));
+  ASSERT_TRUE(PathExists(alternate_input_file));
+
+  EXPECT_FALSE(pref_store->ReadOnly());
+  EXPECT_TRUE(pref_store->IsInitializationComplete());
+
+  // The JSON file looks like this:
+  // {
+  //   "homepage": "http://www.cnn.com",
+  //   "some_directory": "/usr/local/",
+  //   "tabs": {
+  //     "new_windows_in_tabs": true,
+  //     "max_tabs": 20
+  //   }
+  // }
+
+  RunBasicJsonPrefStoreTest(
+      pref_store.get(), input_file, data_dir_.AppendASCII("write.golden.json"));
+}
+
+TEST_F(JsonPrefStoreTest, AlternateFileDNE) {
+  ASSERT_TRUE(
+      base::CopyFile(data_dir_.AppendASCII("read.json"),
+                     temp_dir_.path().AppendASCII("write.json")));
+
+  // Test that the basic read works fine when an alternate file is specified but
+  // does not exist.
+  base::FilePath input_file = temp_dir_.path().AppendASCII("write.json");
+  base::FilePath alternate_input_file =
+      temp_dir_.path().AppendASCII("alternate.json");
+  ASSERT_TRUE(PathExists(input_file));
+  ASSERT_FALSE(PathExists(alternate_input_file));
+  scoped_refptr<JsonPrefStore> pref_store = new JsonPrefStore(
+      input_file,
+      alternate_input_file,
+      message_loop_.message_loop_proxy().get(),
+      scoped_ptr<PrefFilter>());
+
+  ASSERT_TRUE(PathExists(input_file));
+  ASSERT_FALSE(PathExists(alternate_input_file));
+  ASSERT_EQ(PersistentPrefStore::PREF_READ_ERROR_NONE, pref_store->ReadPrefs());
+
+  ASSERT_TRUE(PathExists(input_file));
+  ASSERT_FALSE(PathExists(alternate_input_file));
+
+  EXPECT_FALSE(pref_store->ReadOnly());
+  EXPECT_TRUE(pref_store->IsInitializationComplete());
+
+  // The JSON file looks like this:
+  // {
+  //   "homepage": "http://www.cnn.com",
+  //   "some_directory": "/usr/local/",
+  //   "tabs": {
+  //     "new_windows_in_tabs": true,
+  //     "max_tabs": 20
+  //   }
+  // }
+
+  RunBasicJsonPrefStoreTest(
+      pref_store.get(), input_file, data_dir_.AppendASCII("write.golden.json"));
+}
+
+TEST_F(JsonPrefStoreTest, BasicAsyncWithAlternateFile) {
+  ASSERT_TRUE(
+      base::CopyFile(data_dir_.AppendASCII("read.json"),
+                     temp_dir_.path().AppendASCII("alternate.json")));
+
+  // Test that the alternate file is moved to the main file and read as-is from
+  // there even when the read is made asynchronously.
+  base::FilePath input_file = temp_dir_.path().AppendASCII("write.json");
+  base::FilePath alternate_input_file =
+      temp_dir_.path().AppendASCII("alternate.json");
+  ASSERT_FALSE(PathExists(input_file));
+  ASSERT_TRUE(PathExists(alternate_input_file));
+  scoped_refptr<JsonPrefStore> pref_store = new JsonPrefStore(
+      input_file,
+      alternate_input_file,
+      message_loop_.message_loop_proxy().get(),
+      scoped_ptr<PrefFilter>());
+
+  ASSERT_FALSE(PathExists(input_file));
+  ASSERT_TRUE(PathExists(alternate_input_file));
+
+  {
+    MockPrefStoreObserver mock_observer;
+    pref_store->AddObserver(&mock_observer);
+
+    MockReadErrorDelegate* mock_error_delegate = new MockReadErrorDelegate;
+    pref_store->ReadPrefsAsync(mock_error_delegate);
+
+    EXPECT_CALL(mock_observer, OnInitializationCompleted(true)).Times(1);
+    EXPECT_CALL(*mock_error_delegate,
+                OnError(PersistentPrefStore::PREF_READ_ERROR_NONE)).Times(0);
+    RunLoop().RunUntilIdle();
+    pref_store->RemoveObserver(&mock_observer);
+
+    EXPECT_FALSE(pref_store->ReadOnly());
+    EXPECT_TRUE(pref_store->IsInitializationComplete());
+  }
+
+  ASSERT_TRUE(PathExists(input_file));
+  ASSERT_FALSE(PathExists(alternate_input_file));
+
+  // The JSON file looks like this:
+  // {
+  //   "homepage": "http://www.cnn.com",
+  //   "some_directory": "/usr/local/",
+  //   "tabs": {
+  //     "new_windows_in_tabs": true,
+  //     "max_tabs": 20
+  //   }
+  // }
+
+  RunBasicJsonPrefStoreTest(
+      pref_store.get(), input_file, data_dir_.AppendASCII("write.golden.json"));
+}
+
 }  // namespace base