Revert 45168 - Reland r45028: Factor out reading and writing of preferences into |PrefStore|.

In order to implement platformspecific 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/1687001

TBR=bauerb@chromium.org
Review URL: http://codereview.chromium.org/1688004

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

27 files changed, 314 insertions, 519 deletions

diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc
index f378797..120a7c1 100644
--- a/chrome/browser/browser_main.cc
+++ b/chrome/browser/browser_main.cc
@@ -40,7 +40,6 @@
 #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"
@@ -431,7 +430,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(new JsonPrefStore(parent_profile));
+    PrefService parent_local_state(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 ac623a5..be8b601 100644
--- a/chrome/browser/browser_process_impl.cc
+++ b/chrome/browser/browser_process_impl.cc
@@ -28,7 +28,6 @@
 #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"
@@ -383,7 +382,7 @@ void BrowserProcessImpl::CreateLocalState() {
 
   FilePath local_state_path;
   PathService::Get(chrome::FILE_LOCAL_STATE, &local_state_path);
-  local_state_.reset(new PrefService(new JsonPrefStore(local_state_path)));
+  local_state_.reset(new PrefService(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 d130b4e..0b8f380 100644
--- a/chrome/browser/dom_ui/new_tab_ui_uitest.cc
+++ b/chrome/browser/dom_ui/new_tab_ui_uitest.cc
@@ -7,7 +7,6 @@
 #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"
@@ -71,7 +70,7 @@ TEST_F(NewTabUITest, ChromeInternalLoadsNTP) {
 }
 
 TEST_F(NewTabUITest, UpdateUserPrefsVersion) {
-  PrefService prefs(new JsonPrefStore(FilePath()));
+  PrefService prefs((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 08f3406..a8dfa05 100644
--- a/chrome/browser/dom_ui/shown_sections_handler_unittest.cc
+++ b/chrome/browser/dom_ui/shown_sections_handler_unittest.cc
@@ -6,7 +6,6 @@
 
 #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"
@@ -15,7 +14,7 @@ class ShownSectionsHandlerTest : public testing::Test {
 };
 
 TEST_F(ShownSectionsHandlerTest, MigrateUserPrefs) {
-  PrefService pref(new JsonPrefStore(FilePath()));
+  PrefService pref((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
deleted file mode 100644
index bd728d4..0000000
--- a/chrome/browser/dummy_pref_store.cc
+++ /dev/null
@@ -1,14 +0,0 @@
-// 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
deleted file mode 100644
index 16075c4..0000000
--- a/chrome/browser/dummy_pref_store.h
+++ /dev/null
@@ -1,27 +0,0 @@
-// 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 40ff72a..cd6c88a 100644
--- a/chrome/browser/extensions/extension_prefs_unittest.cc
+++ b/chrome/browser/extensions/extension_prefs_unittest.cc
@@ -7,7 +7,6 @@
 #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"
@@ -21,8 +20,7 @@ class ExtensionPrefsTest : public testing::Test {
   ExtensionPrefsTest() {
     EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
     FilePath preferences_file_ = temp_dir_.path().AppendASCII("Preferences");
-    pref_service_.reset(new PrefService(
-        new JsonPrefStore(preferences_file_)));
+    pref_service_.reset(new PrefService(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 c909a66..88c396d 100644
--- a/chrome/browser/extensions/extension_updater_unittest.cc
+++ b/chrome/browser/extensions/extension_updater_unittest.cc
@@ -14,7 +14,6 @@
 #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"
@@ -119,7 +118,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(new JsonPrefStore(pref_file)));
+    prefs_.reset(new PrefService(pref_file));
   }
 
   ~ScopedTempPrefService() {}
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index 9d3e242..9888e8f 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(new JsonPrefStore(pref_file)));
+  prefs_.reset(new PrefService(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
deleted file mode 100644
index 80258a4..0000000
--- a/chrome/browser/json_pref_store.cc
+++ /dev/null
@@ -1,110 +0,0 @@
-// 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/values.h"
-#include "chrome/common/json_value_serializer.h"
-
-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;
-        // It's possible the user hand-edited the file, so don't clobber it yet.
-        // Give them a chance to recover the file.
-        // TODO(erikkay) maybe we should just move it aside and continue.
-        read_only_ = true;
-        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
deleted file mode 100644
index 7624244..0000000
--- a/chrome/browser/json_pref_store.h
+++ /dev/null
@@ -1,48 +0,0 @@
-// 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
deleted file mode 100644
index 0c8cb06..0000000
--- a/chrome/browser/json_pref_store_unittest.cc
+++ /dev/null
@@ -1,146 +0,0 @@
-// 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.out.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_TRUE(pref_store.ReadOnly());
-  EXPECT_TRUE(pref_store.Prefs()->empty());
-  // Writing to a read-only store should return true, but do nothing.
-  EXPECT_TRUE(pref_store.WritePrefs());
-  EXPECT_TRUE(file_util::TextContentsEqual(invalid_file_original,
-                                           invalid_file));
-  ASSERT_TRUE(file_util::Delete(invalid_file, false));
-}
-
-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 a943c68..636cf1e 100644
--- a/chrome/browser/metrics/metrics_service_uitest.cc
+++ b/chrome/browser/metrics/metrics_service_uitest.cc
@@ -11,7 +11,6 @@
 #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"
@@ -52,7 +51,7 @@ class MetricsServiceTest : public UITest {
     FilePath local_state_path = user_data_dir()
         .Append(chrome::kLocalStateFilename);
 
-    return new PrefService(new JsonPrefStore(local_state_path));
+    return new PrefService(local_state_path);
   }
 };
 
diff --git a/chrome/browser/pref_member_unittest.cc b/chrome/browser/pref_member_unittest.cc
index 4a137e5..96cf68d 100644
--- a/chrome/browser/pref_member_unittest.cc
+++ b/chrome/browser/pref_member_unittest.cc
@@ -3,7 +3,6 @@
 // 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"
@@ -52,7 +51,7 @@ class PrefMemberTestClass : public NotificationObserver {
 }  // anonymous namespace
 
 TEST(PrefMemberTest, BasicGetAndSet) {
-  PrefService prefs(new DummyPrefStore());
+  PrefService prefs((FilePath()));
   RegisterTestPrefs(&prefs);
 
   // Test bool
@@ -142,7 +141,7 @@ TEST(PrefMemberTest, BasicGetAndSet) {
 
 TEST(PrefMemberTest, TwoPrefs) {
   // Make sure two RealPrefMembers stay in sync.
-  PrefService prefs(new DummyPrefStore());
+  PrefService prefs((FilePath()));
   RegisterTestPrefs(&prefs);
 
   RealPrefMember pref1;
@@ -162,7 +161,7 @@ TEST(PrefMemberTest, TwoPrefs) {
 }
 
 TEST(PrefMemberTest, Observer) {
-  PrefService prefs(new DummyPrefStore());
+  PrefService prefs((FilePath()));
   RegisterTestPrefs(&prefs);
 
   PrefMemberTestClass test_obj(&prefs);
diff --git a/chrome/browser/pref_service.cc b/chrome/browser/pref_service.cc
index 62f8340..060e462 100644
--- a/chrome/browser/pref_service.cc
+++ b/chrome/browser/pref_service.cc
@@ -17,6 +17,7 @@
 #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"
@@ -74,8 +75,11 @@ void NotifyReadError(PrefService* pref, int message_id) {
 
 }  // namespace
 
-PrefService::PrefService(PrefStore* storage) : store_(storage) {
-  InitFromStorage();
+PrefService::PrefService(const FilePath& pref_filename)
+    : persistent_(new DictionaryValue),
+      writer_(pref_filename),
+      read_only_(false) {
+  InitFromDisk();
 }
 
 PrefService::~PrefService() {
@@ -95,20 +99,35 @@ PrefService::~PrefService() {
   STLDeleteContainerPairSecondPointers(pref_observers_.begin(),
                                        pref_observers_.end());
   pref_observers_.clear();
+
+  if (writer_.HasPendingWrite() && !read_only_)
+    writer_.DoScheduledWrite();
 }
 
-void PrefService::InitFromStorage() {
-  PrefStore::PrefReadError error = LoadPersistentPrefs();
-  if (error == PrefStore::PREF_READ_ERROR_NONE)
+void PrefService::InitFromDisk() {
+  PrefReadError error = LoadPersistentPrefs();
+  if (error == 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 <= PrefStore::PREF_READ_ERROR_JSON_TYPE) {
+  if (error <= PREF_READ_ERROR_JSON_TYPE) {
+    // JSON errors indicate file corruption of some sort.
+    // It's possible the user hand-edited the file, so don't clobber it yet.
+    // Give them a chance to recover the file.
+    // TODO(erikkay) maybe we should just move it aside and continue.
+    read_only_ = true;
     message_id = IDS_PREFERENCES_CORRUPT_ERROR;
-  } else if (error != PrefStore::PREF_READ_ERROR_NO_FILE) {
+  } 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;
     message_id = IDS_PREFERENCES_UNREADABLE_ERROR;
   }
 
@@ -120,107 +139,153 @@ void PrefService::InitFromStorage() {
 }
 
 bool PrefService::ReloadPersistentPrefs() {
-  return (LoadPersistentPrefs() == PrefStore::PREF_READ_ERROR_NONE);
+  return (LoadPersistentPrefs() == PREF_READ_ERROR_NONE);
 }
 
-PrefStore::PrefReadError PrefService::LoadPersistentPrefs() {
+PrefService::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;
+  }
 
-  PrefStore::PrefReadError pref_error = store_->ReadPrefs();
-
-  persistent_ = store_->Prefs();
+  // Preferences should always have a dictionary root.
+  if (!root->IsType(Value::TYPE_DICTIONARY))
+    return PREF_READ_ERROR_JSON_TYPE;
 
+  persistent_.reset(static_cast<DictionaryValue*>(root.release()));
   for (PreferenceSet::iterator it = prefs_.begin();
        it != prefs_.end(); ++it) {
-    (*it)->root_pref_ = persistent_;
+    (*it)->root_pref_ = persistent_.get();
   }
 
-  return pref_error;
+  return PREF_READ_ERROR_NONE;
 }
 
 bool PrefService::SavePersistentPrefs() {
   DCHECK(CalledOnValidThread());
 
-  return store_->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 PrefService::ScheduleSavePersistentPrefs() {
   DCHECK(CalledOnValidThread());
 
-  store_->ScheduleWritePrefs();
+  if (read_only_)
+    return;
+
+  writer_.ScheduleWrite(this);
 }
 
 void PrefService::RegisterBooleanPref(const wchar_t* path,
                                       bool default_value) {
-  Preference* pref = new Preference(persistent_, path,
+  Preference* pref = new Preference(persistent_.get(), path,
       Value::CreateBooleanValue(default_value));
   RegisterPreference(pref);
 }
 
 void PrefService::RegisterIntegerPref(const wchar_t* path,
                                       int default_value) {
-  Preference* pref = new Preference(persistent_, path,
+  Preference* pref = new Preference(persistent_.get(), path,
       Value::CreateIntegerValue(default_value));
   RegisterPreference(pref);
 }
 
 void PrefService::RegisterRealPref(const wchar_t* path,
                                    double default_value) {
-  Preference* pref = new Preference(persistent_, path,
+  Preference* pref = new Preference(persistent_.get(), path,
       Value::CreateRealValue(default_value));
   RegisterPreference(pref);
 }
 
 void PrefService::RegisterStringPref(const wchar_t* path,
                                      const std::wstring& default_value) {
-  Preference* pref = new Preference(persistent_, path,
+  Preference* pref = new Preference(persistent_.get(), path,
       Value::CreateStringValue(default_value));
   RegisterPreference(pref);
 }
 
 void PrefService::RegisterFilePathPref(const wchar_t* path,
                                        const FilePath& default_value) {
-  Preference* pref = new Preference(persistent_, path,
+  Preference* pref = new Preference(persistent_.get(), path,
       Value::CreateStringValue(default_value.value()));
   RegisterPreference(pref);
 }
 
 void PrefService::RegisterListPref(const wchar_t* path) {
-  Preference* pref = new Preference(persistent_, path,
+  Preference* pref = new Preference(persistent_.get(), path,
       new ListValue);
   RegisterPreference(pref);
 }
 
 void PrefService::RegisterDictionaryPref(const wchar_t* path) {
-  Preference* pref = new Preference(persistent_, path,
+  Preference* pref = new Preference(persistent_.get(), path,
       new DictionaryValue());
   RegisterPreference(pref);
 }
 
 void PrefService::RegisterLocalizedBooleanPref(const wchar_t* path,
                                                int locale_default_message_id) {
-  Preference* pref = new Preference(persistent_, path,
+  Preference* pref = new Preference(persistent_.get(), 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_, path,
+  Preference* pref = new Preference(persistent_.get(), 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_, path,
+  Preference* pref = new Preference(persistent_.get(), 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_, path,
+  Preference* pref = new Preference(persistent_.get(), path,
       CreateLocaleDefaultValue(Value::TYPE_STRING, locale_default_message_id));
   RegisterPreference(pref);
 }
@@ -411,6 +476,7 @@ 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);
@@ -586,7 +652,7 @@ int64 PrefService::GetInt64(const wchar_t* path) const {
 }
 
 void PrefService::RegisterInt64Pref(const wchar_t* path, int64 default_value) {
-  Preference* pref = new Preference(persistent_, path,
+  Preference* pref = new Preference(persistent_.get(), path,
       Value::CreateStringValue(Int64ToWString(default_value)));
   RegisterPreference(pref);
 }
@@ -668,6 +734,15 @@ 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 5b18d9d..75d829e 100644
--- a/chrome/browser/pref_service.h
+++ b/chrome/browser/pref_service.h
@@ -3,6 +3,14 @@
 // 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_
@@ -15,13 +23,14 @@
 #include "base/observer_list.h"
 #include "base/scoped_ptr.h"
 #include "base/values.h"
-#include "chrome/browser/pref_store.h"
+#include "chrome/browser/important_file_writer.h"
 
 class NotificationObserver;
 class Preference;
 class ScopedPrefUpdate;
 
-class PrefService : public NonThreadSafe {
+class PrefService : public NonThreadSafe,
+                    public ImportantFileWriter::DataSerializer {
  public:
 
   // A helper class to store all the information associated with a preference.
@@ -64,8 +73,9 @@ class PrefService : public NonThreadSafe {
     DISALLOW_COPY_AND_ASSIGN(Preference);
   };
 
-  // The |PrefStore| manages reading and writing the preferences.
-  explicit PrefService(PrefStore* store);
+  // |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);
   ~PrefService();
 
   // Reloads the data from file. This should only be called when the importer
@@ -173,9 +183,26 @@ class PrefService : public NonThreadSafe {
   // preference is not registered.
   const Preference* FindPreference(const wchar_t* pref_name) const;
 
-  bool read_only() const { return store_->ReadOnly(); }
+  // ImportantFileWriter::DataSerializer
+  virtual bool SerializeData(std::string* output);
+
+  bool read_only() const { return read_only_; }
 
  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,
+  };
+
   // Add a preference to the PreferenceMap.  If the pref already exists, return
   // false.  This method takes ownership of |pref|.
   void RegisterPreference(Preference* pref);
@@ -193,16 +220,16 @@ class PrefService : public NonThreadSafe {
                               const Value* old_value);
 
   // Load from disk.  Returns a non-zero error code on failure.
-  PrefStore::PrefReadError LoadPersistentPrefs();
+  PrefReadError LoadPersistentPrefs();
 
-  // Load preferences from storage, attempting to diagnose and handle errors.
+  // Load preferences from disk, attempting to diagnose and handle errors.
   // This should only be called from the constructor.
-  void InitFromStorage();
+  void InitFromDisk();
 
-  scoped_ptr<PrefStore> store_;
+  scoped_ptr<DictionaryValue> persistent_;
 
-  // The user-defined preference values. Owned by the |PrefStore|.
-  DictionaryValue* persistent_;
+  // Helper for safe writing pref data.
+  ImportantFileWriter writer_;
 
   // A set of all the registered Preference objects.
   PreferenceSet prefs_;
@@ -216,6 +243,11 @@ 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 fc801d6..e643b8c 100644
--- a/chrome/browser/pref_service_unittest.cc
+++ b/chrome/browser/pref_service_unittest.cc
@@ -5,11 +5,15 @@
 #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/dummy_pref_store.h"
+#include "chrome/browser/chrome_thread.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"
@@ -22,6 +26,33 @@ 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,
@@ -59,10 +90,122 @@ 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(PrefServiceTest, LocalizedPrefs) {
-  PrefService prefs(new DummyPrefStore());
+TEST_F(PrefServiceTest, LocalizedPrefs) {
+  PrefService prefs((FilePath()));
   const wchar_t kBoolean[] = L"boolean";
   const wchar_t kInteger[] = L"integer";
   const wchar_t kString[] = L"string";
@@ -84,8 +227,8 @@ TEST(PrefServiceTest, LocalizedPrefs) {
 }
 #endif
 
-TEST(PrefServiceTest, NoObserverFire) {
-  PrefService prefs(new DummyPrefStore());
+TEST_F(PrefServiceTest, NoObserverFire) {
+  PrefService prefs((FilePath()));
 
   const wchar_t pref_name[] = L"homepage";
   prefs.RegisterStringPref(pref_name, L"");
@@ -119,8 +262,8 @@ TEST(PrefServiceTest, NoObserverFire) {
   prefs.RemovePrefObserver(pref_name, &obs);
 }
 
-TEST(PrefServiceTest, HasPrefPath) {
-  PrefService prefs(new DummyPrefStore());
+TEST_F(PrefServiceTest, HasPrefPath) {
+  PrefService prefs((FilePath()));
 
   const wchar_t path[] = L"fake.path";
 
@@ -137,55 +280,13 @@ TEST(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_(new DummyPrefStore()),
+      : prefs_(FilePath()),
         name_string_(name_),
         null_value_(Value::CreateNullValue()) {}
 
diff --git a/chrome/browser/pref_store.h b/chrome/browser/pref_store.h
deleted file mode 100644
index d7cf8ad..0000000
--- a/chrome/browser/pref_store.h
+++ /dev/null
@@ -1,46 +0,0 @@
-// 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,
-  };
-
-  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 a7a3188..16d2094 100644
--- a/chrome/browser/privacy_blacklist/blacklist_unittest.cc
+++ b/chrome/browser/privacy_blacklist/blacklist_unittest.cc
@@ -9,7 +9,6 @@
 #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"
@@ -23,7 +22,7 @@ class BlacklistTest : public testing::Test {
     source_path = source_path.AppendASCII("profiles")
         .AppendASCII("blacklist_prefs").AppendASCII("Preferences");
 
-    prefs_.reset(new PrefService(new JsonPrefStore(source_path)));
+    prefs_.reset(new PrefService(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 11d2120..92b2ff8 100644
--- a/chrome/browser/profile.cc
+++ b/chrome/browser/profile.cc
@@ -37,7 +37,6 @@
 #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"
@@ -939,7 +938,7 @@ net::TransportSecurityState*
 
 PrefService* ProfileImpl::GetPrefs() {
   if (!prefs_.get()) {
-    prefs_.reset(new PrefService(new JsonPrefStore(GetPrefFilePath())));
+    prefs_.reset(new PrefService(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 f9e731e..0ab3faf 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(new JsonPrefStore(source_path)));
+      prefs_.reset(new PrefService(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 d309b35..08663f8 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -1532,8 +1532,6 @@
         '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',
@@ -1732,7 +1730,6 @@
         '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 1cebe07..aa90b8c 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -68,8 +68,6 @@
         # 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',
@@ -829,7 +827,6 @@
         '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
deleted file mode 100644
index 43392a9..0000000
--- a/chrome/test/data/pref_service/invalid.json
+++ /dev/null
@@ -1 +0,0 @@
-!@#$%^&
\ 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 eba821a..fb673b9 100644
--- a/chrome/test/reliability/page_load_test.cc
+++ b/chrome/test/reliability/page_load_test.cc
@@ -48,7 +48,6 @@
 #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"
@@ -527,8 +526,7 @@ class PageLoadTest : public UITest {
     FilePath local_state_path = user_data_dir()
         .Append(chrome::kLocalStateFilename);
 
-    PrefService* local_state(new PrefService(
-        new JsonPrefStore(local_state_path)));
+    PrefService* local_state(new PrefService(local_state_path));
     return local_state;
   }
 
diff --git a/chrome/test/testing_profile.h b/chrome/test/testing_profile.h
index 7c293e9..b4e97d5 100644
--- a/chrome/test/testing_profile.h
+++ b/chrome/test/testing_profile.h
@@ -18,7 +18,6 @@
 #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"
@@ -147,7 +146,7 @@ class TestingProfile : public Profile {
     if (!prefs_.get()) {
       FilePath prefs_filename =
           path_.Append(FILE_PATH_LITERAL("TestPreferences"));
-      prefs_.reset(new PrefService(new JsonPrefStore(prefs_filename)));
+      prefs_.reset(new PrefService(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 1a2176b..8f31849 100644
--- a/chrome_frame/test/reliability/page_load_test.cc
+++ b/chrome_frame/test/reliability/page_load_test.cc
@@ -35,7 +35,6 @@
 #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"
@@ -468,7 +467,7 @@ class PageLoadTest : public testing::Test {
     FilePath local_state_path;
     chrome::GetChromeFrameUserDataDirectory(&local_state_path);
 
-    PrefService* local_state = new PrefService(new JsonPrefStore(local_state_path));
+    PrefService* local_state = new PrefService(local_state_path);
     return local_state;
   }