update_engine: Make policy UpdateCheckParams processing easier
Currently there are a ton of arguments from UpdateCheckParams that is
passed around in the udpate_attampter.cc. With this CL UpdateCheckParams
is directy passed and this simplies the logic.
BUG=b:171829801
TEST=cros_workon_make --board reef --test update_enigne
Change-Id: If454f6393fc6e28d41fa5d14d184f0db32e8bd19
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/2504453
Commit-Queue: Amin Hassani <ahassani@chromium.org>
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Jae Hoon Kim <kimjae@chromium.org>
diff --git a/mock_update_attempter.h b/mock_update_attempter.h
index d502222..96d93fd 100644
--- a/mock_update_attempter.h
+++ b/mock_update_attempter.h
@@ -32,17 +32,7 @@
MOCK_METHOD(void,
Update,
- (const std::string& app_version,
- const std::string& omaha_url,
- const std::string& target_channel,
- const std::string& lts_tag,
- const std::string& target_version_prefix,
- bool rollback_allowed,
- bool rollback_data_save_requested,
- int rollback_allowed_milestones,
- bool rollback_on_channel_downgrade,
- bool obey_proxies,
- bool interactive),
+ (const chromeos_update_manager::UpdateCheckParams& params),
(override));
MOCK_METHOD1(GetStatus, bool(update_engine::UpdateEngineStatus* out_status));
diff --git a/omaha_request_params.cc b/omaha_request_params.cc
index 8a2e3dc..79d19e8 100644
--- a/omaha_request_params.cc
+++ b/omaha_request_params.cc
@@ -37,9 +37,11 @@
#include "update_engine/common/platform_constants.h"
#include "update_engine/common/utils.h"
#include "update_engine/system_state.h"
+#include "update_engine/update_manager/policy.h"
#define CALL_MEMBER_FN(object, member) ((object).*(member))
+using chromeos_update_manager::UpdateCheckParams;
using std::string;
namespace chromeos_update_engine {
@@ -59,9 +61,9 @@
test::SetImagePropertiesRootPrefix(nullptr);
}
-bool OmahaRequestParams::Init(const string& in_app_version,
- const string& in_update_url,
- bool in_interactive) {
+bool OmahaRequestParams::Init(const string& app_version,
+ const string& update_url,
+ const UpdateCheckParams& params) {
LOG(INFO) << "Initializing parameters for this update attempt";
image_props_ = LoadImageProperties(system_state_);
mutable_image_props_ = LoadMutableImageProperties(system_state_);
@@ -77,15 +79,15 @@
os_platform_ = constants::kOmahaPlatformName;
if (!image_props_.system_version.empty()) {
- if (in_app_version == "ForcedUpdate") {
- image_props_.system_version = in_app_version;
+ if (app_version == "ForcedUpdate") {
+ image_props_.system_version = app_version;
}
os_version_ = image_props_.system_version;
} else {
os_version_ = OmahaRequestParams::kOsVersion;
}
- if (!in_app_version.empty())
- image_props_.version = in_app_version;
+ if (!app_version.empty())
+ image_props_.version = app_version;
os_sp_ = image_props_.version + "_" + GetMachineType();
app_lang_ = "en-US";
@@ -115,17 +117,51 @@
delta_okay_ = false;
}
- if (in_update_url.empty())
+ if (update_url.empty())
update_url_ = image_props_.omaha_url;
else
- update_url_ = in_update_url;
+ update_url_ = update_url;
// Set the interactive flag accordingly.
- interactive_ = in_interactive;
+ interactive_ = params.interactive;
dlc_apps_params_.clear();
// Set false so it will do update by default.
is_install_ = false;
+
+ target_version_prefix_ = params.target_version_prefix;
+
+ lts_tag_ = params.lts_tag;
+
+ rollback_allowed_ = params.rollback_allowed;
+
+ // Set whether saving data over rollback is requested.
+ rollback_data_save_requested_ = params.rollback_data_save_requested;
+
+ // Set how many milestones of rollback are allowed.
+ rollback_allowed_milestones_ = params.rollback_allowed_milestones;
+
+ // Set the target channel, if one was provided.
+ if (params.target_channel.empty()) {
+ LOG(INFO) << "No target channel mandated by policy.";
+ } else {
+ LOG(INFO) << "Setting target channel as mandated: "
+ << params.target_channel;
+ string error_message;
+ if (!SetTargetChannel(params.target_channel,
+ params.rollback_on_channel_downgrade,
+ &error_message)) {
+ LOG(ERROR) << "Setting the channel failed: " << error_message;
+ }
+
+ // Since this is the beginning of a new attempt, update the download
+ // channel. The download channel won't be updated until the next attempt,
+ // even if target channel changes meanwhile, so that how we'll know if we
+ // should cancel the current download attempt if there's such a change in
+ // target channel.
+ UpdateDownloadChannel();
+ }
+
return true;
}
diff --git a/omaha_request_params.h b/omaha_request_params.h
index 1e9ab7d..7e19262 100644
--- a/omaha_request_params.h
+++ b/omaha_request_params.h
@@ -30,6 +30,7 @@
#include "update_engine/common/constants.h"
#include "update_engine/common/platform_constants.h"
#include "update_engine/image_properties.h"
+#include "update_engine/update_manager/policy.h"
// This gathers local system information and prepares info used by the
// Omaha request action.
@@ -249,7 +250,7 @@
// of the parameter. Returns true on success, false otherwise.
bool Init(const std::string& in_app_version,
const std::string& in_update_url,
- bool in_interactive);
+ const chromeos_update_manager::UpdateCheckParams& params);
// Permanently changes the release channel to |channel|. Performs a
// powerwash, if required and allowed.
diff --git a/omaha_request_params_unittest.cc b/omaha_request_params_unittest.cc
index 110fb2b..140cad3 100644
--- a/omaha_request_params_unittest.cc
+++ b/omaha_request_params_unittest.cc
@@ -75,36 +75,36 @@
} // namespace
TEST_F(OmahaRequestParamsTest, MissingChannelTest) {
- EXPECT_TRUE(params_.Init("", "", false));
+ EXPECT_TRUE(params_.Init("", "", {}));
// By default, if no channel is set, we should track the stable-channel.
EXPECT_EQ("stable-channel", params_.target_channel());
}
TEST_F(OmahaRequestParamsTest, ForceVersionTest) {
- EXPECT_TRUE(params_.Init("ForcedVersion", "", false));
+ EXPECT_TRUE(params_.Init("ForcedVersion", "", {}));
EXPECT_EQ(string("ForcedVersion_") + GetMachineType(), params_.os_sp());
EXPECT_EQ("ForcedVersion", params_.app_version());
}
TEST_F(OmahaRequestParamsTest, ForcedURLTest) {
- EXPECT_TRUE(params_.Init("", "http://forced.google.com", false));
+ EXPECT_TRUE(params_.Init("", "http://forced.google.com", {}));
EXPECT_EQ("http://forced.google.com", params_.update_url());
}
TEST_F(OmahaRequestParamsTest, MissingURLTest) {
- EXPECT_TRUE(params_.Init("", "", false));
+ EXPECT_TRUE(params_.Init("", "", {}));
EXPECT_EQ(constants::kOmahaDefaultProductionURL, params_.update_url());
}
TEST_F(OmahaRequestParamsTest, DeltaOKTest) {
- EXPECT_TRUE(params_.Init("", "", false));
+ EXPECT_TRUE(params_.Init("", "", {}));
EXPECT_TRUE(params_.delta_okay());
}
TEST_F(OmahaRequestParamsTest, NoDeltasTest) {
ASSERT_TRUE(
WriteFileString(tempdir_.GetPath().Append(".nodelta").value(), ""));
- EXPECT_TRUE(params_.Init("", "", false));
+ EXPECT_TRUE(params_.Init("", "", {}));
EXPECT_FALSE(params_.delta_okay());
}
@@ -112,12 +112,12 @@
{
OmahaRequestParams params(&fake_system_state_);
params.set_root(tempdir_.GetPath().value());
- EXPECT_TRUE(params.Init("", "", false));
+ EXPECT_TRUE(params.Init("", "", {}));
EXPECT_TRUE(params.SetTargetChannel("canary-channel", false, nullptr));
EXPECT_FALSE(params.mutable_image_props_.is_powerwash_allowed);
}
params_.set_root(tempdir_.GetPath().value());
- EXPECT_TRUE(params_.Init("", "", false));
+ EXPECT_TRUE(params_.Init("", "", {}));
EXPECT_EQ("canary-channel", params_.target_channel());
EXPECT_FALSE(params_.mutable_image_props_.is_powerwash_allowed);
}
@@ -126,12 +126,12 @@
{
OmahaRequestParams params(&fake_system_state_);
params.set_root(tempdir_.GetPath().value());
- EXPECT_TRUE(params.Init("", "", false));
+ EXPECT_TRUE(params.Init("", "", {}));
EXPECT_TRUE(params.SetTargetChannel("canary-channel", true, nullptr));
EXPECT_TRUE(params.mutable_image_props_.is_powerwash_allowed);
}
params_.set_root(tempdir_.GetPath().value());
- EXPECT_TRUE(params_.Init("", "", false));
+ EXPECT_TRUE(params_.Init("", "", {}));
EXPECT_EQ("canary-channel", params_.target_channel());
EXPECT_TRUE(params_.mutable_image_props_.is_powerwash_allowed);
}
@@ -141,7 +141,7 @@
OmahaRequestParams params(&fake_system_state_);
params.set_root(tempdir_.GetPath().value());
SetLockDown(true);
- EXPECT_TRUE(params.Init("", "", false));
+ EXPECT_TRUE(params.Init("", "", {}));
params.image_props_.allow_arbitrary_channels = false;
string error_message;
EXPECT_FALSE(
@@ -151,7 +151,7 @@
EXPECT_FALSE(params.mutable_image_props_.is_powerwash_allowed);
}
params_.set_root(tempdir_.GetPath().value());
- EXPECT_TRUE(params_.Init("", "", false));
+ EXPECT_TRUE(params_.Init("", "", {}));
EXPECT_EQ("stable-channel", params_.target_channel());
EXPECT_FALSE(params_.mutable_image_props_.is_powerwash_allowed);
}
@@ -197,7 +197,7 @@
// When set to a valid value while a change is already pending, it should
// succeed.
- params_.Init("", "", false);
+ params_.Init("", "", {});
EXPECT_TRUE(params_.SetTargetChannel("beta-channel", true, nullptr));
// The target channel should reflect the change, but the download channel
// should continue to retain the old value ...
@@ -237,7 +237,7 @@
}
TEST_F(OmahaRequestParamsTest, TargetChannelHintTest) {
- EXPECT_TRUE(params_.Init("", "", false));
+ EXPECT_TRUE(params_.Init("", "", {}));
const string kHint("foo-hint");
params_.set_lts_tag(kHint);
EXPECT_EQ(kHint, params_.lts_tag());
@@ -266,7 +266,7 @@
}
TEST_F(OmahaRequestParamsTest, RequisitionIsSetTest) {
- EXPECT_TRUE(params_.Init("", "", false));
+ EXPECT_TRUE(params_.Init("", "", {}));
EXPECT_EQ("fake_requisition", params_.device_requisition());
}
} // namespace chromeos_update_engine
diff --git a/payload_state_unittest.cc b/payload_state_unittest.cc
index 8667548..2d571c1 100644
--- a/payload_state_unittest.cc
+++ b/payload_state_unittest.cc
@@ -630,7 +630,7 @@
PayloadState payload_state;
FakeSystemState fake_system_state;
OmahaRequestParams params(&fake_system_state);
- params.Init("", "", true); // interactive = True.
+ params.Init("", "", {.interactive = true});
fake_system_state.set_request_params(¶ms);
EXPECT_TRUE(payload_state.Initialize(&fake_system_state));
@@ -653,7 +653,7 @@
PayloadState payload_state;
FakeSystemState fake_system_state;
OmahaRequestParams params(&fake_system_state);
- params.Init("", "", false); // interactive = False.
+ params.Init("", "", {});
fake_system_state.set_request_params(¶ms);
EXPECT_TRUE(payload_state.Initialize(&fake_system_state));
@@ -1019,7 +1019,7 @@
// Mock out the os version and make sure it's excluded correctly.
string rollback_version = "2345.0.0";
OmahaRequestParams params(&fake_system_state);
- params.Init(rollback_version, "", false);
+ params.Init(rollback_version, "", {});
fake_system_state.set_request_params(¶ms);
EXPECT_TRUE(payload_state.Initialize(&fake_system_state));
diff --git a/real_system_state.cc b/real_system_state.cc
index 74a37f3..924271e 100644
--- a/real_system_state.cc
+++ b/real_system_state.cc
@@ -138,7 +138,7 @@
// will be re-initialized before every request using the actual request
// options. This initialization here pre-loads current channel and version, so
// the DBus service can access it.
- if (!request_params_.Init("", "", false)) {
+ if (!request_params_.Init("", "", {})) {
LOG(WARNING) << "Ignoring OmahaRequestParams initialization error. Some "
"features might not work properly.";
}
diff --git a/update_attempter.cc b/update_attempter.cc
index 14d5837..38b0f82 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -244,17 +244,7 @@
system_state_->metrics_reporter()->ReportDailyMetrics(age);
}
-void UpdateAttempter::Update(const string& app_version,
- const string& omaha_url,
- const string& target_channel,
- const string& lts_tag,
- const string& target_version_prefix,
- bool rollback_allowed,
- bool rollback_data_save_requested,
- int rollback_allowed_milestones,
- bool rollback_on_channel_downgrade,
- bool obey_proxies,
- bool interactive) {
+void UpdateAttempter::Update(const UpdateCheckParams& params) {
// This is normally called frequently enough so it's appropriate to use as a
// hook for reporting daily metrics.
// TODO(garnold) This should be hooked to a separate (reliable and consistent)
@@ -283,21 +273,11 @@
return;
}
- if (!CalculateUpdateParams(app_version,
- omaha_url,
- target_channel,
- lts_tag,
- target_version_prefix,
- rollback_allowed,
- rollback_data_save_requested,
- rollback_allowed_milestones,
- rollback_on_channel_downgrade,
- obey_proxies,
- interactive)) {
+ if (!CalculateUpdateParams(params)) {
return;
}
- BuildUpdateActions(interactive);
+ BuildUpdateActions(params.interactive);
SetStatusAndNotify(UpdateStatus::CHECKING_FOR_UPDATE);
@@ -360,17 +340,7 @@
payload_state->SetUsingP2PForSharing(use_p2p_for_sharing);
}
-bool UpdateAttempter::CalculateUpdateParams(const string& app_version,
- const string& omaha_url,
- const string& target_channel,
- const string& lts_tag,
- const string& target_version_prefix,
- bool rollback_allowed,
- bool rollback_data_save_requested,
- int rollback_allowed_milestones,
- bool rollback_on_channel_downgrade,
- bool obey_proxies,
- bool interactive) {
+bool UpdateAttempter::CalculateUpdateParams(const UpdateCheckParams& params) {
http_response_code_ = 0;
PayloadStateInterface* const payload_state = system_state_->payload_state();
@@ -381,30 +351,13 @@
// policy is available again.
UpdateRollbackHappened();
- // Update the target version prefix.
- omaha_request_params_->set_target_version_prefix(target_version_prefix);
-
- // Update the LTS support.
- omaha_request_params_->set_lts_tag(lts_tag);
-
- // Set whether rollback is allowed.
- omaha_request_params_->set_rollback_allowed(rollback_allowed);
-
- // Set whether saving data over rollback is requested.
- omaha_request_params_->set_rollback_data_save_requested(
- rollback_data_save_requested);
-
- CalculateStagingParams(interactive);
+ CalculateStagingParams(params.interactive);
// If staging_wait_time_ wasn't set, staging is off, use scattering instead.
if (staging_wait_time_.InSeconds() == 0) {
- CalculateScatteringParams(interactive);
+ CalculateScatteringParams(params.interactive);
}
- // Set how many milestones of rollback are allowed.
- omaha_request_params_->set_rollback_allowed_milestones(
- rollback_allowed_milestones);
-
- CalculateP2PParams(interactive);
+ CalculateP2PParams(params.interactive);
if (payload_state->GetUsingP2PForDownloading() ||
payload_state->GetUsingP2PForSharing()) {
// OK, p2p is to be used - start it and perform housekeeping.
@@ -417,30 +370,12 @@
}
}
- if (!omaha_request_params_->Init(app_version, omaha_url, interactive)) {
+ if (!omaha_request_params_->Init(
+ forced_app_version_, forced_omaha_url_, params)) {
LOG(ERROR) << "Unable to initialize Omaha request params.";
return false;
}
- // Set the target channel, if one was provided.
- if (target_channel.empty()) {
- LOG(INFO) << "No target channel mandated by policy.";
- } else {
- LOG(INFO) << "Setting target channel as mandated: " << target_channel;
- string error_message;
- if (!omaha_request_params_->SetTargetChannel(
- target_channel, rollback_on_channel_downgrade, &error_message)) {
- LOG(ERROR) << "Setting the channel failed: " << error_message;
- }
-
- // Since this is the beginning of a new attempt, update the download
- // channel. The download channel won't be updated until the next attempt,
- // even if target channel changes meanwhile, so that how we'll know if we
- // should cancel the current download attempt if there's such a change in
- // target channel.
- omaha_request_params_->UpdateDownloadChannel();
- }
-
// The function |CalculateDlcParams| makes use of the function |GetAppId| from
// |OmahaRequestParams|, so to ensure that the return from |GetAppId|
// doesn't change, no changes to the values |download_channel_|,
@@ -448,8 +383,6 @@
// |omaha_request_params_| shall be made below this line.
CalculateDlcParams();
- omaha_request_params_->set_is_install(is_install_);
-
// Set Quick Fix Build token if policy is set and the device is enterprise
// enrolled.
string token;
@@ -480,7 +413,7 @@
<< payload_state->GetUsingP2PForSharing();
obeying_proxies_ = true;
- if (obey_proxies || proxy_manual_checks_ == 0) {
+ if (proxy_manual_checks_ == 0) {
LOG(INFO) << "forced to obey proxies";
// If forced to obey proxies, every 20th request will not use proxies
proxy_manual_checks_++;
@@ -767,6 +700,7 @@
dlc_apps_params[omaha_request_params_->GetDlcAppId(dlc_id)] = dlc_params;
}
omaha_request_params_->set_dlc_apps_params(dlc_apps_params);
+ omaha_request_params_->set_is_install(is_install_);
}
void UpdateAttempter::BuildUpdateActions(bool interactive) {
@@ -881,7 +815,7 @@
processor_->set_delegate(this);
// Initialize the default request params.
- if (!omaha_request_params_->Init("", "", true)) {
+ if (!omaha_request_params_->Init("", "", {.interactive = true})) {
LOG(ERROR) << "Unable to initialize Omaha request params.";
return false;
}
@@ -1107,17 +1041,7 @@
LOG(INFO) << "Update attempt flags in use = 0x" << std::hex
<< current_update_attempt_flags_;
- Update(forced_app_version_,
- forced_omaha_url_,
- params.target_channel,
- params.lts_tag,
- params.target_version_prefix,
- params.rollback_allowed,
- params.rollback_data_save_requested,
- params.rollback_allowed_milestones,
- params.rollback_on_channel_downgrade,
- /*obey_proxies=*/false,
- params.interactive);
+ Update(params);
// Always clear the forced app_version and omaha_url after an update attempt
// so the next update uses the defaults.
forced_app_version_.clear();
diff --git a/update_attempter.h b/update_attempter.h
index 6c93150..3a1bef4 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -76,23 +76,8 @@
virtual bool ScheduleUpdates();
// Checks for update and, if a newer version is available, attempts to update
- // the system. Non-empty |in_app_version| or |in_update_url| prevents
- // automatic detection of the parameter. |target_channel| denotes a
- // policy-mandated channel we are updating to, if not empty. If |obey_proxies|
- // is true, the update will likely respect Chrome's proxy setting. For
- // security reasons, we may still not honor them. |interactive| should be true
- // if this was called from the user (ie dbus).
- virtual void Update(const std::string& app_version,
- const std::string& omaha_url,
- const std::string& target_channel,
- const std::string& lts_tag,
- const std::string& target_version_prefix,
- bool rollback_allowed,
- bool rollback_data_save_requested,
- int rollback_allowed_milestones,
- bool rollback_on_channel_downgrade,
- bool obey_proxies,
- bool interactive);
+ // the system.
+ virtual void Update(const chromeos_update_manager::UpdateCheckParams& params);
// ActionProcessorDelegate methods:
void ProcessingDone(const ActionProcessor* processor,
@@ -371,17 +356,8 @@
// Helper method of Update() to calculate the update-related parameters
// from various sources and set the appropriate state. Please refer to
// Update() method for the meaning of the parameters.
- bool CalculateUpdateParams(const std::string& app_version,
- const std::string& omaha_url,
- const std::string& target_channel,
- const std::string& lts_tag,
- const std::string& target_version_prefix,
- bool rollback_allowed,
- bool rollback_data_save_requested,
- int rollback_allowed_milestones,
- bool rollback_on_channel_downgrade,
- bool obey_proxies,
- bool interactive);
+ bool CalculateUpdateParams(
+ const chromeos_update_manager::UpdateCheckParams& params);
// Calculates all the scattering related parameters (such as waiting period,
// which type of scattering is enabled, etc.) and also updates/deletes
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 60267f0..767bb82 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -172,30 +172,10 @@
explicit UpdateAttempterUnderTest(SystemState* system_state)
: UpdateAttempter(system_state, nullptr) {}
- void Update(const std::string& app_version,
- const std::string& omaha_url,
- const std::string& target_channel,
- const std::string& lts_tag,
- const std::string& target_version_prefix,
- bool rollback_allowed,
- bool rollback_data_save_requested,
- int rollback_allowed_milestones,
- bool rollback_on_channel_downgrade,
- bool obey_proxies,
- bool interactive) override {
+ void Update(const UpdateCheckParams& params) override {
update_called_ = true;
if (do_update_) {
- UpdateAttempter::Update(app_version,
- omaha_url,
- target_channel,
- lts_tag,
- target_version_prefix,
- rollback_allowed,
- rollback_data_save_requested,
- rollback_allowed_milestones,
- rollback_on_channel_downgrade,
- obey_proxies,
- interactive);
+ UpdateAttempter::Update(params);
return;
}
LOG(INFO) << "[TEST] Update() disabled.";
@@ -430,7 +410,7 @@
void UpdateAttempterTest::SessionIdTestChange() {
EXPECT_NE(UpdateStatus::UPDATED_NEED_REBOOT, attempter_.status());
const auto old_session_id = attempter_.session_id_;
- attempter_.Update("", "", "", "", "", false, false, 0, false, false, false);
+ attempter_.Update({});
EXPECT_NE(old_session_id, attempter_.session_id_);
ScheduleQuitMainLoop();
}
@@ -801,7 +781,7 @@
EXPECT_CALL(*processor_, StartProcessing());
}
- attempter_.Update("", "", "", "", "", false, false, 0, false, false, false);
+ attempter_.Update({});
loop_.PostTask(FROM_HERE,
base::Bind(&UpdateAttempterTest::UpdateTestVerify,
base::Unretained(this)));
@@ -1001,7 +981,7 @@
fake_system_state_.set_p2p_manager(&mock_p2p_manager);
mock_p2p_manager.fake().SetP2PEnabled(false);
EXPECT_CALL(mock_p2p_manager, PerformHousekeeping()).Times(0);
- attempter_.Update("", "", "", "", "", false, false, 0, false, false, false);
+ attempter_.Update({});
EXPECT_FALSE(actual_using_p2p_for_downloading_);
EXPECT_FALSE(actual_using_p2p_for_sharing());
ScheduleQuitMainLoop();
@@ -1023,7 +1003,7 @@
mock_p2p_manager.fake().SetEnsureP2PRunningResult(false);
mock_p2p_manager.fake().SetPerformHousekeepingResult(false);
EXPECT_CALL(mock_p2p_manager, PerformHousekeeping()).Times(0);
- attempter_.Update("", "", "", "", "", false, false, 0, false, false, false);
+ attempter_.Update({});
EXPECT_FALSE(actual_using_p2p_for_downloading());
EXPECT_FALSE(actual_using_p2p_for_sharing());
ScheduleQuitMainLoop();
@@ -1046,7 +1026,7 @@
mock_p2p_manager.fake().SetEnsureP2PRunningResult(true);
mock_p2p_manager.fake().SetPerformHousekeepingResult(false);
EXPECT_CALL(mock_p2p_manager, PerformHousekeeping());
- attempter_.Update("", "", "", "", "", false, false, 0, false, false, false);
+ attempter_.Update({});
EXPECT_FALSE(actual_using_p2p_for_downloading());
EXPECT_FALSE(actual_using_p2p_for_sharing());
ScheduleQuitMainLoop();
@@ -1068,7 +1048,7 @@
mock_p2p_manager.fake().SetEnsureP2PRunningResult(true);
mock_p2p_manager.fake().SetPerformHousekeepingResult(true);
EXPECT_CALL(mock_p2p_manager, PerformHousekeeping());
- attempter_.Update("", "", "", "", "", false, false, 0, false, false, false);
+ attempter_.Update({});
EXPECT_TRUE(actual_using_p2p_for_downloading());
EXPECT_TRUE(actual_using_p2p_for_sharing());
ScheduleQuitMainLoop();
@@ -1091,17 +1071,7 @@
mock_p2p_manager.fake().SetEnsureP2PRunningResult(true);
mock_p2p_manager.fake().SetPerformHousekeepingResult(true);
EXPECT_CALL(mock_p2p_manager, PerformHousekeeping());
- attempter_.Update("",
- "",
- "",
- "",
- "",
- false,
- false,
- /*rollback_allowed_milestones=*/0,
- false,
- false,
- /*interactive=*/true);
+ attempter_.Update({.interactive = true});
EXPECT_FALSE(actual_using_p2p_for_downloading());
EXPECT_TRUE(actual_using_p2p_for_sharing());
ScheduleQuitMainLoop();
@@ -1131,7 +1101,7 @@
attempter_.policy_provider_.reset(
new policy::PolicyProvider(std::move(device_policy)));
- attempter_.Update("", "", "", "", "", false, false, 0, false, false, false);
+ attempter_.Update({});
EXPECT_EQ(scatter_factor_in_seconds, attempter_.scatter_factor_.InSeconds());
ScheduleQuitMainLoop();
@@ -1169,7 +1139,7 @@
attempter_.policy_provider_.reset(
new policy::PolicyProvider(std::move(device_policy)));
- attempter_.Update("", "", "", "", "", false, false, 0, false, false, false);
+ attempter_.Update({});
EXPECT_EQ(scatter_factor_in_seconds, attempter_.scatter_factor_.InSeconds());
// Make sure the file still exists.
@@ -1185,7 +1155,7 @@
// However, if the count is already 0, it's not decremented. Test that.
initial_value = 0;
EXPECT_TRUE(fake_prefs.SetInt64(kPrefsUpdateCheckCount, initial_value));
- attempter_.Update("", "", "", "", "", false, false, 0, false, false, false);
+ attempter_.Update({});
EXPECT_TRUE(fake_prefs.Exists(kPrefsUpdateCheckCount));
EXPECT_TRUE(fake_prefs.GetInt64(kPrefsUpdateCheckCount, &new_value));
EXPECT_EQ(initial_value, new_value);
@@ -1232,17 +1202,7 @@
new policy::PolicyProvider(std::move(device_policy)));
// Trigger an interactive check so we can test that scattering is disabled.
- attempter_.Update("",
- "",
- "",
- "",
- "",
- false,
- false,
- /*rollback_allowed_milestones=*/0,
- false,
- false,
- /*interactive=*/true);
+ attempter_.Update({.interactive = true});
EXPECT_EQ(scatter_factor_in_seconds, attempter_.scatter_factor_.InSeconds());
// Make sure scattering is disabled for manual (i.e. user initiated) update
@@ -1294,7 +1254,7 @@
FakePrefs fake_prefs;
SetUpStagingTest(kValidStagingSchedule, &fake_prefs);
- attempter_.Update("", "", "", "", "", false, false, 0, false, false, false);
+ attempter_.Update({});
// Check that prefs have the correct values.
int64_t update_count;
EXPECT_TRUE(fake_prefs.GetInt64(kPrefsUpdateCheckCount, &update_count));
@@ -1351,17 +1311,7 @@
FakePrefs fake_prefs;
SetUpStagingTest(kValidStagingSchedule, &fake_prefs);
- attempter_.Update("",
- "",
- "",
- "",
- "",
- false,
- false,
- 0,
- false,
- false,
- /* interactive = */ true);
+ attempter_.Update({.interactive = true});
CheckStagingOff();
ScheduleQuitMainLoop();
@@ -1381,17 +1331,7 @@
FakePrefs fake_prefs;
SetUpStagingTest(kValidStagingSchedule, &fake_prefs);
- attempter_.Update("",
- "",
- "",
- "",
- "",
- false,
- false,
- 0,
- false,
- false,
- /* interactive = */ true);
+ attempter_.Update({.interactive = true});
CheckStagingOff();
ScheduleQuitMainLoop();
@@ -1719,64 +1659,38 @@
}
TEST_F(UpdateAttempterTest, TargetVersionPrefixSetAndReset) {
- attempter_.CalculateUpdateParams(
- /*app_version=*/"",
- /*omaha_url=*/"",
- /*target_channel=*/"",
- /*lts_tag=*/"",
- /*target_version_prefix=*/"1234",
- /*rollback_allowed=*/false,
- /*rollback_data_save_requested=*/false,
- /*rollback_allowed_milestones=*/4,
- /*rollback_on_channel_downgrade=*/false,
- /*obey_proxies=*/false,
- /*interactive=*/false);
+ UpdateCheckParams params;
+ attempter_.CalculateUpdateParams({.target_version_prefix = "1234"});
EXPECT_EQ("1234",
fake_system_state_.request_params()->target_version_prefix());
- attempter_.CalculateUpdateParams(
- "", "", "", "", "", false, false, 4, false, false, false);
+ attempter_.CalculateUpdateParams({});
EXPECT_TRUE(
fake_system_state_.request_params()->target_version_prefix().empty());
}
TEST_F(UpdateAttempterTest, TargetChannelHintSetAndReset) {
- attempter_.CalculateUpdateParams(
- "", "", "", "hint", "", false, false, 4, false, false, false);
+ attempter_.CalculateUpdateParams({.lts_tag = "hint"});
EXPECT_EQ("hint", fake_system_state_.request_params()->lts_tag());
- attempter_.CalculateUpdateParams(
- "", "", "", "", "", false, false, 4, false, false, false);
+ attempter_.CalculateUpdateParams({});
EXPECT_TRUE(fake_system_state_.request_params()->lts_tag().empty());
}
TEST_F(UpdateAttempterTest, RollbackAllowedSetAndReset) {
- attempter_.CalculateUpdateParams("",
- "",
- "",
- "",
- "1234",
- /*rollback_allowed=*/true,
- /*rollback_data_save_requested=*/false,
- /*rollback_allowed_milestones=*/4,
- /*rollback_on_channel_downgrade=*/false,
- false,
- false);
+ attempter_.CalculateUpdateParams({
+ .target_version_prefix = "1234",
+ .rollback_allowed = true,
+ .rollback_allowed_milestones = 4,
+ });
EXPECT_TRUE(fake_system_state_.request_params()->rollback_allowed());
EXPECT_EQ(4,
fake_system_state_.request_params()->rollback_allowed_milestones());
- attempter_.CalculateUpdateParams("",
- "",
- "",
- "",
- "1234",
- /*rollback_allowed=*/false,
- /*rollback_data_save_requested=*/false,
- /*rollback_allowed_milestones=*/4,
- /*rollback_on_channel_downgrade=*/false,
- false,
- false);
+ attempter_.CalculateUpdateParams({
+ .target_version_prefix = "1234",
+ .rollback_allowed_milestones = 4,
+ });
EXPECT_FALSE(fake_system_state_.request_params()->rollback_allowed());
EXPECT_EQ(4,
fake_system_state_.request_params()->rollback_allowed_milestones());
@@ -1786,17 +1700,9 @@
base::ScopedTempDir tempdir;
ASSERT_TRUE(tempdir.CreateUniqueTempDir());
fake_system_state_.request_params()->set_root(tempdir.GetPath().value());
- attempter_.CalculateUpdateParams(/*app_version=*/"",
- /*omaha_url=*/"",
- /*target_channel=*/"stable-channel",
- /*lts_tag=*/"",
- /*target_version_prefix=*/"",
- /*rollback_allowed=*/false,
- /*rollback_data_save_requested=*/false,
- /*rollback_allowed_milestones=*/4,
- /*rollback_on_channel_downgrade=*/false,
- /*obey_proxies=*/false,
- /*interactive=*/false);
+ attempter_.CalculateUpdateParams({
+ .target_channel = "stable-channel",
+ });
EXPECT_FALSE(fake_system_state_.request_params()->is_powerwash_allowed());
}
@@ -1804,17 +1710,10 @@
base::ScopedTempDir tempdir;
ASSERT_TRUE(tempdir.CreateUniqueTempDir());
fake_system_state_.request_params()->set_root(tempdir.GetPath().value());
- attempter_.CalculateUpdateParams(/*app_version=*/"",
- /*omaha_url=*/"",
- /*target_channel=*/"stable-channel",
- /*lts_tag=*/"",
- /*target_version_prefix=*/"",
- /*rollback_allowed=*/false,
- /*rollback_data_save_requested=*/false,
- /*rollback_allowed_milestones=*/4,
- /*rollback_on_channel_downgrade=*/true,
- /*obey_proxies=*/false,
- /*interactive=*/false);
+ attempter_.CalculateUpdateParams({
+ .rollback_on_channel_downgrade = true,
+ .target_channel = "stable-channel",
+ });
EXPECT_TRUE(fake_system_state_.request_params()->is_powerwash_allowed());
}
@@ -1932,7 +1831,7 @@
SetRollbackHappened(false))
.Times(expected_reset ? 1 : 0);
attempter_.policy_provider_ = std::move(mock_policy_provider);
- attempter_.Update("", "", "", "", "", false, false, 0, false, false, false);
+ attempter_.Update({});
ScheduleQuitMainLoop();
}
@@ -2273,7 +2172,7 @@
.WillOnce(Return(false));
attempter_.policy_provider_.reset(
new policy::PolicyProvider(std::move(device_policy)));
- attempter_.Update("", "", "", "", "", false, false, 0, false, false, false);
+ attempter_.Update({});
EXPECT_EQ(token, attempter_.omaha_request_params_->autoupdate_token());
ScheduleQuitMainLoop();
diff --git a/update_manager/policy.h b/update_manager/policy.h
index 4b3bfc7..ad6994c 100644
--- a/update_manager/policy.h
+++ b/update_manager/policy.h
@@ -43,21 +43,22 @@
// Parameters of an update check. These parameters are determined by the
// UpdateCheckAllowed policy.
struct UpdateCheckParams {
- bool updates_enabled; // Whether the auto-updates are enabled on this build.
+ // Whether the auto-updates are enabled on this build.
+ bool updates_enabled{true};
// Attributes pertaining to the case where update checks are allowed.
//
// A target version prefix, if imposed by policy; otherwise, an empty string.
std::string target_version_prefix;
// Specifies whether rollback images are allowed by device policy.
- bool rollback_allowed;
+ bool rollback_allowed{false};
// Specifies if rollbacks should attempt to preserve some system state.
- bool rollback_data_save_requested;
+ bool rollback_data_save_requested{false};
// Specifies the number of Chrome milestones rollback should be allowed,
// starting from the stable version at any time. Value is -1 if unspecified
// (e.g. no device policy is available yet), in this case no version
// roll-forward should happen.
- int rollback_allowed_milestones;
+ int rollback_allowed_milestones{0};
// Whether a rollback with data save should be initiated on channel
// downgrade (e.g. beta to stable).
bool rollback_on_channel_downgrade{false};
@@ -67,7 +68,7 @@
std::string lts_tag;
// Whether the allowed update is interactive (user-initiated) or periodic.
- bool interactive;
+ bool interactive{false};
};
// Input arguments to UpdateCanStart.
diff --git a/update_manager/real_updater_provider_unittest.cc b/update_manager/real_updater_provider_unittest.cc
index f0804c4..06808b8 100644
--- a/update_manager/real_updater_provider_unittest.cc
+++ b/update_manager/real_updater_provider_unittest.cc
@@ -327,7 +327,7 @@
TEST_F(UmRealUpdaterProviderTest, GetCurrChannelOkay) {
const string kChannelName("foo-channel");
OmahaRequestParams request_params(&fake_sys_state_);
- request_params.Init("", "", false);
+ request_params.Init("", "", {});
request_params.set_current_channel(kChannelName);
fake_sys_state_.set_request_params(&request_params);
UmTestUtils::ExpectVariableHasValue(kChannelName,
@@ -336,7 +336,7 @@
TEST_F(UmRealUpdaterProviderTest, GetCurrChannelFailEmpty) {
OmahaRequestParams request_params(&fake_sys_state_);
- request_params.Init("", "", false);
+ request_params.Init("", "", {});
request_params.set_current_channel("");
fake_sys_state_.set_request_params(&request_params);
UmTestUtils::ExpectVariableNotSet(provider_->var_curr_channel());
@@ -345,7 +345,7 @@
TEST_F(UmRealUpdaterProviderTest, GetNewChannelOkay) {
const string kChannelName("foo-channel");
OmahaRequestParams request_params(&fake_sys_state_);
- request_params.Init("", "", false);
+ request_params.Init("", "", {});
request_params.set_target_channel(kChannelName);
fake_sys_state_.set_request_params(&request_params);
UmTestUtils::ExpectVariableHasValue(kChannelName,
@@ -354,7 +354,7 @@
TEST_F(UmRealUpdaterProviderTest, GetNewChannelFailEmpty) {
OmahaRequestParams request_params(&fake_sys_state_);
- request_params.Init("", "", false);
+ request_params.Init("", "", {});
request_params.set_target_channel("");
fake_sys_state_.set_request_params(&request_params);
UmTestUtils::ExpectVariableNotSet(provider_->var_new_channel());