8 files changed, 125 insertions, 4 deletions
diff --git a/chrome/browser/prefs/overlay_user_pref_store.cc b/chrome/browser/prefs/overlay_user_pref_store.cc
index ca4f585..8d04ec3 100644
--- a/chrome/browser/prefs/overlay_user_pref_store.cc
+++ b/chrome/browser/prefs/overlay_user_pref_store.cc
@@ -101,6 +101,11 @@ void OverlayUserPrefStore::RemoveValue(const std::string& key) {
     ReportValueChanged(key);
 }
 
+void OverlayUserPrefStore::MarkNeedsEmptyValue(const std::string& key) {
+  if (!ShallBeStoredInOverlay(key))
+    underlay_->MarkNeedsEmptyValue(key);
+}
+
 bool OverlayUserPrefStore::ReadOnly() const {
   return false;
 }
diff --git a/chrome/browser/prefs/overlay_user_pref_store.h b/chrome/browser/prefs/overlay_user_pref_store.h
index f0a1f4b..105dc67 100644
--- a/chrome/browser/prefs/overlay_user_pref_store.h
+++ b/chrome/browser/prefs/overlay_user_pref_store.h
@@ -45,6 +45,7 @@ class OverlayUserPrefStore : public PersistentPrefStore,
   virtual void SetValueSilently(const std::string& key,
                                 base::Value* value) OVERRIDE;
   virtual void RemoveValue(const std::string& key) OVERRIDE;
+  virtual void MarkNeedsEmptyValue(const std::string& key) OVERRIDE;
   virtual bool ReadOnly() const OVERRIDE;
   virtual PrefReadError GetReadError() const OVERRIDE;
   virtual PrefReadError ReadPrefs() OVERRIDE;
diff --git a/chrome/browser/prefs/pref_service.cc b/chrome/browser/prefs/pref_service.cc
index cf9f22e..de7f406 100644
--- a/chrome/browser/prefs/pref_service.cc
+++ b/chrome/browser/prefs/pref_service.cc
@@ -727,6 +727,23 @@ void PrefService::RegisterPreference(const char* path,
   DCHECK(orig_type != Value::TYPE_NULL && orig_type != Value::TYPE_BINARY) <<
          "invalid preference type: " << orig_type;
 
+  // For ListValue and DictionaryValue with non empty default, empty value
+  // for |path| needs to be persisted in |user_pref_store_|. So that
+  // non empty default is not used when user sets an empty ListValue or
+  // DictionaryValue.
+  bool needs_empty_value = false;
+  if (orig_type == base::Value::TYPE_LIST) {
+    const base::ListValue* list = NULL;
+    if (default_value->GetAsList(&list) && !list->empty())
+      needs_empty_value = true;
+  } else if (orig_type == base::Value::TYPE_DICTIONARY) {
+    const base::DictionaryValue* dict = NULL;
+    if (default_value->GetAsDictionary(&dict) && !dict->empty())
+      needs_empty_value = true;
+  }
+  if (needs_empty_value)
+    user_pref_store_->MarkNeedsEmptyValue(path);
+
   // Hand off ownership.
   default_store_->SetDefaultValue(path, scoped_value.release());
 
diff --git a/chrome/browser/prefs/pref_service_mock_builder.cc b/chrome/browser/prefs/pref_service_mock_builder.cc
index 5b4ba24..edfe696 100644
--- a/chrome/browser/prefs/pref_service_mock_builder.cc
+++ b/chrome/browser/prefs/pref_service_mock_builder.cc
@@ -77,10 +77,16 @@ PrefServiceMockBuilder::WithCommandLine(CommandLine* command_line) {
 
 PrefServiceMockBuilder&
 PrefServiceMockBuilder::WithUserFilePrefs(const FilePath& prefs_file) {
-  user_prefs_ =
-      new JsonPrefStore(prefs_file,
-                        BrowserThread::GetMessageLoopProxyForThread(
-                            BrowserThread::FILE));
+  return WithUserFilePrefs(prefs_file,
+                           BrowserThread::GetMessageLoopProxyForThread(
+                               BrowserThread::FILE));
+}
+
+PrefServiceMockBuilder&
+PrefServiceMockBuilder::WithUserFilePrefs(
+    const FilePath& prefs_file,
+    base::MessageLoopProxy* message_loop_proxy) {
+  user_prefs_ = new JsonPrefStore(prefs_file, message_loop_proxy);
   return *this;
 }
 
diff --git a/chrome/browser/prefs/pref_service_mock_builder.h b/chrome/browser/prefs/pref_service_mock_builder.h
index 5ea5ace..2dd764f 100644
--- a/chrome/browser/prefs/pref_service_mock_builder.h
+++ b/chrome/browser/prefs/pref_service_mock_builder.h
@@ -15,6 +15,10 @@ class CommandLine;
 class FilePath;
 class PrefService;
 
+namespace base {
+class MessageLoopProxy;
+}
+
 namespace policy {
 class PolicyService;
 }
@@ -46,6 +50,9 @@ class PrefServiceMockBuilder {
 
   // Specifies to use an actual file-backed user pref store.
   PrefServiceMockBuilder& WithUserFilePrefs(const FilePath& prefs_file);
+  PrefServiceMockBuilder& WithUserFilePrefs(
+      const FilePath& prefs_file,
+      base::MessageLoopProxy* message_loop_proxy);
 
   // Creates the PrefService, invalidating the entire builder configuration.
   PrefService* Create();
diff --git a/chrome/browser/prefs/pref_service_unittest.cc b/chrome/browser/prefs/pref_service_unittest.cc
index 8a7069c..38d6ea96 100644
--- a/chrome/browser/prefs/pref_service_unittest.cc
+++ b/chrome/browser/prefs/pref_service_unittest.cc
@@ -5,7 +5,10 @@
 #include <string>
 
 #include "base/command_line.h"
+#include "base/file_util.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/path_service.h"
+#include "base/scoped_temp_dir.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/policy/configuration_policy_pref_store.h"
@@ -16,9 +19,11 @@
 #include "chrome/browser/prefs/pref_observer_mock.h"
 #include "chrome/browser/prefs/pref_service_mock_builder.h"
 #include "chrome/browser/prefs/pref_value_store.h"
+#include "chrome/browser/prefs/scoped_user_pref_update.h"
 #include "chrome/browser/prefs/testing_pref_store.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/common/json_pref_store.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "chrome/test/base/testing_pref_service.h"
@@ -182,6 +187,82 @@ TEST(PrefServiceTest, UpdateCommandLinePrefStore) {
   EXPECT_TRUE(actual_bool_value);
 }
 
+class PrefServiceUserFilePrefsTest : public testing::Test {
+ protected:
+  virtual void SetUp() {
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+
+    ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &data_dir_));
+    data_dir_ = data_dir_.AppendASCII("pref_service");
+    ASSERT_TRUE(file_util::PathExists(data_dir_));
+  }
+
+  void ClearListValue(PrefService* prefs, const char* key) {
+    ListPrefUpdate updater(prefs, key);
+    updater->Clear();
+  }
+
+  void ClearDictionaryValue(PrefService* prefs, const char* key) {
+    DictionaryPrefUpdate updater(prefs, key);
+    updater->Clear();
+  }
+
+  // The path to temporary directory used to contain the test operations.
+  ScopedTempDir temp_dir_;
+  // The path to the directory where the test data is stored.
+  FilePath data_dir_;
+  // A message loop that we can use as the file thread message loop.
+  MessageLoop message_loop_;
+};
+
+// Verifies that ListValue and DictionaryValue pref with non emtpy default
+// preserves its empty value.
+TEST_F(PrefServiceUserFilePrefsTest, PreserveEmptyValue) {
+  FilePath pref_file = temp_dir_.path().AppendASCII("write.json");
+
+  ASSERT_TRUE(file_util::CopyFile(
+      data_dir_.AppendASCII("read.need_empty_value.json"),
+      pref_file));
+
+  PrefServiceMockBuilder builder;
+  builder.WithUserFilePrefs(pref_file, base::MessageLoopProxy::current());
+  scoped_ptr<PrefService> prefs(builder.Create());
+
+  // Register testing prefs.
+  prefs->RegisterListPref("list",
+                          PrefService::UNSYNCABLE_PREF);
+  prefs->RegisterDictionaryPref("dict",
+                                PrefService::UNSYNCABLE_PREF);
+
+  base::ListValue* non_empty_list = new base::ListValue;
+  non_empty_list->Append(base::Value::CreateStringValue("test"));
+  prefs->RegisterListPref("list_needs_empty_value",
+                          non_empty_list,
+                          PrefService::UNSYNCABLE_PREF);
+
+  base::DictionaryValue* non_empty_dict = new base::DictionaryValue;
+  non_empty_dict->SetString("dummy", "whatever");
+  prefs->RegisterDictionaryPref("dict_needs_empty_value",
+                                non_empty_dict,
+                                PrefService::UNSYNCABLE_PREF);
+
+  // Set all testing prefs to empty.
+  ClearListValue(prefs.get(), "list");
+  ClearListValue(prefs.get(), "list_needs_empty_value");
+  ClearDictionaryValue(prefs.get(), "dict");
+  ClearDictionaryValue(prefs.get(), "dict_needs_empty_value");
+
+  // Write to file.
+  prefs->CommitPendingWrite();
+  MessageLoop::current()->RunAllPending();
+
+  // Compare to expected output.
+  FilePath golden_output_file =
+      data_dir_.AppendASCII("write.golden.need_empty_value.json");
+  ASSERT_TRUE(file_util::PathExists(golden_output_file));
+  EXPECT_TRUE(file_util::TextContentsEqual(golden_output_file, pref_file));
+}
+
 class PrefServiceSetValueTest : public testing::Test {
  protected:
   static const char kName[];
diff --git a/chrome/browser/prefs/testing_pref_store.cc b/chrome/browser/prefs/testing_pref_store.cc
index 8948598..f7ce0f4 100644
--- a/chrome/browser/prefs/testing_pref_store.cc
+++ b/chrome/browser/prefs/testing_pref_store.cc
@@ -53,6 +53,9 @@ void TestingPrefStore::RemoveValue(const std::string& key) {
     NotifyPrefValueChanged(key);
 }
 
+void TestingPrefStore::MarkNeedsEmptyValue(const std::string& key) {
+}
+
 bool TestingPrefStore::ReadOnly() const {
   return read_only_;
 }
diff --git a/chrome/browser/prefs/testing_pref_store.h b/chrome/browser/prefs/testing_pref_store.h
index 85965da..9cda821 100644
--- a/chrome/browser/prefs/testing_pref_store.h
+++ b/chrome/browser/prefs/testing_pref_store.h
@@ -38,6 +38,7 @@ class TestingPrefStore : public PersistentPrefStore {
   virtual void SetValueSilently(const std::string& key,
                                 base::Value* value) OVERRIDE;
   virtual void RemoveValue(const std::string& key) OVERRIDE;
+  virtual void MarkNeedsEmptyValue(const std::string& key) OVERRIDE;
   virtual bool ReadOnly() const OVERRIDE;
   virtual PrefReadError GetReadError() const OVERRIDE;
   virtual PersistentPrefStore::PrefReadError ReadPrefs() OVERRIDE;