update_engine: Reorganize UpdateCheckAllowed policy
The current policy management is through class templates. It has been
quite daunting to deal with the way policies are worked around. Each
policy class has several functions and by addition of new functions, we
will be in a situation that each policy implementation will implemented
one or two functions and ignores the rest. Hence not a very good design.
In this CL we introduced a new PolicyEvaluator concept which is
repsonsible to take a policy and its associated data and evaulate that
policy. Policies can be recursive and one policy can evaluate other
policies too. The new design does not use class templates anymore hence
increases the readability and extensibility of the design.
BUG=b:179419726
TEST=cros_workon_make --board reef --test update_engine
Change-Id: If213fa2dac381ebc1ddb1eb7058aa45d6cb7a4c7
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/1760445
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Jae Hoon Kim <kimjae@chromium.org>
Commit-Queue: Amin Hassani <ahassani@chromium.org>
diff --git a/BUILD.gn b/BUILD.gn
index 80fc867..1678dea 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -226,6 +226,7 @@
"update_manager/official_build_check_policy_impl.cc",
"update_manager/out_of_box_experience_policy_impl.cc",
"update_manager/policy.cc",
+ "update_manager/policy_evaluator.cc",
"update_manager/policy_test_utils.cc",
"update_manager/real_config_provider.cc",
"update_manager/real_device_policy_provider.cc",
@@ -393,6 +394,7 @@
"cros/fake_system_state.cc",
"payload_consumer/fake_file_descriptor.cc",
"payload_generator/fake_filesystem.cc",
+ "testrunner.cc",
"update_manager/umtest_utils.cc",
]
@@ -523,7 +525,6 @@
"payload_generator/payload_signer_unittest.cc",
"payload_generator/squashfs_filesystem_unittest.cc",
"payload_generator/zip_unittest.cc",
- "testrunner.cc",
"update_boot_flags_action_unittest.cc",
"update_manager/boxed_value_unittest.cc",
"update_manager/chromeos_policy_unittest.cc",
@@ -532,6 +533,7 @@
"update_manager/evaluation_context_unittest.cc",
"update_manager/generic_variables_unittest.cc",
"update_manager/minimum_version_policy_impl_unittest.cc",
+ "update_manager/next_update_check_policy_impl_unittest.cc",
"update_manager/prng_unittest.cc",
"update_manager/real_device_policy_provider_unittest.cc",
"update_manager/real_random_provider_unittest.cc",
diff --git a/cros/omaha_request_params.h b/cros/omaha_request_params.h
index 3699e7d..247e782 100644
--- a/cros/omaha_request_params.h
+++ b/cros/omaha_request_params.h
@@ -32,6 +32,7 @@
#include "update_engine/common/prefs.h"
#include "update_engine/cros/image_properties.h"
#include "update_engine/update_manager/policy.h"
+#include "update_engine/update_manager/update_check_allowed_policy.h"
// This gathers local system information and prepares info used by the
// Omaha request action.
diff --git a/cros/update_attempter.cc b/cros/update_attempter.cc
index f696f99..a0290ba 100644
--- a/cros/update_attempter.cc
+++ b/cros/update_attempter.cc
@@ -84,6 +84,8 @@
using chromeos_update_manager::EvalStatus;
using chromeos_update_manager::Policy;
using chromeos_update_manager::StagingCase;
+using chromeos_update_manager::UpdateCheckAllowedPolicy;
+using chromeos_update_manager::UpdateCheckAllowedPolicyData;
using chromeos_update_manager::UpdateCheckParams;
using std::map;
using std::string;
@@ -168,15 +170,14 @@
if (IsBusyOrUpdateScheduled())
return false;
- chromeos_update_manager::UpdateManager* const update_manager =
- SystemState::Get()->update_manager();
- CHECK(update_manager);
- Callback<void(EvalStatus, const UpdateCheckParams&)> callback =
- Bind(&UpdateAttempter::OnUpdateScheduled, base::Unretained(this));
// We limit the async policy request to a reasonably short time, to avoid a
// starvation due to a transient bug.
- update_manager->AsyncPolicyRequestUpdateCheckAllowed(
- callback, &Policy::UpdateCheckAllowed);
+ policy_data_.reset(new UpdateCheckAllowedPolicyData());
+ SystemState::Get()->update_manager()->PolicyRequest2(
+ std::make_unique<UpdateCheckAllowedPolicy>(),
+ policy_data_, // Do not move because we don't want transfer of ownership.
+ base::Bind(&UpdateAttempter::OnUpdateScheduled, base::Unretained(this)));
+
waiting_for_scheduled_check_ = true;
return true;
}
@@ -1041,8 +1042,8 @@
return rc == 0;
}
-void UpdateAttempter::OnUpdateScheduled(EvalStatus status,
- const UpdateCheckParams& params) {
+void UpdateAttempter::OnUpdateScheduled(EvalStatus status) {
+ const UpdateCheckParams& params = policy_data_->update_check_params;
waiting_for_scheduled_check_ = false;
if (status == EvalStatus::kSucceeded) {
diff --git a/cros/update_attempter.h b/cros/update_attempter.h
index e9d44b1..43d89e9 100644
--- a/cros/update_attempter.h
+++ b/cros/update_attempter.h
@@ -47,6 +47,7 @@
#include "update_engine/payload_consumer/postinstall_runner_action.h"
#include "update_engine/update_manager/policy.h"
#include "update_engine/update_manager/staging_utils.h"
+#include "update_engine/update_manager/update_check_allowed_policy.h"
#include "update_engine/update_manager/update_manager.h"
namespace policy {
@@ -413,12 +414,11 @@
// success.
bool RebootDirectly();
- // Callback for the async UpdateCheckAllowed policy request. If |status| is
+ // Callback for the async update check allowed policy request. If |status| is
// |EvalStatus::kSucceeded|, either runs or suppresses periodic update checks,
- // based on the content of |params|. Otherwise, retries the policy request.
- void OnUpdateScheduled(
- chromeos_update_manager::EvalStatus status,
- const chromeos_update_manager::UpdateCheckParams& params);
+ // based on the content of |policy_data_|. Otherwise, retries the policy
+ // request.
+ void OnUpdateScheduled(chromeos_update_manager::EvalStatus status);
// Updates the time an update was last attempted to the current time.
void UpdateLastCheckedTime();
@@ -577,6 +577,10 @@
// Interface for excluder.
std::unique_ptr<ExcluderInterface> excluder_;
+ // Data used by the update check allowed policies.
+ std::shared_ptr<chromeos_update_manager::UpdateCheckAllowedPolicyData>
+ policy_data_;
+
DISALLOW_COPY_AND_ASSIGN(UpdateAttempter);
};
diff --git a/cros/update_attempter_unittest.cc b/cros/update_attempter_unittest.cc
index 90debe3..838ca18 100644
--- a/cros/update_attempter_unittest.cc
+++ b/cros/update_attempter_unittest.cc
@@ -56,13 +56,12 @@
#include "update_engine/payload_consumer/payload_constants.h"
#include "update_engine/payload_consumer/postinstall_runner_action.h"
#include "update_engine/update_boot_flags_action.h"
-#include "update_engine/update_manager/mock_update_manager.h"
using base::Time;
using base::TimeDelta;
using chromeos_update_manager::EvalStatus;
-using chromeos_update_manager::MockUpdateManager;
using chromeos_update_manager::StagingSchedule;
+using chromeos_update_manager::UpdateCheckAllowedPolicyData;
using chromeos_update_manager::UpdateCheckParams;
using policy::DevicePolicy;
using std::map;
@@ -225,7 +224,6 @@
FakeSystemState::Get()->set_connection_manager(&mock_connection_manager);
FakeSystemState::Get()->set_update_attempter(&attempter_);
FakeSystemState::Get()->set_dlcservice(&mock_dlcservice_);
- FakeSystemState::Get()->set_update_manager(&mock_update_manager_);
loop_.SetAsCurrent();
prefs_ = FakeSystemState::Get()->fake_prefs();
@@ -316,7 +314,6 @@
OpenSSLWrapper openssl_wrapper_;
std::unique_ptr<CertificateChecker> certificate_checker_;
MockDlcService mock_dlcservice_;
- MockUpdateManager mock_update_manager_;
NiceMock<MockActionProcessor>* processor_;
NiceMock<MockConnectionManager> mock_connection_manager;
@@ -1753,27 +1750,28 @@
EXPECT_TRUE(attempter_.IsBusyOrUpdateScheduled());
}
+// TODO(ahassani): Refactor these to not use the |policy_data_| of |attempter_|
+// directly.
TEST_F(UpdateAttempterTest, UpdateAttemptFlagsCachedAtUpdateStart) {
attempter_.SetUpdateAttemptFlags(UpdateAttemptFlags::kFlagRestrictDownload);
-
- UpdateCheckParams params = {.updates_enabled = true};
- attempter_.OnUpdateScheduled(EvalStatus::kSucceeded, params);
-
+ attempter_.policy_data_.reset(
+ new UpdateCheckAllowedPolicyData({.updates_enabled = true}));
+ attempter_.OnUpdateScheduled(EvalStatus::kSucceeded);
EXPECT_EQ(UpdateAttemptFlags::kFlagRestrictDownload,
attempter_.GetCurrentUpdateAttemptFlags());
}
TEST_F(UpdateAttempterTest, RollbackNotAllowed) {
- UpdateCheckParams params = {.updates_enabled = true,
- .rollback_allowed = false};
- attempter_.OnUpdateScheduled(EvalStatus::kSucceeded, params);
+ attempter_.policy_data_.reset(new UpdateCheckAllowedPolicyData(
+ {.updates_enabled = true, .rollback_allowed = false}));
+ attempter_.OnUpdateScheduled(EvalStatus::kSucceeded);
EXPECT_FALSE(FakeSystemState::Get()->request_params()->rollback_allowed());
}
TEST_F(UpdateAttempterTest, RollbackAllowed) {
- UpdateCheckParams params = {.updates_enabled = true,
- .rollback_allowed = true};
- attempter_.OnUpdateScheduled(EvalStatus::kSucceeded, params);
+ attempter_.policy_data_.reset(new UpdateCheckAllowedPolicyData(
+ {.updates_enabled = true, .rollback_allowed = true}));
+ attempter_.OnUpdateScheduled(EvalStatus::kSucceeded);
EXPECT_TRUE(FakeSystemState::Get()->request_params()->rollback_allowed());
}
@@ -2143,8 +2141,6 @@
}
TEST_F(UpdateAttempterTest, ScheduleUpdateSpamHandlerTest) {
- EXPECT_CALL(mock_update_manager_, AsyncPolicyRequestUpdateCheckAllowed(_, _))
- .Times(1);
EXPECT_TRUE(attempter_.ScheduleUpdates());
// Now there is an update scheduled which means that all subsequent
// |ScheduleUpdates()| should fail.
@@ -2165,7 +2161,9 @@
attempter_.DisableScheduleUpdates();
// Invocation
- attempter_.OnUpdateScheduled(ous_params_.status, ous_params_.params);
+ attempter_.policy_data_.reset(
+ new UpdateCheckAllowedPolicyData(ous_params_.params));
+ attempter_.OnUpdateScheduled(ous_params_.status);
// Verify
EXPECT_EQ(ous_params_.exit_status, attempter_.status());
diff --git a/update_manager/chromeos_policy.cc b/update_manager/chromeos_policy.cc
index 59761e3..ac227fe 100644
--- a/update_manager/chromeos_policy.cc
+++ b/update_manager/chromeos_policy.cc
@@ -28,6 +28,7 @@
#include "update_engine/common/error_code.h"
#include "update_engine/common/error_code_utils.h"
+#include "update_engine/common/system_state.h"
#include "update_engine/common/utils.h"
#include "update_engine/update_manager/device_policy_provider.h"
#include "update_engine/update_manager/enough_slots_ab_updates_policy_impl.h"
@@ -40,6 +41,8 @@
#include "update_engine/update_manager/policy_utils.h"
#include "update_engine/update_manager/recovery_policy.h"
#include "update_engine/update_manager/shill_provider.h"
+// TODO(b/179419726): Remove.
+#include "update_engine/update_manager/update_check_allowed_policy.h"
#include "update_engine/update_manager/update_time_restrictions_policy_impl.h"
using base::Time;
@@ -48,6 +51,7 @@
using chromeos_update_engine::ConnectionType;
using chromeos_update_engine::ErrorCode;
using chromeos_update_engine::InstallPlan;
+using chromeos_update_engine::SystemState;
using std::get;
using std::min;
using std::set;
@@ -202,23 +206,16 @@
return std::make_unique<ChromeOSPolicy>();
}
-const NextUpdateCheckPolicyConstants
- ChromeOSPolicy::kNextUpdateCheckPolicyConstants = {
- .timeout_initial_interval = 7 * 60,
- .timeout_periodic_interval = 45 * 60,
- .timeout_max_backoff_interval = 4 * 60 * 60,
- .timeout_regular_fuzz = 10 * 60,
- .attempt_backoff_max_interval_in_days = 16,
- .attempt_backoff_fuzz_in_hours = 12,
-};
-
const int ChromeOSPolicy::kMaxP2PAttempts = 10;
const int ChromeOSPolicy::kMaxP2PAttemptsPeriodInSeconds = 5 * 24 * 60 * 60;
-EvalStatus ChromeOSPolicy::UpdateCheckAllowed(EvaluationContext* ec,
+// TODO(b/179419726): Move to update_check_allowed_policy.cc.
+EvalStatus UpdateCheckAllowedPolicy::Evaluate(EvaluationContext* ec,
State* state,
string* error,
- UpdateCheckParams* result) const {
+ PolicyDataInterface* data) const {
+ UpdateCheckParams* result =
+ UpdateCheckAllowedPolicyData::GetUpdateCheckParams(data);
// Set the default return values.
result->updates_enabled = true;
result->target_channel.clear();
@@ -236,10 +233,9 @@
OnlyUpdateOfficialBuildsPolicyImpl only_update_official_builds_policy;
InteractiveUpdatePolicyImpl interactive_update_policy;
OobePolicyImpl oobe_policy;
- NextUpdateCheckTimePolicyImpl next_update_check_time_policy(
- kNextUpdateCheckPolicyConstants);
+ NextUpdateCheckTimePolicyImpl next_update_check_time_policy;
- vector<Policy const*> policies_to_consult = {
+ vector<PolicyInterface* const> policies_to_consult = {
// If in recovery mode, always check for update.
&recovery_policy,
@@ -268,19 +264,14 @@
// has been setup, consult the policies. If none of the policies make a
// definitive decisions about whether or not to check for updates, then allow
// the update check to happen.
- EvalStatus status = ConsultPolicies(policies_to_consult,
- &Policy::UpdateCheckAllowed,
- ec,
- state,
- error,
- result);
- if (EvalStatus::kContinue != status) {
- return status;
- } else {
- // It is time to check for an update.
- LOG(INFO) << "Allowing update check.";
- return EvalStatus::kSucceeded;
+ for (auto policy : policies_to_consult) {
+ EvalStatus status = policy->Evaluate(ec, state, error, data);
+ if (status != EvalStatus::kContinue) {
+ return status;
+ }
}
+ LOG(INFO) << "Allowing update check.";
+ return EvalStatus::kSucceeded;
}
EvalStatus ChromeOSPolicy::UpdateCanBeApplied(EvaluationContext* ec,
@@ -350,12 +341,13 @@
result->scatter_check_threshold = update_state.scatter_check_threshold;
// Make sure that we're not due for an update check.
- UpdateCheckParams check_result;
- EvalStatus check_status = UpdateCheckAllowed(ec, state, error, &check_result);
+ UpdateCheckAllowedPolicy uca_policy;
+ UpdateCheckAllowedPolicyData uca_data;
+ EvalStatus check_status = uca_policy.Evaluate(ec, state, error, &uca_data);
if (check_status == EvalStatus::kFailed)
return EvalStatus::kFailed;
bool is_check_due = (check_status == EvalStatus::kSucceeded &&
- check_result.updates_enabled == true);
+ uca_data.update_check_params.updates_enabled == true);
// Check whether backoff applies, and if not then which URL can be used for
// downloading. These require scanning the download error log, and so they are
diff --git a/update_manager/chromeos_policy.h b/update_manager/chromeos_policy.h
index 3c196da..8111cdd 100644
--- a/update_manager/chromeos_policy.h
+++ b/update_manager/chromeos_policy.h
@@ -17,6 +17,7 @@
#ifndef UPDATE_ENGINE_UPDATE_MANAGER_CHROMEOS_POLICY_H_
#define UPDATE_ENGINE_UPDATE_MANAGER_CHROMEOS_POLICY_H_
+#include <memory>
#include <string>
#include <base/time/time.h>
@@ -47,17 +48,10 @@
int check_threshold;
};
-// ChromeOSPolicy implements the policy-related logic used in ChromeOS.
class ChromeOSPolicy : public Policy {
public:
- ChromeOSPolicy() {}
- ~ChromeOSPolicy() override {}
-
- // Policy overrides.
- EvalStatus UpdateCheckAllowed(EvaluationContext* ec,
- State* state,
- std::string* error,
- UpdateCheckParams* result) const override;
+ ChromeOSPolicy() = default;
+ ~ChromeOSPolicy() override = default;
EvalStatus UpdateCanBeApplied(
EvaluationContext* ec,
@@ -112,8 +106,6 @@
// Auxiliary constant (zero by default).
const base::TimeDelta kZeroInterval;
- static const NextUpdateCheckPolicyConstants kNextUpdateCheckPolicyConstants;
-
// Maximum number of times we'll allow using P2P for the same update payload.
static const int kMaxP2PAttempts;
// Maximum period of time allowed for download a payload via P2P, in seconds.
diff --git a/update_manager/chromeos_policy_unittest.cc b/update_manager/chromeos_policy_unittest.cc
index bfe200f..57b1ebf 100644
--- a/update_manager/chromeos_policy_unittest.cc
+++ b/update_manager/chromeos_policy_unittest.cc
@@ -18,9 +18,14 @@
#include <memory>
#include <set>
+#include <utility>
+// TODO(b/179419726): Remove.
+#include "update_engine/update_manager/enterprise_device_policy_impl.h"
#include "update_engine/update_manager/next_update_check_policy_impl.h"
#include "update_engine/update_manager/policy_test_utils.h"
+#include "update_engine/update_manager/update_check_allowed_policy.h"
+#include "update_engine/update_manager/update_check_allowed_policy_data.h"
#include "update_engine/update_manager/weekly_time.h"
using base::Time;
@@ -38,6 +43,11 @@
protected:
UmChromeOSPolicyTest() : UmPolicyTestBase() {
policy_ = std::make_unique<ChromeOSPolicy>();
+
+ policy_data_.reset(new UpdateCheckAllowedPolicyData());
+ policy_2_.reset(new UpdateCheckAllowedPolicy());
+
+ uca_data_ = static_cast<typeof(uca_data_)>(policy_data_.get());
}
void SetUp() override {
@@ -101,7 +111,7 @@
Time next_update_check;
CallMethodWithContext(&NextUpdateCheckTimePolicyImpl::NextUpdateCheckTime,
&next_update_check,
- ChromeOSPolicy::kNextUpdateCheckPolicyConstants);
+ kNextUpdateCheckPolicyConstants);
SetUpDefaultState();
SetUpDefaultDevicePolicy();
Time curr_time = next_update_check;
@@ -112,6 +122,48 @@
fake_clock_->SetWallclockTime(curr_time);
}
+ // Sets up a test with the given intervals and the current fake wallclock
+ // time.
+ void TestDisallowedTimeIntervals(const WeeklyTimeIntervalVector& intervals,
+ const ErrorCode& expected_error_code,
+ bool is_rollback,
+ bool expected_can_download_be_canceled) {
+ SetUpDefaultTimeProvider();
+ fake_state_.device_policy_provider()
+ ->var_disallowed_time_intervals()
+ ->reset(new WeeklyTimeIntervalVector(intervals));
+
+ // Check that |expected_status| matches the value of |UpdateCanBeApplied|.
+ ErrorCode result;
+ InstallPlan install_plan{.is_rollback = is_rollback};
+ ExpectPolicyStatus(EvalStatus::kSucceeded,
+ &Policy::UpdateCanBeApplied,
+ &result,
+ &install_plan);
+ EXPECT_EQ(result, expected_error_code);
+ EXPECT_EQ(install_plan.can_download_be_canceled,
+ expected_can_download_be_canceled);
+ }
+
+ UpdateCheckAllowedPolicyData* uca_data_;
+};
+
+// TODO(b/179419726): Merge into enterprise_device_policy_impl_unittest.cc.
+class UmEnterprisePolicyTest : public UmPolicyTestBase {
+ protected:
+ UmEnterprisePolicyTest() : UmPolicyTestBase() {
+ policy_data_.reset(new UpdateCheckAllowedPolicyData());
+ policy_2_.reset(new EnterpriseDevicePolicyImpl());
+
+ uca_data_ = static_cast<typeof(uca_data_)>(policy_data_.get());
+ }
+
+ void SetUp() override {
+ UmPolicyTestBase::SetUp();
+ fake_state_.device_policy_provider()->var_device_policy_is_loaded()->reset(
+ new bool(true));
+ }
+
// Sets the policies required for a kiosk app to control Chrome OS version:
// - AllowKioskAppControlChromeVersion = True
// - UpdateDisabled = True
@@ -132,10 +184,6 @@
// UpdateCheckParams.rollback_allowed.
bool TestRollbackAllowed(bool set_policy,
RollbackToTargetVersion rollback_to_target_version) {
- // Update check is allowed, response includes attributes for use in the
- // request.
- SetUpdateCheckAllowed(true);
-
if (set_policy) {
// Override RollbackToTargetVersion device policy attribute.
fake_state_.device_policy_provider()
@@ -143,34 +191,11 @@
->reset(new RollbackToTargetVersion(rollback_to_target_version));
}
- UpdateCheckParams result;
- ExpectPolicyStatus(
- EvalStatus::kSucceeded, &Policy::UpdateCheckAllowed, &result);
- return result.rollback_allowed;
+ EXPECT_EQ(EvalStatus::kContinue, evaluator_->Evaluate());
+ return uca_data_->update_check_params.rollback_allowed;
}
- // Sets up a test with the given intervals and the current fake wallclock
- // time.
- void TestDisallowedTimeIntervals(const WeeklyTimeIntervalVector& intervals,
- const ErrorCode& expected_error_code,
- bool is_rollback,
- bool expected_can_download_be_canceled) {
- SetUpDefaultTimeProvider();
- fake_state_.device_policy_provider()
- ->var_disallowed_time_intervals()
- ->reset(new WeeklyTimeIntervalVector(intervals));
-
- // Check that |expected_status| matches the value of UpdateCanBeApplied.
- ErrorCode result;
- InstallPlan install_plan{.is_rollback = is_rollback};
- ExpectPolicyStatus(EvalStatus::kSucceeded,
- &Policy::UpdateCanBeApplied,
- &result,
- &install_plan);
- EXPECT_EQ(result, expected_error_code);
- EXPECT_EQ(install_plan.can_download_be_canceled,
- expected_can_download_be_canceled);
- }
+ UpdateCheckAllowedPolicyData* uca_data_;
};
TEST_F(UmChromeOSPolicyTest, UpdateCheckAllowedWaitsForTheTimeout) {
@@ -185,9 +210,7 @@
new Time(last_checked_time));
CallMethodWithContext(&NextUpdateCheckTimePolicyImpl::NextUpdateCheckTime,
&next_update_check,
- ChromeOSPolicy::kNextUpdateCheckPolicyConstants);
-
- UpdateCheckParams result;
+ kNextUpdateCheckPolicyConstants);
// Check that the policy blocks until the next_update_check is reached.
SetUpDefaultClock();
@@ -195,18 +218,17 @@
fake_state_.updater_provider()->var_last_checked_time()->reset(
new Time(last_checked_time));
fake_clock_->SetWallclockTime(next_update_check - TimeDelta::FromSeconds(1));
- ExpectPolicyStatus(
- EvalStatus::kAskMeAgainLater, &Policy::UpdateCheckAllowed, &result);
+
+ EXPECT_EQ(EvalStatus::kAskMeAgainLater, evaluator_->Evaluate());
SetUpDefaultClock();
SetUpDefaultState();
fake_state_.updater_provider()->var_last_checked_time()->reset(
new Time(last_checked_time));
fake_clock_->SetWallclockTime(next_update_check + TimeDelta::FromSeconds(1));
- ExpectPolicyStatus(
- EvalStatus::kSucceeded, &Policy::UpdateCheckAllowed, &result);
- EXPECT_TRUE(result.updates_enabled);
- EXPECT_FALSE(result.interactive);
+ EXPECT_EQ(EvalStatus::kSucceeded, evaluator_->Evaluate());
+ EXPECT_TRUE(uca_data_->update_check_params.updates_enabled);
+ EXPECT_FALSE(uca_data_->update_check_params.interactive);
}
TEST_F(UmChromeOSPolicyTest, UpdateCheckAllowedWaitsForOOBE) {
@@ -221,7 +243,7 @@
new Time(last_checked_time));
CallMethodWithContext(&NextUpdateCheckTimePolicyImpl::NextUpdateCheckTime,
&next_update_check,
- ChromeOSPolicy::kNextUpdateCheckPolicyConstants);
+ kNextUpdateCheckPolicyConstants);
SetUpDefaultClock();
SetUpDefaultState();
@@ -230,9 +252,7 @@
fake_clock_->SetWallclockTime(next_update_check + TimeDelta::FromSeconds(1));
fake_state_.system_provider()->var_is_oobe_complete()->reset(new bool(false));
- UpdateCheckParams result;
- ExpectPolicyStatus(
- EvalStatus::kAskMeAgainLater, &Policy::UpdateCheckAllowed, &result);
+ EXPECT_EQ(EvalStatus::kAskMeAgainLater, evaluator_->Evaluate());
// Now check that it is allowed if OOBE is completed.
SetUpDefaultClock();
@@ -240,10 +260,10 @@
fake_state_.updater_provider()->var_last_checked_time()->reset(
new Time(last_checked_time));
fake_clock_->SetWallclockTime(next_update_check + TimeDelta::FromSeconds(1));
- ExpectPolicyStatus(
- EvalStatus::kSucceeded, &Policy::UpdateCheckAllowed, &result);
- EXPECT_TRUE(result.updates_enabled);
- EXPECT_FALSE(result.interactive);
+
+ EXPECT_EQ(EvalStatus::kSucceeded, evaluator_->Evaluate());
+ EXPECT_TRUE(uca_data_->update_check_params.updates_enabled);
+ EXPECT_FALSE(uca_data_->update_check_params.interactive);
}
TEST_F(UmChromeOSPolicyTest, UpdateCheckAllowedWithAttributes) {
@@ -266,64 +286,62 @@
fake_state_.device_policy_provider()->var_quick_fix_build_token()->reset(
new string("foo-token"));
- UpdateCheckParams result;
- ExpectPolicyStatus(
- EvalStatus::kSucceeded, &Policy::UpdateCheckAllowed, &result);
- EXPECT_TRUE(result.updates_enabled);
- EXPECT_EQ("1.2", result.target_version_prefix);
- EXPECT_EQ(5, result.rollback_allowed_milestones);
- EXPECT_EQ("foo-channel", result.target_channel);
- EXPECT_EQ("foo-hint", result.lts_tag);
- EXPECT_EQ("foo-token", result.quick_fix_build_token);
- EXPECT_FALSE(result.interactive);
+ EXPECT_EQ(EvalStatus::kSucceeded, evaluator_->Evaluate());
+ EXPECT_TRUE(uca_data_->update_check_params.updates_enabled);
+ EXPECT_EQ("1.2", uca_data_->update_check_params.target_version_prefix);
+ EXPECT_EQ(5, uca_data_->update_check_params.rollback_allowed_milestones);
+ EXPECT_EQ("foo-channel", uca_data_->update_check_params.target_channel);
+ EXPECT_EQ("foo-hint", uca_data_->update_check_params.lts_tag);
+ EXPECT_EQ("foo-token", uca_data_->update_check_params.quick_fix_build_token);
+ EXPECT_FALSE(uca_data_->update_check_params.interactive);
}
-TEST_F(UmChromeOSPolicyTest, UpdateCheckAllowedRollbackAndPowerwash) {
+TEST_F(UmEnterprisePolicyTest, UpdateCheckAllowedRollbackAndPowerwash) {
EXPECT_TRUE(TestRollbackAllowed(
true, RollbackToTargetVersion::kRollbackAndPowerwash));
}
-TEST_F(UmChromeOSPolicyTest, UpdateCheckAllowedRollbackAndRestoreIfPossible) {
+TEST_F(UmEnterprisePolicyTest, UpdateCheckAllowedRollbackAndRestoreIfPossible) {
// We're doing rollback even if we don't support data save and restore.
EXPECT_TRUE(TestRollbackAllowed(
true, RollbackToTargetVersion::kRollbackAndRestoreIfPossible));
}
-TEST_F(UmChromeOSPolicyTest, UpdateCheckAllowedRollbackDisabled) {
+TEST_F(UmEnterprisePolicyTest, UpdateCheckAllowedRollbackDisabled) {
EXPECT_FALSE(TestRollbackAllowed(true, RollbackToTargetVersion::kDisabled));
}
-TEST_F(UmChromeOSPolicyTest, UpdateCheckAllowedRollbackUnspecified) {
+TEST_F(UmEnterprisePolicyTest, UpdateCheckAllowedRollbackUnspecified) {
EXPECT_FALSE(
TestRollbackAllowed(true, RollbackToTargetVersion::kUnspecified));
}
-TEST_F(UmChromeOSPolicyTest, UpdateCheckAllowedRollbackNotSet) {
+TEST_F(UmEnterprisePolicyTest, UpdateCheckAllowedRollbackNotSet) {
EXPECT_FALSE(
TestRollbackAllowed(false, RollbackToTargetVersion::kUnspecified));
}
-TEST_F(UmChromeOSPolicyTest, UpdateCheckAllowedKioskRollbackAllowed) {
+TEST_F(UmEnterprisePolicyTest, UpdateCheckAllowedKioskRollbackAllowed) {
SetKioskAppControlsChromeOsVersion();
EXPECT_TRUE(TestRollbackAllowed(
true, RollbackToTargetVersion::kRollbackAndPowerwash));
}
-TEST_F(UmChromeOSPolicyTest, UpdateCheckAllowedKioskRollbackDisabled) {
+TEST_F(UmEnterprisePolicyTest, UpdateCheckAllowedKioskRollbackDisabled) {
SetKioskAppControlsChromeOsVersion();
EXPECT_FALSE(TestRollbackAllowed(true, RollbackToTargetVersion::kDisabled));
}
-TEST_F(UmChromeOSPolicyTest, UpdateCheckAllowedKioskRollbackUnspecified) {
+TEST_F(UmEnterprisePolicyTest, UpdateCheckAllowedKioskRollbackUnspecified) {
SetKioskAppControlsChromeOsVersion();
EXPECT_FALSE(
TestRollbackAllowed(true, RollbackToTargetVersion::kUnspecified));
}
-TEST_F(UmChromeOSPolicyTest, UpdateCheckAllowedKioskRollbackNotSet) {
+TEST_F(UmEnterprisePolicyTest, UpdateCheckAllowedKioskRollbackNotSet) {
SetKioskAppControlsChromeOsVersion();
EXPECT_FALSE(
@@ -338,9 +356,7 @@
fake_state_.system_provider()->var_is_official_build()->reset(
new bool(false));
- UpdateCheckParams result;
- ExpectPolicyStatus(
- EvalStatus::kAskMeAgainLater, &Policy::UpdateCheckAllowed, &result);
+ EXPECT_EQ(EvalStatus::kAskMeAgainLater, evaluator_->Evaluate());
}
TEST_F(UmChromeOSPolicyTest, TestUpdateCheckIntervalTimeout) {
@@ -351,16 +367,13 @@
new bool(false));
// The first time, update should not be allowed.
- UpdateCheckParams result;
- ExpectPolicyStatus(
- EvalStatus::kAskMeAgainLater, &Policy::UpdateCheckAllowed, &result);
+ EXPECT_EQ(EvalStatus::kAskMeAgainLater, evaluator_->Evaluate());
// After moving the time forward more than the update check interval, it
// should now allow for update.
fake_clock_->SetWallclockTime(fake_clock_->GetWallclockTime() +
TimeDelta::FromSeconds(11));
- ExpectPolicyStatus(
- EvalStatus::kSucceeded, &Policy::UpdateCheckAllowed, &result);
+ EXPECT_EQ(EvalStatus::kSucceeded, evaluator_->Evaluate());
}
TEST_F(UmChromeOSPolicyTest,
@@ -371,10 +384,8 @@
// NOLINTNEXTLINE(readability/casting)
fake_state_.system_provider()->var_num_slots()->reset(new unsigned int(1));
- UpdateCheckParams result;
- ExpectPolicyStatus(
- EvalStatus::kSucceeded, &Policy::UpdateCheckAllowed, &result);
- EXPECT_FALSE(result.updates_enabled);
+ EXPECT_EQ(EvalStatus::kSucceeded, evaluator_->Evaluate());
+ EXPECT_FALSE(uca_data_->update_check_params.updates_enabled);
}
TEST_F(UmChromeOSPolicyTest, UpdateCheckAllowedUpdatesDisabledByPolicy) {
@@ -385,9 +396,7 @@
fake_state_.device_policy_provider()->var_update_disabled()->reset(
new bool(true));
- UpdateCheckParams result;
- ExpectPolicyStatus(
- EvalStatus::kAskMeAgainLater, &Policy::UpdateCheckAllowed, &result);
+ EXPECT_EQ(EvalStatus::kAskMeAgainLater, evaluator_->Evaluate());
}
TEST_F(UmChromeOSPolicyTest,
@@ -399,11 +408,9 @@
fake_state_.updater_provider()->var_forced_update_requested()->reset(
new UpdateRequestStatus(UpdateRequestStatus::kInteractive));
- UpdateCheckParams result;
- ExpectPolicyStatus(
- EvalStatus::kSucceeded, &Policy::UpdateCheckAllowed, &result);
- EXPECT_TRUE(result.updates_enabled);
- EXPECT_TRUE(result.interactive);
+ EXPECT_EQ(EvalStatus::kSucceeded, evaluator_->Evaluate());
+ EXPECT_TRUE(uca_data_->update_check_params.updates_enabled);
+ EXPECT_TRUE(uca_data_->update_check_params.interactive);
}
TEST_F(UmChromeOSPolicyTest, UpdateCheckAllowedForcedUpdateRequestedPeriodic) {
@@ -414,31 +421,21 @@
fake_state_.updater_provider()->var_forced_update_requested()->reset(
new UpdateRequestStatus(UpdateRequestStatus::kPeriodic));
- UpdateCheckParams result;
- ExpectPolicyStatus(
- EvalStatus::kSucceeded, &Policy::UpdateCheckAllowed, &result);
- EXPECT_TRUE(result.updates_enabled);
- EXPECT_FALSE(result.interactive);
+ EXPECT_EQ(EvalStatus::kSucceeded, evaluator_->Evaluate());
+ EXPECT_TRUE(uca_data_->update_check_params.updates_enabled);
+ EXPECT_FALSE(uca_data_->update_check_params.interactive);
}
-TEST_F(UmChromeOSPolicyTest, UpdateCheckAllowedKioskPin) {
- // Update check is allowed.
- SetUpdateCheckAllowed(true);
-
+TEST_F(UmEnterprisePolicyTest, UpdateCheckAllowedKioskPin) {
SetKioskAppControlsChromeOsVersion();
- UpdateCheckParams result;
- ExpectPolicyStatus(
- EvalStatus::kSucceeded, &Policy::UpdateCheckAllowed, &result);
- EXPECT_TRUE(result.updates_enabled);
- EXPECT_EQ("1234.", result.target_version_prefix);
- EXPECT_FALSE(result.interactive);
+ EXPECT_EQ(EvalStatus::kContinue, evaluator_->Evaluate());
+ EXPECT_TRUE(uca_data_->update_check_params.updates_enabled);
+ EXPECT_EQ("1234.", uca_data_->update_check_params.target_version_prefix);
+ EXPECT_FALSE(uca_data_->update_check_params.interactive);
}
-TEST_F(UmChromeOSPolicyTest, UpdateCheckAllowedDisabledWhenNoKioskPin) {
- // Update check is allowed.
- SetUpdateCheckAllowed(true);
-
+TEST_F(UmEnterprisePolicyTest, UpdateCheckAllowedDisabledWhenNoKioskPin) {
// Disable AU policy is set but kiosk pin policy is set to false. Update is
// disabled in such case.
fake_state_.device_policy_provider()->var_update_disabled()->reset(
@@ -447,15 +444,11 @@
->var_allow_kiosk_app_control_chrome_version()
->reset(new bool(false));
- UpdateCheckParams result;
- ExpectPolicyStatus(
- EvalStatus::kAskMeAgainLater, &Policy::UpdateCheckAllowed, &result);
+ EXPECT_EQ(EvalStatus::kAskMeAgainLater, evaluator_->Evaluate());
}
-TEST_F(UmChromeOSPolicyTest, UpdateCheckAllowedKioskPinWithNoRequiredVersion) {
- // Update check is allowed.
- SetUpdateCheckAllowed(true);
-
+TEST_F(UmEnterprisePolicyTest,
+ UpdateCheckAllowedKioskPinWithNoRequiredVersion) {
// AU disabled, allow kiosk to pin but there is no kiosk required platform
// version (i.e. app does not provide the info). Update to latest in such
// case.
@@ -467,15 +460,13 @@
fake_state_.system_provider()->var_kiosk_required_platform_version()->reset(
new string());
- UpdateCheckParams result;
- ExpectPolicyStatus(
- EvalStatus::kSucceeded, &Policy::UpdateCheckAllowed, &result);
- EXPECT_TRUE(result.updates_enabled);
- EXPECT_TRUE(result.target_version_prefix.empty());
- EXPECT_FALSE(result.interactive);
+ EXPECT_EQ(EvalStatus::kContinue, evaluator_->Evaluate());
+ EXPECT_TRUE(uca_data_->update_check_params.updates_enabled);
+ EXPECT_TRUE(uca_data_->update_check_params.target_version_prefix.empty());
+ EXPECT_FALSE(uca_data_->update_check_params.interactive);
}
-TEST_F(UmChromeOSPolicyTest,
+TEST_F(UmEnterprisePolicyTest,
UpdateCheckAllowedKioskPinWithFailedGetRequiredVersionCall) {
// AU disabled, allow kiosk to pin but D-Bus call to get required platform
// version failed. Defer update check in this case.
@@ -487,9 +478,7 @@
fake_state_.system_provider()->var_kiosk_required_platform_version()->reset(
nullptr);
- UpdateCheckParams result;
- ExpectPolicyStatus(
- EvalStatus::kAskMeAgainLater, &Policy::UpdateCheckAllowed, &result);
+ EXPECT_EQ(EvalStatus::kAskMeAgainLater, evaluator_->Evaluate());
}
TEST_F(UmChromeOSPolicyTest, UpdateCanStartFailsCheckAllowedError) {
diff --git a/update_manager/default_policy.cc b/update_manager/default_policy.cc
index 0713e06..98f5397 100644
--- a/update_manager/default_policy.cc
+++ b/update_manager/default_policy.cc
@@ -16,10 +16,15 @@
#include "update_engine/common/system_state.h"
#include "update_engine/update_manager/default_policy.h"
+// TODO(b/179419726): Remove.
+#include "update_engine/update_manager/update_check_allowed_policy.h"
using chromeos_update_engine::ErrorCode;
using chromeos_update_engine::InstallPlan;
using chromeos_update_engine::SystemState;
+using std::string;
+
+namespace chromeos_update_manager {
namespace {
@@ -31,12 +36,14 @@
} // namespace
-namespace chromeos_update_manager {
-
-EvalStatus DefaultPolicy::UpdateCheckAllowed(EvaluationContext* ec,
- State* state,
- std::string* error,
- UpdateCheckParams* result) const {
+// TODO(b/179419726): Move to update_check_allowed_policy.cc.
+EvalStatus UpdateCheckAllowedPolicy::EvaluateDefault(
+ EvaluationContext* ec,
+ State* state,
+ string* error,
+ PolicyDataInterface* data) const {
+ UpdateCheckParams* result =
+ UpdateCheckAllowedPolicyData::GetUpdateCheckParams(data);
result->updates_enabled = true;
result->target_channel.clear();
result->lts_tag.clear();
diff --git a/update_manager/default_policy.h b/update_manager/default_policy.h
index c93bb46..e2a2b8f 100644
--- a/update_manager/default_policy.h
+++ b/update_manager/default_policy.h
@@ -26,48 +26,15 @@
namespace chromeos_update_manager {
-// Auxiliary state class for DefaultPolicy evaluations.
-//
-// IMPORTANT: The use of a state object in policies is generally forbidden, as
-// it was a design decision to keep policy calls side-effect free. We make an
-// exception here to ensure that DefaultPolicy indeed serves as a safe (and
-// secure) fallback option. This practice should be avoided when imlpementing
-// other policies.
-class DefaultPolicyState {
- public:
- DefaultPolicyState() {}
-
- bool IsLastCheckAllowedTimeSet() const {
- return last_check_allowed_time_ != base::Time::Max();
- }
-
- // Sets/returns the point time on the monotonic time scale when the latest
- // check allowed was recorded.
- void set_last_check_allowed_time(base::Time timestamp) {
- last_check_allowed_time_ = timestamp;
- }
- base::Time last_check_allowed_time() const {
- return last_check_allowed_time_;
- }
-
- private:
- base::Time last_check_allowed_time_ = base::Time::Max();
-};
-
// The DefaultPolicy is a safe Policy implementation that doesn't fail. The
// values returned by this policy are safe default in case of failure of the
// actual policy being used by the UpdateManager.
class DefaultPolicy : public Policy {
public:
- DefaultPolicy() : aux_state_(new DefaultPolicyState()) {}
+ DefaultPolicy() = default;
~DefaultPolicy() override = default;
// Policy overrides.
- EvalStatus UpdateCheckAllowed(EvaluationContext* ec,
- State* state,
- std::string* error,
- UpdateCheckParams* result) const override;
-
EvalStatus UpdateCanBeApplied(
EvaluationContext* ec,
State* state,
@@ -97,9 +64,6 @@
std::string PolicyName() const override { return "DefaultPolicy"; }
private:
- // An auxiliary state object.
- std::unique_ptr<DefaultPolicyState> aux_state_;
-
DISALLOW_COPY_AND_ASSIGN(DefaultPolicy);
};
diff --git a/update_manager/enough_slots_ab_updates_policy_impl.cc b/update_manager/enough_slots_ab_updates_policy_impl.cc
index 70f15d4..1e6b6f4 100644
--- a/update_manager/enough_slots_ab_updates_policy_impl.cc
+++ b/update_manager/enough_slots_ab_updates_policy_impl.cc
@@ -16,15 +16,19 @@
#include "update_engine/update_manager/enough_slots_ab_updates_policy_impl.h"
+#include "update_engine/update_manager/update_check_allowed_policy_data.h"
+
namespace chromeos_update_manager {
// Do not perform any updates if booted from removable device. This decision
// is final.
-EvalStatus EnoughSlotsAbUpdatesPolicyImpl::UpdateCheckAllowed(
+EvalStatus EnoughSlotsAbUpdatesPolicyImpl::Evaluate(
EvaluationContext* ec,
State* state,
std::string* error,
- UpdateCheckParams* result) const {
+ PolicyDataInterface* data) const {
+ UpdateCheckParams* result =
+ UpdateCheckAllowedPolicyData::GetUpdateCheckParams(data);
const auto* num_slots_p =
ec->GetValue(state->system_provider()->var_num_slots());
if (num_slots_p == nullptr || *num_slots_p < 2) {
diff --git a/update_manager/enough_slots_ab_updates_policy_impl.h b/update_manager/enough_slots_ab_updates_policy_impl.h
index 1d45389..fc4551e 100644
--- a/update_manager/enough_slots_ab_updates_policy_impl.h
+++ b/update_manager/enough_slots_ab_updates_policy_impl.h
@@ -19,21 +19,21 @@
#include <string>
-#include "update_engine/update_manager/policy_utils.h"
+#include "update_engine/update_manager/policy_interface.h"
namespace chromeos_update_manager {
// Do not perform any updates if booted from removable device.
-class EnoughSlotsAbUpdatesPolicyImpl : public PolicyImplBase {
+class EnoughSlotsAbUpdatesPolicyImpl : public PolicyInterface {
public:
EnoughSlotsAbUpdatesPolicyImpl() = default;
~EnoughSlotsAbUpdatesPolicyImpl() override = default;
- // Policy overrides.
- EvalStatus UpdateCheckAllowed(EvaluationContext* ec,
- State* state,
- std::string* error,
- UpdateCheckParams* result) const override;
+ // PolicyInterface overrides.
+ EvalStatus Evaluate(EvaluationContext* ec,
+ State* state,
+ std::string* error,
+ PolicyDataInterface* data) const override;
protected:
std::string PolicyName() const override {
diff --git a/update_manager/enterprise_device_policy_impl.cc b/update_manager/enterprise_device_policy_impl.cc
index 242774c..25c48c8 100644
--- a/update_manager/enterprise_device_policy_impl.cc
+++ b/update_manager/enterprise_device_policy_impl.cc
@@ -18,19 +18,23 @@
#include "update_engine/common/utils.h"
+#include "update_engine/update_manager/update_check_allowed_policy_data.h"
+
using std::string;
namespace chromeos_update_manager {
// Check to see if Enterprise-managed (has DevicePolicy) and/or Kiosk-mode. If
// so, then defer to those settings.
-EvalStatus EnterpriseDevicePolicyImpl::UpdateCheckAllowed(
+EvalStatus EnterpriseDevicePolicyImpl::Evaluate(
EvaluationContext* ec,
State* state,
string* error,
- UpdateCheckParams* result) const {
+ PolicyDataInterface* data) const {
DevicePolicyProvider* const dp_provider = state->device_policy_provider();
SystemProvider* const system_provider = state->system_provider();
+ UpdateCheckParams* result =
+ UpdateCheckAllowedPolicyData::GetUpdateCheckParams(data);
const bool* device_policy_is_loaded_p =
ec->GetValue(dp_provider->var_device_policy_is_loaded());
diff --git a/update_manager/enterprise_device_policy_impl.h b/update_manager/enterprise_device_policy_impl.h
index 4b97fda..5a6caff 100644
--- a/update_manager/enterprise_device_policy_impl.h
+++ b/update_manager/enterprise_device_policy_impl.h
@@ -19,13 +19,13 @@
#include <string>
-#include "update_engine/update_manager/policy_utils.h"
+#include "update_engine/update_manager/policy_interface.h"
namespace chromeos_update_manager {
// Check to see if Enterprise-managed (has DevicePolicy) and/or Kiosk-mode. If
// so, then defer to those settings.
-class EnterpriseDevicePolicyImpl : public PolicyImplBase {
+class EnterpriseDevicePolicyImpl : public PolicyInterface {
public:
EnterpriseDevicePolicyImpl() = default;
~EnterpriseDevicePolicyImpl() override = default;
@@ -34,10 +34,10 @@
return "EnterpriseDevicePolicyImpl";
}
// Policy overrides.
- EvalStatus UpdateCheckAllowed(EvaluationContext* ec,
- State* state,
- std::string* error,
- UpdateCheckParams* result) const override;
+ EvalStatus Evaluate(EvaluationContext* ec,
+ State* state,
+ std::string* error,
+ PolicyDataInterface* data) const override;
private:
DISALLOW_COPY_AND_ASSIGN(EnterpriseDevicePolicyImpl);
diff --git a/update_manager/enterprise_device_policy_impl_unittest.cc b/update_manager/enterprise_device_policy_impl_unittest.cc
index 30f54b1..0a2591a 100644
--- a/update_manager/enterprise_device_policy_impl_unittest.cc
+++ b/update_manager/enterprise_device_policy_impl_unittest.cc
@@ -17,15 +17,20 @@
#include "update_engine/update_manager/enterprise_device_policy_impl.h"
#include <memory>
+#include <utility>
#include "update_engine/update_manager/policy_test_utils.h"
+#include "update_engine/update_manager/update_check_allowed_policy_data.h"
namespace chromeos_update_manager {
class UmEnterpriseDevicePolicyImplTest : public UmPolicyTestBase {
protected:
UmEnterpriseDevicePolicyImplTest() : UmPolicyTestBase() {
- policy_ = std::make_unique<EnterpriseDevicePolicyImpl>();
+ policy_data_.reset(new UpdateCheckAllowedPolicyData());
+ policy_2_.reset(new EnterpriseDevicePolicyImpl);
+
+ uca_data_ = static_cast<typeof(uca_data_)>(policy_data_.get());
}
void SetUpDefaultState() override {
@@ -34,6 +39,8 @@
fake_state_.device_policy_provider()->var_device_policy_is_loaded()->reset(
new bool(true));
}
+
+ UpdateCheckAllowedPolicyData* uca_data_;
};
TEST_F(UmEnterpriseDevicePolicyImplTest, KioskAppVersionSet) {
@@ -46,10 +53,8 @@
fake_state_.system_provider()->var_kiosk_required_platform_version()->reset(
new std::string("1234.5.6"));
- UpdateCheckParams result;
- ExpectPolicyStatus(
- EvalStatus::kContinue, &Policy::UpdateCheckAllowed, &result);
- EXPECT_EQ(result.target_version_prefix, "1234.5.6");
+ EXPECT_EQ(EvalStatus::kContinue, evaluator_->Evaluate());
+ EXPECT_EQ(uca_data_->update_check_params.target_version_prefix, "1234.5.6");
}
TEST_F(UmEnterpriseDevicePolicyImplTest, KioskAppVersionUnreadableNoUpdate) {
@@ -62,9 +67,7 @@
fake_state_.system_provider()->var_kiosk_required_platform_version()->reset(
nullptr);
- UpdateCheckParams result;
- ExpectPolicyStatus(
- EvalStatus::kAskMeAgainLater, &Policy::UpdateCheckAllowed, &result);
+ EXPECT_EQ(EvalStatus::kAskMeAgainLater, evaluator_->Evaluate());
}
TEST_F(UmEnterpriseDevicePolicyImplTest, KioskAppVersionUnreadableUpdate) {
@@ -79,10 +82,8 @@
fake_state_.system_provider()->var_kiosk_required_platform_version()->reset(
new std::string(""));
- UpdateCheckParams result;
- ExpectPolicyStatus(
- EvalStatus::kContinue, &Policy::UpdateCheckAllowed, &result);
- EXPECT_EQ(result.target_version_prefix, "");
+ EXPECT_EQ(EvalStatus::kContinue, evaluator_->Evaluate());
+ EXPECT_EQ(uca_data_->update_check_params.target_version_prefix, "");
}
TEST_F(UmEnterpriseDevicePolicyImplTest,
@@ -103,10 +104,8 @@
fake_state_.system_provider()->var_chromeos_version()->reset(
new base::Version("1.0.0"));
- UpdateCheckParams result;
- ExpectPolicyStatus(
- EvalStatus::kContinue, &Policy::UpdateCheckAllowed, &result);
- EXPECT_EQ(result.target_version_prefix, "");
+ EXPECT_EQ(EvalStatus::kContinue, evaluator_->Evaluate());
+ EXPECT_EQ(uca_data_->update_check_params.target_version_prefix, "");
}
TEST_F(UmEnterpriseDevicePolicyImplTest,
@@ -127,9 +126,7 @@
fake_state_.system_provider()->var_chromeos_version()->reset(
new base::Version("2.0.0"));
- UpdateCheckParams result;
- ExpectPolicyStatus(
- EvalStatus::kAskMeAgainLater, &Policy::UpdateCheckAllowed, &result);
+ EXPECT_EQ(EvalStatus::kAskMeAgainLater, evaluator_->Evaluate());
}
TEST_F(UmEnterpriseDevicePolicyImplTest, ChannelDowngradeBehaviorNoRollback) {
@@ -138,10 +135,8 @@
fake_state_.device_policy_provider()->var_release_channel()->reset(
new std::string("stable-channel"));
- UpdateCheckParams result;
- ExpectPolicyStatus(
- EvalStatus::kContinue, &Policy::UpdateCheckAllowed, &result);
- EXPECT_FALSE(result.rollback_on_channel_downgrade);
+ EXPECT_EQ(EvalStatus::kContinue, evaluator_->Evaluate());
+ EXPECT_FALSE(uca_data_->update_check_params.rollback_on_channel_downgrade);
}
TEST_F(UmEnterpriseDevicePolicyImplTest, ChannelDowngradeBehaviorRollback) {
@@ -152,19 +147,15 @@
fake_state_.device_policy_provider()->var_channel_downgrade_behavior()->reset(
new ChannelDowngradeBehavior(ChannelDowngradeBehavior::kRollback));
- UpdateCheckParams result;
- ExpectPolicyStatus(
- EvalStatus::kContinue, &Policy::UpdateCheckAllowed, &result);
- EXPECT_TRUE(result.rollback_on_channel_downgrade);
+ EXPECT_EQ(EvalStatus::kContinue, evaluator_->Evaluate());
+ EXPECT_TRUE(uca_data_->update_check_params.rollback_on_channel_downgrade);
}
TEST_F(UmEnterpriseDevicePolicyImplTest, QuickFixBuildToken) {
fake_state_.device_policy_provider()->var_quick_fix_build_token()->reset(
new std::string("token"));
- UpdateCheckParams result;
- ExpectPolicyStatus(
- EvalStatus::kContinue, &Policy::UpdateCheckAllowed, &result);
- EXPECT_EQ(result.quick_fix_build_token, "token");
+ EXPECT_EQ(EvalStatus::kContinue, evaluator_->Evaluate());
+ EXPECT_EQ(uca_data_->update_check_params.quick_fix_build_token, "token");
}
} // namespace chromeos_update_manager
diff --git a/update_manager/evaluation_context.cc b/update_manager/evaluation_context.cc
index b86f41c..797d124 100644
--- a/update_manager/evaluation_context.cc
+++ b/update_manager/evaluation_context.cc
@@ -212,7 +212,7 @@
timeout = expiration;
// Store the reevaluation callback.
- callback_.reset(new Closure(callback));
+ callback_.reset(new Closure(std::move(callback)));
// Schedule a timeout event, if one is set.
if (!timeout.is_max()) {
diff --git a/update_manager/interactive_update_policy_impl.cc b/update_manager/interactive_update_policy_impl.cc
index 872dc5d..ad749ee 100644
--- a/update_manager/interactive_update_policy_impl.cc
+++ b/update_manager/interactive_update_policy_impl.cc
@@ -16,20 +16,24 @@
#include "update_engine/update_manager/interactive_update_policy_impl.h"
+#include "update_engine/update_manager/update_check_allowed_policy_data.h"
+
using chromeos_update_engine::ErrorCode;
using chromeos_update_engine::InstallPlan;
namespace chromeos_update_manager {
// Check to see if an interactive update was requested.
-EvalStatus InteractiveUpdatePolicyImpl::UpdateCheckAllowed(
+EvalStatus InteractiveUpdatePolicyImpl::Evaluate(
EvaluationContext* ec,
State* state,
std::string* error,
- UpdateCheckParams* result) const {
+ PolicyDataInterface* data) const {
bool interactive;
if (CheckInteractiveUpdateRequested(
ec, state->updater_provider(), &interactive)) {
+ UpdateCheckParams* result =
+ UpdateCheckAllowedPolicyData::GetUpdateCheckParams(data);
result->interactive = interactive;
LOG(INFO) << "Forced update signaled ("
<< (interactive ? "interactive" : "periodic")
diff --git a/update_manager/interactive_update_policy_impl.h b/update_manager/interactive_update_policy_impl.h
index 3690cfb..23d9d75 100644
--- a/update_manager/interactive_update_policy_impl.h
+++ b/update_manager/interactive_update_policy_impl.h
@@ -21,21 +21,23 @@
#include "update_engine/common/error_code.h"
#include "update_engine/payload_consumer/install_plan.h"
+#include "update_engine/update_manager/policy_interface.h"
#include "update_engine/update_manager/policy_utils.h"
namespace chromeos_update_manager {
// Check to see if an interactive update was requested.
-class InteractiveUpdatePolicyImpl : public PolicyImplBase {
+class InteractiveUpdatePolicyImpl : public PolicyImplBase,
+ public PolicyInterface {
public:
InteractiveUpdatePolicyImpl() = default;
~InteractiveUpdatePolicyImpl() override = default;
// Policy overrides.
- EvalStatus UpdateCheckAllowed(EvaluationContext* ec,
- State* state,
- std::string* error,
- UpdateCheckParams* result) const override;
+ EvalStatus Evaluate(EvaluationContext* ec,
+ State* state,
+ std::string* error,
+ PolicyDataInterface* data) const override;
EvalStatus UpdateCanBeApplied(
EvaluationContext* ec,
diff --git a/update_manager/mock_policy.h b/update_manager/mock_policy.h
index 3c6313f..ed30975 100644
--- a/update_manager/mock_policy.h
+++ b/update_manager/mock_policy.h
@@ -32,10 +32,6 @@
MockPolicy() {
// We defer to the corresponding DefaultPolicy methods, by default.
ON_CALL(*this,
- UpdateCheckAllowed(testing::_, testing::_, testing::_, testing::_))
- .WillByDefault(testing::Invoke(&default_policy_,
- &DefaultPolicy::UpdateCheckAllowed));
- ON_CALL(*this,
UpdateCanBeApplied(
testing::_, testing::_, testing::_, testing::_, testing::_))
.WillByDefault(testing::Invoke(&default_policy_,
@@ -56,11 +52,6 @@
}
~MockPolicy() override {}
- // Policy overrides.
- MOCK_CONST_METHOD4(
- UpdateCheckAllowed,
- EvalStatus(EvaluationContext*, State*, std::string*, UpdateCheckParams*));
-
MOCK_CONST_METHOD5(UpdateCanBeApplied,
EvalStatus(EvaluationContext*,
State*,
diff --git a/update_manager/mock_update_manager.h b/update_manager/mock_update_manager.h
deleted file mode 100644
index 06e17d8..0000000
--- a/update_manager/mock_update_manager.h
+++ /dev/null
@@ -1,44 +0,0 @@
-//
-// Copyright (C) 2019 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-//
-
-#ifndef UPDATE_ENGINE_MOCK_UPDATE_MANAGER_H
-#define UPDATE_ENGINE_MOCK_UPDATE_MANAGER_H
-
-#include <string>
-
-#include "update_engine/update_manager/update_manager.h"
-
-#include <gmock/gmock.h>
-
-namespace chromeos_update_manager {
-
-class MockUpdateManager : public UpdateManager {
- public:
- MockUpdateManager()
- : UpdateManager(base::TimeDelta(), base::TimeDelta(), nullptr) {}
-
- MOCK_METHOD2(
- AsyncPolicyRequestUpdateCheckAllowed,
- void(base::Callback<void(EvalStatus, const UpdateCheckParams& result)>
- callback,
- EvalStatus (Policy::*policy_method)(
- EvaluationContext*, State*, std::string*, UpdateCheckParams*)
- const));
-};
-
-} // namespace chromeos_update_manager
-
-#endif // UPDATE_ENGINE_MOCK_UPDATE_MANAGER_H
diff --git a/update_manager/next_update_check_policy_impl.cc b/update_manager/next_update_check_policy_impl.cc
index 0a78718..2ca9af3 100644
--- a/update_manager/next_update_check_policy_impl.cc
+++ b/update_manager/next_update_check_policy_impl.cc
@@ -19,6 +19,7 @@
#include <algorithm>
#include "update_engine/common/utils.h"
+#include "update_engine/update_manager/policy_utils.h"
using base::Time;
using base::TimeDelta;
@@ -27,15 +28,27 @@
namespace chromeos_update_manager {
+const NextUpdateCheckPolicyConstants kNextUpdateCheckPolicyConstants = {
+ .timeout_initial_interval = 7 * 60,
+ .timeout_periodic_interval = 45 * 60,
+ .timeout_max_backoff_interval = 4 * 60 * 60,
+ .timeout_regular_fuzz = 10 * 60,
+ .attempt_backoff_max_interval_in_days = 16,
+ .attempt_backoff_fuzz_in_hours = 12,
+};
+
+NextUpdateCheckTimePolicyImpl::NextUpdateCheckTimePolicyImpl()
+ : NextUpdateCheckTimePolicyImpl(kNextUpdateCheckPolicyConstants) {}
+
NextUpdateCheckTimePolicyImpl::NextUpdateCheckTimePolicyImpl(
const NextUpdateCheckPolicyConstants& constants)
: policy_constants_(constants) {}
-EvalStatus NextUpdateCheckTimePolicyImpl::UpdateCheckAllowed(
+EvalStatus NextUpdateCheckTimePolicyImpl::Evaluate(
EvaluationContext* ec,
State* state,
string* error,
- UpdateCheckParams* result) const {
+ PolicyDataInterface* data) const {
// Ensure that periodic update checks are timed properly.
Time next_update_check;
diff --git a/update_manager/next_update_check_policy_impl.h b/update_manager/next_update_check_policy_impl.h
index 291ea0f..16131fc 100644
--- a/update_manager/next_update_check_policy_impl.h
+++ b/update_manager/next_update_check_policy_impl.h
@@ -21,7 +21,7 @@
#include <base/time/time.h>
-#include "update_engine/update_manager/policy_utils.h"
+#include "update_engine/update_manager/policy_interface.h"
#include "update_engine/update_manager/prng.h"
namespace chromeos_update_manager {
@@ -41,17 +41,20 @@
int attempt_backoff_fuzz_in_hours;
};
+extern const NextUpdateCheckPolicyConstants kNextUpdateCheckPolicyConstants;
+
// Ensure that periodic update checks are timed properly.
-class NextUpdateCheckTimePolicyImpl : public PolicyImplBase {
+class NextUpdateCheckTimePolicyImpl : public PolicyInterface {
public:
- explicit NextUpdateCheckTimePolicyImpl(
+ NextUpdateCheckTimePolicyImpl();
+ NextUpdateCheckTimePolicyImpl(
const NextUpdateCheckPolicyConstants& constants);
// Policy overrides.
- EvalStatus UpdateCheckAllowed(EvaluationContext* ec,
- State* state,
- std::string* error,
- UpdateCheckParams* result) const override;
+ EvalStatus Evaluate(EvaluationContext* ec,
+ State* state,
+ std::string* error,
+ PolicyDataInterface* data) const override;
// A private policy implementation returning the wallclock timestamp when
// the next update check should happen.
diff --git a/update_manager/next_update_check_policy_impl_unittest.cc b/update_manager/next_update_check_policy_impl_unittest.cc
index d80063d..1dbf556 100644
--- a/update_manager/next_update_check_policy_impl_unittest.cc
+++ b/update_manager/next_update_check_policy_impl_unittest.cc
@@ -39,10 +39,7 @@
class UmNextUpdateCheckTimePolicyImplTest : public UmPolicyTestBase {
protected:
- UmNextUpdateCheckTimePolicyImplTest() {
- policy_ =
- std::make_unique<NextUpdateCheckTimePolicyImpl>(policy_test_constants);
- }
+ UmNextUpdateCheckTimePolicyImplTest() = default;
};
TEST_F(UmNextUpdateCheckTimePolicyImplTest,
diff --git a/update_manager/official_build_check_policy_impl.cc b/update_manager/official_build_check_policy_impl.cc
index e80c09f..5973068 100644
--- a/update_manager/official_build_check_policy_impl.cc
+++ b/update_manager/official_build_check_policy_impl.cc
@@ -19,11 +19,11 @@
namespace chromeos_update_manager {
// Unofficial builds should not perform periodic update checks.
-EvalStatus OnlyUpdateOfficialBuildsPolicyImpl::UpdateCheckAllowed(
+EvalStatus OnlyUpdateOfficialBuildsPolicyImpl::Evaluate(
EvaluationContext* ec,
State* state,
std::string* error,
- UpdateCheckParams* result) const {
+ PolicyDataInterface* data) const {
const bool* is_official_build_p =
ec->GetValue(state->system_provider()->var_is_official_build());
if (is_official_build_p != nullptr && !(*is_official_build_p)) {
diff --git a/update_manager/official_build_check_policy_impl.h b/update_manager/official_build_check_policy_impl.h
index 6257209..e7b8073 100644
--- a/update_manager/official_build_check_policy_impl.h
+++ b/update_manager/official_build_check_policy_impl.h
@@ -19,21 +19,21 @@
#include <string>
-#include "update_engine/update_manager/policy_utils.h"
+#include "update_engine/update_manager/policy_interface.h"
namespace chromeos_update_manager {
// Unofficial builds should not perform periodic update checks.
-class OnlyUpdateOfficialBuildsPolicyImpl : public PolicyImplBase {
+class OnlyUpdateOfficialBuildsPolicyImpl : public PolicyInterface {
public:
OnlyUpdateOfficialBuildsPolicyImpl() = default;
~OnlyUpdateOfficialBuildsPolicyImpl() override = default;
// Policy overrides.
- EvalStatus UpdateCheckAllowed(EvaluationContext* ec,
- State* state,
- std::string* error,
- UpdateCheckParams* result) const override;
+ EvalStatus Evaluate(EvaluationContext* ec,
+ State* state,
+ std::string* error,
+ PolicyDataInterface* data) const override;
protected:
std::string PolicyName() const override {
diff --git a/update_manager/out_of_box_experience_policy_impl.cc b/update_manager/out_of_box_experience_policy_impl.cc
index 8dcb560..d24b450 100644
--- a/update_manager/out_of_box_experience_policy_impl.cc
+++ b/update_manager/out_of_box_experience_policy_impl.cc
@@ -20,10 +20,10 @@
namespace chromeos_update_manager {
-EvalStatus OobePolicyImpl::UpdateCheckAllowed(EvaluationContext* ec,
- State* state,
- std::string* error,
- UpdateCheckParams* result) const {
+EvalStatus OobePolicyImpl::Evaluate(EvaluationContext* ec,
+ State* state,
+ std::string* error,
+ PolicyDataInterface* data) const {
SystemProvider* const system_provider = state->system_provider();
// If OOBE is enabled, wait until it is completed.
diff --git a/update_manager/out_of_box_experience_policy_impl.h b/update_manager/out_of_box_experience_policy_impl.h
index ec1997b..013205d 100644
--- a/update_manager/out_of_box_experience_policy_impl.h
+++ b/update_manager/out_of_box_experience_policy_impl.h
@@ -19,12 +19,12 @@
#include <string>
-#include "update_engine/update_manager/policy_utils.h"
+#include "update_engine/update_manager/policy_interface.h"
namespace chromeos_update_manager {
// If OOBE is enabled, wait until it is completed.
-class OobePolicyImpl : public PolicyImplBase {
+class OobePolicyImpl : public PolicyInterface {
public:
OobePolicyImpl() = default;
~OobePolicyImpl() override = default;
@@ -32,10 +32,10 @@
std::string PolicyName() const override { return "OobePolicyImpl"; }
// Policy overrides.
- EvalStatus UpdateCheckAllowed(EvaluationContext* ec,
- State* state,
- std::string* error,
- UpdateCheckParams* result) const override;
+ EvalStatus Evaluate(EvaluationContext* ec,
+ State* state,
+ std::string* error,
+ PolicyDataInterface* data) const override;
private:
DISALLOW_COPY_AND_ASSIGN(OobePolicyImpl);
diff --git a/update_manager/policy.h b/update_manager/policy.h
index fb4a129..11dc017 100644
--- a/update_manager/policy.h
+++ b/update_manager/policy.h
@@ -40,40 +40,6 @@
std::string ToString(EvalStatus status);
-// Parameters of an update check. These parameters are determined by the
-// UpdateCheckAllowed policy.
-struct UpdateCheckParams {
- // 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{false};
- // Specifies if rollbacks should attempt to preserve some system state.
- 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{0};
- // Whether a rollback with data save should be initiated on channel
- // downgrade (e.g. beta to stable).
- bool rollback_on_channel_downgrade{false};
- // A target channel, if so imposed by policy; otherwise, an empty string.
- std::string target_channel;
- // Specifies if the channel hint, e.g. LTS (Long Term Support) updates.
- std::string lts_tag;
- // Specifies a token which maps to a Chrome OS Quick Fix Build, if imposed by
- // policy; otherwise, an empty string.
- std::string quick_fix_build_token;
-
- // Whether the allowed update is interactive (user-initiated) or periodic.
- bool interactive{false};
-};
-
// Input arguments to UpdateCanStart.
//
// A snapshot of the state of the current update process. This includes
@@ -221,9 +187,6 @@
EvaluationContext*, State*, std::string*, R*, Args...) const) const {
std::string class_name = PolicyName() + "::";
- if (reinterpret_cast<typeof(&Policy::UpdateCheckAllowed)>(policy_method) ==
- &Policy::UpdateCheckAllowed)
- return class_name + "UpdateCheckAllowed";
if (reinterpret_cast<typeof(&Policy::UpdateCanBeApplied)>(policy_method) ==
&Policy::UpdateCanBeApplied)
return class_name + "UpdateCanBeApplied";
@@ -248,13 +211,6 @@
// When the implementation fails, the method returns EvalStatus::kFailed and
// sets the |error| string.
- // UpdateCheckAllowed returns whether it is allowed to request an update check
- // to Omaha.
- virtual EvalStatus UpdateCheckAllowed(EvaluationContext* ec,
- State* state,
- std::string* error,
- UpdateCheckParams* result) const = 0;
-
// UpdateCanBeApplied returns whether the given |install_plan| can be acted
// on at this time. The reason for not applying is returned in |result|.
// The Policy may modify the passed-in |install_plan|, based on the
diff --git a/update_manager/policy_evaluator.cc b/update_manager/policy_evaluator.cc
new file mode 100644
index 0000000..bed2bf9
--- /dev/null
+++ b/update_manager/policy_evaluator.cc
@@ -0,0 +1,93 @@
+//
+// Copyright 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+#include "update_engine/update_manager/policy_evaluator.h"
+
+#include <string>
+#include <utility>
+
+namespace chromeos_update_manager {
+
+EvalStatus PolicyEvaluator::Evaluate() {
+ // If expiration timeout fired, dump the context and reset expiration.
+ // IMPORTANT: We must still proceed with evaluation of the policy in this
+ // case, so that the evaluation time (and corresponding reevaluation
+ // timeouts) are readjusted.
+ if (ec_->is_expired()) {
+ LOG(WARNING) << "Request timed out, evaluation context: "
+ << ec_->DumpContext();
+ ec_->ResetExpiration();
+ }
+
+ // Reset the evaluation context.
+ ec_->ResetEvaluation();
+
+ // First try calling the actual policy.
+ std::string error;
+ EvalStatus status = policy_->Evaluate(ec_.get(), state_, &error, data_.get());
+ // If evaluating the main policy failed, defer to the default policy.
+ if (status == EvalStatus::kFailed) {
+ LOG(WARNING) << "Evaluating policy failed: " << error
+ << "\nEvaluation context: " << ec_->DumpContext();
+ error.clear();
+ status = policy_->EvaluateDefault(ec_.get(), state_, &error, data_.get());
+ if (status == EvalStatus::kFailed) {
+ LOG(WARNING) << "Evaluating default policy failed: " << error;
+ } else if (status == EvalStatus::kAskMeAgainLater) {
+ LOG(ERROR)
+ << "Default policy would block; this is a bug, forcing failure.";
+ status = EvalStatus::kFailed;
+ }
+ }
+
+ return status;
+}
+
+void PolicyEvaluator::ScheduleEvaluation(
+ base::Callback<void(EvalStatus)> callback) {
+ base::Closure eval_callback =
+ base::Bind(&PolicyEvaluator::OnPolicyReadyToEvaluate,
+ base::RetainedRef(this),
+ std::move(callback));
+ brillo::MessageLoop::current()->PostTask(FROM_HERE, eval_callback);
+}
+
+void PolicyEvaluator::OnPolicyReadyToEvaluate(
+ base::Callback<void(EvalStatus)> callback) {
+ // Evaluate the policy.
+ EvalStatus status = Evaluate();
+ if (status != EvalStatus::kAskMeAgainLater) {
+ callback.Run(status);
+ return;
+ }
+
+ // Re-schedule the policy request based on used variables.
+ if (ec_->RunOnValueChangeOrTimeout(
+ base::Bind(&PolicyEvaluator::OnPolicyReadyToEvaluate,
+ base::RetainedRef(this),
+ callback)))
+ return; // Reevaluation scheduled successfully.
+
+ // Scheduling a reevaluation can fail because policy method didn't use any
+ // non-const variable nor there's any time-based event that will change the
+ // status of evaluation. Alternatively, this may indicate an error in the
+ // use of the scheduling interface.
+ LOG(ERROR) << "Failed to schedule a reevaluation of policy"
+ << "; this is a bug.";
+ callback.Run(status);
+}
+
+} // namespace chromeos_update_manager
diff --git a/update_manager/policy_evaluator.h b/update_manager/policy_evaluator.h
new file mode 100644
index 0000000..c639682
--- /dev/null
+++ b/update_manager/policy_evaluator.h
@@ -0,0 +1,75 @@
+//
+// Copyright 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+#ifndef UPDATE_ENGINE_UPDATE_MANAGER_POLICY_EVALUATOR_H_
+#define UPDATE_ENGINE_UPDATE_MANAGER_POLICY_EVALUATOR_H_
+
+#include <memory>
+#include <utility>
+
+#include <base/memory/ref_counted.h>
+
+#include "update_engine/update_manager/evaluation_context.h"
+#include "update_engine/update_manager/policy.h"
+#include "update_engine/update_manager/policy_interface.h"
+#include "update_engine/update_manager/state.h"
+
+namespace chromeos_update_manager {
+
+// This class is the main point of entry for evaluating any kind of policy. The
+// reason an instance of this class needs to be ref counted is because normally
+// we don't want to keep a reference to an object of this class
+// around. Specially when we want to evaluate a policy asychronously. Ref
+// counting allows us to pass a pointer to this class in repeated callbacks. To
+// make an instance of this class use base::AdoptRef() or base::MakeRefCounted()
+// (See base/memory/ref_counted.h).
+class PolicyEvaluator : public base::RefCounted<PolicyEvaluator> {
+ public:
+ PolicyEvaluator(State* state,
+ std::unique_ptr<EvaluationContext> ec,
+ std::unique_ptr<PolicyInterface> policy,
+ std::shared_ptr<PolicyDataInterface> data)
+ : state_(state),
+ ec_(std::move(ec)),
+ policy_(std::move(policy)),
+ data_(std::move(data)) {}
+ virtual ~PolicyEvaluator() = default;
+
+ PolicyEvaluator(const PolicyEvaluator&) = delete;
+ PolicyEvaluator& operator=(const PolicyEvaluator&) = delete;
+
+ // Evaluations the policy given in the ctor using the provided |data_| and
+ // returns the result of the evaluation.
+ EvalStatus Evaluate();
+
+ // Same as the above function but the asyncronous version. A call to this
+ // function returns immediately and an evalution is scheduled in the main
+ // message loop. The passed |callback| is called when the policy is evaluated.
+ void ScheduleEvaluation(base::Callback<void(EvalStatus)> callback);
+
+ private:
+ friend class base::RefCounted<PolicyEvaluator>;
+
+ // Internal function to reschedule policy evaluation.
+ void OnPolicyReadyToEvaluate(base::Callback<void(EvalStatus)> callback);
+ State* state_;
+ std::unique_ptr<EvaluationContext> ec_;
+ std::unique_ptr<PolicyInterface> policy_;
+ std::shared_ptr<PolicyDataInterface> data_;
+};
+
+} // namespace chromeos_update_manager
+
+#endif // UPDATE_ENGINE_UPDATE_MANAGER_POLICY_EVALUATOR_H_
diff --git a/update_manager/policy_interface.h b/update_manager/policy_interface.h
new file mode 100644
index 0000000..99e9920
--- /dev/null
+++ b/update_manager/policy_interface.h
@@ -0,0 +1,53 @@
+//
+// Copyright 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+#ifndef UPDATE_ENGINE_UPDATE_MANAGER_POLICY_INTERFACE_H_
+#define UPDATE_ENGINE_UPDATE_MANAGER_POLICY_INTERFACE_H_
+
+#include <string>
+
+#include "update_engine/update_manager/evaluation_context.h"
+#include "update_engine/update_manager/policy.h"
+#include "update_engine/update_manager/state.h"
+
+namespace chromeos_update_manager {
+
+class PolicyDataInterface {};
+
+class PolicyInterface {
+ public:
+ virtual ~PolicyInterface() = default;
+
+ virtual EvalStatus Evaluate(EvaluationContext* ec,
+ State* state,
+ std::string* error,
+ PolicyDataInterface* data) const = 0;
+ virtual EvalStatus EvaluateDefault(EvaluationContext* ec,
+ State* state,
+ std::string* error,
+ PolicyDataInterface* data) const {
+ return EvalStatus::kSucceeded;
+ }
+
+ virtual std::string PolicyName() const = 0;
+
+ protected:
+ PolicyInterface() = default;
+};
+
+} // namespace chromeos_update_manager
+
+#endif // UPDATE_ENGINE_UPDATE_MANAGER_POLICY_INTERFACE_H_
diff --git a/update_manager/policy_test_utils.cc b/update_manager/policy_test_utils.cc
index e8961b1..248c10c 100644
--- a/update_manager/policy_test_utils.cc
+++ b/update_manager/policy_test_utils.cc
@@ -18,6 +18,7 @@
#include <memory>
#include <tuple>
+#include <utility>
#include <vector>
#include "update_engine/cros/fake_system_state.h"
@@ -40,6 +41,12 @@
SetUpDefaultClock();
eval_ctx_.reset(new EvaluationContext(TimeDelta::FromSeconds(5)));
SetUpDefaultState();
+
+ evaluator_ = base::MakeRefCounted<PolicyEvaluator>(
+ &fake_state_,
+ std::make_unique<EvaluationContext>(TimeDelta::FromSeconds(5)),
+ std::move(policy_2_),
+ policy_data_);
}
void UmPolicyTestBase::TearDown() {
diff --git a/update_manager/policy_test_utils.h b/update_manager/policy_test_utils.h
index 72bd3bc..87e9ba9 100644
--- a/update_manager/policy_test_utils.h
+++ b/update_manager/policy_test_utils.h
@@ -27,6 +27,7 @@
#include "update_engine/common/fake_clock.h"
#include "update_engine/update_manager/evaluation_context.h"
#include "update_engine/update_manager/fake_state.h"
+#include "update_engine/update_manager/policy_evaluator.h"
#include "update_engine/update_manager/policy_utils.h"
namespace chromeos_update_manager {
@@ -95,6 +96,10 @@
FakeState fake_state_;
std::shared_ptr<EvaluationContext> eval_ctx_;
std::unique_ptr<Policy> policy_;
+
+ scoped_refptr<PolicyEvaluator> evaluator_;
+ std::unique_ptr<PolicyInterface> policy_2_;
+ std::shared_ptr<PolicyDataInterface> policy_data_;
};
} // namespace chromeos_update_manager
diff --git a/update_manager/policy_utils.h b/update_manager/policy_utils.h
index aedb90c..97e141f 100644
--- a/update_manager/policy_utils.h
+++ b/update_manager/policy_utils.h
@@ -67,14 +67,6 @@
// instead of using pure-virtual methods on that class.
class PolicyImplBase : public Policy {
public:
- // Policy overrides.
- EvalStatus UpdateCheckAllowed(EvaluationContext* ec,
- State* state,
- std::string* error,
- UpdateCheckParams* result) const override {
- return EvalStatus::kContinue;
- };
-
EvalStatus UpdateCanBeApplied(
EvaluationContext* ec,
State* state,
diff --git a/update_manager/real_shill_provider_unittest.cc b/update_manager/real_shill_provider_unittest.cc
index e0e7ef4..49d6bff 100644
--- a/update_manager/real_shill_provider_unittest.cc
+++ b/update_manager/real_shill_provider_unittest.cc
@@ -64,6 +64,7 @@
protected:
// Initialize the RealShillProvider under test.
void SetUp() override {
+ FakeSystemState::CreateInstance();
FakeSystemState::Get()->fake_clock()->SetWallclockTime(InitTime());
loop_.SetAsCurrent();
fake_shill_proxy_ = new chromeos_update_engine::FakeShillProxy();
diff --git a/update_manager/recovery_policy.cc b/update_manager/recovery_policy.cc
index dee0047..48d73c1 100644
--- a/update_manager/recovery_policy.cc
+++ b/update_manager/recovery_policy.cc
@@ -18,10 +18,10 @@
namespace chromeos_update_manager {
-EvalStatus RecoveryPolicy::UpdateCheckAllowed(EvaluationContext* ec,
- State* state,
- std::string* error,
- UpdateCheckParams* result) const {
+EvalStatus RecoveryPolicy::Evaluate(EvaluationContext* ec,
+ State* state,
+ std::string* error,
+ PolicyDataInterface* data) const {
const bool* running_in_minios =
ec->GetValue(state->updater_provider()->var_running_from_minios());
if (running_in_minios && (*running_in_minios)) {
diff --git a/update_manager/recovery_policy.h b/update_manager/recovery_policy.h
index c1d0687..b8a108b 100644
--- a/update_manager/recovery_policy.h
+++ b/update_manager/recovery_policy.h
@@ -19,14 +19,12 @@
#include <string>
-#include "update_engine/common/error_code.h"
-#include "update_engine/payload_consumer/install_plan.h"
-#include "update_engine/update_manager/policy_utils.h"
+#include "update_engine/update_manager/policy_interface.h"
namespace chromeos_update_manager {
// Skip remaining policy checks if in MiniOs recovery.
-class RecoveryPolicy : public PolicyImplBase {
+class RecoveryPolicy : public PolicyInterface {
public:
RecoveryPolicy() = default;
~RecoveryPolicy() override = default;
@@ -34,11 +32,11 @@
RecoveryPolicy(const RecoveryPolicy&) = delete;
RecoveryPolicy& operator=(const RecoveryPolicy&) = delete;
- // Policy overrides.
- EvalStatus UpdateCheckAllowed(EvaluationContext* ec,
- State* state,
- std::string* error,
- UpdateCheckParams* result) const override;
+ // |PolicyInterface| overrides.
+ EvalStatus Evaluate(EvaluationContext* ec,
+ State* state,
+ std::string* error,
+ PolicyDataInterface* data) const override;
protected:
std::string PolicyName() const override { return "RecoveryPolicy"; }
diff --git a/update_manager/recovery_policy_unittest.cc b/update_manager/recovery_policy_unittest.cc
index cc5824e..f613d08 100644
--- a/update_manager/recovery_policy_unittest.cc
+++ b/update_manager/recovery_policy_unittest.cc
@@ -20,30 +20,28 @@
#include "update_engine/update_manager/policy_test_utils.h"
#include "update_engine/update_manager/recovery_policy.h"
+#include "update_engine/update_manager/update_check_allowed_policy_data.h"
namespace chromeos_update_manager {
class UmRecoveryPolicyTest : public UmPolicyTestBase {
protected:
UmRecoveryPolicyTest() : UmPolicyTestBase() {
- policy_ = std::make_unique<RecoveryPolicy>();
+ policy_data_.reset(new UpdateCheckAllowedPolicyData());
+ policy_2_.reset(new RecoveryPolicy());
}
};
TEST_F(UmRecoveryPolicyTest, RecoveryMode) {
fake_state_.updater_provider()->var_running_from_minios()->reset(
new bool(true));
- UpdateCheckParams result;
- ExpectPolicyStatus(
- EvalStatus::kSucceeded, &Policy::UpdateCheckAllowed, &result);
+ EXPECT_EQ(EvalStatus::kSucceeded, evaluator_->Evaluate());
}
TEST_F(UmRecoveryPolicyTest, NotRecoveryMode) {
fake_state_.updater_provider()->var_running_from_minios()->reset(
new bool(false));
- UpdateCheckParams result;
- ExpectPolicyStatus(
- EvalStatus::kContinue, &Policy::UpdateCheckAllowed, &result);
+ EXPECT_EQ(EvalStatus::kContinue, evaluator_->Evaluate());
}
} // namespace chromeos_update_manager
diff --git a/update_manager/update_check_allowed_policy.h b/update_manager/update_check_allowed_policy.h
new file mode 100644
index 0000000..eb96f2e
--- /dev/null
+++ b/update_manager/update_check_allowed_policy.h
@@ -0,0 +1,84 @@
+//
+// Copyright 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+#ifndef UPDATE_ENGINE_UPDATE_MANAGER_UPDATE_CHECK_ALLOWED_POLICY_H_
+#define UPDATE_ENGINE_UPDATE_MANAGER_UPDATE_CHECK_ALLOWED_POLICY_H_
+
+#include <memory>
+#include <string>
+#include <utility>
+
+#include "update_engine/update_manager/policy_interface.h"
+#include "update_engine/update_manager/update_check_allowed_policy_data.h"
+
+namespace chromeos_update_manager {
+
+class UpdateCheckAllowedPolicy : public PolicyInterface {
+ public:
+ // Auxiliary state class for DefaultPolicy evaluations.
+ //
+ // IMPORTANT: The use of a state object in policies is generally forbidden, as
+ // it was a design decision to keep policy calls side-effect free. We make an
+ // exception here to ensure that DefaultPolicy indeed serves as a safe (and
+ // secure) fallback option. This practice should be avoided when imlpementing
+ // other policies.
+ class DefaultPolicyState {
+ public:
+ DefaultPolicyState() {}
+
+ bool IsLastCheckAllowedTimeSet() const {
+ return last_check_allowed_time_ != base::Time::Max();
+ }
+
+ // Sets/returns the point time on the monotonic time scale when the latest
+ // check allowed was recorded.
+ void set_last_check_allowed_time(base::Time timestamp) {
+ last_check_allowed_time_ = timestamp;
+ }
+ base::Time last_check_allowed_time() const {
+ return last_check_allowed_time_;
+ }
+
+ private:
+ base::Time last_check_allowed_time_ = base::Time::Max();
+ };
+
+ UpdateCheckAllowedPolicy() : aux_state_(new DefaultPolicyState()) {}
+ virtual ~UpdateCheckAllowedPolicy() = default;
+
+ UpdateCheckAllowedPolicy(const UpdateCheckAllowedPolicy&) = delete;
+ UpdateCheckAllowedPolicy& operator=(const UpdateCheckAllowedPolicy&) = delete;
+
+ EvalStatus Evaluate(EvaluationContext* ec,
+ State* state,
+ std::string* error,
+ PolicyDataInterface* data) const override;
+
+ EvalStatus EvaluateDefault(EvaluationContext* ec,
+ State* state,
+ std::string* error,
+ PolicyDataInterface* data) const override;
+
+ std::string PolicyName() const override { return "UpdateCheckAllowedPolicy"; }
+
+ private:
+ // An auxiliary state object.
+ std::unique_ptr<DefaultPolicyState> aux_state_;
+};
+
+} // namespace chromeos_update_manager
+
+#endif // UPDATE_ENGINE_UPDATE_MANAGER_UPDATE_CHECK_ALLOWED_POLICY_H_
diff --git a/update_manager/update_check_allowed_policy_data.h b/update_manager/update_check_allowed_policy_data.h
new file mode 100644
index 0000000..ce11052
--- /dev/null
+++ b/update_manager/update_check_allowed_policy_data.h
@@ -0,0 +1,83 @@
+//
+// Copyright 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+#ifndef UPDATE_ENGINE_UPDATE_MANAGER_UPDATE_CHECK_ALLOWED_POLICY_DATA_H_
+#define UPDATE_ENGINE_UPDATE_MANAGER_UPDATE_CHECK_ALLOWED_POLICY_DATA_H_
+
+#include <string>
+#include <utility>
+
+#include "update_engine/update_manager/policy_interface.h"
+
+namespace chromeos_update_manager {
+
+// Parameters of an update check. These parameters are determined by the
+// UpdateCheckAllowed policy.
+struct UpdateCheckParams {
+ // 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{false};
+ // Specifies if rollbacks should attempt to preserve some system state.
+ 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{0};
+ // Whether a rollback with data save should be initiated on channel
+ // downgrade (e.g. beta to stable).
+ bool rollback_on_channel_downgrade{false};
+ // A target channel, if so imposed by policy; otherwise, an empty string.
+ std::string target_channel;
+ // Specifies if the channel hint, e.g. LTS (Long Term Support) updates.
+ std::string lts_tag;
+ // Specifies a token which maps to a Chrome OS Quick Fix Build, if imposed by
+ // policy; otherwise, an empty string.
+ std::string quick_fix_build_token;
+
+ // Whether the allowed update is interactive (user-initiated) or periodic.
+ bool interactive{false};
+};
+
+class UpdateCheckAllowedPolicyData : public PolicyDataInterface {
+ public:
+ UpdateCheckAllowedPolicyData() = default;
+ explicit UpdateCheckAllowedPolicyData(UpdateCheckParams params)
+ : update_check_params(std::move(params)) {}
+ virtual ~UpdateCheckAllowedPolicyData() = default;
+
+ UpdateCheckAllowedPolicyData(const UpdateCheckAllowedPolicyData&) = delete;
+ UpdateCheckAllowedPolicyData& operator=(const UpdateCheckAllowedPolicyData&) =
+ delete;
+
+ // Helper function to convert |PolicyDataInterface| into proper data type.
+ static UpdateCheckParams* GetUpdateCheckParams(PolicyDataInterface* data) {
+ return &(
+ static_cast<UpdateCheckAllowedPolicyData*>(data)->update_check_params);
+ }
+
+ UpdateCheckParams update_check_params;
+};
+
+} // namespace chromeos_update_manager
+
+#endif // UPDATE_ENGINE_UPDATE_MANAGER_UPDATE_CHECK_ALLOWED_POLICY_DATA_H_
diff --git a/update_manager/update_manager.cc b/update_manager/update_manager.cc
index 64fdd1d..d06b05f 100644
--- a/update_manager/update_manager.cc
+++ b/update_manager/update_manager.cc
@@ -15,6 +15,9 @@
//
#include "update_engine/update_manager/update_manager.h"
+
+#include <utility>
+
#include "update_engine/update_manager/state.h"
namespace chromeos_update_manager {
@@ -36,11 +39,27 @@
ec->RemoveObserversAndTimeout();
}
-void UpdateManager::AsyncPolicyRequestUpdateCheckAllowed(
- base::Callback<void(EvalStatus, const UpdateCheckParams& result)> callback,
- EvalStatus (Policy::*policy_method)(
- EvaluationContext*, State*, std::string*, UpdateCheckParams*) const) {
- AsyncPolicyRequest(callback, policy_method);
+EvalStatus UpdateManager::PolicyRequest2(
+ std::unique_ptr<PolicyInterface> policy,
+ std::shared_ptr<PolicyDataInterface> data) {
+ return base::MakeRefCounted<PolicyEvaluator>(
+ state_.get(),
+ std::make_unique<EvaluationContext>(evaluation_timeout_),
+ std::move(policy),
+ std::move(data))
+ ->Evaluate();
+}
+
+void UpdateManager::PolicyRequest2(std::unique_ptr<PolicyInterface> policy,
+ std::shared_ptr<PolicyDataInterface> data,
+ base::Callback<void(EvalStatus)> callback) {
+ auto ec = std::make_unique<EvaluationContext>(
+ evaluation_timeout_,
+ expiration_timeout_,
+ std::unique_ptr<base::Callback<void(EvaluationContext*)>>(nullptr));
+ base::MakeRefCounted<PolicyEvaluator>(
+ state_.get(), std::move(ec), std::move(policy), std::move(data))
+ ->ScheduleEvaluation(std::move(callback));
}
void UpdateManager::UnregisterEvalContext(EvaluationContext* ec) {
diff --git a/update_manager/update_manager.h b/update_manager/update_manager.h
index 40164f9..08aa1d4 100644
--- a/update_manager/update_manager.h
+++ b/update_manager/update_manager.h
@@ -25,35 +25,18 @@
#include <base/time/time.h>
#include "update_engine/common/system_state.h"
+#include "update_engine/update_manager/chromeos_policy.h"
#include "update_engine/update_manager/default_policy.h"
#include "update_engine/update_manager/evaluation_context.h"
#include "update_engine/update_manager/policy.h"
+#include "update_engine/update_manager/policy_evaluator.h"
#include "update_engine/update_manager/state.h"
#include "update_engine/update_manager/update_time_restrictions_monitor.h"
namespace chromeos_update_manager {
-// Please do not move this class into a new file for simplicity.
-// This pure virtual class is purely created for purpose of testing. The reason
-// was that |UpdateManager|'s member functions are templatized, which does not
-// play nicely when testing (mocking + faking). Whenever a specialized member of
-// |UpdateManager| must be tested, please add a specialized template member
-// function within this class for testing.
-class SpecializedPolicyRequestInterface {
- public:
- virtual ~SpecializedPolicyRequestInterface() = default;
-
- virtual void AsyncPolicyRequestUpdateCheckAllowed(
- base::Callback<void(EvalStatus, const UpdateCheckParams& result)>
- callback,
- EvalStatus (Policy::*policy_method)(EvaluationContext*,
- State*,
- std::string*,
- UpdateCheckParams*) const) = 0;
-};
-
// The main Update Manager singleton class.
-class UpdateManager : public SpecializedPolicyRequestInterface {
+class UpdateManager {
public:
// Creates the UpdateManager instance, assuming ownership on the provided
// |state|.
@@ -100,13 +83,29 @@
EvaluationContext*, State*, std::string*, R*, ExpectedArgs...) const,
ActualArgs... args);
- void AsyncPolicyRequestUpdateCheckAllowed(
- base::Callback<void(EvalStatus, const UpdateCheckParams& result)>
- callback,
- EvalStatus (Policy::*policy_method)(EvaluationContext*,
- State*,
- std::string*,
- UpdateCheckParams*) const) override;
+ // This function evaluates a given |policy| and returns the result
+ // immediately. |data| is an input/output argument. When the policy request
+ // succeeds, the |data| is filled and the method returns
+ // |EvalStatus::kSucceeded|, otherwise, the |data| may not be filled. A policy
+ // called with this method should not block (i.e. return
+ // |EvalStatus::kAskMeAgainLater|), which is considered a programming
+ // error. On failure, |EvalStatus::kFailed| is returned.
+ //
+ // TODO(b/179419726): Remove "2" from the name once |PolicyRequest| functions
+ // have been deprecated.
+ EvalStatus PolicyRequest2(std::unique_ptr<PolicyInterface> policy,
+ std::shared_ptr<PolicyDataInterface> data);
+
+ // Similar to the function above but the results are returned at a later time
+ // using the given |callback| function.
+ // If the policy implementation should block, returning a
+ // |EvalStatus::kAskMeAgainLater| status, the policy will be re-evaluated
+ // until another status is returned. If the policy implementation based
+ // its return value solely on const variables, the callback will be called
+ // with the EvalStatus::kAskMeAgainLater status (which indicates an error).
+ void PolicyRequest2(std::unique_ptr<PolicyInterface> policy,
+ std::shared_ptr<PolicyDataInterface> data,
+ base::Callback<void(EvalStatus)> callback);
// Returns instance of update time restrictions monitor if |install_plan|
// requires one. Otherwise returns nullptr.
diff --git a/update_manager/update_manager_unittest.cc b/update_manager/update_manager_unittest.cc
index 8ad5a81..16d64b2 100644
--- a/update_manager/update_manager_unittest.cc
+++ b/update_manager/update_manager_unittest.cc
@@ -100,17 +100,31 @@
unique_ptr<UpdateManager> umut_;
};
+class SimplePolicy : public PolicyInterface {
+ public:
+ SimplePolicy() = default;
+ EvalStatus Evaluate(EvaluationContext* ec,
+ State* state,
+ string* error,
+ PolicyDataInterface* data) const override {
+ return EvalStatus::kSucceeded;
+ }
+
+ protected:
+ string PolicyName() const override { return "SimplePolicy"; }
+};
+
// The FailingPolicy implements a single method and make it always fail. This
// class extends the DefaultPolicy class to allow extensions of the Policy
// class without extending nor changing this test.
-class FailingPolicy : public DefaultPolicy {
+class FailingPolicy : public PolicyInterface {
public:
explicit FailingPolicy(int* num_called_p) : num_called_p_(num_called_p) {}
FailingPolicy() : FailingPolicy(nullptr) {}
- EvalStatus UpdateCheckAllowed(EvaluationContext* ec,
- State* state,
- string* error,
- UpdateCheckParams* result) const override {
+ EvalStatus Evaluate(EvaluationContext* ec,
+ State* state,
+ string* error,
+ PolicyDataInterface* data) const override {
if (num_called_p_)
(*num_called_p_)++;
*error = "FailingPolicy failed.";
@@ -125,11 +139,11 @@
};
// The LazyPolicy always returns EvalStatus::kAskMeAgainLater.
-class LazyPolicy : public DefaultPolicy {
- EvalStatus UpdateCheckAllowed(EvaluationContext* ec,
- State* state,
- string* error,
- UpdateCheckParams* result) const override {
+class LazyPolicy : public PolicyInterface {
+ EvalStatus Evaluate(EvaluationContext* ec,
+ State* state,
+ string* error,
+ PolicyDataInterface* result) const override {
return EvalStatus::kAskMeAgainLater;
}
@@ -142,16 +156,16 @@
// EvalStatus::kAskMeAgainLater if not passed; otherwise, returns
// EvalStatus::kSucceeded. Increments a counter every time it is being queried,
// if a pointer to it is provided.
-class DelayPolicy : public DefaultPolicy {
+class DelayPolicy : public PolicyInterface {
public:
DelayPolicy(int sleep_secs, Time time_threshold, int* num_called_p)
: sleep_secs_(sleep_secs),
time_threshold_(time_threshold),
num_called_p_(num_called_p) {}
- EvalStatus UpdateCheckAllowed(EvaluationContext* ec,
- State* state,
- string* error,
- UpdateCheckParams* result) const override {
+ EvalStatus Evaluate(EvaluationContext* ec,
+ State* state,
+ string* error,
+ PolicyDataInterface* data) const override {
if (num_called_p_)
(*num_called_p_)++;
@@ -181,19 +195,17 @@
// of EvalStatus and T instances. This allows to create a callback that keeps
// track of when it is called and the arguments passed to it, to be used with
// the UpdateManager::AsyncPolicyRequest().
-template <typename T>
-static void AccumulateCallsCallback(vector<pair<EvalStatus, T>>* acc,
- EvalStatus status,
- const T& result) {
- acc->push_back(std::make_pair(status, result));
+static void AccumulateCallsCallback(vector<EvalStatus>* acc,
+ EvalStatus status) {
+ acc->push_back(status);
}
// Tests that policy requests are completed successfully. It is important that
// this tests cover all policy requests as defined in Policy.
TEST_F(UmUpdateManagerTest, PolicyRequestCallUpdateCheckAllowed) {
- UpdateCheckParams result;
EXPECT_EQ(EvalStatus::kSucceeded,
- umut_->PolicyRequest(&Policy::UpdateCheckAllowed, &result));
+ umut_->PolicyRequest2(std::make_unique<SimplePolicy>(),
+ std::make_shared<PolicyDataInterface>()));
}
TEST_F(UmUpdateManagerTest, PolicyRequestCallUpdateCanStart) {
@@ -228,16 +240,11 @@
}
TEST_F(UmUpdateManagerTest, PolicyRequestCallsDefaultOnError) {
- umut_->set_policy(new FailingPolicy());
-
- // Tests that the DefaultPolicy instance is called when the method fails,
- // which will set this as true.
- UpdateCheckParams result;
- result.updates_enabled = false;
- EvalStatus status =
- umut_->PolicyRequest(&Policy::UpdateCheckAllowed, &result);
- EXPECT_EQ(EvalStatus::kSucceeded, status);
- EXPECT_TRUE(result.updates_enabled);
+ // Tests that the default evaluation is called when the method fails, which
+ // will set this as true.
+ EXPECT_EQ(EvalStatus::kSucceeded,
+ umut_->PolicyRequest2(std::make_unique<FailingPolicy>(),
+ std::make_shared<PolicyDataInterface>()));
}
// This test only applies to debug builds where DCHECK is enabled.
@@ -245,9 +252,9 @@
TEST_F(UmUpdateManagerTest, PolicyRequestDoesntBlockDeathTest) {
// The update manager should die (DCHECK) if a policy called synchronously
// returns a kAskMeAgainLater value.
- UpdateCheckParams result;
- umut_->set_policy(new LazyPolicy());
- EXPECT_DEATH(umut_->PolicyRequest(&Policy::UpdateCheckAllowed, &result), "");
+ PolicyDataInterface data;
+ EXPECT_DEATH(umut_->PolicyRequest2(std::make_unique<LazyPolicy>(), &data),
+ "");
}
#endif // DCHECK_IS_ON
@@ -256,13 +263,12 @@
// call on a policy that returns AskMeAgainLater the first time and one that
// succeeds the first time, we ensure that the passed callback is called from
// the main loop in both cases even when we could evaluate it right now.
- umut_->set_policy(new FailingPolicy());
+ vector<EvalStatus> calls;
+ Callback<void(EvalStatus)> callback = Bind(AccumulateCallsCallback, &calls);
- vector<pair<EvalStatus, UpdateCheckParams>> calls;
- Callback<void(EvalStatus, const UpdateCheckParams&)> callback =
- Bind(AccumulateCallsCallback<UpdateCheckParams>, &calls);
-
- umut_->AsyncPolicyRequest(callback, &Policy::UpdateCheckAllowed);
+ umut_->PolicyRequest2(std::make_unique<FailingPolicy>(),
+ std::make_shared<PolicyDataInterface>(),
+ callback);
// The callback should wait until we run the main loop for it to be executed.
EXPECT_EQ(0U, calls.size());
MessageLoopRunMaxIterations(MessageLoop::current(), 100);
@@ -272,20 +278,19 @@
TEST_F(UmUpdateManagerTest, AsyncPolicyRequestTimeoutDoesNotFire) {
// Set up an async policy call to return immediately, then wait a little and
// ensure that the timeout event does not fire.
+ vector<EvalStatus> calls;
+ Callback<void(EvalStatus)> callback = Bind(AccumulateCallsCallback, &calls);
+
int num_called = 0;
- umut_->set_policy(new FailingPolicy(&num_called));
-
- vector<pair<EvalStatus, UpdateCheckParams>> calls;
- Callback<void(EvalStatus, const UpdateCheckParams&)> callback =
- Bind(AccumulateCallsCallback<UpdateCheckParams>, &calls);
-
- umut_->AsyncPolicyRequest(callback, &Policy::UpdateCheckAllowed);
+ umut_->PolicyRequest2(std::make_unique<FailingPolicy>(&num_called),
+ std::make_shared<PolicyDataInterface>(),
+ callback);
// Run the main loop, ensure that policy was attempted once before deferring
// to the default.
MessageLoopRunMaxIterations(MessageLoop::current(), 100);
EXPECT_EQ(1, num_called);
ASSERT_EQ(1U, calls.size());
- EXPECT_EQ(EvalStatus::kSucceeded, calls[0].first);
+ EXPECT_EQ(EvalStatus::kSucceeded, calls[0]);
// Wait for the timeout to expire, run the main loop again, ensure that
// nothing happened.
test_clock_.Advance(TimeDelta::FromSeconds(2));
@@ -299,17 +304,16 @@
// Set up an async policy call to exceed its expiration timeout, make sure
// that the default policy was not used (no callback) and that evaluation is
// reattempted.
+ vector<EvalStatus> calls;
+ Callback<void(EvalStatus)> callback = Bind(AccumulateCallsCallback, &calls);
+
int num_called = 0;
- umut_->set_policy(new DelayPolicy(
+ auto policy = std::make_unique<DelayPolicy>(
0,
fake_clock->GetWallclockTime() + TimeDelta::FromSeconds(3),
- &num_called));
-
- vector<pair<EvalStatus, UpdateCheckParams>> calls;
- Callback<void(EvalStatus, const UpdateCheckParams&)> callback =
- Bind(AccumulateCallsCallback<UpdateCheckParams>, &calls);
-
- umut_->AsyncPolicyRequest(callback, &Policy::UpdateCheckAllowed);
+ &num_called);
+ umut_->PolicyRequest2(
+ std::move(policy), std::make_shared<PolicyDataInterface>(), callback);
// Run the main loop, ensure that policy was attempted once but the callback
// was not invoked.
MessageLoopRunMaxIterations(MessageLoop::current(), 100);
@@ -332,7 +336,7 @@
MessageLoopRunMaxIterations(MessageLoop::current(), 10);
EXPECT_EQ(3, num_called);
ASSERT_EQ(1U, calls.size());
- EXPECT_EQ(EvalStatus::kSucceeded, calls[0].first);
+ EXPECT_EQ(EvalStatus::kSucceeded, calls[0]);
}
TEST_F(UmUpdateManagerTest, UpdateTimeRestrictionsMonitorIsNotNeeded) {