update_engine: Ditch UpdateCheckScheduler, use UpdateCheckAllowed instead.
This change removes the update_check_scheduler module and replaces it
with async requests to the UpdateCheckAllowed policy, done by the
UpdateAttempter directly.
* A new UpdateAttempter::ScheduleUpdates() is used as a replacement for
UpdateCheckScheduler::Run() and rescheduling of periodic checks inside
UpdateCheckScheduler. The callback
UpdateAttempter::OnUpdateScheduled() handles both periodic and
interactive checks.
* The UpdateAttempter keeps track of whether or not an update check is
being waited for (waiting_for_scheduled_check_) so that we can ensure
liveness. This is a similar check to the one performed inside the
UpdateCheckScheduler.
* Inference of the update target version prefix and channel (via device
policy), as well as update disabled, are now performed by the
UpdateManager policy. Also eliminating reference to the list of
network types allowed by policy, which is not enforced anyway and will
be superceded by another policy request (UpdateDownloadAllowed).
* Since update check scheduling is now performed relative to the last
update check time (as recorded by the UpdateAttempter), we care to
update this time as soon as the request is issued (in addition to when
a response is received). This ensures that we won't be scheduling
back-to-back update requests in the case where a response was not
received. Updating the last check time is delegated to a method call;
we replace raw use of time(2) with the ClockInterface abstraction.
* Handling of forced update checks has been revised: the UpdateAttempter
keeps track of the most recent app_version and omaha_url values that
were received through DBus events; it notifies the UpdateManager not
only of whether or not a forced (formerly, "interactive") update
request is pending, but also whether or not it is indeed interactive
or should be treated as a normal periodic one. The UpdateManager
reflects this back to the updater via the result output of
UpdateCheckAllowed, which tells the UpdateManager whether the custom
app_version and omaha_url should be used (interactive) or not.
BUG=chromium:358269
TEST=Unit tests.
Change-Id: Ifa9857b98e58fdd974f91a0fec674fa4472e3a9d
Reviewed-on: https://chromium-review.googlesource.com/209101
Reviewed-by: Gilad Arnold <garnold@chromium.org>
Commit-Queue: Gilad Arnold <garnold@chromium.org>
Tested-by: Gilad Arnold <garnold@chromium.org>
diff --git a/main.cc b/main.cc
index c5ea6e5..5966c25 100644
--- a/main.cc
+++ b/main.cc
@@ -26,7 +26,6 @@
#include "update_engine/subprocess.h"
#include "update_engine/terminator.h"
#include "update_engine/update_attempter.h"
-#include "update_engine/update_check_scheduler.h"
extern "C" {
#include "update_engine/update_engine.dbusserver.h"
}
@@ -194,10 +193,8 @@
update_attempter->set_dbus_service(service);
chromeos_update_engine::SetupDbusService(service);
- // Schedule periodic update checks.
- chromeos_update_engine::UpdateCheckScheduler scheduler(update_attempter,
- &real_system_state);
- scheduler.Run();
+ // Initiate update checks.
+ update_attempter->ScheduleUpdates();
// Update boot flags after 45 seconds.
g_timeout_add_seconds(45,
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index ede4e64..cb436a3 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -534,7 +534,7 @@
// error.
off_t ParseInt(const string& str) {
off_t ret = 0;
- int rc = sscanf(str.c_str(), "%" PRIi64, &ret);
+ int rc = sscanf(str.c_str(), "%" PRIi64, &ret); // NOLINT(runtime/printf)
if (rc < 1) {
// failure
return 0;
@@ -575,18 +575,17 @@
// chromium-os:37289: The PollInterval is not supported by Omaha server
// currently. But still keeping this existing code in case we ever decide to
- // slow down the request rate from the server-side. Note that the
- // PollInterval is not persisted, so it has to be sent by the server on every
- // response to guarantee that the UpdateCheckScheduler uses this value
- // (otherwise, if the device got rebooted after the last server-indicated
- // value, it'll revert to the default value). Also kDefaultMaxUpdateChecks
- // value for the scattering logic is based on the assumption that we perform
- // an update check every hour so that the max value of 8 will roughly be
- // equivalent to one work day. If we decide to use PollInterval permanently,
- // we should update the max_update_checks_allowed to take PollInterval into
- // account. Note: The parsing for PollInterval happens even before parsing
- // of the status because we may want to specify the PollInterval even when
- // there's no update.
+ // slow down the request rate from the server-side. Note that the PollInterval
+ // is not persisted, so it has to be sent by the server on every response to
+ // guarantee that the scheduler uses this value (otherwise, if the device got
+ // rebooted after the last server-indicated value, it'll revert to the default
+ // value). Also kDefaultMaxUpdateChecks value for the scattering logic is
+ // based on the assumption that we perform an update check every hour so that
+ // the max value of 8 will roughly be equivalent to one work day. If we decide
+ // to use PollInterval permanently, we should update the
+ // max_update_checks_allowed to take PollInterval into account. Note: The
+ // parsing for PollInterval happens even before parsing of the status because
+ // we may want to specify the PollInterval even when there's no update.
base::StringToInt(parser_data->updatecheck_poll_interval,
&output_object->poll_interval);
diff --git a/update_attempter.cc b/update_attempter.cc
index c6f1588..db9aefd 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -13,6 +13,7 @@
#include <utility>
#include <vector>
+#include <base/bind.h>
#include <base/file_util.h>
#include <base/logging.h>
#include <base/rand_util.h>
@@ -46,13 +47,20 @@
#include "update_engine/prefs_interface.h"
#include "update_engine/subprocess.h"
#include "update_engine/system_state.h"
-#include "update_engine/update_check_scheduler.h"
+#include "update_engine/update_manager/policy.h"
+#include "update_engine/update_manager/update_manager.h"
#include "update_engine/utils.h"
+using base::Bind;
+using base::Callback;
using base::StringPrintf;
using base::Time;
using base::TimeDelta;
using base::TimeTicks;
+using chromeos_update_manager::EvalStatus;
+using chromeos_update_manager::Policy;
+using chromeos_update_manager::UpdateCheckParams;
+using google::protobuf::NewPermanentCallback;
using std::make_pair;
using std::set;
using std::shared_ptr;
@@ -137,6 +145,10 @@
}
}
+UpdateAttempter::~UpdateAttempter() {
+ CleanupCpuSharesManagement();
+}
+
void UpdateAttempter::Init() {
// Pulling from the SystemState can only be done after construction, since
// this is an aggregate of various objects (such as the UpdateAttempter),
@@ -145,8 +157,16 @@
omaha_request_params_ = system_state_->request_params();
}
-UpdateAttempter::~UpdateAttempter() {
- CleanupCpuSharesManagement();
+void UpdateAttempter::ScheduleUpdates() {
+ chromeos_update_manager::UpdateManager* const update_manager =
+ system_state_->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->AsyncPolicyRequest(callback, &Policy::UpdateCheckAllowed);
+ waiting_for_scheduled_check_ = true;
}
bool UpdateAttempter::CheckAndReportDailyMetrics() {
@@ -222,16 +242,19 @@
void UpdateAttempter::Update(const string& app_version,
const string& omaha_url,
+ const string& target_channel,
+ const string& target_version_prefix,
bool obey_proxies,
bool interactive) {
- // This is called at least every 4 hours (see the constant
- // UpdateCheckScheduler::kTimeoutMaxBackoffInterval) so it's
- // appropriate to use as a hook for reporting daily metrics.
+ // This is normally called frequently enough so it's appropriate to use as a
+ // hook for reporting daily metrics.
+ // TODO(garnold) This should be hooked to a separate (reliable and consistent)
+ // timeout event.
CheckAndReportDailyMetrics();
// Notify of the new update attempt, clearing prior interactive requests.
- if (interactive_update_pending_callback_.get())
- interactive_update_pending_callback_->Run(false);
+ if (forced_update_pending_callback_.get())
+ forced_update_pending_callback_->Run(false, false);
chrome_proxy_resolver_.Init();
fake_update_success_ = false;
@@ -257,6 +280,8 @@
if (!CalculateUpdateParams(app_version,
omaha_url,
+ target_channel,
+ target_version_prefix,
obey_proxies,
interactive)) {
return;
@@ -266,6 +291,11 @@
SetStatusAndNotify(UPDATE_STATUS_CHECKING_FOR_UPDATE);
+ // Update the last check time here; it may be re-updated when an Omaha
+ // response is received, but this will prevent us from repeatedly scheduling
+ // checks in the case where a response is not received.
+ UpdateLastCheckedTime();
+
// Just in case we didn't update boot flags yet, make sure they're updated
// before any update processing starts.
start_action_processor_ = true;
@@ -325,32 +355,15 @@
bool UpdateAttempter::CalculateUpdateParams(const string& app_version,
const string& omaha_url,
+ const string& target_channel,
+ const string& target_version_prefix,
bool obey_proxies,
bool interactive) {
http_response_code_ = 0;
- RefreshDevicePolicy();
- const policy::DevicePolicy* device_policy = system_state_->device_policy();
- if (device_policy) {
- bool update_disabled = false;
- if (device_policy->GetUpdateDisabled(&update_disabled))
- omaha_request_params_->set_update_disabled(update_disabled);
-
- string target_version_prefix;
- if (device_policy->GetTargetVersionPrefix(&target_version_prefix))
- omaha_request_params_->set_target_version_prefix(target_version_prefix);
-
- set<string> allowed_types;
- string allowed_types_str;
- if (device_policy->GetAllowedConnectionTypesForUpdate(&allowed_types)) {
- set<string>::const_iterator iter;
- for (iter = allowed_types.begin(); iter != allowed_types.end(); ++iter)
- allowed_types_str += *iter + " ";
- }
-
- LOG(INFO) << "Networks over which updates are allowed per policy : "
- << (allowed_types_str.empty() ? "all" : allowed_types_str);
- }
+ // Set the target version prefix, if provided.
+ if (!target_version_prefix.empty())
+ omaha_request_params_->set_target_version_prefix(target_version_prefix);
CalculateScatteringParams(interactive);
@@ -374,34 +387,21 @@
return false;
}
- // Set the target channel iff ReleaseChannelDelegated policy is set to
- // false and a non-empty ReleaseChannel policy is present. If delegated
- // is true, we'll ignore ReleaseChannel policy value.
- if (device_policy) {
- bool delegated = false;
- if (!device_policy->GetReleaseChannelDelegated(&delegated) || delegated) {
- LOG(INFO) << "Channel settings are delegated to user by policy. "
- "Ignoring ReleaseChannel policy value";
- } else {
- LOG(INFO) << "Channel settings are not delegated to the user by policy";
- string target_channel;
- if (device_policy->GetReleaseChannel(&target_channel) &&
- !target_channel.empty()) {
- // Pass in false for powerwash_allowed until we add it to the policy
- // protobuf.
- LOG(INFO) << "Setting target channel from ReleaseChannel policy value";
- omaha_request_params_->SetTargetChannel(target_channel, false);
+ // Set the target channel, if one was provided.
+ if (target_channel.empty()) {
+ LOG(INFO) << "No target channel mandated by policy.";
+ } else {
+ LOG(INFO) << "Setting target channel as mandated: " << target_channel;
+ // Pass in false for powerwash_allowed until we add it to the policy
+ // protobuf.
+ omaha_request_params_->SetTargetChannel(target_channel, false);
- // Since this is the beginning of a new attempt, update the download
- // channel. The download channel won't be updated until the next
- // attempt, even if target channel changes meanwhile, so that how we'll
- // know if we should cancel the current download attempt if there's
- // such a change in target channel.
- omaha_request_params_->UpdateDownloadChannel();
- } else {
- LOG(INFO) << "No ReleaseChannel specified in policy";
- }
- }
+ // Since this is the beginning of a new attempt, update the download
+ // channel. The download channel won't be updated until the next attempt,
+ // even if target channel changes meanwhile, so that how we'll know if we
+ // should cancel the current download attempt if there's such a change in
+ // target channel.
+ omaha_request_params_->UpdateDownloadChannel();
}
LOG(INFO) << "update_disabled = "
@@ -805,19 +805,11 @@
void UpdateAttempter::CheckForUpdate(const string& app_version,
const string& omaha_url,
bool interactive) {
- LOG(INFO) << "Interactive update check requested.";
- if (interactive_update_pending_callback_.get())
- interactive_update_pending_callback_->Run(true);
-
- if (status_ != UPDATE_STATUS_IDLE) {
- LOG(INFO) << "Skipping update check because current status is "
- << UpdateStatusToString(status_);
- return;
- }
-
- // Pass through the interactive flag, in case we want to simulate a scheduled
- // test.
- Update(app_version, omaha_url, true, interactive);
+ LOG(INFO) << "Forced update check requested.";
+ forced_app_version_ = app_version;
+ forced_omaha_url_ = omaha_url;
+ if (forced_update_pending_callback_.get())
+ forced_update_pending_callback_->Run(true, interactive);
}
bool UpdateAttempter::RebootIfNeeded() {
@@ -888,6 +880,51 @@
return rc == 0;
}
+void UpdateAttempter::OnUpdateScheduled(EvalStatus status,
+ const UpdateCheckParams& params) {
+ waiting_for_scheduled_check_ = false;
+
+ if (status == EvalStatus::kSucceeded) {
+ if (!params.updates_enabled) {
+ LOG(WARNING) << "Updates permanently disabled.";
+ return;
+ }
+
+ LOG(INFO) << "Running "
+ << (params.is_interactive ? "interactive" : "periodic")
+ << " update.";
+
+ string app_version, omaha_url;
+ if (params.is_interactive) {
+ app_version = forced_app_version_;
+ omaha_url = forced_omaha_url_;
+ } else {
+ // Flush previously generated UMA reports before periodic updates.
+ CertificateChecker::FlushReport();
+ }
+
+ Update(app_version, omaha_url, params.target_channel,
+ params.target_version_prefix, false, params.is_interactive);
+ } else {
+ LOG(WARNING)
+ << "Update check scheduling failed (possibly timed out); retrying.";
+ ScheduleUpdates();
+ }
+
+ // This check ensures that future update checks will be or are already
+ // scheduled. The check should never fail. A check failure means that there's
+ // a bug that will most likely prevent further automatic update checks. It
+ // seems better to crash in such cases and restart the update_engine daemon
+ // into, hopefully, a known good state.
+ CHECK((this->status() != UPDATE_STATUS_IDLE &&
+ this->status() != UPDATE_STATUS_UPDATED_NEED_REBOOT) ||
+ waiting_for_scheduled_check_);
+}
+
+void UpdateAttempter::UpdateLastCheckedTime() {
+ last_checked_time_ = system_state_->clock()->GetWallclockTime().ToTimeT();
+}
+
// Delegate methods:
void UpdateAttempter::ProcessingDone(const ActionProcessor* processor,
ErrorCode code) {
@@ -1012,11 +1049,6 @@
// Store the server-dictated poll interval, if any.
server_dictated_poll_interval_ =
std::max(0, omaha_request_action->GetOutputObject().poll_interval);
- // TODO(garnold) Remove this once we deploy Update Manager.
- if (update_check_scheduler_) {
- update_check_scheduler_->set_poll_interval(
- server_dictated_poll_interval_);
- }
}
}
if (code != ErrorCode::kSuccess) {
@@ -1039,7 +1071,7 @@
// cases when the server and the client are unable to initiate the download.
CHECK(action == response_handler_action_.get());
const InstallPlan& plan = response_handler_action_->install_plan();
- last_checked_time_ = time(nullptr);
+ UpdateLastCheckedTime();
new_version_ = plan.version;
new_payload_size_ = plan.payload_size;
SetupDownload();
@@ -1223,8 +1255,10 @@
void UpdateAttempter::SetStatusAndNotify(UpdateStatus status) {
status_ = status;
- if (update_check_scheduler_) {
- update_check_scheduler_->SetUpdateStatus(status_);
+ // If not updating, schedule subsequent update checks.
+ if (status_ == UPDATE_STATUS_IDLE ||
+ status_ == UPDATE_STATUS_UPDATED_NEED_REBOOT) {
+ ScheduleUpdates();
}
BroadcastStatus();
}
@@ -1424,6 +1458,11 @@
LOG(WARNING) << "Action processor running, Omaha ping suppressed.";
}
+ // Update the last check time here; it may be re-updated when an Omaha
+ // response is received, but this will prevent us from repeatedly scheduling
+ // checks in the case where a response is not received.
+ UpdateLastCheckedTime();
+
// Update the status which will schedule the next update check
SetStatusAndNotify(UPDATE_STATUS_UPDATED_NEED_REBOOT);
}
diff --git a/update_attempter.h b/update_attempter.h
index ce827e3..2d274e5 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -25,6 +25,8 @@
#include "update_engine/omaha_response_handler_action.h"
#include "update_engine/proxy_resolver.h"
#include "update_engine/system_state.h"
+#include "update_engine/update_manager/policy.h"
+#include "update_engine/update_manager/update_manager.h"
class MetricsLibraryInterface;
struct UpdateEngineService;
@@ -36,7 +38,6 @@
namespace chromeos_update_engine {
class DBusWrapperInterface;
-class UpdateCheckScheduler;
enum UpdateStatus {
UPDATE_STATUS_IDLE = 0,
@@ -64,14 +65,20 @@
// Further initialization to be done post construction.
void Init();
+ // Initiates scheduling of update checks.
+ virtual void ScheduleUpdates();
+
// Checks for update and, if a newer version is available, attempts to update
// the system. Non-empty |in_app_version| or |in_update_url| prevents
- // automatic detection of the parameter. If |obey_proxies| is true, the
- // update will likely respect Chrome's proxy setting. For security reasons, we
- // may still not honor them. |interactive| should be true if this was called
- // from the user (ie dbus).
+ // automatic detection of the parameter. |target_channel| denotes a
+ // policy-mandated channel we are updating to, if not empty. If |obey_proxies|
+ // is true, the update will likely respect Chrome's proxy setting. For
+ // security reasons, we may still not honor them. |interactive| should be true
+ // if this was called from the user (ie dbus).
virtual void Update(const std::string& app_version,
const std::string& omaha_url,
+ const std::string& target_channel,
+ const std::string& target_version_prefix,
bool obey_proxies,
bool interactive);
@@ -125,13 +132,6 @@
dbus_service_ = dbus_service;
}
- UpdateCheckScheduler* update_check_scheduler() const {
- return update_check_scheduler_;
- }
- void set_update_check_scheduler(UpdateCheckScheduler* scheduler) {
- update_check_scheduler_ = scheduler;
- }
-
// This is the internal entry point for going through an
// update. If the current status is idle invokes Update.
// This is called by the DBus implementation.
@@ -206,14 +206,17 @@
return server_dictated_poll_interval_;
}
- // Sets a callback to be used when either an interactive update request is
- // received (true) or cleared by an update attempt (false). Takes ownership of
- // the callback object. A null value disables callback on these events. Note
- // that only one callback can be set, so effectively at most one client can be
- // notified.
- virtual void set_interactive_update_pending_callback(
- base::Callback<void(bool)>* callback) { // NOLINT(readability/function)
- interactive_update_pending_callback_.reset(callback);
+ // Sets a callback to be used when either a forced update request is received
+ // (first argument set to true) or cleared by an update attempt (first
+ // argument set to false). The callback further encodes whether the forced
+ // check is an interactive one (second argument set to true). Takes ownership
+ // of the callback object. A null value disables callback on these events.
+ // Note that only one callback can be set, so effectively at most one client
+ // can be notified.
+ virtual void set_forced_update_pending_callback(
+ base::Callback<void(bool, bool)>* // NOLINT(readability/function)
+ callback) {
+ forced_update_pending_callback_.reset(callback);
}
private:
@@ -317,6 +320,8 @@
// Update() method for the meaning of the parametes.
bool CalculateUpdateParams(const std::string& app_version,
const std::string& omaha_url,
+ const std::string& target_channel,
+ const std::string& target_version_prefix,
bool obey_proxies,
bool interactive);
@@ -372,6 +377,16 @@
// success.
bool RebootDirectly();
+ // Callback for the async UpdateCheckAllowed 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);
+
+ // Updates the time an update was last attempted to the current time.
+ void UpdateLastCheckedTime();
+
// Last status notification timestamp used for throttling. Use monotonic
// TimeTicks to ensure that notifications are sent even if the system clock is
// set back in the middle of an update.
@@ -402,9 +417,6 @@
// is convenient this way.
PrefsInterface* prefs_ = nullptr;
- // The current UpdateCheckScheduler to notify of state transitions.
- UpdateCheckScheduler* update_check_scheduler_ = nullptr;
-
// Pending error event, if any.
scoped_ptr<OmahaEvent> error_event_;
@@ -481,10 +493,21 @@
// otherwise. This is needed for calculating the update check interval.
unsigned int server_dictated_poll_interval_ = 0;
- // A callback to use when either an interactive update request is received
- // (true) or cleared by an update attempt (false).
- scoped_ptr<base::Callback<void(bool)>> // NOLINT(readability/function)
- interactive_update_pending_callback_;
+ // Tracks whether we have scheduled update checks.
+ bool waiting_for_scheduled_check_ = false;
+
+ // A callback to use when a forced update request is either received (true) or
+ // cleared by an update attempt (false). The second argument indicates whether
+ // this is an interactive update, and its value is significant iff the first
+ // argument is true.
+ scoped_ptr<base::Callback<void(bool, bool)>> // NOLINT(readability/function)
+ forced_update_pending_callback_;
+
+ // The |app_version| and |omaha_url| parameters received during the latest
+ // forced update request. They are retrieved for use once the update is
+ // actually scheduled.
+ std::string forced_app_version_;
+ std::string forced_omaha_url_;
DISALLOW_COPY_AND_ASSIGN(UpdateAttempter);
};
diff --git a/update_attempter_mock.h b/update_attempter_mock.h
index 37a6275..38bf9a7 100644
--- a/update_attempter_mock.h
+++ b/update_attempter_mock.h
@@ -17,8 +17,10 @@
public:
using UpdateAttempter::UpdateAttempter;
- MOCK_METHOD4(Update, void(const std::string& app_version,
+ MOCK_METHOD6(Update, void(const std::string& app_version,
const std::string& omaha_url,
+ const std::string& target_channel,
+ const std::string& target_version_prefix,
bool obey_proxies,
bool interactive));
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 765d71d..cd83445 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -24,7 +24,6 @@
#include "update_engine/prefs_mock.h"
#include "update_engine/test_utils.h"
#include "update_engine/update_attempter.h"
-#include "update_engine/update_check_scheduler.h"
#include "update_engine/utils.h"
using base::Time;
@@ -57,6 +56,26 @@
DBusWrapperInterface* dbus_iface,
const std::string& update_completed_marker)
: UpdateAttempter(system_state, dbus_iface, update_completed_marker) {}
+
+ // Wrap the update scheduling method, allowing us to opt out of scheduled
+ // updates for testing purposes.
+ void ScheduleUpdates() override {
+ schedule_updates_called_ = true;
+ if (do_schedule_updates_) {
+ UpdateAttempter::ScheduleUpdates();
+ } else {
+ LOG(INFO) << "[TEST] Update scheduling disabled.";
+ }
+ }
+ void EnableScheduleUpdates() { do_schedule_updates_ = true; }
+ void DisableScheduleUpdates() { do_schedule_updates_ = false; }
+
+ // Indicates whether ScheduleUpdates() was called.
+ bool schedule_updates_called() const { return schedule_updates_called_; }
+
+ private:
+ bool schedule_updates_called_ = false;
+ bool do_schedule_updates_ = true;
};
class UpdateAttempterTest : public ::testing::Test {
@@ -84,7 +103,6 @@
EXPECT_EQ(nullptr, attempter_.dbus_service_);
EXPECT_NE(nullptr, attempter_.system_state_);
- EXPECT_EQ(nullptr, attempter_.update_check_scheduler_);
EXPECT_EQ(0, attempter_.http_response_code_);
EXPECT_EQ(utils::kCpuSharesNormal, attempter_.shares_);
EXPECT_EQ(nullptr, attempter_.manage_shares_source_);
@@ -121,16 +139,6 @@
void PingOmahaTestStart();
static gboolean StaticPingOmahaTestStart(gpointer data);
- void ReadChannelFromPolicyTestStart();
- static gboolean StaticReadChannelFromPolicyTestStart(gpointer data);
-
- void ReadUpdateDisabledFromPolicyTestStart();
- static gboolean StaticReadUpdateDisabledFromPolicyTestStart(gpointer data);
-
- void ReadTargetVersionPrefixFromPolicyTestStart();
- static gboolean StaticReadTargetVersionPrefixFromPolicyTestStart(
- gpointer data);
-
void ReadScatterFactorFromPolicyTestStart();
static gboolean StaticReadScatterFactorFromPolicyTestStart(
gpointer data);
@@ -200,13 +208,11 @@
OmahaResponse response;
response.poll_interval = 234;
action.SetOutputObject(response);
- UpdateCheckScheduler scheduler(&attempter_, &fake_system_state_);
- attempter_.set_update_check_scheduler(&scheduler);
EXPECT_CALL(*prefs_, GetInt64(kPrefsDeltaUpdateFailures, _)).Times(0);
attempter_.ActionCompleted(nullptr, &action, ErrorCode::kSuccess);
EXPECT_EQ(500, attempter_.http_response_code());
EXPECT_EQ(UPDATE_STATUS_IDLE, attempter_.status());
- EXPECT_EQ(234, scheduler.poll_interval());
+ EXPECT_EQ(234, attempter_.server_dictated_poll_interval_);
ASSERT_TRUE(attempter_.error_event_.get() == nullptr);
}
@@ -374,27 +380,6 @@
return FALSE;
}
-gboolean UpdateAttempterTest::StaticReadChannelFromPolicyTestStart(
- gpointer data) {
- UpdateAttempterTest* ua_test = reinterpret_cast<UpdateAttempterTest*>(data);
- ua_test->ReadChannelFromPolicyTestStart();
- return FALSE;
-}
-
-gboolean UpdateAttempterTest::StaticReadUpdateDisabledFromPolicyTestStart(
- gpointer data) {
- UpdateAttempterTest* ua_test = reinterpret_cast<UpdateAttempterTest*>(data);
- ua_test->ReadUpdateDisabledFromPolicyTestStart();
- return FALSE;
-}
-
-gboolean UpdateAttempterTest::StaticReadTargetVersionPrefixFromPolicyTestStart(
- gpointer data) {
- UpdateAttempterTest* ua_test = reinterpret_cast<UpdateAttempterTest*>(data);
- ua_test->ReadTargetVersionPrefixFromPolicyTestStart();
- return FALSE;
-}
-
gboolean UpdateAttempterTest::StaticReadScatterFactorFromPolicyTestStart(
gpointer data) {
UpdateAttempterTest* ua_test = reinterpret_cast<UpdateAttempterTest*>(data);
@@ -450,7 +435,7 @@
}
EXPECT_CALL(*processor_, StartProcessing()).Times(1);
- attempter_.Update("", "", false, false);
+ attempter_.Update("", "", "", "", false, false);
g_idle_add(&StaticUpdateTestVerify, this);
}
@@ -587,17 +572,18 @@
}
TEST_F(UpdateAttempterTest, PingOmahaTest) {
- UpdateCheckScheduler scheduler(&attempter_, &fake_system_state_);
- scheduler.enabled_ = true;
- EXPECT_FALSE(scheduler.scheduled_);
- attempter_.set_update_check_scheduler(&scheduler);
+ EXPECT_FALSE(attempter_.waiting_for_scheduled_check_);
+ EXPECT_FALSE(attempter_.schedule_updates_called());
+ // Disable scheduling of subsequnet checks; we're using the DefaultPolicy in
+ // testing, which is more permissive than we want to handle here.
+ attempter_.DisableScheduleUpdates();
loop_ = g_main_loop_new(g_main_context_default(), FALSE);
g_idle_add(&StaticPingOmahaTestStart, this);
g_main_loop_run(loop_);
g_main_loop_unref(loop_);
loop_ = nullptr;
EXPECT_EQ(UPDATE_STATUS_UPDATED_NEED_REBOOT, attempter_.status());
- EXPECT_EQ(true, scheduler.scheduled_);
+ EXPECT_TRUE(attempter_.schedule_updates_called());
}
TEST_F(UpdateAttempterTest, CreatePendingErrorEventTest) {
@@ -632,70 +618,6 @@
attempter_.error_event_->error_code);
}
-TEST_F(UpdateAttempterTest, ReadChannelFromPolicy) {
- loop_ = g_main_loop_new(g_main_context_default(), FALSE);
- g_idle_add(&StaticReadChannelFromPolicyTestStart, this);
- g_main_loop_run(loop_);
- g_main_loop_unref(loop_);
- loop_ = nullptr;
-}
-
-void UpdateAttempterTest::ReadChannelFromPolicyTestStart() {
- // Tests that the update channel (aka release channel) is properly fetched
- // from the device policy.
-
- policy::MockDevicePolicy* device_policy = new policy::MockDevicePolicy();
- attempter_.policy_provider_.reset(new policy::PolicyProvider(device_policy));
-
- EXPECT_CALL(*device_policy, LoadPolicy()).WillRepeatedly(Return(true));
- fake_system_state_.set_device_policy(device_policy);
-
- EXPECT_CALL(*device_policy, GetReleaseChannelDelegated(_)).WillRepeatedly(
- DoAll(SetArgumentPointee<0>(bool(false)), // NOLINT(readability/casting)
- Return(true)));
-
- EXPECT_CALL(*device_policy, GetReleaseChannel(_)).WillRepeatedly(
- DoAll(SetArgumentPointee<0>(std::string("beta-channel")),
- Return(true)));
-
- ASSERT_FALSE(test_dir_.empty());
- attempter_.omaha_request_params_->set_root(test_dir_);
- attempter_.Update("", "", false, false);
- EXPECT_EQ("beta-channel",
- attempter_.omaha_request_params_->target_channel());
-
- g_idle_add(&StaticQuitMainLoop, this);
-}
-
-TEST_F(UpdateAttempterTest, ReadUpdateDisabledFromPolicy) {
- loop_ = g_main_loop_new(g_main_context_default(), FALSE);
- g_idle_add(&StaticReadUpdateDisabledFromPolicyTestStart, this);
- g_main_loop_run(loop_);
- g_main_loop_unref(loop_);
- loop_ = nullptr;
-}
-
-void UpdateAttempterTest::ReadUpdateDisabledFromPolicyTestStart() {
- // Tests that the update_disbled flag is properly fetched
- // from the device policy.
-
- policy::MockDevicePolicy* device_policy = new policy::MockDevicePolicy();
- attempter_.policy_provider_.reset(new policy::PolicyProvider(device_policy));
-
- EXPECT_CALL(*device_policy, LoadPolicy()).WillRepeatedly(Return(true));
- fake_system_state_.set_device_policy(device_policy);
-
- EXPECT_CALL(*device_policy, GetUpdateDisabled(_))
- .WillRepeatedly(DoAll(
- SetArgumentPointee<0>(true),
- Return(true)));
-
- attempter_.Update("", "", false, false);
- EXPECT_TRUE(attempter_.omaha_request_params_->update_disabled());
-
- g_idle_add(&StaticQuitMainLoop, this);
-}
-
TEST_F(UpdateAttempterTest, P2PNotStartedAtStartupWhenNotEnabled) {
MockP2PManager mock_p2p_manager;
fake_system_state_.set_p2p_manager(&mock_p2p_manager);
@@ -740,7 +662,7 @@
fake_system_state_.set_p2p_manager(&mock_p2p_manager);
mock_p2p_manager.fake().SetP2PEnabled(false);
EXPECT_CALL(mock_p2p_manager, PerformHousekeeping()).Times(0);
- attempter_.Update("", "", false, false);
+ attempter_.Update("", "", "", "", false, false);
EXPECT_FALSE(attempter_.omaha_request_params_->use_p2p_for_downloading());
EXPECT_FALSE(attempter_.omaha_request_params_->use_p2p_for_sharing());
g_idle_add(&StaticQuitMainLoop, this);
@@ -768,7 +690,7 @@
mock_p2p_manager.fake().SetEnsureP2PRunningResult(false);
mock_p2p_manager.fake().SetPerformHousekeepingResult(false);
EXPECT_CALL(mock_p2p_manager, PerformHousekeeping()).Times(0);
- attempter_.Update("", "", false, false);
+ attempter_.Update("", "", "", "", false, false);
EXPECT_FALSE(attempter_.omaha_request_params_->use_p2p_for_downloading());
EXPECT_FALSE(attempter_.omaha_request_params_->use_p2p_for_sharing());
g_idle_add(&StaticQuitMainLoop, this);
@@ -796,7 +718,7 @@
mock_p2p_manager.fake().SetEnsureP2PRunningResult(true);
mock_p2p_manager.fake().SetPerformHousekeepingResult(false);
EXPECT_CALL(mock_p2p_manager, PerformHousekeeping()).Times(1);
- attempter_.Update("", "", false, false);
+ attempter_.Update("", "", "", "", false, false);
EXPECT_FALSE(attempter_.omaha_request_params_->use_p2p_for_downloading());
EXPECT_FALSE(attempter_.omaha_request_params_->use_p2p_for_sharing());
g_idle_add(&StaticQuitMainLoop, this);
@@ -823,7 +745,7 @@
mock_p2p_manager.fake().SetEnsureP2PRunningResult(true);
mock_p2p_manager.fake().SetPerformHousekeepingResult(true);
EXPECT_CALL(mock_p2p_manager, PerformHousekeeping()).Times(1);
- attempter_.Update("", "", false, false);
+ attempter_.Update("", "", "", "", false, false);
EXPECT_TRUE(attempter_.omaha_request_params_->use_p2p_for_downloading());
EXPECT_TRUE(attempter_.omaha_request_params_->use_p2p_for_sharing());
g_idle_add(&StaticQuitMainLoop, this);
@@ -851,45 +773,12 @@
mock_p2p_manager.fake().SetEnsureP2PRunningResult(true);
mock_p2p_manager.fake().SetPerformHousekeepingResult(true);
EXPECT_CALL(mock_p2p_manager, PerformHousekeeping()).Times(1);
- attempter_.Update("", "", false, true /* interactive */);
+ attempter_.Update("", "", "", "", false, true /* interactive */);
EXPECT_FALSE(attempter_.omaha_request_params_->use_p2p_for_downloading());
EXPECT_TRUE(attempter_.omaha_request_params_->use_p2p_for_sharing());
g_idle_add(&StaticQuitMainLoop, this);
}
-TEST_F(UpdateAttempterTest, ReadTargetVersionPrefixFromPolicy) {
- loop_ = g_main_loop_new(g_main_context_default(), FALSE);
- g_idle_add(&StaticReadTargetVersionPrefixFromPolicyTestStart, this);
- g_main_loop_run(loop_);
- g_main_loop_unref(loop_);
- loop_ = nullptr;
-}
-
-void UpdateAttempterTest::ReadTargetVersionPrefixFromPolicyTestStart() {
- // Tests that the target_version_prefix value is properly fetched
- // from the device policy.
-
- const std::string target_version_prefix = "1412.";
-
- policy::MockDevicePolicy* device_policy = new policy::MockDevicePolicy();
- attempter_.policy_provider_.reset(new policy::PolicyProvider(device_policy));
-
- EXPECT_CALL(*device_policy, LoadPolicy()).WillRepeatedly(Return(true));
- fake_system_state_.set_device_policy(device_policy);
-
- EXPECT_CALL(*device_policy, GetTargetVersionPrefix(_))
- .WillRepeatedly(DoAll(
- SetArgumentPointee<0>(target_version_prefix),
- Return(true)));
-
- attempter_.Update("", "", false, false);
- EXPECT_EQ(target_version_prefix.c_str(),
- attempter_.omaha_request_params_->target_version_prefix());
-
- g_idle_add(&StaticQuitMainLoop, this);
-}
-
-
TEST_F(UpdateAttempterTest, ReadScatterFactorFromPolicy) {
loop_ = g_main_loop_new(g_main_context_default(), FALSE);
g_idle_add(&StaticReadScatterFactorFromPolicyTestStart, this);
@@ -914,7 +803,7 @@
SetArgumentPointee<0>(scatter_factor_in_seconds),
Return(true)));
- attempter_.Update("", "", false, false);
+ attempter_.Update("", "", "", "", false, false);
EXPECT_EQ(scatter_factor_in_seconds, attempter_.scatter_factor_.InSeconds());
g_idle_add(&StaticQuitMainLoop, this);
@@ -959,7 +848,7 @@
SetArgumentPointee<0>(scatter_factor_in_seconds),
Return(true)));
- attempter_.Update("", "", false, false);
+ attempter_.Update("", "", "", "", false, false);
EXPECT_EQ(scatter_factor_in_seconds, attempter_.scatter_factor_.InSeconds());
// Make sure the file still exists.
@@ -975,7 +864,7 @@
// However, if the count is already 0, it's not decremented. Test that.
initial_value = 0;
EXPECT_TRUE(prefs.SetInt64(kPrefsUpdateCheckCount, initial_value));
- attempter_.Update("", "", false, false);
+ attempter_.Update("", "", "", "", false, false);
EXPECT_TRUE(prefs.Exists(kPrefsUpdateCheckCount));
EXPECT_TRUE(prefs.GetInt64(kPrefsUpdateCheckCount, &new_value));
EXPECT_EQ(initial_value, new_value);
@@ -1026,7 +915,7 @@
Return(true)));
// Trigger an interactive check so we can test that scattering is disabled.
- attempter_.Update("", "", false, true);
+ attempter_.Update("", "", "", "", false, true);
EXPECT_EQ(scatter_factor_in_seconds, attempter_.scatter_factor_.InSeconds());
// Make sure scattering is disabled for manual (i.e. user initiated) update
@@ -1049,7 +938,7 @@
string temp_dir;
// We need persistent preferences for this test
- EXPECT_TRUE(utils::MakeTempDirectory("UpdateCheckScheduler.XXXXXX",
+ EXPECT_TRUE(utils::MakeTempDirectory("UpdateAttempterTest.XXXXXX",
&temp_dir));
prefs.Init(base::FilePath(temp_dir));
fake_system_state_.set_clock(&fake_clock);
diff --git a/update_check_scheduler.cc b/update_check_scheduler.cc
deleted file mode 100644
index 5358416..0000000
--- a/update_check_scheduler.cc
+++ /dev/null
@@ -1,160 +0,0 @@
-// Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "update_engine/update_check_scheduler.h"
-
-#include "update_engine/certificate_checker.h"
-#include "update_engine/hardware_interface.h"
-#include "update_engine/http_common.h"
-#include "update_engine/system_state.h"
-#include "update_engine/utils.h"
-
-namespace chromeos_update_engine {
-
-// Default update check timeout interval/fuzz values, in seconds. Note that
-// actual fuzz is within +/- half of the indicated value.
-const int UpdateCheckScheduler::kTimeoutInitialInterval = 7 * 60;
-const int UpdateCheckScheduler::kTimeoutPeriodicInterval = 45 * 60;
-const int UpdateCheckScheduler::kTimeoutMaxBackoffInterval = 4 * 60 * 60;
-const int UpdateCheckScheduler::kTimeoutRegularFuzz = 10 * 60;
-
-UpdateCheckScheduler::UpdateCheckScheduler(UpdateAttempter* update_attempter,
- SystemState* system_state)
- : update_attempter_(update_attempter),
- enabled_(false),
- scheduled_(false),
- last_interval_(0),
- poll_interval_(0),
- system_state_(system_state) {}
-
-UpdateCheckScheduler::~UpdateCheckScheduler() {}
-
-void UpdateCheckScheduler::Run() {
- enabled_ = false;
- update_attempter_->set_update_check_scheduler(nullptr);
-
- if (!system_state_->hardware()->IsOfficialBuild()) {
- LOG(WARNING) << "Non-official build: periodic update checks disabled.";
- return;
- }
- if (system_state_->hardware()->IsBootDeviceRemovable()) {
- LOG(WARNING) << "Removable device boot: periodic update checks disabled.";
- return;
- }
- enabled_ = true;
-
- // Registers this scheduler with the update attempter so that scheduler can be
- // notified of update status changes.
- update_attempter_->set_update_check_scheduler(this);
-
- // Kicks off periodic update checks. The first check is scheduled
- // |kTimeoutInitialInterval| seconds from now. Subsequent checks are scheduled
- // by ScheduleNextCheck, normally at |kTimeoutPeriodicInterval|-second
- // intervals.
- ScheduleCheck(kTimeoutInitialInterval, kTimeoutRegularFuzz);
-}
-
-guint UpdateCheckScheduler::GTimeoutAddSeconds(guint interval,
- GSourceFunc function) {
- return g_timeout_add_seconds(interval, function, this);
-}
-
-void UpdateCheckScheduler::ScheduleCheck(int interval, int fuzz) {
- if (!CanSchedule()) {
- return;
- }
- last_interval_ = interval;
- interval = utils::FuzzInt(interval, fuzz);
- if (interval < 0) {
- interval = 0;
- }
- GTimeoutAddSeconds(interval, StaticCheck);
- scheduled_ = true;
- LOG(INFO) << "Next update check in " << utils::FormatSecs(interval);
-}
-
-gboolean UpdateCheckScheduler::StaticCheck(void* scheduler) {
- UpdateCheckScheduler* me = reinterpret_cast<UpdateCheckScheduler*>(scheduler);
- CHECK(me->scheduled_);
- me->scheduled_ = false;
-
- if (me->system_state_->hardware()->IsOOBEComplete(nullptr)) {
- // Before updating, we flush any previously generated UMA reports.
- CertificateChecker::FlushReport();
- me->update_attempter_->Update("", "", false, false);
- } else {
- // Skips all automatic update checks if the OOBE process is not complete and
- // schedules a new check as if it is the first one.
- LOG(WARNING) << "Skipping update check because OOBE is not complete.";
- me->ScheduleCheck(kTimeoutInitialInterval, kTimeoutRegularFuzz);
- }
- // This check ensures that future update checks will be or are already
- // scheduled. The check should never fail. A check failure means that there's
- // a bug that will most likely prevent further automatic update checks. It
- // seems better to crash in such cases and restart the update_engine daemon
- // into, hopefully, a known good state.
- CHECK(me->update_attempter_->status() != UPDATE_STATUS_IDLE ||
- !me->CanSchedule());
- return FALSE; // Don't run again.
-}
-
-void UpdateCheckScheduler::ComputeNextIntervalAndFuzz(int* next_interval,
- int* next_fuzz) {
- CHECK(next_interval && next_fuzz);
-
- int interval = 0;
- int fuzz = 0; // Use default fuzz value (see below)
-
- int http_response_code;
- if (poll_interval_ > 0) {
- // Server-dictated poll interval.
- interval = poll_interval_;
- LOG(WARNING) << "Using server-dictated poll interval: " << interval;
- } else if ((http_response_code = update_attempter_->http_response_code()) ==
- kHttpResponseInternalServerError ||
- http_response_code == kHttpResponseServiceUnavailable) {
- // Implements exponential backoff on 500 (Internal Server Error) and 503
- // (Service Unavailable) HTTP response codes.
- interval = 2 * last_interval_;
- LOG(WARNING) << "Exponential backoff due to HTTP response code ("
- << http_response_code << ")";
- }
-
- // Backoff cannot exceed a predetermined maximum period.
- if (interval > kTimeoutMaxBackoffInterval)
- interval = kTimeoutMaxBackoffInterval;
-
- // Ensures that under normal conditions the regular update check interval
- // and fuzz are used. Also covers the case where backoff is required based
- // on the initial update check.
- if (interval < kTimeoutPeriodicInterval) {
- interval = kTimeoutPeriodicInterval;
- fuzz = kTimeoutRegularFuzz;
- }
-
- // Set default fuzz to +/- |interval|/2.
- if (fuzz == 0)
- fuzz = interval;
-
- *next_interval = interval;
- *next_fuzz = fuzz;
-}
-
-void UpdateCheckScheduler::ScheduleNextCheck() {
- int interval, fuzz;
- ComputeNextIntervalAndFuzz(&interval, &fuzz);
- ScheduleCheck(interval, fuzz);
-}
-
-void UpdateCheckScheduler::SetUpdateStatus(UpdateStatus status) {
- // We want to schedule the update checks for when we're idle as well as
- // after we've successfully applied an update and waiting for the user
- // to reboot to ensure our active count is accurate.
- if (status == UPDATE_STATUS_IDLE ||
- status == UPDATE_STATUS_UPDATED_NEED_REBOOT) {
- ScheduleNextCheck();
- }
-}
-
-} // namespace chromeos_update_engine
diff --git a/update_check_scheduler.h b/update_check_scheduler.h
deleted file mode 100644
index 57613f9..0000000
--- a/update_check_scheduler.h
+++ /dev/null
@@ -1,127 +0,0 @@
-// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef UPDATE_ENGINE_UPDATE_CHECK_SCHEDULER_H_
-#define UPDATE_ENGINE_UPDATE_CHECK_SCHEDULER_H_
-
-#include <base/basictypes.h>
-#include <glib.h>
-#include <gtest/gtest_prod.h> // for FRIEND_TEST
-
-#include "update_engine/system_state.h"
-#include "update_engine/update_attempter.h"
-
-namespace chromeos_update_engine {
-
-// UpdateCheckScheduler manages the periodic background update checks. This is
-// the basic update check cycle:
-//
-// Run
-// |
-// v
-// /->ScheduleCheck
-// | |
-// | v
-// | StaticCheck (invoked through a GLib timeout source)
-// | |
-// | v
-// | UpdateAttempter::Update
-// | |
-// | v
-// | SetUpdateStatus (invoked by UpdateAttempter on state transitions)
-// | |
-// | v
-// | ScheduleNextCheck (invoked when UpdateAttempter becomes idle)
-// \---/
-class UpdateCheckScheduler {
- public:
- static const int kTimeoutInitialInterval;
- static const int kTimeoutPeriodicInterval;
- static const int kTimeoutRegularFuzz;
- static const int kTimeoutMaxBackoffInterval;
-
- UpdateCheckScheduler(UpdateAttempter* update_attempter,
- SystemState* system_state);
- virtual ~UpdateCheckScheduler();
-
- // Initiates the periodic update checks, if necessary.
- void Run();
-
- // Sets the new update status. This is invoked by UpdateAttempter.
- void SetUpdateStatus(UpdateStatus status);
-
- void set_poll_interval(int interval) { poll_interval_ = interval; }
- int poll_interval() const { return poll_interval_; }
-
- private:
- friend class UpdateCheckSchedulerTest;
- FRIEND_TEST(UpdateCheckSchedulerTest, CanScheduleTest);
- FRIEND_TEST(UpdateCheckSchedulerTest, ComputeNextIntervalAndFuzzBackoffTest);
- FRIEND_TEST(UpdateCheckSchedulerTest, ComputeNextIntervalAndFuzzPollTest);
- FRIEND_TEST(UpdateCheckSchedulerTest, ComputeNextIntervalAndFuzzPriorityTest);
- FRIEND_TEST(UpdateCheckSchedulerTest, ComputeNextIntervalAndFuzzTest);
- FRIEND_TEST(UpdateCheckSchedulerTest, GTimeoutAddSecondsTest);
- FRIEND_TEST(UpdateCheckSchedulerTest, RunBootDeviceRemovableTest);
- FRIEND_TEST(UpdateCheckSchedulerTest, RunNonOfficialBuildTest);
- FRIEND_TEST(UpdateCheckSchedulerTest, RunTest);
- FRIEND_TEST(UpdateCheckSchedulerTest, ScheduleCheckDisabledTest);
- FRIEND_TEST(UpdateCheckSchedulerTest, ScheduleCheckEnabledTest);
- FRIEND_TEST(UpdateCheckSchedulerTest, ScheduleCheckNegativeIntervalTest);
- FRIEND_TEST(UpdateCheckSchedulerTest, ScheduleNextCheckDisabledTest);
- FRIEND_TEST(UpdateCheckSchedulerTest, ScheduleNextCheckEnabledTest);
- FRIEND_TEST(UpdateCheckSchedulerTest, SetUpdateStatusIdleDisabledTest);
- FRIEND_TEST(UpdateCheckSchedulerTest, SetUpdateStatusIdleEnabledTest);
- FRIEND_TEST(UpdateCheckSchedulerTest, SetUpdateStatusNonIdleTest);
- FRIEND_TEST(UpdateCheckSchedulerTest, StaticCheckOOBECompleteTest);
- FRIEND_TEST(UpdateCheckSchedulerTest, StaticCheckOOBENotCompleteTest);
- FRIEND_TEST(UpdateAttempterTest, PingOmahaTest);
-
- // Wraps GLib's g_timeout_add_seconds so that it can be mocked in tests.
- virtual guint GTimeoutAddSeconds(guint interval, GSourceFunc function);
-
- // Returns true if an update check can be scheduled. An update check should
- // not be scheduled if periodic update checks are disabled or if one is
- // already scheduled.
- bool CanSchedule() { return enabled_ && !scheduled_; }
-
- // Schedules the next periodic update check |interval| seconds from now
- // randomized by +/- |fuzz|/2.
- void ScheduleCheck(int interval, int fuzz);
-
- // GLib timeout source callback. Initiates an update check through the update
- // attempter.
- static gboolean StaticCheck(void* scheduler);
-
- // Schedules the next update check by setting up a timeout source;
- void ScheduleNextCheck();
-
- // Computes the timeout interval along with its random fuzz range for the next
- // update check by taking into account the last timeout interval as well as
- // the last update status.
- void ComputeNextIntervalAndFuzz(int* next_interval, int* next_fuzz);
-
- // The UpdateAttempter to use for update checks.
- UpdateAttempter* update_attempter_;
-
- // True if automatic update checks should be scheduled, false otherwise.
- bool enabled_;
-
- // True if there's an update check scheduled already, false otherwise.
- bool scheduled_;
-
- // The timeout interval (before fuzzing) for the last update check.
- int last_interval_;
-
- // Server dictated poll interval in seconds, if positive.
- int poll_interval_;
-
- // The external state of the system outside the update_engine process.
- SystemState* system_state_;
-
- DISALLOW_COPY_AND_ASSIGN(UpdateCheckScheduler);
-};
-
-} // namespace chromeos_update_engine
-
-#endif // UPDATE_ENGINE_UPDATE_CHECK_SCHEDULER_H_
diff --git a/update_check_scheduler_unittest.cc b/update_check_scheduler_unittest.cc
deleted file mode 100644
index dda3b40..0000000
--- a/update_check_scheduler_unittest.cc
+++ /dev/null
@@ -1,298 +0,0 @@
-// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "update_engine/update_check_scheduler.h"
-
-#include <gtest/gtest.h>
-
-#include "update_engine/certificate_checker.h"
-#include "update_engine/certificate_checker_mock.h"
-#include "update_engine/fake_system_state.h"
-#include "update_engine/update_attempter_mock.h"
-
-using base::Time;
-using testing::_;
-using testing::AllOf;
-using testing::Assign;
-using testing::Ge;
-using testing::Le;
-using testing::MockFunction;
-using testing::Return;
-using testing::SetArgumentPointee;
-
-namespace chromeos_update_engine {
-
-namespace {
-void FuzzRange(int interval, int fuzz, int* interval_min, int* interval_max) {
- *interval_min = interval - fuzz / 2;
- *interval_max = interval + fuzz - fuzz / 2;
-}
-} // namespace
-
-// Test a subclass rather than the main class directly so that we can mock out
-// GLib and utils in tests. There're explicit unit test for the wrapper methods.
-class UpdateCheckSchedulerUnderTest : public UpdateCheckScheduler {
- public:
- UpdateCheckSchedulerUnderTest(UpdateAttempter* update_attempter,
- FakeSystemState* fake_system_state)
- : UpdateCheckScheduler(update_attempter, fake_system_state),
- fake_system_state_(fake_system_state) {}
-
- MOCK_METHOD2(GTimeoutAddSeconds, guint(guint seconds, GSourceFunc function));
-
- FakeSystemState* fake_system_state_;
-};
-
-class UpdateCheckSchedulerTest : public ::testing::Test {
- public:
- UpdateCheckSchedulerTest()
- : attempter_(&fake_system_state_, &dbus_),
- scheduler_(&attempter_, &fake_system_state_) {}
-
- protected:
- virtual void SetUp() {
- test_ = this;
- loop_ = nullptr;
- EXPECT_EQ(&attempter_, scheduler_.update_attempter_);
- EXPECT_FALSE(scheduler_.enabled_);
- EXPECT_FALSE(scheduler_.scheduled_);
- EXPECT_EQ(0, scheduler_.last_interval_);
- EXPECT_EQ(0, scheduler_.poll_interval_);
- // Make sure singleton CertificateChecker has its members properly setup.
- CertificateChecker::set_system_state(&fake_system_state_);
- CertificateChecker::set_openssl_wrapper(&openssl_wrapper_);
- }
-
- virtual void TearDown() {
- test_ = nullptr;
- loop_ = nullptr;
- }
-
- static gboolean SourceCallback(gpointer data) {
- g_main_loop_quit(test_->loop_);
- // Forwards the call to the function mock so that expectations can be set.
- return test_->source_callback_.Call(data);
- }
-
- FakeSystemState fake_system_state_;
- MockDBusWrapper dbus_;
- OpenSSLWrapperMock openssl_wrapper_;
- UpdateAttempterMock attempter_;
- UpdateCheckSchedulerUnderTest scheduler_;
- MockFunction<gboolean(gpointer data)> source_callback_;
- GMainLoop* loop_;
- static UpdateCheckSchedulerTest* test_;
-};
-
-UpdateCheckSchedulerTest* UpdateCheckSchedulerTest::test_ = nullptr;
-
-TEST_F(UpdateCheckSchedulerTest, CanScheduleTest) {
- EXPECT_FALSE(scheduler_.CanSchedule());
- scheduler_.enabled_ = true;
- EXPECT_TRUE(scheduler_.CanSchedule());
- scheduler_.scheduled_ = true;
- EXPECT_FALSE(scheduler_.CanSchedule());
- scheduler_.enabled_ = false;
- EXPECT_FALSE(scheduler_.CanSchedule());
-}
-
-TEST_F(UpdateCheckSchedulerTest, ComputeNextIntervalAndFuzzBackoffTest) {
- int interval, fuzz;
- attempter_.set_http_response_code(500);
- int last_interval = UpdateCheckScheduler::kTimeoutPeriodicInterval + 50;
- scheduler_.last_interval_ = last_interval;
- scheduler_.ComputeNextIntervalAndFuzz(&interval, &fuzz);
- EXPECT_EQ(2 * last_interval, interval);
- EXPECT_EQ(2 * last_interval, fuzz);
-
- attempter_.set_http_response_code(503);
- scheduler_.last_interval_ =
- UpdateCheckScheduler::kTimeoutMaxBackoffInterval / 2 + 1;
- scheduler_.ComputeNextIntervalAndFuzz(&interval, &fuzz);
- EXPECT_EQ(UpdateCheckScheduler::kTimeoutMaxBackoffInterval, interval);
- EXPECT_EQ(UpdateCheckScheduler::kTimeoutMaxBackoffInterval, fuzz);
-}
-
-TEST_F(UpdateCheckSchedulerTest, ComputeNextIntervalAndFuzzPollTest) {
- int interval, fuzz;
- int poll_interval = UpdateCheckScheduler::kTimeoutPeriodicInterval + 50;
- scheduler_.set_poll_interval(poll_interval);
- scheduler_.ComputeNextIntervalAndFuzz(&interval, &fuzz);
- EXPECT_EQ(poll_interval, interval);
- EXPECT_EQ(poll_interval, fuzz);
-
- scheduler_.set_poll_interval(
- UpdateCheckScheduler::kTimeoutMaxBackoffInterval + 1);
- scheduler_.ComputeNextIntervalAndFuzz(&interval, &fuzz);
- EXPECT_EQ(UpdateCheckScheduler::kTimeoutMaxBackoffInterval, interval);
- EXPECT_EQ(UpdateCheckScheduler::kTimeoutMaxBackoffInterval, fuzz);
-
- scheduler_.set_poll_interval(
- UpdateCheckScheduler::kTimeoutPeriodicInterval - 1);
- scheduler_.ComputeNextIntervalAndFuzz(&interval, &fuzz);
- EXPECT_EQ(UpdateCheckScheduler::kTimeoutPeriodicInterval, interval);
- EXPECT_EQ(UpdateCheckScheduler::kTimeoutRegularFuzz, fuzz);
-}
-
-TEST_F(UpdateCheckSchedulerTest, ComputeNextIntervalAndFuzzPriorityTest) {
- int interval, fuzz;
- attempter_.set_http_response_code(500);
- scheduler_.last_interval_ =
- UpdateCheckScheduler::kTimeoutPeriodicInterval + 50;
- int poll_interval = UpdateCheckScheduler::kTimeoutPeriodicInterval + 100;
- scheduler_.set_poll_interval(poll_interval);
- scheduler_.ComputeNextIntervalAndFuzz(&interval, &fuzz);
- EXPECT_EQ(poll_interval, interval);
- EXPECT_EQ(poll_interval, fuzz);
-}
-
-TEST_F(UpdateCheckSchedulerTest, ComputeNextIntervalAndFuzzTest) {
- int interval, fuzz;
- scheduler_.ComputeNextIntervalAndFuzz(&interval, &fuzz);
- EXPECT_EQ(UpdateCheckScheduler::kTimeoutPeriodicInterval, interval);
- EXPECT_EQ(UpdateCheckScheduler::kTimeoutRegularFuzz, fuzz);
-}
-
-TEST_F(UpdateCheckSchedulerTest, GTimeoutAddSecondsTest) {
- loop_ = g_main_loop_new(g_main_context_default(), FALSE);
- // Invokes the actual GLib wrapper method rather than the subclass mock.
- scheduler_.UpdateCheckScheduler::GTimeoutAddSeconds(0, SourceCallback);
- EXPECT_CALL(source_callback_, Call(&scheduler_)).Times(1);
- g_main_loop_run(loop_);
- g_main_loop_unref(loop_);
-}
-
-TEST_F(UpdateCheckSchedulerTest, RunBootDeviceRemovableTest) {
- scheduler_.enabled_ = true;
- fake_system_state_.fake_hardware()->SetIsOfficialBuild(true);
- fake_system_state_.fake_hardware()->SetIsBootDeviceRemovable(true);
- scheduler_.Run();
- EXPECT_FALSE(scheduler_.enabled_);
- EXPECT_EQ(nullptr, attempter_.update_check_scheduler());
-}
-
-TEST_F(UpdateCheckSchedulerTest, RunNonOfficialBuildTest) {
- scheduler_.enabled_ = true;
- fake_system_state_.fake_hardware()->SetIsOfficialBuild(false);
- scheduler_.Run();
- EXPECT_FALSE(scheduler_.enabled_);
- EXPECT_EQ(nullptr, attempter_.update_check_scheduler());
-}
-
-TEST_F(UpdateCheckSchedulerTest, RunTest) {
- int interval_min, interval_max;
- FuzzRange(UpdateCheckScheduler::kTimeoutInitialInterval,
- UpdateCheckScheduler::kTimeoutRegularFuzz,
- &interval_min,
- &interval_max);
- fake_system_state_.fake_hardware()->SetIsOfficialBuild(true);
- fake_system_state_.fake_hardware()->SetIsBootDeviceRemovable(false);
- EXPECT_CALL(scheduler_,
- GTimeoutAddSeconds(AllOf(Ge(interval_min), Le(interval_max)),
- scheduler_.StaticCheck)).Times(1);
- scheduler_.Run();
- EXPECT_TRUE(scheduler_.enabled_);
- EXPECT_EQ(&scheduler_, attempter_.update_check_scheduler());
-}
-
-TEST_F(UpdateCheckSchedulerTest, ScheduleCheckDisabledTest) {
- EXPECT_CALL(scheduler_, GTimeoutAddSeconds(_, _)).Times(0);
- scheduler_.ScheduleCheck(250, 30);
- EXPECT_EQ(0, scheduler_.last_interval_);
- EXPECT_FALSE(scheduler_.scheduled_);
-}
-
-TEST_F(UpdateCheckSchedulerTest, ScheduleCheckEnabledTest) {
- int interval_min, interval_max;
- FuzzRange(100, 10, &interval_min, &interval_max);
- EXPECT_CALL(scheduler_,
- GTimeoutAddSeconds(AllOf(Ge(interval_min), Le(interval_max)),
- scheduler_.StaticCheck)).Times(1);
- scheduler_.enabled_ = true;
- scheduler_.ScheduleCheck(100, 10);
- EXPECT_EQ(100, scheduler_.last_interval_);
- EXPECT_TRUE(scheduler_.scheduled_);
-}
-
-TEST_F(UpdateCheckSchedulerTest, ScheduleCheckNegativeIntervalTest) {
- EXPECT_CALL(scheduler_, GTimeoutAddSeconds(0, scheduler_.StaticCheck))
- .Times(1);
- scheduler_.enabled_ = true;
- scheduler_.ScheduleCheck(-50, 20);
- EXPECT_TRUE(scheduler_.scheduled_);
-}
-
-TEST_F(UpdateCheckSchedulerTest, ScheduleNextCheckDisabledTest) {
- EXPECT_CALL(scheduler_, GTimeoutAddSeconds(_, _)).Times(0);
- scheduler_.ScheduleNextCheck();
-}
-
-TEST_F(UpdateCheckSchedulerTest, ScheduleNextCheckEnabledTest) {
- int interval_min, interval_max;
- FuzzRange(UpdateCheckScheduler::kTimeoutPeriodicInterval,
- UpdateCheckScheduler::kTimeoutRegularFuzz,
- &interval_min,
- &interval_max);
- EXPECT_CALL(scheduler_,
- GTimeoutAddSeconds(AllOf(Ge(interval_min), Le(interval_max)),
- scheduler_.StaticCheck)).Times(1);
- scheduler_.enabled_ = true;
- scheduler_.ScheduleNextCheck();
-}
-
-TEST_F(UpdateCheckSchedulerTest, SetUpdateStatusIdleDisabledTest) {
- EXPECT_CALL(scheduler_, GTimeoutAddSeconds(_, _)).Times(0);
- scheduler_.SetUpdateStatus(UPDATE_STATUS_IDLE);
-}
-
-TEST_F(UpdateCheckSchedulerTest, SetUpdateStatusIdleEnabledTest) {
- int interval_min, interval_max;
- FuzzRange(UpdateCheckScheduler::kTimeoutPeriodicInterval,
- UpdateCheckScheduler::kTimeoutRegularFuzz,
- &interval_min,
- &interval_max);
- EXPECT_CALL(scheduler_,
- GTimeoutAddSeconds(AllOf(Ge(interval_min), Le(interval_max)),
- scheduler_.StaticCheck)).Times(1);
- scheduler_.enabled_ = true;
- scheduler_.SetUpdateStatus(UPDATE_STATUS_IDLE);
-}
-
-TEST_F(UpdateCheckSchedulerTest, SetUpdateStatusNonIdleTest) {
- EXPECT_CALL(scheduler_, GTimeoutAddSeconds(_, _)).Times(0);
- scheduler_.SetUpdateStatus(UPDATE_STATUS_DOWNLOADING);
- scheduler_.enabled_ = true;
- scheduler_.SetUpdateStatus(UPDATE_STATUS_DOWNLOADING);
-}
-
-TEST_F(UpdateCheckSchedulerTest, StaticCheckOOBECompleteTest) {
- scheduler_.scheduled_ = true;
- EXPECT_NE(nullptr, scheduler_.fake_system_state_);
- scheduler_.fake_system_state_->fake_hardware()->SetIsOOBEComplete(
- Time::UnixEpoch());
- EXPECT_CALL(attempter_, Update("", "", false, false))
- .Times(1)
- .WillOnce(Assign(&scheduler_.scheduled_, true));
- scheduler_.enabled_ = true;
- EXPECT_CALL(scheduler_, GTimeoutAddSeconds(_, _)).Times(0);
- UpdateCheckSchedulerUnderTest::StaticCheck(&scheduler_);
-}
-
-TEST_F(UpdateCheckSchedulerTest, StaticCheckOOBENotCompleteTest) {
- scheduler_.scheduled_ = true;
- scheduler_.fake_system_state_->fake_hardware()->UnsetIsOOBEComplete();
- EXPECT_CALL(attempter_, Update("", "", _, _)).Times(0);
- int interval_min, interval_max;
- FuzzRange(UpdateCheckScheduler::kTimeoutInitialInterval,
- UpdateCheckScheduler::kTimeoutRegularFuzz,
- &interval_min,
- &interval_max);
- scheduler_.enabled_ = true;
- EXPECT_CALL(scheduler_,
- GTimeoutAddSeconds(AllOf(Ge(interval_min), Le(interval_max)),
- scheduler_.StaticCheck)).Times(1);
- UpdateCheckSchedulerUnderTest::StaticCheck(&scheduler_);
-}
-
-} // namespace chromeos_update_engine
diff --git a/update_engine.gyp b/update_engine.gyp
index 44ce67e..d2d9ca1 100644
--- a/update_engine.gyp
+++ b/update_engine.gyp
@@ -174,7 +174,6 @@
'subprocess.cc',
'terminator.cc',
'update_attempter.cc',
- 'update_check_scheduler.cc',
'update_manager/boxed_value.cc',
'update_manager/chromeos_policy.cc',
'update_manager/default_policy.cc',
@@ -377,7 +376,6 @@
'terminator_unittest.cc',
'test_utils.cc',
'update_attempter_unittest.cc',
- 'update_check_scheduler_unittest.cc',
'update_manager/boxed_value_unittest.cc',
'update_manager/chromeos_policy_unittest.cc',
'update_manager/evaluation_context_unittest.cc',
diff --git a/update_manager/boxed_value.cc b/update_manager/boxed_value.cc
index 56b42b8..773431c 100644
--- a/update_manager/boxed_value.cc
+++ b/update_manager/boxed_value.cc
@@ -24,56 +24,56 @@
// Keep in sync with boxed_value_unitttest.cc.
template<>
-string BoxedValue::ValuePrinter<string>(const void *value) {
+string BoxedValue::ValuePrinter<string>(const void* value) {
const string* val = reinterpret_cast<const string*>(value);
return *val;
}
template<>
-string BoxedValue::ValuePrinter<int>(const void *value) {
+string BoxedValue::ValuePrinter<int>(const void* value) {
const int* val = reinterpret_cast<const int*>(value);
return base::IntToString(*val);
}
template<>
-string BoxedValue::ValuePrinter<unsigned int>(const void *value) {
+string BoxedValue::ValuePrinter<unsigned int>(const void* value) {
const unsigned int* val = reinterpret_cast<const unsigned int*>(value);
return base::UintToString(*val);
}
template<>
-string BoxedValue::ValuePrinter<int64_t>(const void *value) {
+string BoxedValue::ValuePrinter<int64_t>(const void* value) {
const int64_t* val = reinterpret_cast<const int64_t*>(value);
return base::Int64ToString(*val);
}
template<>
-string BoxedValue::ValuePrinter<uint64_t>(const void *value) {
+string BoxedValue::ValuePrinter<uint64_t>(const void* value) {
const uint64_t* val =
reinterpret_cast<const uint64_t*>(value);
return base::Uint64ToString(static_cast<uint64_t>(*val));
}
template<>
-string BoxedValue::ValuePrinter<bool>(const void *value) {
+string BoxedValue::ValuePrinter<bool>(const void* value) {
const bool* val = reinterpret_cast<const bool*>(value);
return *val ? "true" : "false";
}
template<>
-string BoxedValue::ValuePrinter<double>(const void *value) {
+string BoxedValue::ValuePrinter<double>(const void* value) {
const double* val = reinterpret_cast<const double*>(value);
return base::DoubleToString(*val);
}
template<>
-string BoxedValue::ValuePrinter<base::Time>(const void *value) {
+string BoxedValue::ValuePrinter<base::Time>(const void* value) {
const base::Time* val = reinterpret_cast<const base::Time*>(value);
return chromeos_update_engine::utils::ToString(*val);
}
template<>
-string BoxedValue::ValuePrinter<base::TimeDelta>(const void *value) {
+string BoxedValue::ValuePrinter<base::TimeDelta>(const void* value) {
const base::TimeDelta* val = reinterpret_cast<const base::TimeDelta*>(value);
return chromeos_update_engine::utils::FormatTimeDelta(*val);
}
@@ -98,13 +98,13 @@
}
template<>
-string BoxedValue::ValuePrinter<ConnectionType>(const void *value) {
+string BoxedValue::ValuePrinter<ConnectionType>(const void* value) {
const ConnectionType* val = reinterpret_cast<const ConnectionType*>(value);
return ConnectionTypeToString(*val);
}
template<>
-string BoxedValue::ValuePrinter<set<ConnectionType>>(const void *value) {
+string BoxedValue::ValuePrinter<set<ConnectionType>>(const void* value) {
string ret = "";
const set<ConnectionType>* val =
reinterpret_cast<const set<ConnectionType>*>(value);
@@ -118,7 +118,7 @@
}
template<>
-string BoxedValue::ValuePrinter<ConnectionTethering>(const void *value) {
+string BoxedValue::ValuePrinter<ConnectionTethering>(const void* value) {
const ConnectionTethering* val =
reinterpret_cast<const ConnectionTethering*>(value);
switch (*val) {
@@ -136,7 +136,7 @@
}
template<>
-string BoxedValue::ValuePrinter<Stage>(const void *value) {
+string BoxedValue::ValuePrinter<Stage>(const void* value) {
const Stage* val = reinterpret_cast<const Stage*>(value);
switch (*val) {
case Stage::kIdle:
@@ -162,4 +162,20 @@
return "Unknown";
}
+template<>
+string BoxedValue::ValuePrinter<UpdateRequestStatus>(const void* value) {
+ const UpdateRequestStatus* val =
+ reinterpret_cast<const UpdateRequestStatus*>(value);
+ switch (*val) {
+ case UpdateRequestStatus::kNone:
+ return "None";
+ case UpdateRequestStatus::kInteractive:
+ return "Interactive";
+ case UpdateRequestStatus::kPeriodic:
+ return "Periodic";
+ }
+ NOTREACHED();
+ return "Unknown";
+}
+
} // namespace chromeos_update_manager
diff --git a/update_manager/boxed_value_unittest.cc b/update_manager/boxed_value_unittest.cc
index dff6738..77b44c2 100644
--- a/update_manager/boxed_value_unittest.cc
+++ b/update_manager/boxed_value_unittest.cc
@@ -74,7 +74,7 @@
TEST(UmBoxedValueTest, MixedList) {
list<BoxedValue> lst;
// This is mostly a compile test.
- lst.emplace_back(new const int(42)); // NOLINT(readability/casting)
+ lst.emplace_back(new const int{42});
lst.emplace_back(new const string("Hello world!"));
bool marker;
lst.emplace_back(new const DeleterMarker(&marker));
diff --git a/update_manager/chromeos_policy.cc b/update_manager/chromeos_policy.cc
index 0dc0ad6..67799ce 100644
--- a/update_manager/chromeos_policy.cc
+++ b/update_manager/chromeos_policy.cc
@@ -168,6 +168,7 @@
const bool* is_boot_device_removable_p = ec->GetValue(
system_provider->var_is_boot_device_removable());
if (is_boot_device_removable_p && *is_boot_device_removable_p) {
+ LOG(INFO) << "Booted from removable device, disabling update checks.";
result->updates_enabled = false;
return EvalStatus::kSucceeded;
}
@@ -178,8 +179,10 @@
// Check whether updates are disabled by policy.
const bool* update_disabled_p = ec->GetValue(
dp_provider->var_update_disabled());
- if (update_disabled_p && *update_disabled_p)
+ if (update_disabled_p && *update_disabled_p) {
+ LOG(INFO) << "Updates disabled by policy, blocking update checks.";
return EvalStatus::kAskMeAgainLater;
+ }
// Determine whether a target version prefix is dictated by policy.
const string* target_version_prefix_p = ec->GetValue(
@@ -199,10 +202,15 @@
}
// First, check to see if an interactive update was requested.
- const bool* interactive_update_requested_p = ec->GetValue(
- updater_provider->var_interactive_update_requested());
- if (interactive_update_requested_p && *interactive_update_requested_p) {
- result->is_interactive = true;
+ const UpdateRequestStatus* forced_update_requested_p = ec->GetValue(
+ updater_provider->var_forced_update_requested());
+ if (forced_update_requested_p &&
+ *forced_update_requested_p != UpdateRequestStatus::kNone) {
+ result->is_interactive =
+ (*forced_update_requested_p == UpdateRequestStatus::kInteractive);
+ LOG(INFO) << "Forced update signaled ("
+ << (result->is_interactive ? "interactive" : "periodic")
+ << "), allowing update check.";
return EvalStatus::kSucceeded;
}
@@ -214,6 +222,7 @@
const bool* is_official_build_p = ec->GetValue(
system_provider->var_is_official_build());
if (is_official_build_p && !(*is_official_build_p)) {
+ LOG(INFO) << "Unofficial build, blocking periodic update checks.";
return EvalStatus::kAskMeAgainLater;
}
@@ -223,8 +232,10 @@
if (is_oobe_enabled_p && *is_oobe_enabled_p) {
const bool* is_oobe_complete_p = ec->GetValue(
system_provider->var_is_oobe_complete());
- if (is_oobe_complete_p && !(*is_oobe_complete_p))
+ if (is_oobe_complete_p && !(*is_oobe_complete_p)) {
+ LOG(INFO) << "OOBE not completed, blocking update checks.";
return EvalStatus::kAskMeAgainLater;
+ }
}
// Ensure that periodic update checks are timed properly.
@@ -233,10 +244,14 @@
EvalStatus::kSucceeded) {
return EvalStatus::kFailed;
}
- if (!ec->IsWallclockTimeGreaterThan(next_update_check))
+ if (!ec->IsWallclockTimeGreaterThan(next_update_check)) {
+ LOG(INFO) << "Periodic check interval not satisfied, blocking until "
+ << chromeos_update_engine::utils::ToString(next_update_check);
return EvalStatus::kAskMeAgainLater;
+ }
// It is time to check for an update.
+ LOG(INFO) << "Allowing update check.";
return EvalStatus::kSucceeded;
}
diff --git a/update_manager/chromeos_policy_unittest.cc b/update_manager/chromeos_policy_unittest.cc
index d231a7c..25a091e 100644
--- a/update_manager/chromeos_policy_unittest.cc
+++ b/update_manager/chromeos_policy_unittest.cc
@@ -47,11 +47,11 @@
fake_state_.updater_provider()->var_last_checked_time()->reset(
new Time(fake_clock_.GetWallclockTime()));
fake_state_.updater_provider()->var_consecutive_failed_update_checks()->
- reset(new unsigned int(0)); // NOLINT(readability/casting)
+ reset(new unsigned int{0});
fake_state_.updater_provider()->var_server_dictated_poll_interval()->
- reset(new unsigned int(0)); // NOLINT(readability/casting)
- fake_state_.updater_provider()->var_interactive_update_requested()->
- reset(new bool(false)); // NOLINT(readability/casting)
+ reset(new unsigned int{0});
+ fake_state_.updater_provider()->var_forced_update_requested()->
+ reset(new UpdateRequestStatus{UpdateRequestStatus::kNone});
fake_state_.random_provider()->var_seed()->reset(
new uint64_t(4)); // chosen by fair dice roll.
@@ -199,7 +199,7 @@
Time next_update_check;
fake_state_.updater_provider()->var_consecutive_failed_update_checks()->
- reset(new unsigned int(2)); // NOLINT(readability/casting)
+ reset(new unsigned int{2});
ExpectPolicyStatus(EvalStatus::kSucceeded,
&ChromeOSPolicy::NextUpdateCheckTime, &next_update_check);
@@ -221,10 +221,10 @@
const unsigned int kInterval = ChromeOSPolicy::kTimeoutPeriodicInterval * 4;
fake_state_.updater_provider()->var_server_dictated_poll_interval()->
- reset(new unsigned int(kInterval)); // NOLINT(readability/casting)
+ reset(new unsigned int{kInterval});
// We should not be backing off in this case.
fake_state_.updater_provider()->var_consecutive_failed_update_checks()->
- reset(new unsigned int(2)); // NOLINT(readability/casting)
+ reset(new unsigned int{2});
ExpectPolicyStatus(EvalStatus::kSucceeded,
&ChromeOSPolicy::NextUpdateCheckTime, &next_update_check);
@@ -243,7 +243,7 @@
Time next_update_check;
fake_state_.updater_provider()->var_consecutive_failed_update_checks()->
- reset(new unsigned int(100)); // NOLINT(readability/casting)
+ reset(new unsigned int{100});
ExpectPolicyStatus(EvalStatus::kSucceeded,
&ChromeOSPolicy::NextUpdateCheckTime, &next_update_check);
@@ -394,13 +394,14 @@
&Policy::UpdateCheckAllowed, &result);
}
-TEST_F(UmChromeOSPolicyTest, UpdateCheckAllowedInteractiveUpdateRequested) {
- // UpdateCheckAllowed should return true because an interactive update request
- // was signaled.
+TEST_F(UmChromeOSPolicyTest,
+ UpdateCheckAllowedForcedUpdateRequestedInteractive) {
+ // UpdateCheckAllowed should return true because a forced update request was
+ // signaled for an interactive update.
SetUpdateCheckAllowed(true);
- fake_state_.updater_provider()->var_interactive_update_requested()->reset(
- new bool(true));
+ fake_state_.updater_provider()->var_forced_update_requested()->reset(
+ new UpdateRequestStatus(UpdateRequestStatus::kInteractive));
UpdateCheckParams result;
ExpectPolicyStatus(EvalStatus::kSucceeded,
@@ -409,6 +410,21 @@
EXPECT_TRUE(result.is_interactive);
}
+TEST_F(UmChromeOSPolicyTest, UpdateCheckAllowedForcedUpdateRequestedPeriodic) {
+ // UpdateCheckAllowed should return true because a forced update request was
+ // signaled for a periodic check.
+
+ SetUpdateCheckAllowed(true);
+ 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.is_interactive);
+}
+
TEST_F(UmChromeOSPolicyTest, UpdateCanStartFailsCheckAllowedError) {
// The UpdateCanStart policy fails, not being able to query
// UpdateCheckAllowed.
diff --git a/update_manager/fake_updater_provider.h b/update_manager/fake_updater_provider.h
index f2295a5..0820858 100644
--- a/update_manager/fake_updater_provider.h
+++ b/update_manager/fake_updater_provider.h
@@ -69,8 +69,8 @@
return &var_server_dictated_poll_interval_;
}
- FakeVariable<bool>* var_interactive_update_requested() override {
- return &var_interactive_update_requested_;
+ FakeVariable<UpdateRequestStatus>* var_forced_update_requested() override {
+ return &var_forced_update_requested_;
}
private:
@@ -104,9 +104,9 @@
FakeVariable<unsigned int>
var_server_dictated_poll_interval_{ // NOLINT(whitespace/braces)
"server_dictated_poll_interval", kVariableModePoll};
- FakeVariable<bool>
- var_interactive_update_requested_{ // NOLINT(whitespace/braces)
- "interactive_update_requested", kVariableModeAsync};
+ FakeVariable<UpdateRequestStatus>
+ var_forced_update_requested_{ // NOLINT(whitespace/braces)
+ "forced_update_requested", kVariableModeAsync};
DISALLOW_COPY_AND_ASSIGN(FakeUpdaterProvider);
};
diff --git a/update_manager/real_updater_provider.cc b/update_manager/real_updater_provider.cc
index 004b51a..ac70746 100644
--- a/update_manager/real_updater_provider.cc
+++ b/update_manager/real_updater_provider.cc
@@ -350,35 +350,39 @@
DISALLOW_COPY_AND_ASSIGN(ServerDictatedPollIntervalVariable);
};
-// An async variable that tracks changes to interactive update requests.
-class InteractiveUpdateRequestedVariable : public UpdaterVariableBase<bool> {
+// An async variable that tracks changes to forced update requests.
+class ForcedUpdateRequestedVariable
+ : public UpdaterVariableBase<UpdateRequestStatus> {
public:
- InteractiveUpdateRequestedVariable(const string& name,
- SystemState* system_state)
- : UpdaterVariableBase<bool>::UpdaterVariableBase(name, kVariableModeAsync,
- system_state) {
- system_state->update_attempter()->set_interactive_update_pending_callback(
- new base::Callback<void(bool)>( // NOLINT(readability/function)
- base::Bind(&InteractiveUpdateRequestedVariable::Reset,
+ ForcedUpdateRequestedVariable(const string& name, SystemState* system_state)
+ : UpdaterVariableBase<UpdateRequestStatus>::UpdaterVariableBase(
+ name, kVariableModeAsync, system_state) {
+ system_state->update_attempter()->set_forced_update_pending_callback(
+ new base::Callback<void(bool, bool)>( // NOLINT(readability/function)
+ base::Bind(&ForcedUpdateRequestedVariable::Reset,
base::Unretained(this))));
}
private:
- const bool* GetValue(TimeDelta /* timeout */,
- string* /* errmsg */) override {
- return new bool(interactive_update_requested_);
+ const UpdateRequestStatus* GetValue(TimeDelta /* timeout */,
+ string* /* errmsg */) override {
+ return new UpdateRequestStatus(update_request_status_);
}
- void Reset(bool value) {
- if (interactive_update_requested_ != value) {
- interactive_update_requested_ = value;
+ void Reset(bool forced_update_requested, bool is_interactive) {
+ UpdateRequestStatus new_value = UpdateRequestStatus::kNone;
+ if (forced_update_requested)
+ new_value = (is_interactive ? UpdateRequestStatus::kInteractive :
+ UpdateRequestStatus::kPeriodic);
+ if (update_request_status_ != new_value) {
+ update_request_status_ = new_value;
NotifyValueChanged();
}
}
- bool interactive_update_requested_ = false;
+ UpdateRequestStatus update_request_status_ = UpdateRequestStatus::kNone;
- DISALLOW_COPY_AND_ASSIGN(InteractiveUpdateRequestedVariable);
+ DISALLOW_COPY_AND_ASSIGN(ForcedUpdateRequestedVariable);
};
// RealUpdaterProvider methods.
@@ -413,8 +417,8 @@
var_server_dictated_poll_interval_(
new ServerDictatedPollIntervalVariable(
"server_dictated_poll_interval", system_state_)),
- var_interactive_update_requested_(
- new InteractiveUpdateRequestedVariable(
- "interactive_update_requested", system_state_)) {}
+ var_forced_update_requested_(
+ new ForcedUpdateRequestedVariable(
+ "forced_update_requested", system_state_)) {}
} // namespace chromeos_update_manager
diff --git a/update_manager/real_updater_provider.h b/update_manager/real_updater_provider.h
index 5c40acd..c61032e 100644
--- a/update_manager/real_updater_provider.h
+++ b/update_manager/real_updater_provider.h
@@ -81,8 +81,8 @@
return var_server_dictated_poll_interval_.get();
}
- Variable<bool>* var_interactive_update_requested() override {
- return var_interactive_update_requested_.get();
+ Variable<UpdateRequestStatus>* var_forced_update_requested() override {
+ return var_forced_update_requested_.get();
}
private:
@@ -103,7 +103,7 @@
scoped_ptr<Variable<bool>> var_cellular_enabled_;
scoped_ptr<Variable<unsigned int>> var_consecutive_failed_update_checks_;
scoped_ptr<Variable<unsigned int>> var_server_dictated_poll_interval_;
- scoped_ptr<Variable<bool>> var_interactive_update_requested_;
+ scoped_ptr<Variable<UpdateRequestStatus>> var_forced_update_requested_;
DISALLOW_COPY_AND_ASSIGN(RealUpdaterProvider);
};
diff --git a/update_manager/updater_provider.h b/update_manager/updater_provider.h
index ec1ca8a..535b3e7 100644
--- a/update_manager/updater_provider.h
+++ b/update_manager/updater_provider.h
@@ -26,6 +26,12 @@
kAttemptingRollback,
};
+enum class UpdateRequestStatus {
+ kNone,
+ kInteractive,
+ kPeriodic,
+};
+
// Provider for Chrome OS update related information.
class UpdaterProvider : public Provider {
public:
@@ -82,9 +88,10 @@
// A server-dictated update check interval in seconds, if one was given.
virtual Variable<unsigned int>* var_server_dictated_poll_interval() = 0;
- // A variable denoting whether a request for an interactive update was
- // received but no update check performed yet.
- virtual Variable<bool>* var_interactive_update_requested() = 0;
+ // A variable denoting whether a forced update was request but no update check
+ // performed yet; also tells whether this request is for an interactive or
+ // scheduled update.
+ virtual Variable<UpdateRequestStatus>* var_forced_update_requested() = 0;
protected:
UpdaterProvider() {}