Keep emtpy List/Dictionary pref value with non-empty default.

chrome/browser/prefs/pref_service_unittest.cc

Index: chrome/browser/prefs/pref_service_unittest.cc
diff --git a/chrome/browser/prefs/pref_service_unittest.cc b/chrome/browser/prefs/pref_service_unittest.cc
index 8a7069cedf50bd3a2c362359a339e7051b1aef51..fad22f81e40d13f06e4940ef366b9d4fa117b3bc 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,86 @@ TEST(PrefServiceTest, UpdateCommandLinePrefStore) {
   EXPECT_TRUE(actual_bool_value);
 }
 
+class PrefServiceUserFilePrefsTest : public testing::Test {
+ protected:
+  virtual void SetUp() {
+    message_loop_proxy_ = base::MessageLoopProxy::current();
+
+    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.Get()->Clear();

[battre, 2012/04/11 21:55:51]

nit, optional: you can simplify this to updater->Clear();

[xiyuan, 2012/04/11 22:36:46]

Done.

+  }
+
+  void ClearDictionaryValue(PrefService* prefs, const char* key) {
+    DictionaryPrefUpdate updater(prefs, key);
+    updater.Get()->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_;
+  scoped_refptr<base::MessageLoopProxy> message_loop_proxy_;
+};
+
+// 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"),

[battre, 2012/04/11 21:55:51]

nit: indentation

[xiyuan, 2012/04/11 22:36:46]

Done.

+          pref_file));
+
+  PrefServiceMockBuilder builder;
+  builder.WithUserFilePrefs(pref_file, message_loop_proxy_.get());

[battre, 2012/04/11 21:55:51]

optional: do you want to inline message_loop_proxy_? It is written and read
exactly once.

[xiyuan, 2012/04/11 22:36:46]

Done.

+  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));
+  ASSERT_TRUE(file_util::Delete(pref_file, false));

[battre, 2012/04/11 21:55:51]

nit: necessary? should be deleted by ScopedTempDir

[xiyuan, 2012/04/11 22:36:46]

Removed.

+}
+
 class PrefServiceSetValueTest : public testing::Test {
  protected:
   static const char kName[];