Reland "Factor out reading and writing of preferences into |PrefStore|."

The CL now applies after r45225 (Throw out preferences files that are corrupt rather than keeping them in read-only mode), which means that the changes in that commit moved to JsonPrefStore.

I updated JsonPrefStoreTest.InvalidFile to test the new behavior.

***

In order to implement platform-specific policies, reading and writing preferences needs to be abstracted from the |PrefService|. The interface for that is now |PrefStore|, with an implementation |JsonPrefStore|, which stores the pref data in a JSON file. There is another implementation, |DummyPrefStore|, which doesn't store any persistent preferences, and is currently used for tests.

Most of the changes are for using the new interface, which is |new PrefService(new JsonPrefStore(filename))| instead of |new PrefService(filename)|.

BUG=40259
TEST=PrefServiceTest.*:PrefServiceSetValueTest.*:PrefMemberTest.*:JsonPrefStoreTest.*

Review URL: http://codereview.chromium.org/1717007

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

1 files changed, 50 insertions, 151 deletions

diff --git a/chrome/browser/pref_service_unittest.cc b/chrome/browser/pref_service_unittest.cc
index e643b8c..fc801d6 100644
--- a/chrome/browser/pref_service_unittest.cc
+++ b/chrome/browser/pref_service_unittest.cc
@@ -5,15 +5,11 @@
 #include <string>
 
 #include "app/test/data/resource.h"
-#include "base/file_util.h"
-#include "base/message_loop.h"
-#include "base/path_service.h"
 #include "base/scoped_ptr.h"
 #include "base/values.h"
-#include "chrome/browser/chrome_thread.h"
+#include "chrome/browser/dummy_pref_store.h"
 #include "chrome/browser/pref_service.h"
 #include "chrome/common/chrome_paths.h"
-#include "chrome/common/json_value_serializer.h"
 #include "chrome/common/notification_observer_mock.h"
 #include "chrome/common/notification_service.h"
 #include "chrome/common/notification_type.h"
@@ -26,33 +22,6 @@ using testing::Mock;
 using testing::Pointee;
 using testing::Property;
 
-class PrefServiceTest : public testing::Test {
- protected:
-  virtual void SetUp() {
-    // Name a subdirectory of the temp directory.
-    ASSERT_TRUE(PathService::Get(base::DIR_TEMP, &test_dir_));
-    test_dir_ = test_dir_.AppendASCII("PrefServiceTest");
-
-    // Create a fresh, empty copy of this directory.
-    file_util::Delete(test_dir_, true);
-    file_util::CreateDirectory(test_dir_);
-
-    ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &data_dir_));
-    data_dir_ = data_dir_.AppendASCII("pref_service");
-    ASSERT_TRUE(file_util::PathExists(data_dir_));
-  }
-  virtual void TearDown() {
-    // Clean up test directory
-    ASSERT_TRUE(file_util::Delete(test_dir_, true));
-    ASSERT_FALSE(file_util::PathExists(test_dir_));
-  }
-
-  // the path to temporary directory used to contain the test operations
-  FilePath test_dir_;
-  // the path to the directory where the test data is stored
-  FilePath data_dir_;
-};
-
 class TestPrefObserver : public NotificationObserver {
  public:
   TestPrefObserver(const PrefService* prefs, const std::wstring& pref_name,
@@ -90,122 +59,10 @@ class TestPrefObserver : public NotificationObserver {
   std::wstring new_pref_value_;
 };
 
-TEST_F(PrefServiceTest, Basic) {
-  {
-    // Test that it fails on nonexistent file.
-    FilePath bogus_input_file = data_dir_.AppendASCII("read.txt");
-    PrefService prefs(bogus_input_file);
-    EXPECT_FALSE(prefs.ReloadPersistentPrefs());
-  }
-
-  ASSERT_TRUE(file_util::CopyFile(data_dir_.AppendASCII("read.json"),
-                                  test_dir_.AppendASCII("write.json")));
-
-  // Test that the persistent value can be loaded.
-  FilePath input_file = test_dir_.AppendASCII("write.json");
-  ASSERT_TRUE(file_util::PathExists(input_file));
-  PrefService prefs(input_file);
-  ASSERT_TRUE(prefs.ReloadPersistentPrefs());
-
-  // Register test prefs.
-  const wchar_t kNewWindowsInTabs[] = L"tabs.new_windows_in_tabs";
-  const wchar_t kMaxTabs[] = L"tabs.max_tabs";
-  const wchar_t kLongIntPref[] = L"long_int.pref";
-  prefs.RegisterStringPref(prefs::kHomePage, L"");
-  prefs.RegisterBooleanPref(kNewWindowsInTabs, false);
-  prefs.RegisterIntegerPref(kMaxTabs, 0);
-  prefs.RegisterStringPref(kLongIntPref, L"2147483648");
-
-  std::wstring microsoft(L"http://www.microsoft.com");
-  std::wstring cnn(L"http://www.cnn.com");
-  std::wstring homepage(L"http://www.example.com");
-
-  EXPECT_EQ(cnn, prefs.GetString(prefs::kHomePage));
-
-  const wchar_t kSomeDirectory[] = L"some_directory";
-  FilePath some_path(FILE_PATH_LITERAL("/usr/sbin/"));
-  prefs.RegisterFilePathPref(kSomeDirectory, FilePath());
-
-  // Test reading some other data types from sub-dictionaries, and
-  // writing to the persistent store.
-  EXPECT_TRUE(prefs.GetBoolean(kNewWindowsInTabs));
-  prefs.SetBoolean(kNewWindowsInTabs, false);
-  EXPECT_FALSE(prefs.GetBoolean(kNewWindowsInTabs));
-
-  EXPECT_EQ(20, prefs.GetInteger(kMaxTabs));
-  prefs.SetInteger(kMaxTabs, 10);
-  EXPECT_EQ(10, prefs.GetInteger(kMaxTabs));
-
-  EXPECT_EQ(2147483648LL, prefs.GetInt64(kLongIntPref));
-  prefs.SetInt64(kLongIntPref, 214748364842LL);
-  EXPECT_EQ(214748364842LL, prefs.GetInt64(kLongIntPref));
-
-  EXPECT_EQ(FilePath::StringType(FILE_PATH_LITERAL("/usr/local/")),
-      prefs.GetFilePath(kSomeDirectory).value());
-  prefs.SetFilePath(kSomeDirectory, some_path);
-  EXPECT_EQ(some_path.value(), prefs.GetFilePath(kSomeDirectory).value());
-
-  // Serialize and compare to expected output.
-  // SavePersistentPrefs uses ImportantFileWriter which needs a file thread.
-  MessageLoop message_loop;
-  ChromeThread file_thread(ChromeThread::FILE, &message_loop);
-  FilePath output_file = test_dir_.AppendASCII("write.json");
-  FilePath golden_output_file = data_dir_.AppendASCII("write.golden.json");
-  ASSERT_TRUE(file_util::PathExists(golden_output_file));
-  ASSERT_TRUE(prefs.SavePersistentPrefs());
-  MessageLoop::current()->RunAllPending();
-  EXPECT_TRUE(file_util::TextContentsEqual(golden_output_file, output_file));
-  ASSERT_TRUE(file_util::Delete(output_file, false));
-}
-
-TEST_F(PrefServiceTest, Observers) {
-  FilePath input_file = data_dir_.AppendASCII("read.json");
-  EXPECT_TRUE(file_util::PathExists(input_file));
-
-  PrefService prefs(input_file);
-
-  EXPECT_TRUE(prefs.ReloadPersistentPrefs());
-
-  const wchar_t pref_name[] = L"homepage";
-  prefs.RegisterStringPref(pref_name, L"");
-  EXPECT_EQ(std::wstring(L"http://www.cnn.com"), prefs.GetString(pref_name));
-
-  const std::wstring new_pref_value(L"http://www.google.com/");
-  TestPrefObserver obs(&prefs, pref_name, new_pref_value);
-  prefs.AddPrefObserver(pref_name, &obs);
-  // This should fire the checks in TestPrefObserver::Observe.
-  prefs.SetString(pref_name, new_pref_value);
-
-  // Make sure the tests were actually run.
-  EXPECT_TRUE(obs.observer_fired());
-
-  // Now try adding a second pref observer.
-  const std::wstring new_pref_value2(L"http://www.youtube.com/");
-  obs.Reset(new_pref_value2);
-  TestPrefObserver obs2(&prefs, pref_name, new_pref_value2);
-  prefs.AddPrefObserver(pref_name, &obs2);
-  // This should fire the checks in obs and obs2.
-  prefs.SetString(pref_name, new_pref_value2);
-  EXPECT_TRUE(obs.observer_fired());
-  EXPECT_TRUE(obs2.observer_fired());
-
-  // Make sure obs2 still works after removing obs.
-  prefs.RemovePrefObserver(pref_name, &obs);
-  obs.Reset(L"");
-  obs2.Reset(new_pref_value);
-  // This should only fire the observer in obs2.
-  prefs.SetString(pref_name, new_pref_value);
-  EXPECT_FALSE(obs.observer_fired());
-  EXPECT_TRUE(obs2.observer_fired());
-
-  // Ok, clean up.
-  prefs.RemovePrefObserver(pref_name, &obs2);
-}
-
 // TODO(port): port this test to POSIX.
 #if defined(OS_WIN)
-TEST_F(PrefServiceTest, LocalizedPrefs) {
-  PrefService prefs((FilePath()));
+TEST(PrefServiceTest, LocalizedPrefs) {
+  PrefService prefs(new DummyPrefStore());
   const wchar_t kBoolean[] = L"boolean";
   const wchar_t kInteger[] = L"integer";
   const wchar_t kString[] = L"string";
@@ -227,8 +84,8 @@ TEST_F(PrefServiceTest, LocalizedPrefs) {
 }
 #endif
 
-TEST_F(PrefServiceTest, NoObserverFire) {
-  PrefService prefs((FilePath()));
+TEST(PrefServiceTest, NoObserverFire) {
+  PrefService prefs(new DummyPrefStore());
 
   const wchar_t pref_name[] = L"homepage";
   prefs.RegisterStringPref(pref_name, L"");
@@ -262,8 +119,8 @@ TEST_F(PrefServiceTest, NoObserverFire) {
   prefs.RemovePrefObserver(pref_name, &obs);
 }
 
-TEST_F(PrefServiceTest, HasPrefPath) {
-  PrefService prefs((FilePath()));
+TEST(PrefServiceTest, HasPrefPath) {
+  PrefService prefs(new DummyPrefStore());
 
   const wchar_t path[] = L"fake.path";
 
@@ -280,13 +137,55 @@ TEST_F(PrefServiceTest, HasPrefPath) {
   EXPECT_TRUE(prefs.HasPrefPath(path));
 }
 
+TEST(PrefServiceTest, Observers) {
+  const wchar_t pref_name[] = L"homepage";
+
+  DictionaryValue* dict = new DictionaryValue();
+  dict->SetString(pref_name, std::wstring(L"http://www.cnn.com"));
+  DummyPrefStore* pref_store = new DummyPrefStore();
+  pref_store->SetPrefs(dict);
+  PrefService prefs(pref_store);
+  prefs.RegisterStringPref(pref_name, L"");
+
+  const std::wstring new_pref_value(L"http://www.google.com/");
+  TestPrefObserver obs(&prefs, pref_name, new_pref_value);
+  prefs.AddPrefObserver(pref_name, &obs);
+  // This should fire the checks in TestPrefObserver::Observe.
+  prefs.SetString(pref_name, new_pref_value);
+
+  // Make sure the tests were actually run.
+  EXPECT_TRUE(obs.observer_fired());
+
+  // Now try adding a second pref observer.
+  const std::wstring new_pref_value2(L"http://www.youtube.com/");
+  obs.Reset(new_pref_value2);
+  TestPrefObserver obs2(&prefs, pref_name, new_pref_value2);
+  prefs.AddPrefObserver(pref_name, &obs2);
+  // This should fire the checks in obs and obs2.
+  prefs.SetString(pref_name, new_pref_value2);
+  EXPECT_TRUE(obs.observer_fired());
+  EXPECT_TRUE(obs2.observer_fired());
+
+  // Make sure obs2 still works after removing obs.
+  prefs.RemovePrefObserver(pref_name, &obs);
+  obs.Reset(L"");
+  obs2.Reset(new_pref_value);
+  // This should only fire the observer in obs2.
+  prefs.SetString(pref_name, new_pref_value);
+  EXPECT_FALSE(obs.observer_fired());
+  EXPECT_TRUE(obs2.observer_fired());
+
+  // Ok, clean up.
+  prefs.RemovePrefObserver(pref_name, &obs2);
+}
+
 class PrefServiceSetValueTest : public testing::Test {
  protected:
   static const wchar_t name_[];
   static const wchar_t value_[];
 
   PrefServiceSetValueTest()
-      : prefs_(FilePath()),
+      : prefs_(new DummyPrefStore()),
         name_string_(name_),
         null_value_(Value::CreateNullValue()) {}