From 4ccf9ed3ce54674419d134392490363e4ee7ec92 Mon Sep 17 00:00:00 2001 From: esmael Date: Mon, 6 May 2024 10:48:45 -0700 Subject: [PATCH] Simplify PersistentSettings. Building off the work of @briantting: https://github.com/youtube/cobalt/pull/3095 b/305057554 --- cobalt/browser/application.cc | 11 +- cobalt/browser/browser_module.cc | 2 +- cobalt/h5vcc/h5vcc_metrics.cc | 10 +- cobalt/h5vcc/h5vcc_settings.cc | 11 +- cobalt/h5vcc/h5vcc_storage.cc | 8 +- cobalt/network/network_module.cc | 5 +- cobalt/network/url_request_context.cc | 21 +- .../persistent_storage/persistent_settings.cc | 229 +++---- .../persistent_storage/persistent_settings.h | 92 +-- .../persistent_settings_test.cc | 622 ++---------------- cobalt/watchdog/watchdog.cc | 30 +- cobalt/watchdog/watchdog_test.cc | 68 +- .../service_worker_persistent_settings.cc | 190 +++--- .../service_worker_persistent_settings.h | 5 +- .../worker/service_worker_registration_map.cc | 8 +- 15 files changed, 331 insertions(+), 981 deletions(-) diff --git a/cobalt/browser/application.cc b/cobalt/browser/application.cc index b94411d99085..9f34df3de47f 100644 --- a/cobalt/browser/application.cc +++ b/cobalt/browser/application.cc @@ -1513,10 +1513,13 @@ void Application::InitMetrics() { metrics::CobaltMetricsServicesManager::GetInstance(); // Before initializing metrics manager, set any persisted settings like if // it's enabled or upload interval. - bool is_metrics_enabled = persistent_settings_->GetPersistentSettingAsBool( - metrics::kMetricEnabledSettingName, false); - auto metric_event_interval = persistent_settings_->GetPersistentSettingAsInt( - metrics::kMetricEventIntervalSettingName, 300); + bool is_metrics_enabled = persistent_settings_->dict() + .FindBool(metrics::kMetricEnabledSettingName) + .value_or(false); + auto metric_event_interval = + persistent_settings_->dict() + .FindInt(metrics::kMetricEventIntervalSettingName) + .value_or(300); metrics_services_manager_->SetEventDispatcher(&event_dispatcher_); metrics_services_manager_->SetUploadInterval(metric_event_interval); metrics_services_manager_->ToggleMetricsEnabled(is_metrics_enabled); diff --git a/cobalt/browser/browser_module.cc b/cobalt/browser/browser_module.cc index 5e5c7da3cdfc..64b0d53c74e7 100644 --- a/cobalt/browser/browser_module.cc +++ b/cobalt/browser/browser_module.cc @@ -793,7 +793,7 @@ void BrowserModule::OnLoad() { web_module_loaded_.Signal(); - options_.persistent_settings->ValidatePersistentSettings(); + options_.persistent_settings->Validate(); ValidateCacheBackendSettings(); } diff --git a/cobalt/h5vcc/h5vcc_metrics.cc b/cobalt/h5vcc/h5vcc_metrics.cc index 0b22b176983c..04846ca5cb12 100644 --- a/cobalt/h5vcc/h5vcc_metrics.cc +++ b/cobalt/h5vcc/h5vcc_metrics.cc @@ -119,9 +119,8 @@ script::HandlePromiseVoid H5vccMetrics::Disable( void H5vccMetrics::ToggleMetricsEnabled(bool is_enabled, base::OnceClosure done_callback) { - persistent_settings_->SetPersistentSetting( - browser::metrics::kMetricEnabledSettingName, - std::make_unique(is_enabled)); + persistent_settings_->Set(browser::metrics::kMetricEnabledSettingName, + base::Value(is_enabled)); browser::metrics::CobaltMetricsServicesManager::GetInstance() ->ToggleMetricsEnabled(is_enabled, std::move(done_callback)); } @@ -132,9 +131,8 @@ bool H5vccMetrics::IsEnabled() { } void H5vccMetrics::SetMetricEventInterval(uint32_t interval_seconds) { - persistent_settings_->SetPersistentSetting( - browser::metrics::kMetricEventIntervalSettingName, - std::make_unique(static_cast(interval_seconds))); + persistent_settings_->Set(browser::metrics::kMetricEventIntervalSettingName, + base::Value(static_cast(interval_seconds))); browser::metrics::CobaltMetricsServicesManager::GetInstance() ->SetUploadInterval(interval_seconds); } diff --git a/cobalt/h5vcc/h5vcc_settings.cc b/cobalt/h5vcc/h5vcc_settings.cc index 59d76c863024..b12d925d280b 100644 --- a/cobalt/h5vcc/h5vcc_settings.cc +++ b/cobalt/h5vcc/h5vcc_settings.cc @@ -84,9 +84,8 @@ bool H5vccSettings::Set(const std::string& name, SetValueType value) const { if (!persistent_settings_) { return false; } else { - persistent_settings_->SetPersistentSetting( - network::kQuicEnabledPersistentSettingsKey, - std::make_unique(value.AsType() != 0)); + persistent_settings_->Set(network::kQuicEnabledPersistentSettingsKey, + base::Value(value.AsType() != 0)); // Tell NetworkModule (if exists) to re-query persistent settings. if (network_module_) { network_module_->SetEnableQuicFromPersistentSettings(); @@ -107,16 +106,14 @@ bool H5vccSettings::Set(const std::string& name, SetValueType value) const { void H5vccSettings::SetPersistentSettingAsInt(const std::string& key, int value) const { if (persistent_settings_) { - persistent_settings_->SetPersistentSetting( - key, std::make_unique(value)); + persistent_settings_->Set(key, base::Value(value)); } } int H5vccSettings::GetPersistentSettingAsInt(const std::string& key, int default_setting) const { if (persistent_settings_) { - return persistent_settings_->GetPersistentSettingAsInt(key, - default_setting); + return persistent_settings_->dict().FindInt(key).value_or(default_setting); } return default_setting; } diff --git a/cobalt/h5vcc/h5vcc_storage.cc b/cobalt/h5vcc/h5vcc_storage.cc index 9b88ae9dcc2f..fdd660e0ff67 100644 --- a/cobalt/h5vcc/h5vcc_storage.cc +++ b/cobalt/h5vcc/h5vcc_storage.cc @@ -372,9 +372,9 @@ H5vccStorageResourceTypeQuotaBytesDictionary H5vccStorage::GetQuota() { } void H5vccStorage::EnableCache() { - persistent_settings_->SetPersistentSetting( + persistent_settings_->Set( network::disk_cache::kCacheEnabledPersistentSettingsKey, - std::make_unique(true)); + base::Value(true)); network::disk_cache::settings::SetCacheEnabled(true); @@ -384,9 +384,9 @@ void H5vccStorage::EnableCache() { } void H5vccStorage::DisableCache() { - persistent_settings_->SetPersistentSetting( + persistent_settings_->Set( network::disk_cache::kCacheEnabledPersistentSettingsKey, - std::make_unique(false)); + base::Value(false)); network::disk_cache::settings::SetCacheEnabled(false); diff --git a/cobalt/network/network_module.cc b/cobalt/network/network_module.cc index de6b98cc9849..d2e1c886e4eb 100644 --- a/cobalt/network/network_module.cc +++ b/cobalt/network/network_module.cc @@ -109,8 +109,9 @@ void NetworkModule::SetProxy(const std::string& custom_proxy_rules) { void NetworkModule::SetEnableQuicFromPersistentSettings() { // Called on initialization and when the persistent setting is changed. if (options_.persistent_settings != nullptr) { - bool enable_quic = options_.persistent_settings->GetPersistentSettingAsBool( - kQuicEnabledPersistentSettingsKey, false); + bool enable_quic = options_.persistent_settings->dict() + .FindBool(kQuicEnabledPersistentSettingsKey) + .value_or(false); task_runner()->PostTask( FROM_HERE, base::Bind(&URLRequestContext::SetEnableQuic, diff --git a/cobalt/network/url_request_context.cc b/cobalt/network/url_request_context.cc index ed047d13e58c..92c954abdc83 100644 --- a/cobalt/network/url_request_context.cc +++ b/cobalt/network/url_request_context.cc @@ -72,8 +72,8 @@ void LoadDiskCacheQuotaSettings( std::string directory = disk_cache::defaults::GetSubdirectory(resource_type); uint32_t bucket_size = - static_cast(settings->GetPersistentSettingAsDouble( - directory, disk_cache::defaults::GetQuota(resource_type))); + static_cast(settings->dict().FindDouble(directory).value_or( + disk_cache::defaults::GetQuota(resource_type))); quotas[resource_type] = bucket_size; total_size += bucket_size; } @@ -93,9 +93,7 @@ void LoadDiskCacheQuotaSettings( disk_cache::settings::SetQuota(resource_type, default_quota); std::string directory = disk_cache::defaults::GetSubdirectory(resource_type); - settings->SetPersistentSetting( - directory, - std::make_unique(static_cast(default_quota))); + settings->Set(directory, base::Value(static_cast(default_quota))); } } @@ -290,8 +288,10 @@ URLRequestContext::URLRequestContext( http_cache->set_can_disable_by_mime_type(true); if (persistent_settings != nullptr) { auto cache_enabled = - persistent_settings->GetPersistentSettingAsBool( - disk_cache::kCacheEnabledPersistentSettingsKey, true); + persistent_settings->dict() + .FindBool( + disk_cache::kCacheEnabledPersistentSettingsKey) + .value_or(true); disk_cache::settings::SetCacheEnabled(cache_enabled); if (!cache_enabled) { http_cache->set_mode(net::HttpCache::Mode::DISABLE); @@ -344,13 +344,12 @@ void URLRequestContext::OnQuicToggle(const std::string& message) { void URLRequestContext::UpdateCacheSizeSetting(disk_cache::ResourceType type, uint32_t bytes) { CHECK(cache_persistent_settings_); - cache_persistent_settings_->SetPersistentSetting( - disk_cache::defaults::GetSubdirectory(type), - std::make_unique(static_cast(bytes))); + cache_persistent_settings_->Set(disk_cache::defaults::GetSubdirectory(type), + base::Value(static_cast(bytes))); } void URLRequestContext::ValidateCachePersistentSettings() { - cache_persistent_settings_->ValidatePersistentSettings(); + cache_persistent_settings_->Validate(); } void URLRequestContext::AssociateKeyWithResourceType( diff --git a/cobalt/persistent_storage/persistent_settings.cc b/cobalt/persistent_storage/persistent_settings.cc index 4fefafbe0b7e..465f5fe2c533 100644 --- a/cobalt/persistent_storage/persistent_settings.cc +++ b/cobalt/persistent_storage/persistent_settings.cc @@ -31,11 +31,14 @@ namespace persistent_storage { void PersistentSettings::WillDestroyCurrentMessageLoop() { // Clear all member variables allocated from the thread. + base::AutoLock auto_lock(pref_store_lock_); pref_store_.reset(); } PersistentSettings::PersistentSettings(const std::string& file_name) - : thread_("PersistentSettings"), validated_initial_settings_(false) { + : thread_("PersistentSettings"), + validated_initial_settings_(false), + pending_write_count_(0) { if (!thread_.Start()) return; DCHECK(task_runner()); @@ -46,11 +49,10 @@ PersistentSettings::PersistentSettings(const std::string& file_name) std::string(storage_dir.data()) + kSbFileSepString + file_name; LOG(INFO) << "Persistent settings file path: " << persistent_settings_file_; - task_runner()->PostTask(FROM_HERE, - base::Bind(&PersistentSettings::InitializePrefStore, - base::Unretained(this))); - pref_store_initialized_.Wait(); - destruction_observer_added_.Wait(); + Run(FROM_HERE, + base::Bind(&PersistentSettings::InitializePrefStore, + base::Unretained(this)), + /*blocking=*/true); } PersistentSettings::~PersistentSettings() { @@ -58,7 +60,7 @@ PersistentSettings::~PersistentSettings() { DCHECK(thread_.IsRunning()); // Wait for all previously posted tasks to finish. - base::task_runner_util::WaitForFence(thread_.task_runner(), FROM_HERE); + Fence(FROM_HERE); thread_.Stop(); } @@ -76,169 +78,116 @@ void PersistentSettings::InitializePrefStore() { pref_store_ = base::MakeRefCounted( base::FilePath(persistent_settings_file_)); pref_store_->ReadPrefs(); - pref_store_initialized_.Signal(); + persistent_settings_ = pref_store_->GetValues(); } // Remove settings file and do not allow writes to the file until the // |ValidatePersistentSettings()| is called. starboard::SbFileDeleteRecursive(persistent_settings_file_.c_str(), true); - destruction_observer_added_.Signal(); -} - -void PersistentSettings::ValidatePersistentSettings(bool blocking) { - task_runner()->PostTask( - FROM_HERE, - base::BindOnce(&PersistentSettings::ValidatePersistentSettingsHelper, - base::Unretained(this), blocking)); } -void PersistentSettings::ValidatePersistentSettingsHelper(bool blocking) { - DCHECK_EQ(base::SequencedTaskRunner::GetCurrentDefault(), task_runner()); - if (!validated_initial_settings_) { - base::AutoLock auto_lock(pref_store_lock_); - validated_initial_settings_ = true; - CommitPendingWrite(blocking); +void PersistentSettings::Validate() { + if (validated_initial_settings_) { + return; + } + if (base::SequencedTaskRunner::GetCurrentDefault() != task_runner()) { + Run(FROM_HERE, + base::BindOnce(&PersistentSettings::Validate, base::Unretained(this))); + return; } -} - -bool PersistentSettings::GetPersistentSettingAsBool(const std::string& key, - bool default_setting) { - base::AutoLock auto_lock(pref_store_lock_); - auto persistent_settings = pref_store_->GetValues(); - absl::optional result = persistent_settings.FindBool(key); - return result.value_or(default_setting); -} - -int PersistentSettings::GetPersistentSettingAsInt(const std::string& key, - int default_setting) { - base::AutoLock auto_lock(pref_store_lock_); - auto persistent_settings = pref_store_->GetValues(); - absl::optional result = persistent_settings.FindInt(key); - return result.value_or(default_setting); -} - -double PersistentSettings::GetPersistentSettingAsDouble( - const std::string& key, double default_setting) { - base::AutoLock auto_lock(pref_store_lock_); - auto persistent_settings = pref_store_->GetValues(); - 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) { base::AutoLock auto_lock(pref_store_lock_); - auto persistent_settings = pref_store_->GetValues(); - const std::string* result = persistent_settings.FindString(key); - if (result) return *result; - return default_setting; + validated_initial_settings_ = true; + CommitPendingWrite(); } -std::vector PersistentSettings::GetPersistentSettingAsList( - const std::string& key) { +void PersistentSettings::Set(const std::string& key, const base::Value& value) { + if (!pref_store_) { + return; + } + if (base::SequencedTaskRunner::GetCurrentDefault() != task_runner()) { + Run(FROM_HERE, base::BindOnce(&PersistentSettings::Set, + base::Unretained(this), key, value.Clone())); + return; + } base::AutoLock auto_lock(pref_store_lock_); - auto persistent_settings = pref_store_->GetValues(); - const base::Value::List* result = persistent_settings.FindList(key); - std::vector values; - if (result) { - for (const auto& value : *result) { - values.emplace_back(value.Clone()); - } + pref_store_->SetValue(key, value.Clone(), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); + persistent_settings_ = pref_store_->GetValues(); + if (validated_initial_settings_) { + CommitPendingWrite(); } - return values; } -base::flat_map> -PersistentSettings::GetPersistentSettingAsDictionary(const std::string& key) { +void PersistentSettings::Remove(const std::string& key) { + if (!pref_store_) { + return; + } + if (base::SequencedTaskRunner::GetCurrentDefault() != task_runner()) { + Run(FROM_HERE, base::BindOnce(&PersistentSettings::Remove, + base::Unretained(this), key)); + return; + } base::AutoLock auto_lock(pref_store_lock_); - auto persistent_settings = pref_store_->GetValues(); - 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(); - ++it) { - dict.insert(std::make_pair( - it->first, base::Value::ToUniquePtrValue(std::move(it->second)))); - } - return dict; + pref_store_->RemoveValue(key, WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); + persistent_settings_ = pref_store_->GetValues(); + if (validated_initial_settings_) { + CommitPendingWrite(); } - return dict; -} - -void PersistentSettings::SetPersistentSetting( - const std::string& key, std::unique_ptr value, - base::OnceClosure closure, bool blocking) { - task_runner()->PostTask( - FROM_HERE, base::BindOnce(&PersistentSettings::SetPersistentSettingHelper, - base::Unretained(this), key, std::move(value), - std::move(closure), blocking)); } -void PersistentSettings::SetPersistentSettingHelper( - const std::string& key, std::unique_ptr value, - base::OnceClosure closure, 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); - } +void PersistentSettings::RemoveAll() { + if (!pref_store_) { + return; } - 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)); + if (base::SequencedTaskRunner::GetCurrentDefault() != task_runner()) { + Run(FROM_HERE, + base::BindOnce(&PersistentSettings::RemoveAll, base::Unretained(this))); + return; + } + 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::RemovePersistentSettingHelper( - const std::string& key, base::OnceClosure closure, 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); - } +void PersistentSettings::CommitPendingWrite() { + base::AutoLock auto_lock(file_write_lock_); + if (pending_write_count_ == 0) { + file_write_done_.Reset(); } - std::move(closure).Run(); + pending_write_count_++; + pref_store_->CommitPendingWrite( + base::OnceClosure(), + base::BindOnce(&PersistentSettings::OnFileWriteDone, AsWeakPtr())); } -void PersistentSettings::DeletePersistentSettings(base::OnceClosure closure) { - task_runner()->PostTask( - FROM_HERE, - base::BindOnce(&PersistentSettings::DeletePersistentSettingsHelper, - base::Unretained(this), std::move(closure))); +void PersistentSettings::OnFileWriteDone() { + base::AutoLock auto_lock(file_write_lock_); + pending_write_count_--; + if (pending_write_count_ == 0) { + file_write_done_.Signal(); + } } -void PersistentSettings::DeletePersistentSettingsHelper( - base::OnceClosure closure) { - 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(); +void PersistentSettings::Fence(const base::Location& location) { + Run(location, base::OnceClosure(), /*blocking=*/true); } -void PersistentSettings::CommitPendingWrite(bool blocking) { +void PersistentSettings::Run(const base::Location& location, + base::OnceClosure closure, bool blocking) { if (blocking) { - base::WaitableEvent written; - pref_store_->CommitPendingWrite( - base::OnceClosure(), - base::BindOnce(&base::WaitableEvent::Signal, Unretained(&written))); - written.Wait(); + base::WaitableEvent done; + task_runner()->PostTask( + location, base::BindOnce( + [](base::OnceClosure closure, base::WaitableEvent* done) { + if (closure) { + std::move(closure).Run(); + } + done->Signal(); + }, + std::move(closure), &done)); + done.Wait(); } else { - pref_store_->CommitPendingWrite(); + task_runner()->PostTask(location, std::move(closure)); } } diff --git a/cobalt/persistent_storage/persistent_settings.h b/cobalt/persistent_storage/persistent_settings.h index 30f3b6be2ffe..c2628272ab88 100644 --- a/cobalt/persistent_storage/persistent_settings.h +++ b/cobalt/persistent_storage/persistent_settings.h @@ -32,6 +32,11 @@ #include "components/prefs/persistent_pref_store.h" namespace cobalt { + +namespace watchdog { +class WatchdogTest; +} + namespace persistent_storage { // PersistentSettings manages Cobalt settings that persist from one application @@ -40,68 +45,45 @@ namespace persistent_storage { // PersistentSettings uses JsonPrefStore for most of its functionality. // JsonPrefStore maintains thread safety by requiring that all access occurs on // the Sequence that created it. -class PersistentSettings : public base::CurrentThread::DestructionObserver { +class PersistentSettings : public base::CurrentThread::DestructionObserver, + public base::SupportsWeakPtr { public: explicit PersistentSettings(const std::string& file_name); ~PersistentSettings(); - // The task runner this object is running on. - base::SequencedTaskRunner* task_runner() const { - return thread_.task_runner(); - } - - // From base::CurrentThread::DestructionObserver. + // base::CurrentThread::DestructionObserver void WillDestroyCurrentMessageLoop() override; // Validates persistent settings by restoring the file on successful start up. - void ValidatePersistentSettings(bool blocking = false); + void Validate(); - // Getters and Setters for persistent settings. - bool GetPersistentSettingAsBool(const std::string& key, bool default_setting); + void Set(const std::string& key, const base::Value& value); - int GetPersistentSettingAsInt(const std::string& key, int default_setting); + void Remove(const std::string& key); - double GetPersistentSettingAsDouble(const std::string& key, - double default_setting); + void RemoveAll(); - std::string GetPersistentSettingAsString(const std::string& key, - const std::string& default_setting); - - std::vector GetPersistentSettingAsList(const std::string& key); - - 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 RemovePersistentSetting( - const std::string& key, - base::OnceClosure closure = std::move(base::DoNothing()), - bool blocking = false); - - void DeletePersistentSettings( - base::OnceClosure closure = std::move(base::DoNothing())); + const base::Value::Dict& dict() const { return persistent_settings_; } private: - // Called by the constructor to initialize pref_store_ from - // the dedicated thread_ as a JSONPrefStore. + friend class PersistentSettingTest; + friend class cobalt::watchdog::WatchdogTest; + void InitializePrefStore(); - void ValidatePersistentSettingsHelper(bool blocking); + void CommitPendingWrite(); - void SetPersistentSettingHelper(const std::string& key, - std::unique_ptr value, - base::OnceClosure closure, bool blocking); + void Run(const base::Location& location, base::OnceClosure closure, + bool blocking = false); - void RemovePersistentSettingHelper(const std::string& key, - base::OnceClosure closure, bool blocking); + void Fence(const base::Location& location); - void DeletePersistentSettingsHelper(base::OnceClosure closure); + // The task runner this object is running on. + base::SequencedTaskRunner* task_runner() const { + return thread_.task_runner(); + } - void CommitPendingWrite(bool blocking); + void OnFileWriteDone(); // Persistent settings file path. std::string persistent_settings_file_; @@ -109,28 +91,22 @@ 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_; - // This event is used to signal when Initialize has been called and - // pref_store_ mutations can now occur. - base::WaitableEvent pref_store_initialized_ = { - base::WaitableEvent::ResetPolicy::MANUAL, - base::WaitableEvent::InitialState::NOT_SIGNALED}; - - // This event is used to signal when the destruction observers have been - // added to the task runner. This is then used in Stop() to ensure that - // processing doesn't continue until the thread is cleanly shutdown. - base::WaitableEvent destruction_observer_added_ = { - base::WaitableEvent::ResetPolicy::MANUAL, - base::WaitableEvent::InitialState::NOT_SIGNALED}; + // Flag indicating whether or not initial persistent settings have been + // validated. + bool validated_initial_settings_; base::Lock pref_store_lock_; + + base::WaitableEvent file_write_done_; + base::Lock file_write_lock_; + int pending_write_count_; }; } // namespace persistent_storage diff --git a/cobalt/persistent_storage/persistent_settings_test.cc b/cobalt/persistent_storage/persistent_settings_test.cc index 9da560444661..8ddcf1ca6aca 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,607 +42,78 @@ 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); + persistent_settings_ = + std::make_unique(kPersistentSettingsJson); } void TearDown() final { - test_done_.Reset(); starboard::SbFileDeleteRecursive(persistent_settings_file_.c_str(), true); } + void Fence(const base::Location& location) { + persistent_settings_->Fence(location); + } + + void WaitForFileWrite() { persistent_settings_->file_write_done_.Wait(); } + base::test::TaskEnvironment scoped_task_environment_; std::string persistent_settings_file_; - base::WaitableEvent test_done_ = { - base::WaitableEvent::ResetPolicy::MANUAL, - base::WaitableEvent::InitialState::NOT_SIGNALED}; + std::unique_ptr persistent_settings_; }; -TEST_F(PersistentSettingTest, GetDefaultBool) { - auto persistent_settings = - std::make_unique(kPersistentSettingsJson); - persistent_settings->ValidatePersistentSettings(); - - // does not exist - ASSERT_TRUE(persistent_settings->GetPersistentSettingAsBool("key", true)); - 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(); - 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) { - auto persistent_settings = - 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_); - - persistent_settings->SetPersistentSetting( - "key", std::make_unique(false), std::move(closure), true); - - test_done_.Wait(); +TEST_F(PersistentSettingTest, InitiallyEmpty) { + EXPECT_TRUE(persistent_settings_->dict().empty()); } -TEST_F(PersistentSettingTest, GetDefaultInt) { - auto persistent_settings = - std::make_unique(kPersistentSettingsJson); - persistent_settings->ValidatePersistentSettings(); - - // does not exist - ASSERT_EQ(-1, persistent_settings->GetPersistentSettingAsInt("key", -1)); - ASSERT_EQ(0, persistent_settings->GetPersistentSettingAsInt("key", 0)); - 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(); +TEST_F(PersistentSettingTest, Set) { + EXPECT_FALSE(persistent_settings_->dict().contains("key")); + persistent_settings_->Set("key", base::Value(4.2)); + Fence(FROM_HERE); + EXPECT_EQ(4.2, persistent_settings_->dict().FindDouble("key").value_or(0)); } -TEST_F(PersistentSettingTest, GetSetInt) { - auto persistent_settings = - 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), 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_); - 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_); - persistent_settings->SetPersistentSetting( - "key", std::make_unique(42), std::move(closure), true); - test_done_.Wait(); +TEST_F(PersistentSettingTest, Remove) { + persistent_settings_->Set("key", base::Value(true)); + Fence(FROM_HERE); + EXPECT_EQ(true, persistent_settings_->dict().FindBool("key").value_or(false)); + persistent_settings_->Remove("key"); + Fence(FROM_HERE); + EXPECT_FALSE(persistent_settings_->dict().contains("key")); } -TEST_F(PersistentSettingTest, GetDefaultString) { - auto persistent_settings = - std::make_unique(kPersistentSettingsJson); - persistent_settings->ValidatePersistentSettings(); - - // does not exist - ASSERT_EQ("", persistent_settings->GetPersistentSettingAsString("key", "")); - ASSERT_EQ("hello there", persistent_settings->GetPersistentSettingAsString( - "key", "hello there")); - ASSERT_EQ("42", - persistent_settings->GetPersistentSettingAsString("key", "42")); - ASSERT_EQ("\n", - persistent_settings->GetPersistentSettingAsString("key", "\n")); - ASSERT_EQ("\\n", - 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(); +TEST_F(PersistentSettingTest, RemoveAll) { + persistent_settings_->Set("key", base::Value(true)); + Fence(FROM_HERE); + EXPECT_EQ(true, persistent_settings_->dict().FindBool("key").value_or(false)); + persistent_settings_->RemoveAll(); + Fence(FROM_HERE); + EXPECT_FALSE(persistent_settings_->dict().contains("key")); } -TEST_F(PersistentSettingTest, GetSetString) { - auto persistent_settings = - 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->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_); - persistent_settings->SetPersistentSetting( - "key", std::make_unique("42"), std::move(closure), true); - test_done_.Wait(); - test_done_.Reset(); +TEST_F(PersistentSettingTest, PersistValidatedSettings) { + EXPECT_TRUE(persistent_settings_->dict().empty()); + persistent_settings_->Set("key", base::Value(true)); + Fence(FROM_HERE); + EXPECT_EQ(true, persistent_settings_->dict().FindBool("key").value_or(false)); - 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_); - 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_); - persistent_settings->SetPersistentSetting( - "key", std::make_unique("\\n"), std::move(closure), true); - test_done_.Wait(); - test_done_.Reset(); -} - -TEST_F(PersistentSettingTest, GetSetList) { - auto persistent_settings = + persistent_settings_ = 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_); - - 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_); - - list.GetList().Append(42); - persistent_settings->SetPersistentSetting( - "key", base::Value::ToUniquePtrValue(list.Clone()), std::move(closure), - true); - test_done_.Wait(); - test_done_.Reset(); -} - -TEST_F(PersistentSettingTest, GetSetDictionary) { - auto persistent_settings = + persistent_settings_->Validate(); + EXPECT_TRUE(persistent_settings_->dict().empty()); + persistent_settings_->Set("key", base::Value(true)); + Fence(FROM_HERE); + EXPECT_EQ(true, persistent_settings_->dict().FindBool("key").value_or(false)); + WaitForFileWrite(); + + persistent_settings_ = 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_); - - 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(); - - 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(); -} - -TEST_F(PersistentSettingTest, RemoveSetting) { - auto persistent_settings = - std::make_unique(kPersistentSettingsJson); - persistent_settings->ValidatePersistentSettings(); - - 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(); -} - -TEST_F(PersistentSettingTest, DeleteSettings) { - auto persistent_settings = - std::make_unique(kPersistentSettingsJson); - persistent_settings->ValidatePersistentSettings(); - - 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(); -} - -TEST_F(PersistentSettingTest, InvalidSettings) { - auto persistent_settings = - std::make_unique(kPersistentSettingsJson); - persistent_settings->ValidatePersistentSettings(); - - 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(); - - 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(); - - 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)); - } + EXPECT_EQ(true, persistent_settings_->dict().FindBool("key").value_or(false)); } } // namespace persistent_storage diff --git a/cobalt/watchdog/watchdog.cc b/cobalt/watchdog/watchdog.cc index 0d5daf8296dd..e724a292f13b 100644 --- a/cobalt/watchdog/watchdog.cc +++ b/cobalt/watchdog/watchdog.cc @@ -759,8 +759,9 @@ bool Watchdog::GetPersistentSettingWatchdogEnable() { if (!persistent_settings_) return kDefaultSettingWatchdogEnable; // Gets the boolean that controls whether or not Watchdog is enabled. - return persistent_settings_->GetPersistentSettingAsBool( - kPersistentSettingWatchdogEnable, kDefaultSettingWatchdogEnable); + return persistent_settings_->dict() + .FindBool(kPersistentSettingWatchdogEnable) + .value_or(kDefaultSettingWatchdogEnable); } void Watchdog::SetPersistentSettingWatchdogEnable(bool enable_watchdog) { @@ -768,9 +769,8 @@ void Watchdog::SetPersistentSettingWatchdogEnable(bool enable_watchdog) { if (!persistent_settings_) return; // Sets the boolean that controls whether or not Watchdog is enabled. - persistent_settings_->SetPersistentSetting( - kPersistentSettingWatchdogEnable, - std::make_unique(enable_watchdog)); + persistent_settings_->Set(kPersistentSettingWatchdogEnable, + base::Value(enable_watchdog)); } bool Watchdog::GetPersistentSettingWatchdogCrash() { @@ -778,8 +778,9 @@ bool Watchdog::GetPersistentSettingWatchdogCrash() { if (!persistent_settings_) return kDefaultSettingWatchdogCrash; // Gets the boolean that controls whether or not crashes can be triggered. - return persistent_settings_->GetPersistentSettingAsBool( - kPersistentSettingWatchdogCrash, kDefaultSettingWatchdogCrash); + return persistent_settings_->dict() + .FindBool(kPersistentSettingWatchdogCrash) + .value_or(kDefaultSettingWatchdogCrash); } void Watchdog::SetPersistentSettingWatchdogCrash(bool can_trigger_crash) { @@ -787,9 +788,8 @@ void Watchdog::SetPersistentSettingWatchdogCrash(bool can_trigger_crash) { if (!persistent_settings_) return; // Sets the boolean that controls whether or not crashes can be triggered. - persistent_settings_->SetPersistentSetting( - kPersistentSettingWatchdogCrash, - std::make_unique(can_trigger_crash)); + persistent_settings_->Set(kPersistentSettingWatchdogCrash, + base::Value(can_trigger_crash)); } bool Watchdog::LogEvent(const std::string& event) { @@ -820,17 +820,17 @@ bool Watchdog::GetPersistentSettingLogtraceEnable() { if (!persistent_settings_) return kDefaultSettingLogtraceEnable; // Gets the boolean that controls whether or not LogTrace is enabled. - return persistent_settings_->GetPersistentSettingAsBool( - kPersistentSettingLogtraceEnable, kDefaultSettingLogtraceEnable); + return persistent_settings_->dict() + .FindBool(kPersistentSettingLogtraceEnable) + .value_or(kDefaultSettingLogtraceEnable); } void Watchdog::SetPersistentSettingLogtraceEnable(bool enable_logtrace) { if (!persistent_settings_) return; // Sets the boolean that controls whether or not LogTrace is enabled. - persistent_settings_->SetPersistentSetting( - kPersistentSettingLogtraceEnable, - std::make_unique(enable_logtrace)); + persistent_settings_->Set(kPersistentSettingLogtraceEnable, + base::Value(enable_logtrace)); } #if defined(_DEBUG) diff --git a/cobalt/watchdog/watchdog_test.cc b/cobalt/watchdog/watchdog_test.cc index 95e3fa2d4567..f7ec9934ba5d 100644 --- a/cobalt/watchdog/watchdog_test.cc +++ b/cobalt/watchdog/watchdog_test.cc @@ -99,6 +99,11 @@ class WatchdogTest : public testing::Test { starboard::SbFileDeleteRecursive(path.c_str(), true); } + void Fence(PersistentSettings* persistent_settings, + const base::Location& location) { + persistent_settings->Fence(location); + } + const std::string kSettingsFileName = "test-settings.json"; watchdog::Watchdog* watchdog_; @@ -258,61 +263,61 @@ TEST_F(WatchdogTest, ViolationsJsonShouldPersistAndBeValid) { absl::optional violations_map_optional = base::JSONReader::Read(json); ASSERT_TRUE(violations_map_optional.has_value()); - std::unique_ptr violations_map = - std::make_unique( - std::move(*violations_map_optional.value().GetIfDict())); + const base::Value::Dict* violations_map = + violations_map_optional->GetIfDict(); ASSERT_NE(violations_map, nullptr); - base::Value::Dict* violation_dict = violations_map->FindDict("test-name"); + const base::Value::Dict* violation_dict = + violations_map->FindDict("test-name"); ASSERT_NE(violation_dict, nullptr); - std::string* description = violation_dict->FindString("description"); + const std::string* description = violation_dict->FindString("description"); ASSERT_NE(description, nullptr); ASSERT_EQ(*description, "test-desc"); - base::Value::List* violations = violation_dict->FindList("violations"); + const base::Value::List* violations = violation_dict->FindList("violations"); ASSERT_NE(violations, nullptr); ASSERT_EQ(violations->size(), 1); - std::string* monitor_state = + const std::string* monitor_state = (*violations)[0].GetDict().FindString("monitorState"); ASSERT_NE(monitor_state, nullptr); ASSERT_EQ( *monitor_state, std::string(GetApplicationStateString(base::kApplicationStateStarted))); - base::Value::List* ping_infos = + const base::Value::List* ping_infos = (*violations)[0].GetDict().FindList("pingInfos"); ASSERT_NE(ping_infos, nullptr); ASSERT_EQ(ping_infos->size(), 1); - std::string* info = (*ping_infos)[0].GetDict().FindString("info"); + const std::string* info = (*ping_infos)[0].GetDict().FindString("info"); ASSERT_NE(info, nullptr); ASSERT_EQ(*info, "test-ping"); - std::string* timestamp_milliseconds = + const std::string* timestamp_milliseconds = (*ping_infos)[0].GetDict().FindString("timestampMilliseconds"); ASSERT_NE(timestamp_milliseconds, nullptr); std::stoll(*timestamp_milliseconds); - base::Value::List* registered_clients = + const base::Value::List* registered_clients = (*violations)[0].GetDict().FindList("registeredClients"); ASSERT_NE(registered_clients, nullptr); ASSERT_EQ(registered_clients->size(), 1); ASSERT_EQ((*registered_clients)[0].GetString(), "test-name"); - std::string* time_interval_milliseconds = + const std::string* time_interval_milliseconds = (*violations)[0].GetDict().FindString("timeIntervalMilliseconds"); ASSERT_NE(time_interval_milliseconds, nullptr); std::stoll(*time_interval_milliseconds); - std::string* time_wait_milliseconds = + const std::string* time_wait_milliseconds = (*violations)[0].GetDict().FindString("timeWaitMilliseconds"); ASSERT_NE(time_wait_milliseconds, nullptr); std::stoll(*time_wait_milliseconds); - std::string* timestamp_last_pinged_milliseconds = + const std::string* timestamp_last_pinged_milliseconds = (*violations)[0].GetDict().FindString("timestampLastPingedMilliseconds"); ASSERT_NE(timestamp_last_pinged_milliseconds, nullptr); std::stoll(*timestamp_last_pinged_milliseconds); - std::string* timestamp_registered_milliseconds = + const std::string* timestamp_registered_milliseconds = (*violations)[0].GetDict().FindString("timestampRegisteredMilliseconds"); ASSERT_NE(timestamp_registered_milliseconds, nullptr); std::stoll(*timestamp_registered_milliseconds); - std::string* timestamp_violation_milliseconds = + const std::string* timestamp_violation_milliseconds = (*violations)[0].GetDict().FindString("timestampViolationMilliseconds"); ASSERT_NE(timestamp_violation_milliseconds, nullptr); std::stoll(*timestamp_violation_milliseconds); - std::string* violation_duration_milliseconds = + const std::string* violation_duration_milliseconds = (*violations)[0].GetDict().FindString("violationDurationMilliseconds"); ASSERT_NE(violation_duration_milliseconds, nullptr); std::stoll(*violation_duration_milliseconds); @@ -751,14 +756,12 @@ TEST_F(WatchdogTest, WatchdogMethodsAreNoopWhenWatchdogIsDisabled) { // PersistentSettings doesn't have interface so it's not mockable auto persistent_settings = 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(); + persistent_settings->Set(kPersistentSettingWatchdogEnable, + base::Value(false)); + Fence(persistent_settings.get(), FROM_HERE); + ASSERT_FALSE(persistent_settings->dict() + .FindBool(kPersistentSettingWatchdogEnable) + .value_or(true)); watchdog_ = new watchdog::Watchdog(); watchdog_->InitializeCustom(persistent_settings.get(), @@ -787,14 +790,13 @@ TEST_F(WatchdogTest, LogtraceMethodsAreNoopWhenLogtraceIsDisabled) { // PersistentSettings doesn't have interface so it's not mockable auto persistent_settings = 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(); + + persistent_settings->Set(kPersistentSettingLogtraceEnable, + base::Value(false)); + Fence(persistent_settings.get(), FROM_HERE); + ASSERT_FALSE(persistent_settings->dict() + .FindBool(kPersistentSettingLogtraceEnable) + .value_or(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..8bbbd4e00f97 100644 --- a/cobalt/worker/service_worker_persistent_settings.cc +++ b/cobalt/worker/service_worker_persistent_settings.cc @@ -66,22 +66,6 @@ const char kSettingsSetOfUsedScriptsKey[] = "set_of_used_scripts"; const char kSettingsSkipWaitingKey[] = "skip_waiting"; const char kSettingsClassicScriptsImportedKey[] = "classic_scripts_imported"; const char kSettingsRawHeadersKey[] = "raw_headers"; - -bool CheckPersistentValue( - std::string key_string, std::string settings_key, - base::flat_map>& dict, - base::Value::Type type) { - if (!dict.contains(settings_key)) { - DLOG(INFO) << "Key: " << key_string << " does not contain " << settings_key; - return false; - } else if (!(dict[settings_key]->type() == type)) { - DLOG(INFO) << "Key: " << key_string << " " << settings_key - << " is of type: " << dict[settings_key]->type() - << ", but expected type is: " << type; - return false; - } - return true; -} } // namespace ServiceWorkerPersistentSettings::ServiceWorkerPersistentSettings( @@ -89,7 +73,7 @@ ServiceWorkerPersistentSettings::ServiceWorkerPersistentSettings( : options_(options) { persistent_settings_.reset(new cobalt::persistent_storage::PersistentSettings( WorkerConsts::kSettingsJson)); - persistent_settings_->ValidatePersistentSettings(); + persistent_settings_->Validate(); DCHECK(persistent_settings_); } @@ -97,69 +81,61 @@ void ServiceWorkerPersistentSettings::ReadServiceWorkerRegistrationMapSettings( std::map>& registration_map) { - std::vector key_list = - persistent_settings_->GetPersistentSettingAsList(kSettingsKeyList); + const base::Value::List* key_list = + persistent_settings_->dict().FindList(kSettingsKeyList); + if (!key_list || key_list->empty()) { + return; + } std::set unverified_key_set; - for (auto& key : key_list) { + for (auto& key : *key_list) { if (key.is_string()) { unverified_key_set.insert(key.GetString()); } } for (auto& key_string : unverified_key_set) { - auto dict = - persistent_settings_->GetPersistentSettingAsDictionary(key_string); - if (dict.empty()) { + const base::Value::Dict* dict = + persistent_settings_->dict().FindDict(key_string); + if (!dict || dict->empty()) { DLOG(INFO) << "Key: " << key_string << " does not exist in " << WorkerConsts::kSettingsJson; continue; } - if (!CheckPersistentValue(key_string, kSettingsStorageKeyKey, dict, - base::Value::Type::STRING)) - continue; - url::Origin storage_key = - url::Origin::Create(GURL(dict[kSettingsStorageKeyKey]->GetString())); + auto storage_key = dict->FindString(kSettingsStorageKeyKey); + if (!storage_key) continue; + url::Origin storage_key_origin = url::Origin::Create(GURL(*storage_key)); - if (!CheckPersistentValue(key_string, kSettingsScopeUrlKey, dict, - base::Value::Type::STRING)) - continue; - GURL scope(dict[kSettingsScopeUrlKey]->GetString()); + auto scope_url = dict->FindString(kSettingsScopeUrlKey); + if (!scope_url) continue; + GURL scope(*scope_url); - if (!CheckPersistentValue(key_string, kSettingsUpdateViaCacheModeKey, dict, - base::Value::Type::INTEGER)) - continue; + auto update_via_cache_mode = dict->FindInt(kSettingsUpdateViaCacheModeKey); + if (!update_via_cache_mode) continue; ServiceWorkerUpdateViaCache update_via_cache = - static_cast( - dict[kSettingsUpdateViaCacheModeKey]->GetInt()); + static_cast(*update_via_cache_mode); - if (!CheckPersistentValue(key_string, kSettingsScopeStringKey, dict, - base::Value::Type::STRING)) - continue; - std::string scope_string(dict[kSettingsScopeStringKey]->GetString()); + auto scope_string = dict->FindString(kSettingsScopeStringKey); + if (!scope_string) continue; - RegistrationMapKey key(storage_key, scope_string); + RegistrationMapKey key(storage_key_origin, *scope_string); scoped_refptr registration( - new ServiceWorkerRegistrationObject(storage_key, scope, + new ServiceWorkerRegistrationObject(storage_key_origin, scope, update_via_cache)); - auto worker_key = kSettingsWaitingWorkerKey; - if (!CheckPersistentValue(key_string, worker_key, dict, - base::Value::Type::DICT)) { - worker_key = kSettingsActiveWorkerKey; - if (!CheckPersistentValue(key_string, worker_key, dict, - base::Value::Type::DICT)) - continue; + const base::Value::Dict* worker = dict->FindDict(kSettingsWaitingWorkerKey); + if (!worker) { + worker = dict->FindDict(kSettingsActiveWorkerKey); + if (!worker) continue; } - if (!ReadServiceWorkerObjectSettings( - registration, key_string, std::move(dict[worker_key]), worker_key)) + if (!ReadServiceWorkerObjectSettings(registration, key_string, *worker)) continue; - if (CheckPersistentValue(key_string, kSettingsLastUpdateCheckTimeKey, dict, - base::Value::Type::STRING)) { - int64_t last_update_check_time = - std::atol(dict[kSettingsLastUpdateCheckTimeKey]->GetString().c_str()); + auto last_update_check_time = + dict->FindString(kSettingsLastUpdateCheckTimeKey); + if (last_update_check_time) { registration->set_last_update_check_time( base::Time::FromDeltaSinceWindowsEpoch( - base::TimeDelta::FromMicroseconds(last_update_check_time))); + base::TimeDelta::FromMicroseconds( + std::atol(last_update_check_time->c_str())))); } key_set_.insert(key_string); @@ -173,7 +149,7 @@ void ServiceWorkerPersistentSettings::ReadServiceWorkerRegistrationMapSettings( registration)); auto job = options_.service_worker_context->jobs()->CreateJobWithoutPromise( - ServiceWorkerJobs::JobType::kUpdate, storage_key, scope, + ServiceWorkerJobs::JobType::kUpdate, storage_key_origin, scope, registration->waiting_worker()->script_url()); options_.service_worker_context->task_runner()->PostTask( FROM_HERE, base::BindOnce(&ServiceWorkerJobs::ScheduleJob, @@ -185,11 +161,10 @@ void ServiceWorkerPersistentSettings::ReadServiceWorkerRegistrationMapSettings( bool ServiceWorkerPersistentSettings::ReadServiceWorkerObjectSettings( scoped_refptr registration, - std::string key_string, std::unique_ptr value_dict, - std::string worker_key_string) { - std::string* options_name_value = - value_dict->FindStringKey(kSettingsOptionsNameKey); - if (options_name_value == nullptr) return false; + std::string key_string, const base::Value::Dict& value_dict) { + const std::string* options_name_value = + value_dict.FindString(kSettingsOptionsNameKey); + if (!options_name_value) return false; ServiceWorkerObject::Options options(*options_name_value, options_.web_settings, options_.network_module, registration); @@ -197,42 +172,37 @@ bool ServiceWorkerPersistentSettings::ReadServiceWorkerObjectSettings( options.web_options.service_worker_context = options_.service_worker_context; scoped_refptr worker(new ServiceWorkerObject(options)); - std::string* script_url_value = - value_dict->FindStringKey(kSettingsScriptUrlKey); - if (script_url_value == nullptr) return false; + const std::string* script_url_value = + value_dict.FindString(kSettingsScriptUrlKey); + if (!script_url_value) return false; worker->set_script_url(GURL(*script_url_value)); absl::optional skip_waiting_value = - value_dict->FindBoolKey(kSettingsSkipWaitingKey); - if (!skip_waiting_value.has_value()) return false; + value_dict.FindBool(kSettingsSkipWaitingKey); + if (!skip_waiting_value) return false; if (skip_waiting_value.value()) worker->set_skip_waiting(); absl::optional classic_scripts_imported_value = - value_dict->FindBoolKey(kSettingsClassicScriptsImportedKey); - if (!classic_scripts_imported_value.has_value()) return false; + value_dict.FindBool(kSettingsClassicScriptsImportedKey); + if (!classic_scripts_imported_value) return false; if (classic_scripts_imported_value.value()) worker->set_classic_scripts_imported(); worker->set_start_status(nullptr); - base::Value* used_scripts_value = - value_dict->FindListKey(kSettingsSetOfUsedScriptsKey); - if (used_scripts_value == nullptr) return false; - base::Value::List used_scripts_list = - std::move(*used_scripts_value).TakeList(); - for (int i = 0; i < used_scripts_list.size(); i++) { - auto script_value = std::move(used_scripts_list[i]); + const base::Value::List* used_scripts_list = + value_dict.FindList(kSettingsSetOfUsedScriptsKey); + if (!used_scripts_list) return false; + for (const auto& script_value : *used_scripts_list) { if (script_value.is_string()) { worker->AppendToSetOfUsedScripts(GURL(script_value.GetString())); } } - base::Value* script_urls_value = - value_dict->FindListKey(kSettingsScriptResourceMapScriptUrlsKey); - if (script_urls_value == nullptr) return false; - base::Value::List script_urls_list = std::move(*script_urls_value).TakeList(); + const base::Value::List* script_urls_list = + value_dict.FindList(kSettingsScriptResourceMapScriptUrlsKey); + if (!script_urls_list) return false; ScriptResourceMap script_resource_map; - for (int i = 0; i < script_urls_list.size(); i++) { - auto script_url_value = std::move(script_urls_list[i]); + for (const auto& script_url_value : *script_urls_list) { if (script_url_value.is_string()) { auto script_url_string = script_url_value.GetString(); auto script_url = GURL(script_url_string); @@ -248,9 +218,9 @@ bool ServiceWorkerPersistentSettings::ReadServiceWorkerObjectSettings( if (script_url == worker->script_url()) { // Get the persistent headers for the ServiceWorkerObject script_url_. // This is used in ServiceWorkerObject::Initialize(). - std::string* raw_header_value = - value_dict->FindStringKey(kSettingsRawHeadersKey); - if (raw_header_value == nullptr) return false; + const std::string* raw_header_value = + value_dict.FindString(kSettingsRawHeadersKey); + if (!raw_header_value) return false; const scoped_refptr headers = scoped_refptr( new net::HttpResponseHeaders(*raw_header_value)); @@ -306,8 +276,7 @@ void ServiceWorkerPersistentSettings:: for (auto& key : key_set_) { key_list.Append(key); } - persistent_settings_->SetPersistentSetting( - kSettingsKeyList, std::make_unique(std::move(key_list))); + persistent_settings_->Set(kSettingsKeyList, base::Value(std::move(key_list))); // Persist ServiceWorkerRegistrationObject's fields. dict.Set(kSettingsStorageKeyKey, registration->storage_key().GetURL().spec()); @@ -324,8 +293,7 @@ void ServiceWorkerPersistentSettings:: .ToDeltaSinceWindowsEpoch() .InMicroseconds())); - persistent_settings_->SetPersistentSetting( - key_string, std::make_unique(std::move(dict))); + persistent_settings_->Set(key_string, base::Value(std::move(dict))); } base::Value::Dict @@ -396,38 +364,32 @@ void ServiceWorkerPersistentSettings:: for (auto& key : key_set_) { key_list.Append(key); } - persistent_settings_->SetPersistentSetting( - kSettingsKeyList, std::make_unique(std::move(key_list))); + persistent_settings_->Set(kSettingsKeyList, base::Value(std::move(key_list))); // Remove the registration dictionary. - persistent_settings_->RemovePersistentSetting(key_string); + persistent_settings_->Remove(key_string); } void ServiceWorkerPersistentSettings::RemoveServiceWorkerObjectSettings( std::string key_string) { - auto dict = - persistent_settings_->GetPersistentSettingAsDictionary(key_string); - if (dict.empty()) return; + const base::Value::Dict* dict = + persistent_settings_->dict().FindDict(key_string); + if (!dict || dict->empty()) return; std::vector worker_keys{kSettingsWaitingWorkerKey, kSettingsActiveWorkerKey}; for (std::string worker_key : worker_keys) { - if (!CheckPersistentValue(key_string, worker_key, dict, - base::Value::Type::DICT)) - continue; - auto worker_dict = std::move(dict[worker_key]); - base::Value* script_urls_value = - worker_dict->FindListKey(kSettingsScriptResourceMapScriptUrlsKey); - if (script_urls_value == nullptr) return; - base::Value::List script_urls_list = - std::move(*script_urls_value).TakeList(); - - for (int i = 0; i < script_urls_list.size(); i++) { - auto script_url_value = std::move(script_urls_list[i]); - if (script_url_value.is_string()) { - auto script_url_string = script_url_value.GetString(); + const base::Value::Dict* worker = dict->FindDict(worker_key); + if (!worker) continue; + const base::Value::List* script_urls = + worker->FindList(kSettingsScriptResourceMapScriptUrlsKey); + if (!script_urls) return; + + for (const auto& script_url : *script_urls) { + auto script_url_string = script_url.GetIfString(); + if (script_url_string) { cobalt::cache::Cache::GetInstance()->Delete( network::disk_cache::ResourceType::kServiceWorkerScript, - web::cache_utils::GetKey(key_string + script_url_string)); + web::cache_utils::GetKey(key_string + *script_url_string)); } } } @@ -435,12 +397,12 @@ void ServiceWorkerPersistentSettings::RemoveServiceWorkerObjectSettings( void ServiceWorkerPersistentSettings::RemoveAll() { for (auto& key : key_set_) { - persistent_settings_->RemovePersistentSetting(key); + persistent_settings_->Remove(key); } } -void ServiceWorkerPersistentSettings::DeleteAll(base::OnceClosure closure) { - persistent_settings_->DeletePersistentSettings(std::move(closure)); +void ServiceWorkerPersistentSettings::DeleteAll() { + persistent_settings_->RemoveAll(); } } // namespace worker diff --git a/cobalt/worker/service_worker_persistent_settings.h b/cobalt/worker/service_worker_persistent_settings.h index 459d45b296b7..a0da02d2cdf9 100644 --- a/cobalt/worker/service_worker_persistent_settings.h +++ b/cobalt/worker/service_worker_persistent_settings.h @@ -68,8 +68,7 @@ class ServiceWorkerPersistentSettings { bool ReadServiceWorkerObjectSettings( scoped_refptr registration, - std::string key_string, std::unique_ptr value_dict, - std::string worker_type_string); + std::string key_string, const base::Value::Dict& value_dict); void WriteServiceWorkerRegistrationObjectSettings( RegistrationMapKey key, @@ -85,7 +84,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