From 7212ab841b0a94b6445216d0312716764e320dfe Mon Sep 17 00:00:00 2001 From: Ying Yu Date: Mon, 2 Dec 2024 20:00:22 -0800 Subject: [PATCH] Force update check if channel is set If SetUpdaterChannel is called with the same channel as the current channel, an update check is forced just like the channel has changed, as long as the channel is not "prod". b/381954273 Change-Id: I042ea07029c56b810f7b4e273fe621d1ab75a9e2 --- chrome/updater/configurator.cc | 13 +++++++++---- chrome/updater/configurator.h | 4 ++-- chrome/updater/updater_module.cc | 10 +++++++--- chrome/updater/updater_module.h | 2 +- cobalt/h5vcc/h5vcc_updater.cc | 8 ++++++-- components/update_client/configurator.h | 2 +- components/update_client/test_configurator.h | 2 +- components/update_client/update_checker.cc | 4 ++-- 8 files changed, 29 insertions(+), 16 deletions(-) diff --git a/chrome/updater/configurator.cc b/chrome/updater/configurator.cc index 36a57de64372..a3e8789d83c3 100644 --- a/chrome/updater/configurator.cc +++ b/chrome/updater/configurator.cc @@ -56,7 +56,7 @@ Configurator::Configurator(network::NetworkModule* network_module) : pref_service_(CreatePrefService()), persisted_data_(std::make_unique( pref_service_.get(), nullptr)), - is_channel_changed_(0), + is_forced_update_(0), unzip_factory_(base::MakeRefCounted()), network_fetcher_factory_( base::MakeRefCounted(network_module)), @@ -172,9 +172,14 @@ base::flat_map Configurator::ExtraRequestParams() params.insert(std::make_pair("sbversion", std::to_string(SB_API_VERSION))); params.insert( std::make_pair("jsengine", script::GetJavaScriptEngineNameAndVersion())); + // The flag name to force an update is changed to "is_forced_update_" to cover + // the cases where update is requested by client but channel stays the same, but + // the flag name sent to Omaha remains "updaterchannelchanged" to isolate + // the changes to Omaha configs. If it's decided to change this flag name + // on Omaha, it'll be done in a separate PR after the Omaha configs are updated. params.insert(std::make_pair( "updaterchannelchanged", - SbAtomicNoBarrier_Load(&is_channel_changed_) == 1 ? "True" : "False")); + SbAtomicNoBarrier_Load(&is_forced_update_) == 1 ? "True" : "False")); // Brand name params.insert( std::make_pair("brand", GetDeviceProperty(kSbSystemPropertyBrandName))); @@ -318,8 +323,8 @@ update_client::RecoveryCRXElevator Configurator::GetRecoveryCRXElevator() return {}; } -void Configurator::CompareAndSwapChannelChanged(int old_value, int new_value) { - SbAtomicNoBarrier_CompareAndSwap(&is_channel_changed_, old_value, new_value); +void Configurator::CompareAndSwapForcedUpdate(int old_value, int new_value) { + SbAtomicNoBarrier_CompareAndSwap(&is_forced_update_, old_value, new_value); } // The updater channel is get and set by main web module thread and update diff --git a/chrome/updater/configurator.h b/chrome/updater/configurator.h index 4aff09e7ac90..60953d829d09 100644 --- a/chrome/updater/configurator.h +++ b/chrome/updater/configurator.h @@ -79,7 +79,7 @@ class Configurator : public update_client::Configurator { void SetChannel(const std::string& updater_channel) override; - void CompareAndSwapChannelChanged(int old_value, int new_value) override; + void CompareAndSwapForcedUpdate(int old_value, int new_value) override; std::string GetUpdaterStatus() const override; void SetUpdaterStatus(const std::string& status) override; @@ -117,7 +117,7 @@ class Configurator : public update_client::Configurator { scoped_refptr patch_factory_; std::string updater_channel_; base::Lock updater_channel_lock_; - SbAtomic32 is_channel_changed_; + SbAtomic32 is_forced_update_; std::string updater_status_; base::Lock updater_status_lock_; std::string previous_updater_status_; diff --git a/chrome/updater/updater_module.cc b/chrome/updater/updater_module.cc index fd89b1791cb9..3cf6fbbe609e 100644 --- a/chrome/updater/updater_module.cc +++ b/chrome/updater/updater_module.cc @@ -330,9 +330,11 @@ void UpdaterModule::Update() { // The following methods are called by other threads than the updater_thread_. -void UpdaterModule::CompareAndSwapChannelChanged(int old_value, int new_value) { +void UpdaterModule::CompareAndSwapForcedUpdate(int old_value, int new_value) { auto config = updater_configurator_; - if (config) config->CompareAndSwapChannelChanged(old_value, new_value); + if (config) { + config->CompareAndSwapForcedUpdate(old_value, new_value); + } } std::string UpdaterModule::GetUpdaterChannel() const { @@ -352,7 +354,9 @@ void UpdaterModule::SetUpdaterChannel(const std::string& updater_channel) { LOG(INFO) << "UpdaterModule::SetUpdaterChannel updater_channel=" << updater_channel; auto config = updater_configurator_; - if (config) config->SetChannel(updater_channel); + if (config) { + config->SetChannel(updater_channel); + } } std::string UpdaterModule::GetUpdaterStatus() const { diff --git a/chrome/updater/updater_module.h b/chrome/updater/updater_module.h index db901342718a..99daadfedbb1 100644 --- a/chrome/updater/updater_module.h +++ b/chrome/updater/updater_module.h @@ -141,7 +141,7 @@ class UpdaterModule { std::string GetUpdaterChannel() const; void SetUpdaterChannel(const std::string& updater_channel); - void CompareAndSwapChannelChanged(int old_value, int new_value); + void CompareAndSwapForcedUpdate(int old_value, int new_value); void RunUpdateCheck(); diff --git a/cobalt/h5vcc/h5vcc_updater.cc b/cobalt/h5vcc/h5vcc_updater.cc index 6f2e614dbdd2..0afe97c9b224 100644 --- a/cobalt/h5vcc/h5vcc_updater.cc +++ b/cobalt/h5vcc/h5vcc_updater.cc @@ -41,11 +41,15 @@ void H5vccUpdater::SetUpdaterChannel(const std::string& channel) { return; } + // Force an update if SetUpdaterChannel is called, even if + // the channel doesn't change, as long as it's not "prod" channel if (updater_module_->GetUpdaterChannel().compare(channel) != 0) { updater_module_->SetUpdaterChannel(channel); - updater_module_->CompareAndSwapChannelChanged(0, 1); - updater_module_->RunUpdateCheck(); + } else if (channel == "prod") { + return; } + updater_module_->CompareAndSwapForcedUpdate(0, 1); + updater_module_->RunUpdateCheck(); } std::string H5vccUpdater::GetUpdateStatus() const { diff --git a/components/update_client/configurator.h b/components/update_client/configurator.h index df96c4bce7c0..103eddd2c3bc 100644 --- a/components/update_client/configurator.h +++ b/components/update_client/configurator.h @@ -178,7 +178,7 @@ class Configurator : public base::RefCountedThreadSafe { virtual void SetUpdaterStatus(const std::string& status) = 0; // Compare and swap the is_channel_changed flag. - virtual void CompareAndSwapChannelChanged(int old_value, int new_value) = 0; + virtual void CompareAndSwapForcedUpdate(int old_value, int new_value) = 0; virtual void SetMinFreeSpaceBytes(uint64_t bytes) = 0; virtual uint64_t GetMinFreeSpaceBytes() = 0; diff --git a/components/update_client/test_configurator.h b/components/update_client/test_configurator.h index 6fe5cbe6445c..a2fd9e7d44ce 100644 --- a/components/update_client/test_configurator.h +++ b/components/update_client/test_configurator.h @@ -128,7 +128,7 @@ class TestConfigurator : public Configurator { #if defined(STARBOARD) // TODO: add unit tests for updater channels, status, and compressed updates. void SetChannel(const std::string& channel) override {} - void CompareAndSwapChannelChanged(int old_value, int new_value) override {} + void CompareAndSwapForcedUpdate(int old_value, int new_value) override {} std::string GetUpdaterStatus() const override { return ""; } void SetUpdaterStatus(const std::string& status) override {} diff --git a/components/update_client/update_checker.cc b/components/update_client/update_checker.cc index d42a1d447a7e..78f6c54cf25d 100644 --- a/components/update_client/update_checker.cc +++ b/components/update_client/update_checker.cc @@ -303,8 +303,8 @@ void UpdateCheckerImpl::CheckForUpdatesHelper( base::BindOnce(&UpdateCheckerImpl::OnRequestSenderComplete, base::Unretained(this))); #if defined(STARBOARD) - // Reset is_channel_changed flag to false if it is true - config_->CompareAndSwapChannelChanged(1, 0); + // Reset |is_forced_update| flag to false if it is true + config_->CompareAndSwapForcedUpdate(/*old_value=*/1, /*new_value=*/0); #endif }