Implement lossy pref behavior for JsonPrefStore.

base/prefs/json_pref_store_unittest.cc

Index: base/prefs/json_pref_store_unittest.cc
diff --git a/base/prefs/json_pref_store_unittest.cc b/base/prefs/json_pref_store_unittest.cc
index 0fab8685e9b21410731955169bad35b5d98ceef2..c1009b3a4473567ee60ee76cbdac042da8b4fc31 100644
--- a/base/prefs/json_pref_store_unittest.cc
+++ b/base/prefs/json_pref_store_unittest.cc
@@ -809,4 +809,145 @@ TEST_F(JsonPrefStoreTest, WriteCountHistogramTestPeriodWithGaps) {
   ASSERT_EQ(6, samples->TotalCount());
 }
 
+class JsonPrefStoreLossyWriteTest : public JsonPrefStoreTest {
+ protected:
+  // A struct used for conveniently setting values in the PrefStore
+  struct PrefDetails {

[Mattias Nissler (ping if slow), 2015/05/08 07:04:25]

This is still here, but I don't see an answer to my previous comment. Can you
comment?

[raymes, 2015/05/11 00:57:27]

I removed this.

+    std::string key;
+    scoped_ptr<base::Value> value;
+    uint32 flags;
+  };
+
+  static const base::FilePath::CharType kTestFile[];
+
+  static PrefDetails GetNormalPrefDetails() {
+    return {"normal",
+            scoped_ptr<base::Value>(new base::StringValue("normal")),
+            WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS};
+  }
+
+  static PrefDetails GetLossyPrefDetails() {
+    return {"lossy",
+            scoped_ptr<base::Value>(new base::StringValue("lossy")),
+            WriteablePrefStore::LOSSY_PREF_WRITE_FLAG};
+  }
+
+  void SetUp() override {
+    JsonPrefStoreTest::SetUp();
+    base::DeleteFile(base::FilePath(kTestFile), false);
+  }
+
+  void TearDown() override {
+    base::DeleteFile(base::FilePath(kTestFile), false);
+    JsonPrefStoreTest::TearDown();
+  }
+
+  // Creates a JsonPrefStore with the given |file_writer|.
+  scoped_refptr<JsonPrefStore> CreatePrefStore() {
+    return new JsonPrefStore(base::FilePath(kTestFile),
+                             message_loop_.task_runner(),
+                             scoped_ptr<PrefFilter>());
+  }
+
+  // Return the ImportantFileWriter for a given JsonPrefStore.
+  ImportantFileWriter* GetImportantFileWriter(
+      scoped_refptr<JsonPrefStore> pref_store) {
+    return &(pref_store->writer_);
+  }
+
+  // Get the contents of kTestFile. Pumps the message loop before returning the
+  // result.
+  std::string GetTestFileContents() {
+    MessageLoop::current()->RunUntilIdle();
+    std::string file_contents;
+    CHECK(ReadFileToString(base::FilePath(kTestFile), &file_contents));
+    return file_contents;
+  }
+};
+
+// static
+const base::FilePath::CharType JsonPrefStoreLossyWriteTest::kTestFile[] =
+    FILE_PATH_LITERAL("/tmp/Local_State");
+
+TEST_F(JsonPrefStoreLossyWriteTest, LossyWriteBasic) {
+  scoped_refptr<JsonPrefStore> pref_store = CreatePrefStore();
+  ImportantFileWriter* file_writer = GetImportantFileWriter(pref_store);
+
+  PrefDetails normal = GetNormalPrefDetails();
+  PrefDetails lossy = GetLossyPrefDetails();
+
+  // Set a normal pref and check that it gets scheduled to be written.
+  ASSERT_FALSE(file_writer->HasPendingWrite());
+  pref_store->SetValue(normal.key, normal.value->DeepCopy(), normal.flags);
+  ASSERT_TRUE(file_writer->HasPendingWrite());
+  file_writer->DoScheduledWrite();
+  ASSERT_EQ("{\"normal\":\"normal\"}", GetTestFileContents());
+  ASSERT_FALSE(file_writer->HasPendingWrite());
+
+  // Set a lossy pref and check that it is not scheduled to be written.
+  // SetValue/RemoveValue

[Mattias Nissler (ping if slow), 2015/05/08 07:04:25]

Missing period?

[raymes, 2015/05/11 00:57:27]

Done.

+  pref_store->SetValue(lossy.key, lossy.value->DeepCopy(), lossy.flags);
+  ASSERT_FALSE(file_writer->HasPendingWrite());
+  pref_store->RemoveValue(lossy.key, lossy.flags);
+  ASSERT_FALSE(file_writer->HasPendingWrite());
+
+  // SetValueSilently/RemoveValueSilently.
+  pref_store->SetValueSilently(lossy.key, lossy.value->DeepCopy(), lossy.flags);
+  ASSERT_FALSE(file_writer->HasPendingWrite());
+  pref_store->RemoveValueSilently(lossy.key, lossy.flags);
+  ASSERT_FALSE(file_writer->HasPendingWrite());
+
+  // ReportValueChanged.
+  pref_store->SetValue(lossy.key, lossy.value->DeepCopy(), lossy.flags);
+  ASSERT_FALSE(file_writer->HasPendingWrite());
+  pref_store->ReportValueChanged(lossy.key, lossy.flags);
+  ASSERT_FALSE(file_writer->HasPendingWrite());
+
+  // Call CommitPendingWrite and check that the lossy pref and the normal pref
+  // are there with the last values set above.
+  pref_store->CommitPendingWrite();
+  ASSERT_FALSE(file_writer->HasPendingWrite());
+  ASSERT_EQ("{\"lossy\":\"lossy\",\"normal\":\"normal\"}",
+            GetTestFileContents());
+}
+
+TEST_F(JsonPrefStoreLossyWriteTest, LossyWriteMixed) {
+  scoped_refptr<JsonPrefStore> pref_store = CreatePrefStore();
+  ImportantFileWriter* file_writer = GetImportantFileWriter(pref_store);
+
+  PrefDetails normal = GetNormalPrefDetails();
+  PrefDetails lossy = GetLossyPrefDetails();
+
+  // Set a normal pref and check that it is scheduled to be written.
+  ASSERT_FALSE(file_writer->HasPendingWrite());
+  pref_store->SetValue(normal.key, normal.value->DeepCopy(), normal.flags);
+  ASSERT_TRUE(file_writer->HasPendingWrite());
+  // Set a lossy pref and check that the write is still scheduled.
+  pref_store->SetValue(lossy.key, lossy.value->DeepCopy(), lossy.flags);
+  ASSERT_TRUE(file_writer->HasPendingWrite());
+  // Call DoScheduledWrite and check both prefs get written.
+  file_writer->DoScheduledWrite();
+  ASSERT_EQ("{\"lossy\":\"lossy\",\"normal\":\"normal\"}",
+            GetTestFileContents());
+  ASSERT_FALSE(file_writer->HasPendingWrite());
+
+  // Cleanup.

[Mattias Nissler (ping if slow), 2015/05/08 07:04:25]

Instead of doing a cleanup and starting fresh, why not put the second test
sequence in a separate TEST_F()?

[raymes, 2015/05/11 00:57:27]

Done.

+  pref_store->RemoveValue(normal.key, normal.flags);
+  pref_store->RemoveValue(lossy.key, lossy.flags);
+  pref_store->CommitPendingWrite();
+  ASSERT_FALSE(file_writer->HasPendingWrite());
+
+  // Set a lossy pref and check that it is not scheduled to be written.
+  pref_store->SetValue(lossy.key, lossy.value->DeepCopy(), lossy.flags);
+  ASSERT_FALSE(file_writer->HasPendingWrite());
+  // Set a normal pref and check that it is scheduled to be written.
+  pref_store->SetValue(normal.key, normal.value->DeepCopy(), normal.flags);
+  ASSERT_TRUE(file_writer->HasPendingWrite());
+  // Call DoScheduledWrite and check both prefs get written.
+  file_writer->DoScheduledWrite();
+  ASSERT_EQ("{\"lossy\":\"lossy\",\"normal\":\"normal\"}",
+            GetTestFileContents());
+  ASSERT_FALSE(file_writer->HasPendingWrite());
+}
+
 }  // namespace base