Commit 90a76d83 authored by Abhishek Bhardwaj's avatar Abhishek Bhardwaj Committed by Commit Bot

DeviceScheduledUpdateChecker: Use ICU time library and handle DST transitions

This change uses ICU time library to handle time calculations. This is
done because it handles rollovers, DST calculations and other tricky
edge cases. Each time calculation now also uses the time zone of the
device instead of relying on base::Time::LocalExplode and
base::Time::FromLocalExploded.

BUG=924762
TEST=Unit tests.

Change-Id: I6fd8a17cd9f96994516cbaea06158d17abb467fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1684642
Commit-Queue: Abhishek Bhardwaj <abhishekbh@chromium.org>
Reviewed-by: default avatarGreg Thompson <grt@chromium.org>
Reviewed-by: default avatarPavol Marko <pmarko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#678942}
parent 9300bb2a
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#define CHROME_BROWSER_CHROMEOS_POLICY_DEVICE_SCHEDULED_UPDATE_CHECKER_H_ #define CHROME_BROWSER_CHROMEOS_POLICY_DEVICE_SCHEDULED_UPDATE_CHECKER_H_
#include <memory> #include <memory>
#include <string>
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
...@@ -15,32 +16,12 @@ ...@@ -15,32 +16,12 @@
#include "chrome/browser/chromeos/policy/task_executor_with_retries.h" #include "chrome/browser/chromeos/policy/task_executor_with_retries.h"
#include "chrome/browser/chromeos/settings/cros_settings.h" #include "chrome/browser/chromeos/settings/cros_settings.h"
#include "chromeos/dbus/power/native_timer.h" #include "chromeos/dbus/power/native_timer.h"
#include "chromeos/settings/timezone_settings.h"
#include "third_party/icu/source/i18n/unicode/calendar.h"
#include "third_party/icu/source/i18n/unicode/timezone.h"
namespace policy { namespace policy {
namespace update_checker_internal {
// Increments month (taking care of year rollovers) and sets day_of_month field
// to |month| in |local_exploded_time| and returns the corresponding base::Time.
// |local_exploded_time| should be local time. In case of an error, due to
// concurrent DST or time zone change, returns base::nullopt.
base::Optional<base::Time> IncrementMonthAndSetDayOfMonth(
base::Time::Exploded exploded_time,
int day_of_month);
// The maximum iterations allowed to start an update check timer if the
// operation fails.
constexpr int kMaxStartUpdateCheckTimerRetryIterations = 5;
// Time to call |StartUpdateCheckTimer| again in case it failed.
constexpr base::TimeDelta kStartUpdateCheckTimerRetryTime =
base::TimeDelta::FromMinutes(5);
// Number of days in a week.
constexpr int kDaysInAWeek = 7;
} // namespace update_checker_internal
// This class listens for changes in the scheduled update check policy and then // This class listens for changes in the scheduled update check policy and then
// manages recurring update checks based on the policy. // manages recurring update checks based on the policy.
class DeviceScheduledUpdateChecker { class DeviceScheduledUpdateChecker {
...@@ -61,18 +42,20 @@ class DeviceScheduledUpdateChecker { ...@@ -61,18 +42,20 @@ class DeviceScheduledUpdateChecker {
ScheduledUpdateCheckData(const ScheduledUpdateCheckData&); ScheduledUpdateCheckData(const ScheduledUpdateCheckData&);
~ScheduledUpdateCheckData(); ~ScheduledUpdateCheckData();
// Corresponds to UCAL_HOUR_OF_DAY in icu::Calendar.
int hour; int hour;
// Corresponds to UCAL_MINUTE in icu::Calendar.
int minute; int minute;
Frequency frequency; Frequency frequency;
// Only set when frequency is |kWeekly|. Corresponds to day_of_week in // Only set when frequency is |kWeekly|. Corresponds to UCAL_DAY_OF_WEEK in
// base::Time::Exploded. Values between 0 (SUNDAY) to 6 (SATURDAY). // icu::Calendar. Values between 1 (SUNDAY) to 7 (SATURDAY).
base::Optional<int> day_of_week; base::Optional<int> day_of_week;
// Only set when frequency is |kMonthly|. Corresponds to day_of_month in // Only set when frequency is |kMonthly|. Corresponds to UCAL_DAY_OF_MONTH
// base::Time::Exploded i.e. values between 1 to 28. // in icu::Calendar i.e. values between 1 to 31.
base::Optional<int> day_of_month; base::Optional<int> day_of_month;
// Absolute time ticks when the next update check (i.e. |UpdateCheck|) will // Absolute time ticks when the next update check (i.e. |UpdateCheck|) will
...@@ -85,12 +68,11 @@ class DeviceScheduledUpdateChecker { ...@@ -85,12 +68,11 @@ class DeviceScheduledUpdateChecker {
// schedules the next update check based on |scheduled_update_check_data_|. // schedules the next update check based on |scheduled_update_check_data_|.
virtual void OnUpdateCheckTimerExpired(); virtual void OnUpdateCheckTimerExpired();
// Calculates next update check time based on |scheduled_update_check_data_| // Calculates the delay from |cur_time| at which |update_check_timer_| should
// and |cur_local_time|. Returns |base::nullopt| if calculation failed due to // run next. Returns 0 delay if the calculation failed due to a concurrent DST
// a concurrent DST or Time Zone change. Requires // or Time Zone change. Requires |scheduled_update_check_data_| to be set.
// |scheduled_update_check_data_| to be set. virtual base::TimeDelta CalculateNextUpdateCheckTimerDelay(
virtual base::Optional<base::Time> CalculateNextUpdateCheckTime( base::Time cur_time);
base::Time cur_local_time);
// Called when |os_and_policies_update_checker_| has finished successfully or // Called when |os_and_policies_update_checker_| has finished successfully or
// unsuccessfully after retrying. // unsuccessfully after retrying.
...@@ -103,7 +85,7 @@ class DeviceScheduledUpdateChecker { ...@@ -103,7 +85,7 @@ class DeviceScheduledUpdateChecker {
// Must only be run via |start_update_check_timer_task_executor_|. Sets // Must only be run via |start_update_check_timer_task_executor_|. Sets
// |update_check_timer_| based on |scheduled_update_check_data_|. If the // |update_check_timer_| based on |scheduled_update_check_data_|. If the
// |update_check_timer_| can't be started due to an error in // |update_check_timer_| can't be started due to an error in
// |CalculateNextUpdateCheckTime| then reschedules itself via // |CalculateNextUpdateCheckTimerDelay| then reschedules itself via
// |start_update_check_timer_task_executor_|. Requires // |start_update_check_timer_task_executor_|. Requires
// |scheduled_update_check_data_| to be set. // |scheduled_update_check_data_| to be set.
void StartUpdateCheckTimer(); void StartUpdateCheckTimer();
...@@ -130,6 +112,9 @@ class DeviceScheduledUpdateChecker { ...@@ -130,6 +112,9 @@ class DeviceScheduledUpdateChecker {
// Returns time ticks from boot including time ticks spent during sleeping. // Returns time ticks from boot including time ticks spent during sleeping.
virtual base::TimeTicks GetTicksSinceBoot(); virtual base::TimeTicks GetTicksSinceBoot();
// Returns the current time zone.
virtual const icu::TimeZone& GetTimeZone();
// Used to retrieve Chrome OS settings. Not owned. // Used to retrieve Chrome OS settings. Not owned.
chromeos::CrosSettings* const cros_settings_; chromeos::CrosSettings* const cros_settings_;
...@@ -152,6 +137,55 @@ class DeviceScheduledUpdateChecker { ...@@ -152,6 +137,55 @@ class DeviceScheduledUpdateChecker {
DISALLOW_COPY_AND_ASSIGN(DeviceScheduledUpdateChecker); DISALLOW_COPY_AND_ASSIGN(DeviceScheduledUpdateChecker);
}; };
namespace update_checker_internal {
// Parses |value| into a |ScheduledUpdateCheckData|. Returns nullopt if there
// is any error while parsing |value|.
base::Optional<DeviceScheduledUpdateChecker::ScheduledUpdateCheckData>
ParseScheduledUpdate(const base::Value& value);
// Converts an icu::Calendar to base::Time. Assumes |time| is valid.
base::Time IcuToBaseTime(const icu::Calendar& time);
// Calculates the difference in milliseconds of |a| - |b|.
double GetDiffInMs(const icu::Calendar& a, const icu::Calendar& b);
// Converts |cur_time| to ICU time in the time zone |tz|.
std::unique_ptr<icu::Calendar> ConvertUtcToTzIcuTime(base::Time cur_time,
const icu::TimeZone& tz);
// Modifies |time| to the next monthly date at |hour|, |minute| and
// |day_of_month| if it is greater than equal to |hour|:|minute| at
// |day_of_month|. It takes into account variance in days in different months
// i.e. 31st Jan is forwarded to 28th Feb in non leap years. However, if
// |day_of_month| equals |time|'s day of month and the |hour|:|minute| is
// greater than the |hour|:|minute| of |time| then only the |hour| and |minute|
// in |time| is set. Returns false iff there is a failure setting any of the
// fields.
//
// Some examples -
// - hour - 9 minute - 0 day_of_month - 31 time - 31st Jan 08:00 1970. |time|
// will be 31st Jan 09:00 1970.
// - hour - 7 minute - 0 day_of_month - 31 time - 31st Jan 08:00 1970. |time|
// will be 28th Feb 07:00 1970.
bool SetNextMonthlyDate(int hour,
int minute,
int day_of_month,
icu::Calendar* time);
// The maximum iterations allowed to start an update check timer if the
// operation fails.
constexpr int kMaxStartUpdateCheckTimerRetryIterations = 5;
// Time to call |StartUpdateCheckTimer| again in case it failed.
constexpr base::TimeDelta kStartUpdateCheckTimerRetryTime =
base::TimeDelta::FromMinutes(5);
// Number of days in a week.
constexpr int kDaysInAWeek = 7;
} // namespace update_checker_internal
} // namespace policy } // namespace policy
#endif // CHROME_BROWSER_CHROMEOS_POLICY_DEVICE_SCHEDULED_UPDATE_CHECKER_H_ #endif // CHROME_BROWSER_CHROMEOS_POLICY_DEVICE_SCHEDULED_UPDATE_CHECKER_H_
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment