Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2052)

Unified Diff: base/prefs/json_pref_store_unittest.cc

Issue 1127963002: Implement lossy pref behavior for JsonPrefStore. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@prefs-fix-flags
Patch Set: Created 5 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..4f4bf8f2e69235a9ce68018dc003862bc261d215 100644
--- a/base/prefs/json_pref_store_unittest.cc
+++ b/base/prefs/json_pref_store_unittest.cc
@@ -213,7 +213,7 @@ void RunBasicJsonPrefStoreTest(JsonPrefStore* pref_store,
EXPECT_FALSE(boolean);
EXPECT_TRUE(pref_store->GetValue(kMaxTabs, &actual));
- int integer = 0;
+ int integer;
EXPECT_TRUE(actual->GetAsInteger(&integer));
EXPECT_EQ(20, integer);
pref_store->SetValue(kMaxTabs, new FundamentalValue(10),
@@ -809,4 +809,160 @@ TEST_F(JsonPrefStoreTest, WriteCountHistogramTestPeriodWithGaps) {
ASSERT_EQ(6, samples->TotalCount());
}
+class TestImportantFileWriter : public ImportantFileWriter {
+ public:
+ TestImportantFileWriter() {}
+ ~TestImportantFileWriter() override {}
+
+ // ImportantFileWriter overrides.
+ bool HasPendingWrite() const override { return !!serializer_; }
+
+ void WriteNow(scoped_ptr<std::string> data) override { NOTREACHED(); }
+
+ void ScheduleWrite(DataSerializer* serializer) override {
+ serializer_ = serializer;
+ }
+
+ void DoScheduledWrite() override {
+ CHECK(serializer_ && !has_last_write_);
+ if (!serializer_->SerializeData(&last_write_))
+ NOTREACHED();
+ serializer_ = nullptr;
+ has_last_write_ = true;
+ }
+
+ void RegisterOnNextSuccessfulWriteCallback(
+ const base::Closure& on_next_successful_write) override {
+ NOTREACHED();
+ }
+
+ TimeDelta CommitInterval() const override {
+ return TimeDelta::FromSeconds(5);
+ }
+
+ std::string GetAndClearLastWrite() {
+ std::string result = last_write_;
+ last_write_ = std::string();
+ has_last_write_ = false;
+ return result;
+ }
+
+ private:
+ DataSerializer* serializer_ = nullptr;
+ std::string last_write_;
+ bool has_last_write_ = false;
+};
+
+// This test subclass just contains some helper functions.
+class JsonPrefStoreLossyWriteTest : public JsonPrefStoreTest {
+ protected:
+ // A struct used for conveniently setting values in the PrefStore
+ struct PrefDetails {
Mattias Nissler (ping if slow) 2015/05/07 07:59:53 This is actually not providing any benefit that I
+ std::string key;
+ scoped_ptr<base::Value> value;
+ uint32 flags;
+ };
+
+ 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};
+ }
+
+ // Creates a JsonPrefStore with the given |file_writer|.
+ scoped_refptr<JsonPrefStore> CreatePrefStore(
+ TestImportantFileWriter* file_writer) {
+ base::FilePath file(FILE_PATH_LITERAL("abc.txt"));
+ return new JsonPrefStore(file, file, message_loop_.task_runner(),
+ scoped_ptr<base::ImportantFileWriter>(file_writer),
+ scoped_ptr<PrefFilter>());
+ }
+};
+
+TEST_F(JsonPrefStoreLossyWriteTest, LossyWriteBasic) {
+ TestImportantFileWriter* file_writer = new TestImportantFileWriter;
+ scoped_refptr<JsonPrefStore> pref_store = CreatePrefStore(file_writer);
+ 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\"}", file_writer->GetAndClearLastWrite());
+ ASSERT_FALSE(file_writer->HasPendingWrite());
Mattias Nissler (ping if slow) 2015/05/07 07:59:53 Instead of mocking the file writer, you could just
+
+ // Set a lossy pref and check that it is not scheduled to be written.
+ // SetValue/RemoveValue
+ 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());
Mattias Nissler (ping if slow) 2015/05/07 07:59:53 Same here, could just spin the loop and check the
+
+ // 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\"}",
+ file_writer->GetAndClearLastWrite());
+}
+
+TEST_F(JsonPrefStoreLossyWriteTest, LossyWriteMixed) {
+ TestImportantFileWriter* file_writer = new TestImportantFileWriter;
+ scoped_refptr<JsonPrefStore> pref_store = CreatePrefStore(file_writer);
+ 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\"}",
+ file_writer->GetAndClearLastWrite());
+ ASSERT_FALSE(file_writer->HasPendingWrite());
+
+ // Cleanup.
+ pref_store->RemoveValue(normal.key, normal.flags);
+ pref_store->RemoveValue(lossy.key, lossy.flags);
+ pref_store->CommitPendingWrite();
+ file_writer->GetAndClearLastWrite();
+ 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\"}",
+ file_writer->GetAndClearLastWrite());
+ ASSERT_FALSE(file_writer->HasPendingWrite());
+}
+
} // namespace base
« no previous file with comments | « base/prefs/json_pref_store.cc ('k') | chrome/browser/safe_browsing/incident_reporting/download_metadata_manager.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698