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/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);
};