Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run update check if channel is set to be the same #4521

Merged
merged 1 commit into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions chrome/updater/configurator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand Down Expand Up @@ -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.
yuying-y marked this conversation as resolved.
Show resolved Hide resolved
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)));
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions chrome/updater/configurator.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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_;
Expand Down
10 changes: 7 additions & 3 deletions chrome/updater/updater_module.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion chrome/updater/updater_module.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
8 changes: 6 additions & 2 deletions cobalt/h5vcc/h5vcc_updater.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
yuying-y marked this conversation as resolved.
Show resolved Hide resolved
updater_module_->RunUpdateCheck();
}

std::string H5vccUpdater::GetUpdateStatus() const {
Expand Down
2 changes: 1 addition & 1 deletion components/update_client/configurator.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion components/update_client/test_configurator.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}

Expand Down
4 changes: 2 additions & 2 deletions components/update_client/update_checker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
Loading