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

27 files changed, 541 insertions, 330 deletions

diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc
index 7d7e6ab..5f70f8e 100644
--- a/chrome/browser/browser_main.cc
+++ b/chrome/browser/browser_main.cc
@@ -40,6 +40,7 @@
 #include "chrome/browser/extensions/extensions_service.h"
 #include "chrome/browser/first_run.h"
 #include "chrome/browser/jankometer.h"
+#include "chrome/browser/json_pref_store.h"
 #include "chrome/browser/metrics/histogram_synchronizer.h"
 #include "chrome/browser/metrics/metrics_service.h"
 #include "chrome/browser/net/dns_global.h"
@@ -430,7 +431,7 @@ PrefService* InitializeLocalState(const CommandLine& parsed_command_line,
       parsed_command_line.HasSwitch(switches::kParentProfile)) {
     FilePath parent_profile =
         parsed_command_line.GetSwitchValuePath(switches::kParentProfile);
-    PrefService parent_local_state(parent_profile);
+    PrefService parent_local_state(new JsonPrefStore(parent_profile));
     parent_local_state.RegisterStringPref(prefs::kApplicationLocale,
                                           std::wstring());
     // Right now, we only inherit the locale setting from the parent profile.
diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc
index be8b601..ac623a5 100644
--- a/chrome/browser/browser_process_impl.cc
+++ b/chrome/browser/browser_process_impl.cc
@@ -28,6 +28,7 @@
 #include "chrome/browser/in_process_webkit/dom_storage_context.h"
 #include "chrome/browser/intranet_redirect_detector.h"
 #include "chrome/browser/io_thread.h"
+#include "chrome/browser/json_pref_store.h"
 #include "chrome/browser/metrics/metrics_service.h"
 #include "chrome/browser/net/dns_global.h"
 #include "chrome/browser/net/sdch_dictionary_fetcher.h"
@@ -382,7 +383,7 @@ void BrowserProcessImpl::CreateLocalState() {
 
   FilePath local_state_path;
   PathService::Get(chrome::FILE_LOCAL_STATE, &local_state_path);
-  local_state_.reset(new PrefService(local_state_path));
+  local_state_.reset(new PrefService(new JsonPrefStore(local_state_path)));
 }
 
 void BrowserProcessImpl::CreateIconManager() {
diff --git a/chrome/browser/dom_ui/new_tab_ui_uitest.cc b/chrome/browser/dom_ui/new_tab_ui_uitest.cc
index 0b8f380..d130b4e 100644
--- a/chrome/browser/dom_ui/new_tab_ui_uitest.cc
+++ b/chrome/browser/dom_ui/new_tab_ui_uitest.cc
@@ -7,6 +7,7 @@
 #include "base/file_path.h"
 #include "chrome/app/chrome_dll_resource.h"
 #include "chrome/browser/dom_ui/new_tab_ui.h"
+#include "chrome/browser/json_pref_store.h"
 #include "chrome/browser/pref_service.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/automation/browser_proxy.h"
@@ -70,7 +71,7 @@ TEST_F(NewTabUITest, ChromeInternalLoadsNTP) {
 }
 
 TEST_F(NewTabUITest, UpdateUserPrefsVersion) {
-  PrefService prefs((FilePath()));
+  PrefService prefs(new JsonPrefStore(FilePath()));
 
   // Does the migration
   NewTabUI::RegisterUserPrefs(&prefs);
diff --git a/chrome/browser/dom_ui/shown_sections_handler_unittest.cc b/chrome/browser/dom_ui/shown_sections_handler_unittest.cc
index 4a54e71..08f3406 100644
--- a/chrome/browser/dom_ui/shown_sections_handler_unittest.cc
+++ b/chrome/browser/dom_ui/shown_sections_handler_unittest.cc
@@ -6,6 +6,7 @@
 
 #include "base/file_path.h"
 #include "base/scoped_ptr.h"
+#include "chrome/browser/json_pref_store.h"
 #include "chrome/browser/pref_service.h"
 #include "chrome/common/pref_names.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -14,7 +15,7 @@ class ShownSectionsHandlerTest : public testing::Test {
 };
 
 TEST_F(ShownSectionsHandlerTest, MigrateUserPrefs) {
-  PrefService pref((FilePath()));
+  PrefService pref(new JsonPrefStore(FilePath()));
 
   // Set an *old* value
   pref.RegisterIntegerPref(prefs::kNTPShownSections, 0);
@@ -32,7 +33,7 @@ TEST_F(ShownSectionsHandlerTest, MigrateUserPrefs) {
 }
 
 TEST_F(ShownSectionsHandlerTest, MigrateUserPrefs1To2) {
-  PrefService pref((FilePath()));
+  PrefService pref(new JsonPrefStore(FilePath()));
 
   // Set an *old* value
   pref.RegisterIntegerPref(prefs::kNTPShownSections, 0);
diff --git a/chrome/browser/dummy_pref_store.cc b/chrome/browser/dummy_pref_store.cc
new file mode 100644
index 0000000..bd728d4
--- /dev/null
+++ b/chrome/browser/dummy_pref_store.cc
@@ -0,0 +1,14 @@
+// Copyright (c) 2010 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.
+
+#include "chrome/browser/dummy_pref_store.h"
+
+#include "base/values.h"
+
+DummyPrefStore::DummyPrefStore() : prefs_(new DictionaryValue()) { }
+
+PrefStore::PrefReadError DummyPrefStore::ReadPrefs() {
+  prefs_.reset(new DictionaryValue());
+  return PrefStore::PREF_READ_ERROR_NONE;
+}
diff --git a/chrome/browser/dummy_pref_store.h b/chrome/browser/dummy_pref_store.h
new file mode 100644
index 0000000..16075c4
--- /dev/null
+++ b/chrome/browser/dummy_pref_store.h
@@ -0,0 +1,27 @@
+// Copyright (c) 2010 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_DUMMY_PREF_STORE_H_
+#define CHROME_BROWSER_DUMMY_PREF_STORE_H_
+
+#include "base/basictypes.h"
+#include "base/scoped_ptr.h"
+#include "chrome/browser/pref_store.h"
+
+class DummyPrefStore : public PrefStore {
+ public:
+  DummyPrefStore();
+
+  virtual DictionaryValue* Prefs() { return prefs_.get(); }
+  void SetPrefs(DictionaryValue* prefs) { prefs_.reset(prefs); }
+
+  virtual PrefStore::PrefReadError ReadPrefs();
+
+ private:
+  scoped_ptr<DictionaryValue> prefs_;
+
+  DISALLOW_COPY_AND_ASSIGN(DummyPrefStore);
+};
+
+#endif  // CHROME_BROWSER_DUMMY_PREF_STORE_H_
diff --git a/chrome/browser/extensions/extension_prefs_unittest.cc b/chrome/browser/extensions/extension_prefs_unittest.cc
index cd6c88a..40ff72a 100644
--- a/chrome/browser/extensions/extension_prefs_unittest.cc
+++ b/chrome/browser/extensions/extension_prefs_unittest.cc
@@ -7,6 +7,7 @@
 #include "base/string_util.h"
 #include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/extensions/extension_prefs.h"
+#include "chrome/browser/json_pref_store.h"
 #include "chrome/browser/pref_service.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -20,7 +21,8 @@ class ExtensionPrefsTest : public testing::Test {
   ExtensionPrefsTest() {
     EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
     FilePath preferences_file_ = temp_dir_.path().AppendASCII("Preferences");
-    pref_service_.reset(new PrefService(preferences_file_));
+    pref_service_.reset(new PrefService(
+        new JsonPrefStore(preferences_file_)));
     ExtensionPrefs::RegisterUserPrefs(pref_service_.get());
     CreateExtensionPrefs();
   }
diff --git a/chrome/browser/extensions/extension_updater_unittest.cc b/chrome/browser/extensions/extension_updater_unittest.cc
index 88c396d..c909a66 100644
--- a/chrome/browser/extensions/extension_updater_unittest.cc
+++ b/chrome/browser/extensions/extension_updater_unittest.cc
@@ -14,6 +14,7 @@
 #include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/extensions/extension_updater.h"
 #include "chrome/browser/extensions/extensions_service.h"
+#include "chrome/browser/json_pref_store.h"
 #include "chrome/browser/net/test_url_fetcher_factory.h"
 #include "chrome/browser/pref_service.h"
 #include "chrome/common/extensions/extension.h"
@@ -118,7 +119,7 @@ class ScopedTempPrefService {
     // problem when different tests are running in parallel.
     temp_dir_.CreateUniqueTempDir();
     FilePath pref_file = temp_dir_.path().AppendASCII("prefs");
-    prefs_.reset(new PrefService(pref_file));
+    prefs_.reset(new PrefService(new JsonPrefStore(pref_file)));
   }
 
   ~ScopedTempPrefService() {}
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index 9888e8f..9d3e242 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -240,7 +240,7 @@ ExtensionsServiceTestBase::~ExtensionsServiceTestBase() {
 void ExtensionsServiceTestBase::InitializeExtensionsService(
     const FilePath& pref_file, const FilePath& extensions_install_dir) {
   ExtensionTestingProfile* profile = new ExtensionTestingProfile();
-  prefs_.reset(new PrefService(pref_file));
+  prefs_.reset(new PrefService(new JsonPrefStore(pref_file)));
   Profile::RegisterUserPrefs(prefs_.get());
   browser::RegisterUserPrefs(prefs_.get());
   profile_.reset(profile);
diff --git a/chrome/browser/json_pref_store.cc b/chrome/browser/json_pref_store.cc
new file mode 100644
index 0000000..389986b
--- /dev/null
+++ b/chrome/browser/json_pref_store.cc
@@ -0,0 +1,128 @@
+// Copyright (c) 2010 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.
+
+#include "chrome/browser/json_pref_store.h"
+
+#include <algorithm>
+
+#include "base/file_util.h"
+#include "base/values.h"
+#include "chrome/common/json_value_serializer.h"
+
+namespace {
+
+// Some extensions we'll tack on to copies of the Preferences files.
+const FilePath::CharType* kBadExtension = FILE_PATH_LITERAL("bad");
+
+}  // namespace
+
+JsonPrefStore::JsonPrefStore(const FilePath& filename)
+    : path_(filename),
+      prefs_(new DictionaryValue()),
+      read_only_(false),
+      writer_(filename) { }
+
+JsonPrefStore::~JsonPrefStore() {
+  if (writer_.HasPendingWrite() && !read_only_)
+    writer_.DoScheduledWrite();
+}
+
+PrefStore::PrefReadError JsonPrefStore::ReadPrefs() {
+  JSONFileValueSerializer serializer(path_);
+
+  int error_code = 0;
+  std::string error_msg;
+  scoped_ptr<Value> value(serializer.Deserialize(&error_code, &error_msg));
+  if (!value.get()) {
+#if defined(GOOGLE_CHROME_BUILD)
+    // This log could be used for more detailed client-side error diagnosis,
+    // but since this triggers often with unit tests, we need to disable it
+    // in non-official builds.
+    PLOG(ERROR) << "Error reading Preferences: " << error_msg << " " <<
+        path_.value();
+#endif
+    PrefReadError error;
+    switch (error_code) {
+      case JSONFileValueSerializer::JSON_ACCESS_DENIED:
+        // If the file exists but is simply unreadable, put the file into a
+        // state where we don't try to save changes.  Otherwise, we could
+        // clobber the existing prefs.
+        error = PREF_READ_ERROR_ACCESS_DENIED;
+        read_only_ = true;
+        break;
+      case JSONFileValueSerializer::JSON_CANNOT_READ_FILE:
+        error = PREF_READ_ERROR_FILE_OTHER;
+        read_only_ = true;
+        break;
+      case JSONFileValueSerializer::JSON_FILE_LOCKED:
+        error = PREF_READ_ERROR_FILE_LOCKED;
+        read_only_ = true;
+        break;
+      case JSONFileValueSerializer::JSON_NO_SUCH_FILE:
+        // If the file just doesn't exist, maybe this is first run.  In any case
+        // there's no harm in writing out default prefs in this case.
+        error = PREF_READ_ERROR_NO_FILE;
+        break;
+      default:
+        error = PREF_READ_ERROR_JSON_PARSE;
+        // JSON errors indicate file corruption of some sort.
+        // Since the file is corrupt, move it to the side and continue with
+        // empty preferences.  This will result in them losing their settings.
+        // We keep the old file for possible support and debugging assistance
+        // as well as to detect if they're seeing these errors repeatedly.
+        // TODO(erikkay) Instead, use the last known good file.
+        FilePath bad = path_.ReplaceExtension(kBadExtension);
+
+        // If they've ever had a parse error before, put them in another bucket.
+        // TODO(erikkay) if we keep this error checking for very long, we may
+        // want to differentiate between recent and long ago errors.
+        if (file_util::PathExists(bad))
+          error = PREF_READ_ERROR_JSON_REPEAT;
+        file_util::Move(path_, bad);
+        break;
+    }
+    return error;
+  }
+
+  // Preferences should always have a dictionary root.
+  if (!value->IsType(Value::TYPE_DICTIONARY)) {
+    // See comment for the default case above.
+    read_only_ = true;
+    return PREF_READ_ERROR_JSON_TYPE;
+  }
+
+  prefs_.reset(static_cast<DictionaryValue*>(value.release()));
+
+  return PREF_READ_ERROR_NONE;
+}
+
+bool JsonPrefStore::WritePrefs() {
+  std::string data;
+  if (!SerializeData(&data))
+    return false;
+
+  // Lie about our ability to save.
+  if (read_only_)
+    return true;
+
+  writer_.WriteNow(data);
+  return true;
+}
+
+void JsonPrefStore::ScheduleWritePrefs() {
+  if (read_only_)
+    return;
+
+  writer_.ScheduleWrite(this);
+}
+
+bool JsonPrefStore::SerializeData(std::string* output) {
+  // TODO(tc): Do we want to prune webkit preferences that match the default
+  // value?
+  JSONStringValueSerializer serializer(output);
+  serializer.set_pretty_print(true);
+  scoped_ptr<DictionaryValue> copy(prefs_->DeepCopyWithoutEmptyChildren());
+  return serializer.Serialize(*(copy.get()));
+}
+
diff --git a/chrome/browser/json_pref_store.h b/chrome/browser/json_pref_store.h
new file mode 100644
index 0000000..7624244
--- /dev/null
+++ b/chrome/browser/json_pref_store.h
@@ -0,0 +1,48 @@
+// Copyright (c) 2010 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_JSON_PREF_STORE_H_
+#define CHROME_BROWSER_JSON_PREF_STORE_H_
+
+#include <string>
+
+#include "base/scoped_ptr.h"
+#include "chrome/browser/important_file_writer.h"
+#include "chrome/browser/pref_store.h"
+
+class DictionaryValue;
+class FilePath;
+
+class JsonPrefStore : public PrefStore,
+                      public ImportantFileWriter::DataSerializer {
+ public:
+  explicit JsonPrefStore(const FilePath& pref_filename);
+  virtual ~JsonPrefStore();
+
+  // PrefStore methods:
+  virtual bool ReadOnly() { return read_only_; }
+
+  virtual DictionaryValue* Prefs() { return prefs_.get(); }
+
+  virtual PrefReadError ReadPrefs();
+
+  virtual bool WritePrefs();
+
+  virtual void ScheduleWritePrefs();
+
+  // ImportantFileWriter::DataSerializer methods:
+  virtual bool SerializeData(std::string* data);
+
+ private:
+  FilePath path_;
+
+  scoped_ptr<DictionaryValue> prefs_;
+
+  bool read_only_;
+
+  // Helper for safely writing pref data.
+  ImportantFileWriter writer_;
+};
+
+#endif  // CHROME_BROWSER_JSON_PREF_STORE_H_
diff --git a/chrome/browser/json_pref_store_unittest.cc b/chrome/browser/json_pref_store_unittest.cc
new file mode 100644
index 0000000..05fcf66
--- /dev/null
+++ b/chrome/browser/json_pref_store_unittest.cc
@@ -0,0 +1,148 @@
+// Copyright (c) 2010 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.
+
+#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/string_util.h"
+#include "base/utf_string_conversions.h"
+#include "base/values.h"
+#include "chrome/browser/chrome_thread.h"
+#include "chrome/browser/json_pref_store.h"
+#include "chrome/common/chrome_paths.h"
+#include "chrome/common/pref_names.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class JsonPrefStoreTest : 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("JsonPrefStoreTest");
+
+    // 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_;
+};
+
+// Test fallback behavior for a nonexistent file.
+TEST_F(JsonPrefStoreTest, NonExistentFile) {
+  FilePath bogus_input_file = data_dir_.AppendASCII("read.txt");
+  ASSERT_FALSE(file_util::PathExists(bogus_input_file));
+  JsonPrefStore pref_store(bogus_input_file);
+  EXPECT_EQ(PrefStore::PREF_READ_ERROR_NO_FILE, pref_store.ReadPrefs());
+  EXPECT_FALSE(pref_store.ReadOnly());
+  EXPECT_TRUE(pref_store.Prefs()->empty());
+}
+
+// Test fallback behavior for an invalid file.
+TEST_F(JsonPrefStoreTest, InvalidFile) {
+  FilePath invalid_file_original = data_dir_.AppendASCII("invalid.json");
+  FilePath invalid_file = test_dir_.AppendASCII("invalid.json");
+  ASSERT_TRUE(file_util::CopyFile(invalid_file_original, invalid_file));
+  JsonPrefStore pref_store(invalid_file);
+  EXPECT_EQ(PrefStore::PREF_READ_ERROR_JSON_PARSE, pref_store.ReadPrefs());
+  EXPECT_FALSE(pref_store.ReadOnly());
+  EXPECT_TRUE(pref_store.Prefs()->empty());
+
+  // The file should have been moved aside.
+  EXPECT_FALSE(file_util::PathExists(invalid_file));
+  FilePath moved_aside = test_dir_.AppendASCII("invalid.bad");
+  EXPECT_TRUE(file_util::PathExists(moved_aside));
+  EXPECT_TRUE(file_util::TextContentsEqual(invalid_file_original,
+                                           moved_aside));
+}
+
+TEST_F(JsonPrefStoreTest, Basic) {
+  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));
+  JsonPrefStore pref_store(input_file);
+  ASSERT_EQ(PrefStore::PREF_READ_ERROR_NONE, pref_store.ReadPrefs());
+  ASSERT_FALSE(pref_store.ReadOnly());
+  DictionaryValue* prefs = pref_store.Prefs();
+
+  // The JSON file looks like this:
+  // {
+  //   "homepage": "http://www.cnn.com",
+  //   "some_directory": "/usr/local/",
+  //   "tabs": {
+  //     "new_windows_in_tabs": true,
+  //     "max_tabs": 20
+  //   }
+  // }
+
+  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";
+
+  std::wstring cnn(L"http://www.cnn.com");
+
+  std::wstring string_value;
+  EXPECT_TRUE(prefs->GetString(prefs::kHomePage, &string_value));
+  EXPECT_EQ(cnn, string_value);
+
+  const wchar_t kSomeDirectory[] = L"some_directory";
+
+  FilePath::StringType path;
+  EXPECT_TRUE(prefs->GetString(kSomeDirectory, &path));
+  EXPECT_EQ(FilePath::StringType(FILE_PATH_LITERAL("/usr/local/")), path);
+  FilePath some_path(FILE_PATH_LITERAL("/usr/sbin/"));
+  prefs->SetString(kSomeDirectory, some_path.value());
+  EXPECT_TRUE(prefs->GetString(kSomeDirectory, &path));
+  EXPECT_EQ(some_path.value(), path);
+
+  // Test reading some other data types from sub-dictionaries.
+  bool boolean;
+  EXPECT_TRUE(prefs->GetBoolean(kNewWindowsInTabs, &boolean));
+  EXPECT_TRUE(boolean);
+
+  prefs->SetBoolean(kNewWindowsInTabs, false);
+  EXPECT_TRUE(prefs->GetBoolean(kNewWindowsInTabs, &boolean));
+  EXPECT_FALSE(boolean);
+
+  int integer;
+  EXPECT_TRUE(prefs->GetInteger(kMaxTabs, &integer));
+  EXPECT_EQ(20, integer);
+  prefs->SetInteger(kMaxTabs, 10);
+  EXPECT_TRUE(prefs->GetInteger(kMaxTabs, &integer));
+  EXPECT_EQ(10, integer);
+
+  prefs->SetString(kLongIntPref, Int64ToWString(214748364842LL));
+  EXPECT_TRUE(prefs->GetString(kLongIntPref, &string_value));
+  EXPECT_EQ(214748364842LL, StringToInt64(WideToUTF16Hack(string_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 = input_file;
+  FilePath golden_output_file = data_dir_.AppendASCII("write.golden.json");
+  ASSERT_TRUE(file_util::PathExists(golden_output_file));
+  ASSERT_TRUE(pref_store.WritePrefs());
+  MessageLoop::current()->RunAllPending();
+  EXPECT_TRUE(file_util::TextContentsEqual(golden_output_file, output_file));
+  ASSERT_TRUE(file_util::Delete(output_file, false));
+}
diff --git a/chrome/browser/metrics/metrics_service_uitest.cc b/chrome/browser/metrics/metrics_service_uitest.cc
index 636cf1e..a943c68 100644
--- a/chrome/browser/metrics/metrics_service_uitest.cc
+++ b/chrome/browser/metrics/metrics_service_uitest.cc
@@ -11,6 +11,7 @@
 #include "base/file_util.h"
 #include "base/path_service.h"
 #include "base/platform_thread.h"
+#include "chrome/browser/json_pref_store.h"
 #include "chrome/browser/pref_service.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths.h"
@@ -51,7 +52,7 @@ class MetricsServiceTest : public UITest {
     FilePath local_state_path = user_data_dir()
         .Append(chrome::kLocalStateFilename);
 
-    return new PrefService(local_state_path);
+    return new PrefService(new JsonPrefStore(local_state_path));
   }
 };
 
diff --git a/chrome/browser/pref_member_unittest.cc b/chrome/browser/pref_member_unittest.cc
index 96cf68d..4a137e5 100644
--- a/chrome/browser/pref_member_unittest.cc
+++ b/chrome/browser/pref_member_unittest.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/file_path.h"
+#include "chrome/browser/dummy_pref_store.h"
 #include "chrome/browser/pref_member.h"
 #include "chrome/browser/pref_service.h"
 #include "chrome/common/notification_service.h"
@@ -51,7 +52,7 @@ class PrefMemberTestClass : public NotificationObserver {
 }  // anonymous namespace
 
 TEST(PrefMemberTest, BasicGetAndSet) {
-  PrefService prefs((FilePath()));
+  PrefService prefs(new DummyPrefStore());
   RegisterTestPrefs(&prefs);
 
   // Test bool
@@ -141,7 +142,7 @@ TEST(PrefMemberTest, BasicGetAndSet) {
 
 TEST(PrefMemberTest, TwoPrefs) {
   // Make sure two RealPrefMembers stay in sync.
-  PrefService prefs((FilePath()));
+  PrefService prefs(new DummyPrefStore());
   RegisterTestPrefs(&prefs);
 
   RealPrefMember pref1;
@@ -161,7 +162,7 @@ TEST(PrefMemberTest, TwoPrefs) {
 }
 
 TEST(PrefMemberTest, Observer) {
-  PrefService prefs((FilePath()));
+  PrefService prefs(new DummyPrefStore());
   RegisterTestPrefs(&prefs);
 
   PrefMemberTestClass test_obj(&prefs);
diff --git a/chrome/browser/pref_service.cc b/chrome/browser/pref_service.cc
index 74dc53d..62f8340 100644
--- a/chrome/browser/pref_service.cc
+++ b/chrome/browser/pref_service.cc
@@ -8,7 +8,6 @@
 #include <string>
 
 #include "app/l10n_util.h"
-#include "base/file_util.h"
 #include "base/histogram.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
@@ -18,16 +17,12 @@
 #include "base/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "chrome/browser/chrome_thread.h"
-#include "chrome/common/json_value_serializer.h"
 #include "chrome/common/notification_service.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 
 namespace {
 
-// Some extensions we'll tack on to copies of the Preferences files.
-const FilePath::CharType* kBadExtension = FILE_PATH_LITERAL("bad");
-
 // A helper function for RegisterLocalized*Pref that creates a Value* based on
 // the string value in the locale dll.  Because we control the values in a
 // locale dll, this should always return a Value of the appropriate type.
@@ -79,11 +74,8 @@ void NotifyReadError(PrefService* pref, int message_id) {
 
 }  // namespace
 
-PrefService::PrefService(const FilePath& pref_filename)
-    : persistent_(new DictionaryValue),
-      writer_(pref_filename),
-      read_only_(false) {
-  InitFromDisk();
+PrefService::PrefService(PrefStore* storage) : store_(storage) {
+  InitFromStorage();
 }
 
 PrefService::~PrefService() {
@@ -103,45 +95,20 @@ PrefService::~PrefService() {
   STLDeleteContainerPairSecondPointers(pref_observers_.begin(),
                                        pref_observers_.end());
   pref_observers_.clear();
-
-  if (writer_.HasPendingWrite() && !read_only_)
-    writer_.DoScheduledWrite();
 }
 
-void PrefService::InitFromDisk() {
-  PrefReadError error = LoadPersistentPrefs();
-  if (error == PREF_READ_ERROR_NONE)
+void PrefService::InitFromStorage() {
+  PrefStore::PrefReadError error = LoadPersistentPrefs();
+  if (error == PrefStore::PREF_READ_ERROR_NONE)
     return;
 
   // Failing to load prefs on startup is a bad thing(TM). See bug 38352 for
   // an example problem that this can cause.
   // Do some diagnosis and try to avoid losing data.
   int message_id = 0;
-  if (error <= PREF_READ_ERROR_JSON_TYPE) {
-    // JSON errors indicate file corruption of some sort.
+  if (error <= PrefStore::PREF_READ_ERROR_JSON_TYPE) {
     message_id = IDS_PREFERENCES_CORRUPT_ERROR;
-
-    // Since the file is corrupt, move it to the side and continue with
-    // empty preferences.  This will result in them losing their settings.
-    // We keep the old file for possible support and debugging assistance
-    // as well as to detect if they're seeing these errors repeatedly.
-    // TODO(erikkay) Instead, use the last known good file.
-    FilePath bad = writer_.path().ReplaceExtension(kBadExtension);
-
-    // If they've ever had a parse error before, put them in another bucket.
-    // TODO(erikkay) if we keep this error checking for very long, we may want
-    // to differentiate between recent and long ago errors.
-    if (file_util::PathExists(bad))
-      error = PREF_READ_ERROR_JSON_REPEAT;
-    file_util::Move(writer_.path(), bad);
-  } else if (error == PREF_READ_ERROR_NO_FILE) {
-    // If the file just doesn't exist, maybe this is first run.  In any case
-    // there's no harm in writing out default prefs in this case.
-  } else {
-    // If the file exists but is simply unreadable, put the file into a state
-    // where we don't try to save changes.  Otherwise, we could clobber the
-    // existing prefs.
-    read_only_ = true;
+  } else if (error != PrefStore::PREF_READ_ERROR_NO_FILE) {
     message_id = IDS_PREFERENCES_UNREADABLE_ERROR;
   }
 
@@ -153,153 +120,107 @@ void PrefService::InitFromDisk() {
 }
 
 bool PrefService::ReloadPersistentPrefs() {
-  return (LoadPersistentPrefs() == PREF_READ_ERROR_NONE);
+  return (LoadPersistentPrefs() == PrefStore::PREF_READ_ERROR_NONE);
 }
 
-PrefService::PrefReadError PrefService::LoadPersistentPrefs() {
+PrefStore::PrefReadError PrefService::LoadPersistentPrefs() {
   DCHECK(CalledOnValidThread());
-  JSONFileValueSerializer serializer(writer_.path());
-
-  int error_code = 0;
-  std::string error_msg;
-  scoped_ptr<Value> root(serializer.Deserialize(&error_code, &error_msg));
-  if (!root.get()) {
-#if defined(GOOGLE_CHROME_BUILD)
-    // This log could be used for more detailed client-side error diagnosis,
-    // but since this triggers often with unit tests, we need to disable it
-    // in non-official builds.
-    PLOG(ERROR) << "Error reading Preferences: " << error_msg << " " <<
-        writer_.path().value();
-#endif
-    PrefReadError pref_error;
-    switch (error_code) {
-      case JSONFileValueSerializer::JSON_ACCESS_DENIED:
-        pref_error = PREF_READ_ERROR_ACCESS_DENIED;
-        break;
-      case JSONFileValueSerializer::JSON_CANNOT_READ_FILE:
-        pref_error = PREF_READ_ERROR_FILE_OTHER;
-        break;
-      case JSONFileValueSerializer::JSON_FILE_LOCKED:
-        pref_error = PREF_READ_ERROR_FILE_LOCKED;
-        break;
-      case JSONFileValueSerializer::JSON_NO_SUCH_FILE:
-        pref_error = PREF_READ_ERROR_NO_FILE;
-        break;
-      default:
-        pref_error = PREF_READ_ERROR_JSON_PARSE;
-        break;
-    }
-    return pref_error;
-  }
 
-  // Preferences should always have a dictionary root.
-  if (!root->IsType(Value::TYPE_DICTIONARY))
-    return PREF_READ_ERROR_JSON_TYPE;
+  PrefStore::PrefReadError pref_error = store_->ReadPrefs();
+
+  persistent_ = store_->Prefs();
 
-  persistent_.reset(static_cast<DictionaryValue*>(root.release()));
   for (PreferenceSet::iterator it = prefs_.begin();
        it != prefs_.end(); ++it) {
-    (*it)->root_pref_ = persistent_.get();
+    (*it)->root_pref_ = persistent_;
   }
 
-  return PREF_READ_ERROR_NONE;
+  return pref_error;
 }
 
 bool PrefService::SavePersistentPrefs() {
   DCHECK(CalledOnValidThread());
 
-  std::string data;
-  if (!SerializeData(&data))
-    return false;
-
-  // Lie about our ability to save.
-  if (read_only_)
-    return true;
-
-  writer_.WriteNow(data);
-  return true;
+  return store_->WritePrefs();
 }
 
 void PrefService::ScheduleSavePersistentPrefs() {
   DCHECK(CalledOnValidThread());
 
-  if (read_only_)
-    return;
-
-  writer_.ScheduleWrite(this);
+  store_->ScheduleWritePrefs();
 }
 
 void PrefService::RegisterBooleanPref(const wchar_t* path,
                                       bool default_value) {
-  Preference* pref = new Preference(persistent_.get(), path,
+  Preference* pref = new Preference(persistent_, path,
       Value::CreateBooleanValue(default_value));
   RegisterPreference(pref);
 }
 
 void PrefService::RegisterIntegerPref(const wchar_t* path,
                                       int default_value) {
-  Preference* pref = new Preference(persistent_.get(), path,
+  Preference* pref = new Preference(persistent_, path,
       Value::CreateIntegerValue(default_value));
   RegisterPreference(pref);
 }
 
 void PrefService::RegisterRealPref(const wchar_t* path,
                                    double default_value) {
-  Preference* pref = new Preference(persistent_.get(), path,
+  Preference* pref = new Preference(persistent_, path,
       Value::CreateRealValue(default_value));
   RegisterPreference(pref);
 }
 
 void PrefService::RegisterStringPref(const wchar_t* path,
                                      const std::wstring& default_value) {
-  Preference* pref = new Preference(persistent_.get(), path,
+  Preference* pref = new Preference(persistent_, path,
       Value::CreateStringValue(default_value));
   RegisterPreference(pref);
 }
 
 void PrefService::RegisterFilePathPref(const wchar_t* path,
                                        const FilePath& default_value) {
-  Preference* pref = new Preference(persistent_.get(), path,
+  Preference* pref = new Preference(persistent_, path,
       Value::CreateStringValue(default_value.value()));
   RegisterPreference(pref);
 }
 
 void PrefService::RegisterListPref(const wchar_t* path) {
-  Preference* pref = new Preference(persistent_.get(), path,
+  Preference* pref = new Preference(persistent_, path,
       new ListValue);
   RegisterPreference(pref);
 }
 
 void PrefService::RegisterDictionaryPref(const wchar_t* path) {
-  Preference* pref = new Preference(persistent_.get(), path,
+  Preference* pref = new Preference(persistent_, path,
       new DictionaryValue());
   RegisterPreference(pref);
 }
 
 void PrefService::RegisterLocalizedBooleanPref(const wchar_t* path,
                                                int locale_default_message_id) {
-  Preference* pref = new Preference(persistent_.get(), path,
+  Preference* pref = new Preference(persistent_, path,
       CreateLocaleDefaultValue(Value::TYPE_BOOLEAN, locale_default_message_id));
   RegisterPreference(pref);
 }
 
 void PrefService::RegisterLocalizedIntegerPref(const wchar_t* path,
                                                int locale_default_message_id) {
-  Preference* pref = new Preference(persistent_.get(), path,
+  Preference* pref = new Preference(persistent_, path,
       CreateLocaleDefaultValue(Value::TYPE_INTEGER, locale_default_message_id));
   RegisterPreference(pref);
 }
 
 void PrefService::RegisterLocalizedRealPref(const wchar_t* path,
                                             int locale_default_message_id) {
-  Preference* pref = new Preference(persistent_.get(), path,
+  Preference* pref = new Preference(persistent_, path,
       CreateLocaleDefaultValue(Value::TYPE_REAL, locale_default_message_id));
   RegisterPreference(pref);
 }
 
 void PrefService::RegisterLocalizedStringPref(const wchar_t* path,
                                               int locale_default_message_id) {
-  Preference* pref = new Preference(persistent_.get(), path,
+  Preference* pref = new Preference(persistent_, path,
       CreateLocaleDefaultValue(Value::TYPE_STRING, locale_default_message_id));
   RegisterPreference(pref);
 }
@@ -490,7 +411,6 @@ void PrefService::ClearPref(const wchar_t* path) {
     NOTREACHED() << "Trying to clear an unregistered pref: " << path;
     return;
   }
-
   Value* value;
   bool has_old_value = persistent_->Get(path, &value);
   persistent_->Remove(path, NULL);
@@ -666,7 +586,7 @@ int64 PrefService::GetInt64(const wchar_t* path) const {
 }
 
 void PrefService::RegisterInt64Pref(const wchar_t* path, int64 default_value) {
-  Preference* pref = new Preference(persistent_.get(), path,
+  Preference* pref = new Preference(persistent_, path,
       Value::CreateStringValue(Int64ToWString(default_value)));
   RegisterPreference(pref);
 }
@@ -748,15 +668,6 @@ void PrefService::FireObservers(const wchar_t* path) {
   }
 }
 
-bool PrefService::SerializeData(std::string* output) {
-  // TODO(tc): Do we want to prune webkit preferences that match the default
-  // value?
-  JSONStringValueSerializer serializer(output);
-  serializer.set_pretty_print(true);
-  scoped_ptr<DictionaryValue> copy(persistent_->DeepCopyWithoutEmptyChildren());
-  return serializer.Serialize(*(copy.get()));
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 // PrefService::Preference
 
diff --git a/chrome/browser/pref_service.h b/chrome/browser/pref_service.h
index 48c8438..5b18d9d 100644
--- a/chrome/browser/pref_service.h
+++ b/chrome/browser/pref_service.h
@@ -3,14 +3,6 @@
 // found in the LICENSE file.
 
 // This provides a way to access the application's current preferences.
-// This service has two preference stores, one for "persistent" preferences,
-// which get serialized for use in the next session, and one for "transient"
-// preferences, which are in effect for only the current session
-// (this usually encodes things like command-line switches).
-//
-// Calling the getter functions in this class basically looks at both the
-// persistent and transient stores, where any corresponding value in the
-// transient store overrides the one in the persistent store.
 
 #ifndef CHROME_BROWSER_PREF_SERVICE_H_
 #define CHROME_BROWSER_PREF_SERVICE_H_
@@ -23,14 +15,13 @@
 #include "base/observer_list.h"
 #include "base/scoped_ptr.h"
 #include "base/values.h"
-#include "chrome/browser/important_file_writer.h"
+#include "chrome/browser/pref_store.h"
 
 class NotificationObserver;
 class Preference;
 class ScopedPrefUpdate;
 
-class PrefService : public NonThreadSafe,
-                    public ImportantFileWriter::DataSerializer {
+class PrefService : public NonThreadSafe {
  public:
 
   // A helper class to store all the information associated with a preference.
@@ -73,9 +64,8 @@ class PrefService : public NonThreadSafe,
     DISALLOW_COPY_AND_ASSIGN(Preference);
   };
 
-  // |pref_filename| is the path to the prefs file we will try to load or save
-  // to. Saves will be executed on the file thread.
-  explicit PrefService(const FilePath& pref_filename);
+  // The |PrefStore| manages reading and writing the preferences.
+  explicit PrefService(PrefStore* store);
   ~PrefService();
 
   // Reloads the data from file. This should only be called when the importer
@@ -183,27 +173,9 @@ class PrefService : public NonThreadSafe,
   // preference is not registered.
   const Preference* FindPreference(const wchar_t* pref_name) const;
 
-  // ImportantFileWriter::DataSerializer
-  virtual bool SerializeData(std::string* output);
-
-  bool read_only() const { return read_only_; }
+  bool read_only() const { return store_->ReadOnly(); }
 
  private:
-  // Unique integer code for each type of error so we can report them
-  // distinctly in a histogram.
-  // NOTE: Don't change the order here as it will change the server's meaning
-  // of the histogram.
-  enum PrefReadError {
-    PREF_READ_ERROR_NONE = 0,
-    PREF_READ_ERROR_JSON_PARSE,
-    PREF_READ_ERROR_JSON_TYPE,
-    PREF_READ_ERROR_ACCESS_DENIED,
-    PREF_READ_ERROR_FILE_OTHER,
-    PREF_READ_ERROR_FILE_LOCKED,
-    PREF_READ_ERROR_NO_FILE,
-    PREF_READ_ERROR_JSON_REPEAT,
-  };
-
   // Add a preference to the PreferenceMap.  If the pref already exists, return
   // false.  This method takes ownership of |pref|.
   void RegisterPreference(Preference* pref);
@@ -221,16 +193,16 @@ class PrefService : public NonThreadSafe,
                               const Value* old_value);
 
   // Load from disk.  Returns a non-zero error code on failure.
-  PrefReadError LoadPersistentPrefs();
+  PrefStore::PrefReadError LoadPersistentPrefs();
 
-  // Load preferences from disk, attempting to diagnose and handle errors.
+  // Load preferences from storage, attempting to diagnose and handle errors.
   // This should only be called from the constructor.
-  void InitFromDisk();
+  void InitFromStorage();
 
-  scoped_ptr<DictionaryValue> persistent_;
+  scoped_ptr<PrefStore> store_;
 
-  // Helper for safe writing pref data.
-  ImportantFileWriter writer_;
+  // The user-defined preference values. Owned by the |PrefStore|.
+  DictionaryValue* persistent_;
 
   // A set of all the registered Preference objects.
   PreferenceSet prefs_;
@@ -244,11 +216,6 @@ class PrefService : public NonThreadSafe,
 
   friend class ScopedPrefUpdate;
 
-  // Whether the service is in a pseudo-read-only mode where changes are not
-  // actually persisted to disk.  This happens in some cases when there are
-  // read errors during startup.
-  bool read_only_;
-
   DISALLOW_COPY_AND_ASSIGN(PrefService);
 };
 
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()) {}
 
diff --git a/chrome/browser/pref_store.h b/chrome/browser/pref_store.h
new file mode 100644
index 0000000..77130d6
--- /dev/null
+++ b/chrome/browser/pref_store.h
@@ -0,0 +1,47 @@
+// Copyright (c) 2010 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_PREF_STORE_H_
+#define CHROME_BROWSER_PREF_STORE_H_
+
+class DictionaryValue;
+
+// This is an abstract interface for reading and writing from/to a persistent
+// preference store, used by |PrefService|. An implementation using a JSON file
+// can be found in |JsonPrefStore|, while an implementation without any backing
+// store (currently used for testing) can be found in |DummyPrefStore|.
+class PrefStore {
+ public:
+  // Unique integer code for each type of error so we can report them
+  // distinctly in a histogram.
+  // NOTE: Don't change the order here as it will change the server's meaning
+  // of the histogram.
+  enum PrefReadError {
+    PREF_READ_ERROR_NONE = 0,
+    PREF_READ_ERROR_JSON_PARSE,
+    PREF_READ_ERROR_JSON_TYPE,
+    PREF_READ_ERROR_ACCESS_DENIED,
+    PREF_READ_ERROR_FILE_OTHER,
+    PREF_READ_ERROR_FILE_LOCKED,
+    PREF_READ_ERROR_NO_FILE,
+    PREF_READ_ERROR_JSON_REPEAT,
+  };
+
+  virtual ~PrefStore() { }
+
+  // Whether the store is in a pseudo-read-only mode where changes are not
+  // actually persisted to disk.  This happens in some cases when there are
+  // read errors during startup.
+  virtual bool ReadOnly() { return true; }
+
+  virtual DictionaryValue* Prefs() = 0;
+
+  virtual PrefReadError ReadPrefs() = 0;
+
+  virtual bool WritePrefs() { return true; }
+
+  virtual void ScheduleWritePrefs() { }
+};
+
+#endif  // CHROME_BROWSER_PREF_STORE_H_
diff --git a/chrome/browser/privacy_blacklist/blacklist_unittest.cc b/chrome/browser/privacy_blacklist/blacklist_unittest.cc
index 16d2094..a7a3188 100644
--- a/chrome/browser/privacy_blacklist/blacklist_unittest.cc
+++ b/chrome/browser/privacy_blacklist/blacklist_unittest.cc
@@ -9,6 +9,7 @@
 #include "base/path_service.h"
 #include "base/string_util.h"
 #include "chrome/browser/browser_prefs.h"
+#include "chrome/browser/json_pref_store.h"
 #include "chrome/browser/pref_service.h"
 #include "chrome/browser/profile.h"
 #include "chrome/common/chrome_paths.h"
@@ -22,7 +23,7 @@ class BlacklistTest : public testing::Test {
     source_path = source_path.AppendASCII("profiles")
         .AppendASCII("blacklist_prefs").AppendASCII("Preferences");
 
-    prefs_.reset(new PrefService(source_path));
+    prefs_.reset(new PrefService(new JsonPrefStore(source_path)));
     Profile::RegisterUserPrefs(prefs_.get());
     browser::RegisterAllPrefs(prefs_.get(), prefs_.get());
   }
diff --git a/chrome/browser/profile.cc b/chrome/browser/profile.cc
index ed480e8..0777837 100644
--- a/chrome/browser/profile.cc
+++ b/chrome/browser/profile.cc
@@ -36,6 +36,7 @@
 #include "chrome/browser/host_content_settings_map.h"
 #include "chrome/browser/host_zoom_map.h"
 #include "chrome/browser/in_process_webkit/webkit_context.h"
+#include "chrome/browser/json_pref_store.h"
 #include "chrome/browser/net/chrome_url_request_context.h"
 #include "chrome/browser/net/ssl_config_service_manager.h"
 #include "chrome/browser/notifications/desktop_notification_service.h"
@@ -938,7 +939,7 @@ net::TransportSecurityState*
 
 PrefService* ProfileImpl::GetPrefs() {
   if (!prefs_.get()) {
-    prefs_.reset(new PrefService(GetPrefFilePath()));
+    prefs_.reset(new PrefService(new JsonPrefStore(GetPrefFilePath())));
 
     // The Profile class and ProfileManager class may read some prefs so
     // register known prefs as soon as possible.
diff --git a/chrome/browser/tab_contents/web_contents_unittest.cc b/chrome/browser/tab_contents/web_contents_unittest.cc
index 0ab3faf..f9e731e 100644
--- a/chrome/browser/tab_contents/web_contents_unittest.cc
+++ b/chrome/browser/tab_contents/web_contents_unittest.cc
@@ -51,7 +51,7 @@ class TabContentsTestingProfile : public TestingProfile {
       source_path = source_path.AppendASCII("profiles")
           .AppendASCII("chrome_prefs").AppendASCII("Preferences");
 
-      prefs_.reset(new PrefService(source_path));
+      prefs_.reset(new PrefService(new JsonPrefStore(source_path)));
       Profile::RegisterUserPrefs(prefs_.get());
       browser::RegisterAllPrefs(prefs_.get(), prefs_.get());
     }
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 43d9aac..750ee58 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -1535,6 +1535,8 @@
         'browser/js_modal_dialog_mac.mm',
         'browser/js_modal_dialog_win.cc',
         'browser/jsmessage_box_client.h',
+        'browser/json_pref_store.cc',
+        'browser/json_pref_store.h',
         'browser/keychain_mac.cc',
         'browser/keychain_mac.h',
         'browser/language_combobox_model.cc',
@@ -1733,6 +1735,7 @@
         'browser/pref_member.h',
         'browser/pref_service.cc',
         'browser/pref_service.h',
+        'browser/pref_store.h',
         'browser/printing/print_dialog_gtk.cc',
         'browser/printing/print_dialog_gtk.h',
         'browser/printing/print_job.cc',
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 5f34f37..257223c 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -67,6 +67,8 @@
         # The only thing used from browser is Browser::Type.
         'browser/browser.h',
         'browser/cocoa/browser_test_helper.h',
+        'browser/dummy_pref_store.cc',
+        'browser/dummy_pref_store.h',
         'browser/geolocation/mock_location_provider.cc',
         'browser/geolocation/mock_location_provider.h',
         'browser/mock_browsing_data_appcache_helper.cc',
@@ -826,6 +828,7 @@
         'browser/in_process_webkit/dom_storage_dispatcher_host_unittest.cc',
         'browser/in_process_webkit/webkit_context_unittest.cc',
         'browser/in_process_webkit/webkit_thread_unittest.cc',
+        'browser/json_pref_store_unittest.cc',
         'browser/keychain_mock_mac.cc',
         'browser/keychain_mock_mac.h',
         'browser/login_prompt_unittest.cc',
diff --git a/chrome/test/data/pref_service/invalid.json b/chrome/test/data/pref_service/invalid.json
new file mode 100644
index 0000000..43392a9
--- /dev/null
+++ b/chrome/test/data/pref_service/invalid.json
@@ -0,0 +1 @@
+!@#$%^&
\ No newline at end of file
diff --git a/chrome/test/reliability/page_load_test.cc b/chrome/test/reliability/page_load_test.cc
index fb673b9..eba821a 100644
--- a/chrome/test/reliability/page_load_test.cc
+++ b/chrome/test/reliability/page_load_test.cc
@@ -48,6 +48,7 @@
 #include "base/test/test_file_util.h"
 #include "base/time.h"
 #include "chrome/app/chrome_version_info.h"
+#include "chrome/browser/json_pref_store.h"
 #include "chrome/browser/net/url_fixer_upper.h"
 #include "chrome/browser/pref_service.h"
 #include "chrome/common/chrome_constants.h"
@@ -526,7 +527,8 @@ class PageLoadTest : public UITest {
     FilePath local_state_path = user_data_dir()
         .Append(chrome::kLocalStateFilename);
 
-    PrefService* local_state(new PrefService(local_state_path));
+    PrefService* local_state(new PrefService(
+        new JsonPrefStore(local_state_path)));
     return local_state;
   }
 
diff --git a/chrome/test/testing_profile.h b/chrome/test/testing_profile.h
index f3c37f0..acb724e 100644
--- a/chrome/test/testing_profile.h
+++ b/chrome/test/testing_profile.h
@@ -17,6 +17,7 @@
 #include "chrome/browser/host_content_settings_map.h"
 #include "chrome/browser/history/history.h"
 #include "chrome/browser/in_process_webkit/webkit_context.h"
+#include "chrome/browser/json_pref_store.h"
 #include "chrome/browser/net/url_request_context_getter.h"
 #include "chrome/browser/pref_service.h"
 #include "chrome/browser/profile.h"
@@ -143,7 +144,7 @@ class TestingProfile : public Profile {
     if (!prefs_.get()) {
       FilePath prefs_filename =
           path_.Append(FILE_PATH_LITERAL("TestPreferences"));
-      prefs_.reset(new PrefService(prefs_filename));
+      prefs_.reset(new PrefService(new JsonPrefStore(prefs_filename)));
       Profile::RegisterUserPrefs(prefs_.get());
       browser::RegisterAllPrefs(prefs_.get(), prefs_.get());
     }
diff --git a/chrome_frame/test/reliability/page_load_test.cc b/chrome_frame/test/reliability/page_load_test.cc
index 8f31849..1a2176b 100644
--- a/chrome_frame/test/reliability/page_load_test.cc
+++ b/chrome_frame/test/reliability/page_load_test.cc
@@ -35,6 +35,7 @@
 #include "base/string_util.h"
 #include "base/test/test_file_util.h"
 #include "base/time.h"
+#include "chrome/browser/json_pref_store.h"
 #include "chrome/browser/net/url_fixer_upper.h"
 #include "chrome/browser/pref_service.h"
 #include "chrome/common/chrome_constants.h"
@@ -467,7 +468,7 @@ class PageLoadTest : public testing::Test {
     FilePath local_state_path;
     chrome::GetChromeFrameUserDataDirectory(&local_state_path);
 
-    PrefService* local_state = new PrefService(local_state_path);
+    PrefService* local_state = new PrefService(new JsonPrefStore(local_state_path));
     return local_state;
   }