diff --git a/cobalt/persistent_storage/persistent_settings.cc b/cobalt/persistent_storage/persistent_settings.cc index 4fefafbe0b7e..9aaa73f5610e 100644 --- a/cobalt/persistent_storage/persistent_settings.cc +++ b/cobalt/persistent_storage/persistent_settings.cc @@ -76,6 +76,7 @@ void PersistentSettings::InitializePrefStore() { pref_store_ = base::MakeRefCounted( base::FilePath(persistent_settings_file_)); pref_store_->ReadPrefs(); + persistent_settings_ = pref_store_->GetValues(); pref_store_initialized_.Signal(); } // Remove settings file and do not allow writes to the file until the @@ -102,42 +103,47 @@ void PersistentSettings::ValidatePersistentSettingsHelper(bool blocking) { bool PersistentSettings::GetPersistentSettingAsBool(const std::string& key, bool default_setting) { + // Wait for all previously posted tasks to finish. + PostBlockingTask(); base::AutoLock auto_lock(pref_store_lock_); - auto persistent_settings = pref_store_->GetValues(); - absl::optional result = persistent_settings.FindBool(key); + absl::optional result = persistent_settings_.FindBool(key); return result.value_or(default_setting); } int PersistentSettings::GetPersistentSettingAsInt(const std::string& key, int default_setting) { + // Wait for all previously posted tasks to finish. + PostBlockingTask(); base::AutoLock auto_lock(pref_store_lock_); - auto persistent_settings = pref_store_->GetValues(); - absl::optional result = persistent_settings.FindInt(key); + absl::optional result = persistent_settings_.FindInt(key); return result.value_or(default_setting); } double PersistentSettings::GetPersistentSettingAsDouble( const std::string& key, double default_setting) { + // Wait for all previously posted tasks to finish. + PostBlockingTask(); base::AutoLock auto_lock(pref_store_lock_); - auto persistent_settings = pref_store_->GetValues(); - absl::optional result = persistent_settings.FindDouble(key); + absl::optional result = persistent_settings_.FindDouble(key); return result.value_or(default_setting); } std::string PersistentSettings::GetPersistentSettingAsString( const std::string& key, const std::string& default_setting) { + // Wait for all previously posted tasks to finish. + PostBlockingTask(); base::AutoLock auto_lock(pref_store_lock_); - auto persistent_settings = pref_store_->GetValues(); - const std::string* result = persistent_settings.FindString(key); + const std::string* result = persistent_settings_.FindString(key); if (result) return *result; return default_setting; } std::vector PersistentSettings::GetPersistentSettingAsList( const std::string& key) { + // Wait for all previously posted tasks to finish. + PostBlockingTask(); base::AutoLock auto_lock(pref_store_lock_); - auto persistent_settings = pref_store_->GetValues(); - const base::Value::List* result = persistent_settings.FindList(key); + const base::Value::List* result = persistent_settings_.FindList(key); std::vector values; if (result) { for (const auto& value : *result) { @@ -149,9 +155,10 @@ std::vector PersistentSettings::GetPersistentSettingAsList( base::flat_map> PersistentSettings::GetPersistentSettingAsDictionary(const std::string& key) { + // Wait for all previously posted tasks to finish. + PostBlockingTask(); base::AutoLock auto_lock(pref_store_lock_); - auto persistent_settings = pref_store_->GetValues(); - base::Value::Dict* result = persistent_settings.FindDict(key); + base::Value::Dict* result = persistent_settings_.FindDict(key); base::flat_map> dict; if (result) { for (base::Value::Dict::iterator it = result->begin(); it != result->end(); @@ -164,70 +171,74 @@ PersistentSettings::GetPersistentSettingAsDictionary(const std::string& key) { return dict; } +void PersistentSettings::PostBlockingTask() { + base::WaitableEvent task_finished( + base::WaitableEvent::ResetPolicy::MANUAL, + base::WaitableEvent::InitialState::NOT_SIGNALED); + task_runner()->PostTask( + FROM_HERE, base::BindOnce(&PersistentSettings::PostBlockingTaskHelper, + base::Unretained(this), &task_finished)); + task_finished.Wait(); +} + +void PersistentSettings::PostBlockingTaskHelper( + base::WaitableEvent* task_finished) { + DCHECK_EQ(base::SequencedTaskRunner::GetCurrentDefault(), task_runner()); + task_finished->Signal(); +} + void PersistentSettings::SetPersistentSetting( - const std::string& key, std::unique_ptr value, - base::OnceClosure closure, bool blocking) { + const std::string& key, std::unique_ptr value, bool blocking) { task_runner()->PostTask( - FROM_HERE, base::BindOnce(&PersistentSettings::SetPersistentSettingHelper, - base::Unretained(this), key, std::move(value), - std::move(closure), blocking)); + FROM_HERE, + base::BindOnce(&PersistentSettings::SetPersistentSettingHelper, + base::Unretained(this), key, std::move(value), blocking)); } void PersistentSettings::SetPersistentSettingHelper( - const std::string& key, std::unique_ptr value, - base::OnceClosure closure, bool blocking) { + const std::string& key, std::unique_ptr value, bool blocking) { DCHECK_EQ(base::SequencedTaskRunner::GetCurrentDefault(), task_runner()); - { - base::AutoLock auto_lock(pref_store_lock_); - pref_store_->SetValue(key, - base::Value::FromUniquePtrValue(std::move(value)), - WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); - if (validated_initial_settings_) { - CommitPendingWrite(blocking); - } + base::AutoLock auto_lock(pref_store_lock_); + pref_store_->SetValue(key, base::Value::FromUniquePtrValue(std::move(value)), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); + persistent_settings_ = pref_store_->GetValues(); + if (validated_initial_settings_) { + CommitPendingWrite(blocking); } - std::move(closure).Run(); } void PersistentSettings::RemovePersistentSetting(const std::string& key, - base::OnceClosure closure, bool blocking) { task_runner()->PostTask( FROM_HERE, base::BindOnce(&PersistentSettings::RemovePersistentSettingHelper, - base::Unretained(this), key, std::move(closure), - blocking)); + base::Unretained(this), key, blocking)); } -void PersistentSettings::RemovePersistentSettingHelper( - const std::string& key, base::OnceClosure closure, bool blocking) { +void PersistentSettings::RemovePersistentSettingHelper(const std::string& key, + bool blocking) { DCHECK_EQ(base::SequencedTaskRunner::GetCurrentDefault(), task_runner()); - { - base::AutoLock auto_lock(pref_store_lock_); - pref_store_->RemoveValue(key, WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); - if (validated_initial_settings_) { - CommitPendingWrite(blocking); - } + base::AutoLock auto_lock(pref_store_lock_); + pref_store_->RemoveValue(key, WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); + persistent_settings_ = pref_store_->GetValues(); + if (validated_initial_settings_) { + CommitPendingWrite(blocking); } - std::move(closure).Run(); } -void PersistentSettings::DeletePersistentSettings(base::OnceClosure closure) { +void PersistentSettings::DeletePersistentSettings() { task_runner()->PostTask( FROM_HERE, base::BindOnce(&PersistentSettings::DeletePersistentSettingsHelper, - base::Unretained(this), std::move(closure))); + base::Unretained(this))); } -void PersistentSettings::DeletePersistentSettingsHelper( - base::OnceClosure closure) { +void PersistentSettings::DeletePersistentSettingsHelper() { DCHECK_EQ(base::SequencedTaskRunner::GetCurrentDefault(), task_runner()); - { - base::AutoLock auto_lock(pref_store_lock_); - starboard::SbFileDeleteRecursive(persistent_settings_file_.c_str(), true); - pref_store_->ReadPrefs(); - } - std::move(closure).Run(); + base::AutoLock auto_lock(pref_store_lock_); + starboard::SbFileDeleteRecursive(persistent_settings_file_.c_str(), true); + pref_store_->ReadPrefs(); + persistent_settings_ = pref_store_->GetValues(); } void PersistentSettings::CommitPendingWrite(bool blocking) { diff --git a/cobalt/persistent_storage/persistent_settings.h b/cobalt/persistent_storage/persistent_settings.h index 30f3b6be2ffe..6b869d03b751 100644 --- a/cobalt/persistent_storage/persistent_settings.h +++ b/cobalt/persistent_storage/persistent_settings.h @@ -72,18 +72,13 @@ class PersistentSettings : public base::CurrentThread::DestructionObserver { base::flat_map> GetPersistentSettingAsDictionary(const std::string& key); - void SetPersistentSetting( - const std::string& key, std::unique_ptr value, - base::OnceClosure closure = std::move(base::DoNothing()), - bool blocking = false); + void SetPersistentSetting(const std::string& key, + std::unique_ptr value, + bool blocking = false); - void RemovePersistentSetting( - const std::string& key, - base::OnceClosure closure = std::move(base::DoNothing()), - bool blocking = false); + void RemovePersistentSetting(const std::string& key, bool blocking = false); - void DeletePersistentSettings( - base::OnceClosure closure = std::move(base::DoNothing())); + void DeletePersistentSettings(); private: // Called by the constructor to initialize pref_store_ from @@ -92,14 +87,17 @@ class PersistentSettings : public base::CurrentThread::DestructionObserver { void ValidatePersistentSettingsHelper(bool blocking); + void PostBlockingTask(); + + void PostBlockingTaskHelper(base::WaitableEvent* task_finished); + void SetPersistentSettingHelper(const std::string& key, std::unique_ptr value, - base::OnceClosure closure, bool blocking); + bool blocking); - void RemovePersistentSettingHelper(const std::string& key, - base::OnceClosure closure, bool blocking); + void RemovePersistentSettingHelper(const std::string& key, bool blocking); - void DeletePersistentSettingsHelper(base::OnceClosure closure); + void DeletePersistentSettingsHelper(); void CommitPendingWrite(bool blocking); @@ -109,14 +107,17 @@ class PersistentSettings : public base::CurrentThread::DestructionObserver { // PrefStore used for persistent settings. scoped_refptr pref_store_; - // Flag indicating whether or not initial persistent settings have been - // validated. - bool validated_initial_settings_; + // Persistent settings dictionary. + base::Value::Dict persistent_settings_; // The thread created and owned by PersistentSettings. All pref_store_ // methods must be called from this thread. base::Thread thread_; + // Flag indicating whether or not initial persistent settings have been + // validated. + bool validated_initial_settings_; + // This event is used to signal when Initialize has been called and // pref_store_ mutations can now occur. base::WaitableEvent pref_store_initialized_ = { diff --git a/cobalt/persistent_storage/persistent_settings_test.cc b/cobalt/persistent_storage/persistent_settings_test.cc index 9da560444661..d653c9233f7e 100644 --- a/cobalt/persistent_storage/persistent_settings_test.cc +++ b/cobalt/persistent_storage/persistent_settings_test.cc @@ -17,9 +17,6 @@ #include #include -#include "base/bind.h" -#include "base/functional/callback_forward.h" -#include "base/synchronization/waitable_event.h" #include "base/test/task_environment.h" #include "base/values.h" #include "starboard/common/file.h" @@ -45,26 +42,20 @@ class PersistentSettingTest : public testing::Test { std::vector storage_dir(kSbFileMaxPath + 1, 0); SbSystemGetPath(kSbSystemPathCacheDirectory, storage_dir.data(), kSbFileMaxPath); - persistent_settings_file_ = std::string(storage_dir.data()) + kSbFileSepString + kPersistentSettingsJson; } void SetUp() final { - test_done_.Reset(); starboard::SbFileDeleteRecursive(persistent_settings_file_.c_str(), true); } void TearDown() final { - test_done_.Reset(); starboard::SbFileDeleteRecursive(persistent_settings_file_.c_str(), true); } base::test::TaskEnvironment scoped_task_environment_; std::string persistent_settings_file_; - base::WaitableEvent test_done_ = { - base::WaitableEvent::ResetPolicy::MANUAL, - base::WaitableEvent::InitialState::NOT_SIGNALED}; }; TEST_F(PersistentSettingTest, GetDefaultBool) { @@ -77,17 +68,9 @@ TEST_F(PersistentSettingTest, GetDefaultBool) { ASSERT_FALSE(persistent_settings->GetPersistentSettingAsBool("key", false)); // exists but invalid - base::OnceClosure closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { test_done->Signal(); }, - persistent_settings.get(), &test_done_); - persistent_settings->SetPersistentSetting( - "key", std::make_unique(4.2), std::move(closure), true); - test_done_.Wait(); + persistent_settings->SetPersistentSetting("key", + std::make_unique(4.2)); EXPECT_TRUE(persistent_settings->GetPersistentSettingAsBool("key", true)); - EXPECT_EQ(-1, persistent_settings->GetPersistentSettingAsInt("key", -1)); - EXPECT_EQ(4.2, - persistent_settings->GetPersistentSettingAsDouble("key", -1.0)); } TEST_F(PersistentSettingTest, GetSetBool) { @@ -95,37 +78,14 @@ TEST_F(PersistentSettingTest, GetSetBool) { std::make_unique(kPersistentSettingsJson); persistent_settings->ValidatePersistentSettings(); - base::OnceClosure closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - EXPECT_TRUE( - persistent_settings->GetPersistentSettingAsBool("key", true)); - EXPECT_TRUE( - persistent_settings->GetPersistentSettingAsBool("key", false)); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); - persistent_settings->SetPersistentSetting( - "key", std::make_unique(true), std::move(closure), true); - test_done_.Wait(); - test_done_.Reset(); - - closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - EXPECT_FALSE( - persistent_settings->GetPersistentSettingAsBool("key", true)); - EXPECT_FALSE( - persistent_settings->GetPersistentSettingAsBool("key", false)); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); - + "key", std::make_unique(true)); + EXPECT_TRUE(persistent_settings->GetPersistentSettingAsBool("key", true)); + EXPECT_TRUE(persistent_settings->GetPersistentSettingAsBool("key", false)); persistent_settings->SetPersistentSetting( - "key", std::make_unique(false), std::move(closure), true); - - test_done_.Wait(); + "key", std::make_unique(false)); + EXPECT_FALSE(persistent_settings->GetPersistentSettingAsBool("key", true)); + EXPECT_FALSE(persistent_settings->GetPersistentSettingAsBool("key", false)); } TEST_F(PersistentSettingTest, GetDefaultInt) { @@ -139,17 +99,9 @@ TEST_F(PersistentSettingTest, GetDefaultInt) { ASSERT_EQ(42, persistent_settings->GetPersistentSettingAsInt("key", 42)); // exists but invalid - base::OnceClosure closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - EXPECT_EQ(8, persistent_settings->GetPersistentSettingAsInt("key", 8)); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); - - persistent_settings->SetPersistentSetting( - "key", std::make_unique(4.2), std::move(closure), true); - test_done_.Wait(); + persistent_settings->SetPersistentSetting("key", + std::make_unique(4.2)); + EXPECT_EQ(8, persistent_settings->GetPersistentSettingAsInt("key", 8)); } TEST_F(PersistentSettingTest, GetSetInt) { @@ -157,40 +109,51 @@ TEST_F(PersistentSettingTest, GetSetInt) { std::make_unique(kPersistentSettingsJson); persistent_settings->ValidatePersistentSettings(); - base::OnceClosure closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - EXPECT_EQ(-1, persistent_settings->GetPersistentSettingAsInt("key", 8)); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); + persistent_settings->SetPersistentSetting("key", + std::make_unique(-1)); + EXPECT_EQ(-1, persistent_settings->GetPersistentSettingAsInt("key", 8)); + persistent_settings->SetPersistentSetting("key", + std::make_unique(0)); + EXPECT_EQ(0, persistent_settings->GetPersistentSettingAsInt("key", 8)); + persistent_settings->SetPersistentSetting("key", + std::make_unique(42)); + EXPECT_EQ(42, persistent_settings->GetPersistentSettingAsInt("key", 8)); +} + +TEST_F(PersistentSettingTest, GetDefaultDouble) { + auto persistent_settings = + std::make_unique(kPersistentSettingsJson); + persistent_settings->ValidatePersistentSettings(); + + // does not exist + ASSERT_EQ(-1.1, + persistent_settings->GetPersistentSettingAsDouble("key", -1.1)); + ASSERT_EQ(0.1, persistent_settings->GetPersistentSettingAsDouble("key", 0.1)); + ASSERT_EQ(42.1, + persistent_settings->GetPersistentSettingAsDouble("key", 42.1)); + + // exists but invalid persistent_settings->SetPersistentSetting( - "key", std::make_unique(-1), std::move(closure), true); - test_done_.Wait(); - test_done_.Reset(); - - closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - EXPECT_EQ(0, persistent_settings->GetPersistentSettingAsInt("key", 8)); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); + "key", std::make_unique(true)); + EXPECT_EQ(8.1, persistent_settings->GetPersistentSettingAsDouble("key", 8.1)); +} + +TEST_F(PersistentSettingTest, GetSetDouble) { + auto persistent_settings = + std::make_unique(kPersistentSettingsJson); + persistent_settings->ValidatePersistentSettings(); + persistent_settings->SetPersistentSetting( - "key", std::make_unique(0), std::move(closure), true); - test_done_.Wait(); - test_done_.Reset(); - - closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - EXPECT_EQ(42, persistent_settings->GetPersistentSettingAsInt("key", 8)); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); + "key", std::make_unique(-1.1)); + EXPECT_EQ(-1.1, + persistent_settings->GetPersistentSettingAsDouble("key", 8.1)); + persistent_settings->SetPersistentSetting("key", + std::make_unique(0.1)); + EXPECT_EQ(0.1, persistent_settings->GetPersistentSettingAsDouble("key", 8.1)); persistent_settings->SetPersistentSetting( - "key", std::make_unique(42), std::move(closure), true); - test_done_.Wait(); + "key", std::make_unique(42.1)); + EXPECT_EQ(42.1, + persistent_settings->GetPersistentSettingAsDouble("key", 8.1)); } TEST_F(PersistentSettingTest, GetDefaultString) { @@ -210,17 +173,10 @@ TEST_F(PersistentSettingTest, GetDefaultString) { persistent_settings->GetPersistentSettingAsString("key", "\\n")); // exists but invalid - base::OnceClosure closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - EXPECT_EQ("hello", persistent_settings->GetPersistentSettingAsString( - "key", "hello")); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); - persistent_settings->SetPersistentSetting( - "key", std::make_unique(4.2), std::move(closure), true); - test_done_.Wait(); + persistent_settings->SetPersistentSetting("key", + std::make_unique(4.2)); + EXPECT_EQ("hello", + persistent_settings->GetPersistentSettingAsString("key", "hello")); } TEST_F(PersistentSettingTest, GetSetString) { @@ -228,74 +184,26 @@ TEST_F(PersistentSettingTest, GetSetString) { std::make_unique(kPersistentSettingsJson); persistent_settings->ValidatePersistentSettings(); - base::OnceClosure closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - EXPECT_EQ("", persistent_settings->GetPersistentSettingAsString( - "key", "hello")); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); - - persistent_settings->SetPersistentSetting( - "key", std::make_unique(""), std::move(closure), true); - test_done_.Wait(); - test_done_.Reset(); - - closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - EXPECT_EQ( - "hello there", + persistent_settings->SetPersistentSetting("key", + std::make_unique("")); + EXPECT_EQ("", persistent_settings->GetPersistentSettingAsString("key", "hello")); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); - persistent_settings->SetPersistentSetting( - "key", std::make_unique("hello there"), std::move(closure), - true); - test_done_.Wait(); - test_done_.Reset(); - - closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - EXPECT_EQ("42", persistent_settings->GetPersistentSettingAsString( - "key", "hello")); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); + "key", std::make_unique("hello there")); + EXPECT_EQ("hello there", + persistent_settings->GetPersistentSettingAsString("key", "hello")); persistent_settings->SetPersistentSetting( - "key", std::make_unique("42"), std::move(closure), true); - test_done_.Wait(); - test_done_.Reset(); - - closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - EXPECT_EQ("\n", persistent_settings->GetPersistentSettingAsString( - "key", "hello")); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); + "key", std::make_unique("42")); + EXPECT_EQ("42", + persistent_settings->GetPersistentSettingAsString("key", "hello")); persistent_settings->SetPersistentSetting( - "key", std::make_unique("\n"), std::move(closure), true); - test_done_.Wait(); - test_done_.Reset(); - - closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - EXPECT_EQ("\\n", persistent_settings->GetPersistentSettingAsString( - "key", "hello")); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); + "key", std::make_unique("\n")); + EXPECT_EQ("\n", + persistent_settings->GetPersistentSettingAsString("key", "hello")); persistent_settings->SetPersistentSetting( - "key", std::make_unique("\\n"), std::move(closure), true); - test_done_.Wait(); - test_done_.Reset(); + "key", std::make_unique("\\n")); + EXPECT_EQ("\\n", + persistent_settings->GetPersistentSettingAsString("key", "hello")); } TEST_F(PersistentSettingTest, GetSetList) { @@ -303,69 +211,39 @@ TEST_F(PersistentSettingTest, GetSetList) { std::make_unique(kPersistentSettingsJson); persistent_settings->ValidatePersistentSettings(); - base::OnceClosure closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - auto test_list = persistent_settings->GetPersistentSettingAsList("key"); - EXPECT_FALSE(test_list.empty()); - EXPECT_EQ(1, test_list.size()); - EXPECT_TRUE(test_list[0].is_string()); - EXPECT_EQ("hello", test_list[0].GetString()); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); - base::Value list(base::Value::Type::LIST); list.GetList().Append("hello"); persistent_settings->SetPersistentSetting( - "key", base::Value::ToUniquePtrValue(list.Clone()), std::move(closure), - true); - test_done_.Wait(); - test_done_.Reset(); - - closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - auto test_list = persistent_settings->GetPersistentSettingAsList("key"); - EXPECT_FALSE(test_list.empty()); - EXPECT_EQ(2, test_list.size()); - EXPECT_TRUE(test_list[0].is_string()); - EXPECT_EQ("hello", test_list[0].GetString()); - EXPECT_TRUE(test_list[1].is_string()); - EXPECT_EQ("there", test_list[1].GetString()); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); + "key", base::Value::ToUniquePtrValue(list.Clone())); + auto test_list = persistent_settings->GetPersistentSettingAsList("key"); + EXPECT_FALSE(test_list.empty()); + EXPECT_EQ(1, test_list.size()); + EXPECT_TRUE(test_list[0].is_string()); + EXPECT_EQ("hello", test_list[0].GetString()); list.GetList().Append("there"); persistent_settings->SetPersistentSetting( - "key", base::Value::ToUniquePtrValue(list.Clone()), std::move(closure), - true); - test_done_.Wait(); - test_done_.Reset(); - - closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - auto test_list = persistent_settings->GetPersistentSettingAsList("key"); - EXPECT_FALSE(test_list.empty()); - EXPECT_EQ(3, test_list.size()); - EXPECT_TRUE(test_list[0].is_string()); - EXPECT_EQ("hello", test_list[0].GetString()); - EXPECT_TRUE(test_list[1].is_string()); - EXPECT_EQ("there", test_list[1].GetString()); - EXPECT_TRUE(test_list[2].is_int()); - EXPECT_EQ(42, test_list[2].GetInt()); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); + "key", base::Value::ToUniquePtrValue(list.Clone())); + test_list = persistent_settings->GetPersistentSettingAsList("key"); + EXPECT_FALSE(test_list.empty()); + EXPECT_EQ(2, test_list.size()); + EXPECT_TRUE(test_list[0].is_string()); + EXPECT_EQ("hello", test_list[0].GetString()); + EXPECT_TRUE(test_list[1].is_string()); + EXPECT_EQ("there", test_list[1].GetString()); list.GetList().Append(42); persistent_settings->SetPersistentSetting( - "key", base::Value::ToUniquePtrValue(list.Clone()), std::move(closure), - true); - test_done_.Wait(); - test_done_.Reset(); + "key", base::Value::ToUniquePtrValue(list.Clone())); + test_list = persistent_settings->GetPersistentSettingAsList("key"); + EXPECT_FALSE(test_list.empty()); + EXPECT_EQ(3, test_list.size()); + EXPECT_TRUE(test_list[0].is_string()); + EXPECT_EQ("hello", test_list[0].GetString()); + EXPECT_TRUE(test_list[1].is_string()); + EXPECT_EQ("there", test_list[1].GetString()); + EXPECT_TRUE(test_list[2].is_int()); + EXPECT_EQ(42, test_list[2].GetInt()); } TEST_F(PersistentSettingTest, GetSetDictionary) { @@ -373,95 +251,40 @@ TEST_F(PersistentSettingTest, GetSetDictionary) { std::make_unique(kPersistentSettingsJson); persistent_settings->ValidatePersistentSettings(); - base::OnceClosure closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - auto test_dict = - persistent_settings->GetPersistentSettingAsDictionary("key"); - EXPECT_FALSE(test_dict.empty()); - EXPECT_EQ(1, test_dict.size()); - EXPECT_TRUE(test_dict["key_string"]->is_string()); - EXPECT_EQ("hello", test_dict["key_string"]->GetString()); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); - base::Value dict(base::Value::Type::DICT); dict.GetDict().Set("key_string", "hello"); persistent_settings->SetPersistentSetting( - "key", base::Value::ToUniquePtrValue(dict.Clone()), std::move(closure), - true); - test_done_.Wait(); - test_done_.Reset(); - - closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - auto test_dict = - persistent_settings->GetPersistentSettingAsDictionary("key"); - EXPECT_FALSE(test_dict.empty()); - EXPECT_EQ(2, test_dict.size()); - EXPECT_TRUE(test_dict["key_string"]->is_string()); - EXPECT_EQ("hello", test_dict["key_string"]->GetString()); - EXPECT_TRUE(test_dict["key_int"]->is_int()); - EXPECT_EQ(42, test_dict["key_int"]->GetInt()); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); + "key", base::Value::ToUniquePtrValue(dict.Clone())); + auto test_dict = persistent_settings->GetPersistentSettingAsDictionary("key"); + EXPECT_FALSE(test_dict.empty()); + EXPECT_EQ(1, test_dict.size()); + EXPECT_TRUE(test_dict["key_string"]->is_string()); + EXPECT_EQ("hello", test_dict["key_string"]->GetString()); dict.GetDict().Set("key_int", 42); persistent_settings->SetPersistentSetting( - "key", base::Value::ToUniquePtrValue(dict.Clone()), std::move(closure), - true); - test_done_.Wait(); - test_done_.Reset(); -} - -TEST_F(PersistentSettingTest, URLAsKey) { - // Tests that json_pref_store has the correct SetValue and - // RemoveValue changes for using a URL as a PersistentSettings - // Key. - auto persistent_settings = - std::make_unique(kPersistentSettingsJson); - persistent_settings->ValidatePersistentSettings(); + "key", base::Value::ToUniquePtrValue(dict.Clone())); + test_dict = persistent_settings->GetPersistentSettingAsDictionary("key"); + EXPECT_FALSE(test_dict.empty()); + EXPECT_EQ(2, test_dict.size()); + EXPECT_TRUE(test_dict["key_string"]->is_string()); + EXPECT_EQ("hello", test_dict["key_string"]->GetString()); + EXPECT_TRUE(test_dict["key_int"]->is_int()); + EXPECT_EQ(42, test_dict["key_int"]->GetInt()); - base::OnceClosure closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - auto test_dict = persistent_settings->GetPersistentSettingAsDictionary( - "http://127.0.0.1:45019/"); - EXPECT_FALSE(test_dict.empty()); - EXPECT_EQ(1, test_dict.size()); - EXPECT_TRUE(test_dict["http://127.0.0.1:45019/"]->is_string()); - EXPECT_EQ("Dictionary URL Key Works!", - test_dict["http://127.0.0.1:45019/"]->GetString()); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); - - // Test that json_pref_store uses SetKey instead of Set, making URL - // keys viable. - base::Value dict(base::Value::Type::DICT); dict.GetDict().Set("http://127.0.0.1:45019/", "Dictionary URL Key Works!"); persistent_settings->SetPersistentSetting( - "http://127.0.0.1:45019/", base::Value::ToUniquePtrValue(dict.Clone()), - std::move(closure), true); - test_done_.Wait(); - test_done_.Reset(); - - closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - auto test_dict = persistent_settings->GetPersistentSettingAsDictionary( - "http://127.0.0.1:45019/"); - EXPECT_TRUE(test_dict.empty()); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); - persistent_settings->RemovePersistentSetting("http://127.0.0.1:45019/", - std::move(closure), true); - test_done_.Wait(); - test_done_.Reset(); + "key", base::Value::ToUniquePtrValue(dict.Clone())); + test_dict = persistent_settings->GetPersistentSettingAsDictionary("key"); + EXPECT_FALSE(test_dict.empty()); + EXPECT_EQ(3, test_dict.size()); + EXPECT_TRUE(test_dict["key_string"]->is_string()); + EXPECT_EQ("hello", test_dict["key_string"]->GetString()); + EXPECT_TRUE(test_dict["key_int"]->is_int()); + EXPECT_EQ(42, test_dict["key_int"]->GetInt()); + EXPECT_TRUE(test_dict["http://127.0.0.1:45019/"]->is_string()); + EXPECT_EQ("Dictionary URL Key Works!", + test_dict["http://127.0.0.1:45019/"]->GetString()); } TEST_F(PersistentSettingTest, RemoveSetting) { @@ -471,35 +294,13 @@ TEST_F(PersistentSettingTest, RemoveSetting) { ASSERT_TRUE(persistent_settings->GetPersistentSettingAsBool("key", true)); ASSERT_FALSE(persistent_settings->GetPersistentSettingAsBool("key", false)); - - base::OnceClosure closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - EXPECT_TRUE( - persistent_settings->GetPersistentSettingAsBool("key", true)); - EXPECT_TRUE( - persistent_settings->GetPersistentSettingAsBool("key", false)); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); persistent_settings->SetPersistentSetting( - "key", std::make_unique(true), std::move(closure), true); - test_done_.Wait(); - test_done_.Reset(); - - closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - EXPECT_TRUE( - persistent_settings->GetPersistentSettingAsBool("key", true)); - EXPECT_FALSE( - persistent_settings->GetPersistentSettingAsBool("key", false)); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); - persistent_settings->RemovePersistentSetting("key", std::move(closure), true); - test_done_.Wait(); - test_done_.Reset(); + "key", std::make_unique(true)); + EXPECT_TRUE(persistent_settings->GetPersistentSettingAsBool("key", true)); + EXPECT_TRUE(persistent_settings->GetPersistentSettingAsBool("key", false)); + persistent_settings->RemovePersistentSetting("key"); + EXPECT_TRUE(persistent_settings->GetPersistentSettingAsBool("key", true)); + EXPECT_FALSE(persistent_settings->GetPersistentSettingAsBool("key", false)); } TEST_F(PersistentSettingTest, DeleteSettings) { @@ -509,35 +310,13 @@ TEST_F(PersistentSettingTest, DeleteSettings) { ASSERT_TRUE(persistent_settings->GetPersistentSettingAsBool("key", true)); ASSERT_FALSE(persistent_settings->GetPersistentSettingAsBool("key", false)); - - base::OnceClosure closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - EXPECT_TRUE( - persistent_settings->GetPersistentSettingAsBool("key", true)); - EXPECT_TRUE( - persistent_settings->GetPersistentSettingAsBool("key", false)); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); persistent_settings->SetPersistentSetting( - "key", std::make_unique(true), std::move(closure), true); - test_done_.Wait(); - test_done_.Reset(); - - closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - EXPECT_TRUE( - persistent_settings->GetPersistentSettingAsBool("key", true)); - EXPECT_FALSE( - persistent_settings->GetPersistentSettingAsBool("key", false)); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); - persistent_settings->DeletePersistentSettings(std::move(closure)); - test_done_.Wait(); - test_done_.Reset(); + "key", std::make_unique(true)); + EXPECT_TRUE(persistent_settings->GetPersistentSettingAsBool("key", true)); + EXPECT_TRUE(persistent_settings->GetPersistentSettingAsBool("key", false)); + persistent_settings->DeletePersistentSettings(); + EXPECT_TRUE(persistent_settings->GetPersistentSettingAsBool("key", true)); + EXPECT_FALSE(persistent_settings->GetPersistentSettingAsBool("key", false)); } TEST_F(PersistentSettingTest, InvalidSettings) { @@ -547,105 +326,29 @@ TEST_F(PersistentSettingTest, InvalidSettings) { ASSERT_TRUE(persistent_settings->GetPersistentSettingAsBool("key", true)); ASSERT_FALSE(persistent_settings->GetPersistentSettingAsBool("key", false)); - - base::OnceClosure closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - EXPECT_TRUE( - persistent_settings->GetPersistentSettingAsBool("key", true)); - EXPECT_TRUE( - persistent_settings->GetPersistentSettingAsBool("key", false)); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); persistent_settings->SetPersistentSetting( - "key", std::make_unique(true), std::move(closure), true); - test_done_.Wait(); - test_done_.Reset(); + "key", std::make_unique(true), true); + EXPECT_TRUE(persistent_settings->GetPersistentSettingAsBool("key", true)); + EXPECT_TRUE(persistent_settings->GetPersistentSettingAsBool("key", false)); persistent_settings = std::make_unique(kPersistentSettingsJson); ASSERT_TRUE(persistent_settings->GetPersistentSettingAsBool("key", true)); ASSERT_TRUE(persistent_settings->GetPersistentSettingAsBool("key", false)); - - closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - EXPECT_FALSE( - persistent_settings->GetPersistentSettingAsBool("key", true)); - EXPECT_FALSE( - persistent_settings->GetPersistentSettingAsBool("key", false)); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); persistent_settings->SetPersistentSetting( - "key", std::make_unique(false), std::move(closure), true); - test_done_.Wait(); - test_done_.Reset(); + "key", std::make_unique(false), true); + EXPECT_FALSE(persistent_settings->GetPersistentSettingAsBool("key", true)); + EXPECT_FALSE(persistent_settings->GetPersistentSettingAsBool("key", false)); persistent_settings = std::make_unique(kPersistentSettingsJson); persistent_settings->ValidatePersistentSettings(); ASSERT_TRUE(persistent_settings->GetPersistentSettingAsBool("key", true)); ASSERT_FALSE(persistent_settings->GetPersistentSettingAsBool("key", false)); - closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { - EXPECT_TRUE( - persistent_settings->GetPersistentSettingAsBool("key", true)); - EXPECT_TRUE( - persistent_settings->GetPersistentSettingAsBool("key", false)); - test_done->Signal(); - }, - persistent_settings.get(), &test_done_); persistent_settings->SetPersistentSetting( - "key", std::make_unique(true), std::move(closure), true); - test_done_.Wait(); - test_done_.Reset(); -} - -TEST_F(PersistentSettingTest, WriteToFileOnlyWhenValidated) { - { - auto persistent_settings = - std::make_unique(kPersistentSettingsJson); - auto closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { test_done->Signal(); }, - persistent_settings.get(), &test_done_); - // Write to memory, but not file. - persistent_settings->SetPersistentSetting( - "key", std::make_unique(true), std::move(closure), true); - test_done_.Wait(); - test_done_.Reset(); - EXPECT_TRUE(persistent_settings->GetPersistentSettingAsBool("key", false)); - } - { - auto persistent_settings = - std::make_unique(kPersistentSettingsJson); - EXPECT_TRUE(persistent_settings->GetPersistentSettingAsBool("key", true)); - EXPECT_FALSE(persistent_settings->GetPersistentSettingAsBool("key", false)); - } - - { - auto persistent_settings = - std::make_unique(kPersistentSettingsJson); - auto closure = base::BindOnce( - [](PersistentSettings* persistent_settings, - base::WaitableEvent* test_done) { test_done->Signal(); }, - persistent_settings.get(), &test_done_); - // Write to memory, but not file. - persistent_settings->SetPersistentSetting( - "key", std::make_unique(true), std::move(closure), true); - test_done_.Wait(); - test_done_.Reset(); - EXPECT_TRUE(persistent_settings->GetPersistentSettingAsBool("key", false)); - persistent_settings->ValidatePersistentSettings(/*blocking=*/true); - } - { - auto persistent_settings = - std::make_unique(kPersistentSettingsJson); - EXPECT_TRUE(persistent_settings->GetPersistentSettingAsBool("key", false)); - } + "key", std::make_unique(true)); + EXPECT_TRUE(persistent_settings->GetPersistentSettingAsBool("key", true)); + EXPECT_TRUE(persistent_settings->GetPersistentSettingAsBool("key", false)); } } // namespace persistent_storage diff --git a/cobalt/watchdog/watchdog_test.cc b/cobalt/watchdog/watchdog_test.cc index 95e3fa2d4567..c5e93d5afb14 100644 --- a/cobalt/watchdog/watchdog_test.cc +++ b/cobalt/watchdog/watchdog_test.cc @@ -753,12 +753,11 @@ TEST_F(WatchdogTest, WatchdogMethodsAreNoopWhenWatchdogIsDisabled) { std::make_unique(kSettingsFileName); persistent_settings->ValidatePersistentSettings(); - base::OnceClosure closure = base::BindOnce( - [](base::WaitableEvent* task_done) { task_done->Signal(); }, &task_done_); persistent_settings->SetPersistentSetting( kPersistentSettingWatchdogEnable, std::make_unique(false), - std::move(closure), true); - task_done_.Wait(); + true); + ASSERT_FALSE(persistent_settings->GetPersistentSettingAsBool( + kPersistentSettingWatchdogEnable, true)); watchdog_ = new watchdog::Watchdog(); watchdog_->InitializeCustom(persistent_settings.get(), @@ -789,12 +788,11 @@ TEST_F(WatchdogTest, LogtraceMethodsAreNoopWhenLogtraceIsDisabled) { std::make_unique(kSettingsFileName); persistent_settings->ValidatePersistentSettings(); - base::OnceClosure closure = base::BindOnce( - [](base::WaitableEvent* task_done) { task_done->Signal(); }, &task_done_); persistent_settings->SetPersistentSetting( kPersistentSettingLogtraceEnable, std::make_unique(false), - std::move(closure), true); - task_done_.Wait(); + true); + ASSERT_FALSE(persistent_settings->GetPersistentSettingAsBool( + kPersistentSettingLogtraceEnable, true)); watchdog_ = new watchdog::Watchdog(); watchdog_->InitializeCustom(persistent_settings.get(), diff --git a/cobalt/worker/service_worker_persistent_settings.cc b/cobalt/worker/service_worker_persistent_settings.cc index cb803be6a091..0a2112a3a65a 100644 --- a/cobalt/worker/service_worker_persistent_settings.cc +++ b/cobalt/worker/service_worker_persistent_settings.cc @@ -439,8 +439,8 @@ void ServiceWorkerPersistentSettings::RemoveAll() { } } -void ServiceWorkerPersistentSettings::DeleteAll(base::OnceClosure closure) { - persistent_settings_->DeletePersistentSettings(std::move(closure)); +void ServiceWorkerPersistentSettings::DeleteAll() { + persistent_settings_->DeletePersistentSettings(); } } // namespace worker diff --git a/cobalt/worker/service_worker_persistent_settings.h b/cobalt/worker/service_worker_persistent_settings.h index 459d45b296b7..90b150934a9b 100644 --- a/cobalt/worker/service_worker_persistent_settings.h +++ b/cobalt/worker/service_worker_persistent_settings.h @@ -85,7 +85,7 @@ class ServiceWorkerPersistentSettings { void RemoveAll(); - void DeleteAll(base::OnceClosure closure); + void DeleteAll(); private: Options options_; diff --git a/cobalt/worker/service_worker_registration_map.cc b/cobalt/worker/service_worker_registration_map.cc index 27ea623c6ef9..e632377dea2b 100644 --- a/cobalt/worker/service_worker_registration_map.cc +++ b/cobalt/worker/service_worker_registration_map.cc @@ -70,12 +70,8 @@ void ServiceWorkerRegistrationMap::ReadPersistentSettings() { } void ServiceWorkerRegistrationMap::DeletePersistentSettings() { - base::OnceClosure closure = base::BindOnce( - [](std::map>* - registration_map) { (*registration_map).clear(); }, - ®istration_map_); - service_worker_persistent_settings_->DeleteAll(std::move(closure)); + service_worker_persistent_settings_->DeleteAll(); + registration_map_.clear(); } scoped_refptr