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
youtube · Dec 11, 2024 · ce42f3c · ce42f3c
1 parent 9307a5f
commit ce42f3c
Show file tree

Hide file tree

Showing 8 changed files with 29 additions and 16 deletions.
diff --git 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<update_client::PersistedData>(
           pref_service_.get(), nullptr)),
-      is_channel_changed_(0),
+      is_forced_update_(0),
       unzip_factory_(base::MakeRefCounted<UnzipperFactory>()),
       network_fetcher_factory_(
           base::MakeRefCounted<NetworkFetcherFactoryCobalt>(network_module)),
@@ -172,9 +172,14 @@ base::flat_map<std::string, std::string> 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
@@ -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<update_client::PatcherFactory> 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
@@ -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
@@ -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
@@ -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
@@ -178,7 +178,7 @@ class Configurator : public base::RefCountedThreadSafe<Configurator> {
   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
@@ -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
@@ -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
 }