Add DeviceOffHoursController unittests

Add FakeClock classes
Add unittests for:
* basic DeviceOffHoursController
* Convert*
* Apply*
* UpdateOffHoursMode
* SuspendDone observer method

Original CL written by Daria Yakovleva: https://crrev.com/c/675243

Bug: 739713

Author: Daria Yakovleva <yakovleva@google.com>
Change-Id: I35929e374d05713066027b86946f77c4c50f8473
Reviewed-on: https://chromium-review.googlesource.com/723744
Commit-Queue: Sergey Poromov <poromov@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510202}

chrome/browser/chromeos/BUILD.gn

@@ -1862,6 +1862,8 @@ source_set("unit_tests") {
     "policy/network_configuration_updater_unittest.cc",
     "policy/off_hours/device_off_hours_controller_unittest.cc",
     "policy/off_hours/off_hours_interval_unittest.cc",
+    "policy/off_hours/off_hours_policy_applier_unittest.cc",
+    "policy/off_hours/off_hours_proto_parser_unittest.cc",
     "policy/off_hours/weekly_time_unittest.cc",
     "policy/pre_signin_policy_fetcher_unittest.cc",
     "policy/recommendation_restorer_unittest.cc",

chrome/browser/chromeos/policy/off_hours/device_off_hours_controller.cc

@@ -5,11 +5,14 @@
 #include "chrome/browser/chromeos/policy/off_hours/device_off_hours_controller.h"
 
 #include <string>
+#include <tuple>
 #include <utility>
 
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/optional.h"
+#include "base/time/default_clock.h"
+#include "base/time/tick_clock.h"
 #include "base/time/time.h"
 #include "chrome/browser/chromeos/policy/off_hours/off_hours_proto_parser.h"
 #include "chrome/browser/chromeos/policy/off_hours/time_utils.h"
@@ -22,7 +25,9 @@ namespace em = enterprise_management;
 namespace policy {
 namespace off_hours {
 
-DeviceOffHoursController::DeviceOffHoursController() {
+DeviceOffHoursController::DeviceOffHoursController()
+    : timer_(base::MakeUnique<base::OneShotTimer>()),
+      clock_(base::MakeUnique<base::DefaultClock>()) {
   // IsInitialized() check is used for testing. Otherwise it has to be already
   // initialized.
   if (chromeos::DBusThreadManager::IsInitialized()) {
@@ -57,6 +62,13 @@ void DeviceOffHoursController::RemoveObserver(Observer* observer) {
   observers_.RemoveObserver(observer);
 }
 
+void DeviceOffHoursController::SetClockForTesting(
+    std::unique_ptr<base::Clock> clock,
+    base::TickClock* timer_clock) {
+  clock_ = std::move(clock);
+  timer_ = base::MakeUnique<base::OneShotTimer>(timer_clock);
+}
+
 void DeviceOffHoursController::UpdateOffHoursPolicy(
     const em::ChromeDeviceSettingsProto& device_settings_proto) {
   std::vector<OffHoursInterval> off_hours_intervals;
@@ -65,8 +77,9 @@ void DeviceOffHoursController::UpdateOffHoursPolicy(
         device_settings_proto.device_off_hours());
     base::Optional<std::string> timezone = ExtractTimezoneFromProto(container);
     if (timezone) {
-      off_hours_intervals = ConvertIntervalsToGmt(
-          ExtractOffHoursIntervalsFromProto(container), *timezone);
+      off_hours_intervals =
+          ConvertIntervalsToGmt(ExtractOffHoursIntervalsFromProto(container),
+                                clock_.get(), *timezone);
     }
   }
   off_hours_intervals_.swap(off_hours_intervals);
@@ -101,7 +114,7 @@ void DeviceOffHoursController::UpdateOffHoursMode() {
     SetOffHoursMode(false);
     return;
   }
-  WeeklyTime current_time = WeeklyTime::GetCurrentWeeklyTime();
+  WeeklyTime current_time = WeeklyTime::GetCurrentWeeklyTime(clock_.get());
   for (const auto& interval : off_hours_intervals_) {
     if (interval.Contains(current_time)) {
       base::TimeDelta remaining_off_hours_duration =
@@ -138,13 +151,13 @@ void DeviceOffHoursController::SetOffHoursMode(bool off_hours_enabled) {
 void DeviceOffHoursController::StartOffHoursTimer(base::TimeDelta delay) {
   DCHECK_GT(delay, base::TimeDelta());
   DVLOG(1) << "OffHours mode timer starts for " << delay;
-  timer_.Start(FROM_HERE, delay,
-               base::Bind(&DeviceOffHoursController::UpdateOffHoursMode,
-                          weak_ptr_factory_.GetWeakPtr()));
+  timer_->Start(FROM_HERE, delay,
+                base::Bind(&DeviceOffHoursController::UpdateOffHoursMode,
+                           weak_ptr_factory_.GetWeakPtr()));
 }
 
 void DeviceOffHoursController::StopOffHoursTimer() {
-  timer_.Stop();
+  timer_->Stop();
 }
 
 void DeviceOffHoursController::SystemClockUpdated() {

chrome/browser/chromeos/policy/off_hours/device_off_hours_controller.h

@@ -11,6 +11,7 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
+#include "base/time/clock.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
 #include "chrome/browser/chromeos/policy/off_hours/off_hours_interval.h"
@@ -75,6 +76,11 @@ class DeviceOffHoursController : public chromeos::SystemClockClient::Observer,
   // chromeos::SystemClockClient::Observer:
   void SystemClockUpdated() override;
 
+  // |timer_clock| is not owned and its lifetime should cover lifetime of
+  // DeviceOffHoursContoller.
+  void SetClockForTesting(std::unique_ptr<base::Clock> clock,
+                          base::TickClock* timer_clock);
+
  private:
   // Run OnOffHoursEndTimeChanged() for observers.
   void NotifyOffHoursEndTimeChanged() const;
@@ -119,7 +125,11 @@ class DeviceOffHoursController : public chromeos::SystemClockClient::Observer,
 
   // Timer for updating device settings at the begin of next “OffHours” interval
   // or at the end of current "OffHours" interval.
-  base::OneShotTimer timer_;
+  std::unique_ptr<base::OneShotTimer> timer_;
+
+  // Used for testing purposes, otherwise it's an instance of
+  // base::DefaultClock.
+  std::unique_ptr<base::Clock> clock_;
 
   // Value is false until the system time is synchronized with network time.
   bool network_synchronized_ = false;

chrome/browser/chromeos/policy/off_hours/off_hours_interval_unittest.cc

@@ -11,10 +11,8 @@
 #include "base/values.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-namespace chromeos {
-
-using policy::off_hours::WeeklyTime;
-using policy::off_hours::OffHoursInterval;
+namespace policy {
+namespace off_hours {
 
 namespace {
 
@@ -182,4 +180,5 @@ INSTANTIATE_TEST_CASE_P(RandomInterval,
                                                         14 * kMinutesInHour,
                                                         false)));
 
-}  // namespace chromeos
+}  // namespace off_hours
+}  // namespace policy

chrome/browser/chromeos/policy/off_hours/off_hours_policy_applier_unittest.cc

+// Copyright 2017 The Chromium 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 "chrome/browser/chromeos/policy/off_hours/off_hours_policy_applier.h"
+
+#include <utility>
+
+#include "base/logging.h"
+#include "base/memory/ptr_util.h"
+#include "chrome/browser/chromeos/policy/off_hours/off_hours_interval.h"
+#include "chrome/browser/chromeos/policy/off_hours/weekly_time.h"
+#include "chrome/browser/chromeos/policy/proto/chrome_device_policy.pb.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace em = enterprise_management;
+
+namespace policy {
+namespace off_hours {
+
+namespace {
+
+constexpr em::WeeklyTimeProto_DayOfWeek kWeekdays[] = {
+    em::WeeklyTimeProto::DAY_OF_WEEK_UNSPECIFIED,
+    em::WeeklyTimeProto::MONDAY,
+    em::WeeklyTimeProto::TUESDAY,
+    em::WeeklyTimeProto::WEDNESDAY,
+    em::WeeklyTimeProto::THURSDAY,
+    em::WeeklyTimeProto::FRIDAY,
+    em::WeeklyTimeProto::SATURDAY,
+    em::WeeklyTimeProto::SUNDAY};
+
+const char kUtcTimezone[] = "UTC";
+
+const int kDeviceAllowNewUsersTag = 3;
+const int kDeviceGuestModeEnabledTag = 8;
+
+const std::vector<int> kDefaultIgnoredPolicies = {kDeviceAllowNewUsersTag,
+                                                  kDeviceGuestModeEnabledTag};
+
+struct OffHoursPolicy {
+  std::string timezone;
+  std::vector<OffHoursInterval> intervals;
+  std::vector<int> ignored_policy_proto_tags;
+
+  OffHoursPolicy(const std::string& timezone,
+                 const std::vector<OffHoursInterval>& intervals,
+                 const std::vector<int>& ignored_policy_proto_tags)
+      : timezone(timezone),
+        intervals(intervals),
+        ignored_policy_proto_tags(ignored_policy_proto_tags) {}
+};
+
+em::DeviceOffHoursIntervalProto ConvertOffHoursIntervalToProto(
+    const OffHoursInterval& off_hours_interval) {
+  em::DeviceOffHoursIntervalProto interval_proto;
+  em::WeeklyTimeProto* start = interval_proto.mutable_start();
+  em::WeeklyTimeProto* end = interval_proto.mutable_end();
+  start->set_day_of_week(kWeekdays[off_hours_interval.start().day_of_week()]);
+  start->set_time(off_hours_interval.start().milliseconds());
+  end->set_day_of_week(kWeekdays[off_hours_interval.end().day_of_week()]);
+  end->set_time(off_hours_interval.end().milliseconds());
+  return interval_proto;
+}
+
+void RemoveOffHoursPolicyFromProto(em::ChromeDeviceSettingsProto* proto) {
+  proto->clear_device_off_hours();
+}
+
+void SetOffHoursPolicyToProto(em::ChromeDeviceSettingsProto* proto,
+                              const OffHoursPolicy& off_hours_policy) {
+  RemoveOffHoursPolicyFromProto(proto);
+  auto* off_hours = proto->mutable_device_off_hours();
+  for (auto interval : off_hours_policy.intervals) {
+    auto interval_proto = ConvertOffHoursIntervalToProto(interval);
+    auto* cur = off_hours->add_intervals();
+    *cur = interval_proto;
+  }
+  off_hours->set_timezone(off_hours_policy.timezone);
+  for (auto p : off_hours_policy.ignored_policy_proto_tags) {
+    off_hours->add_ignored_policy_proto_tags(p);
+  }
+}
+
+}  // namespace
+
+class ApplyOffHoursPolicyToProtoTest : public testing::Test {
+ protected:
+  ApplyOffHoursPolicyToProtoTest() = default;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ApplyOffHoursPolicyToProtoTest);
+};
+
+TEST_F(ApplyOffHoursPolicyToProtoTest, Test) {
+  em::ChromeDeviceSettingsProto proto;
+  proto.mutable_guest_mode_enabled()->set_guest_mode_enabled(false);
+  proto.mutable_allow_new_users()->set_allow_new_users(false);
+  proto.mutable_camera_enabled()->set_camera_enabled(false);
+  SetOffHoursPolicyToProto(
+      &proto, OffHoursPolicy(kUtcTimezone, {}, kDefaultIgnoredPolicies));
+  std::unique_ptr<em::ChromeDeviceSettingsProto> off_hours_proto =
+      policy::off_hours::ApplyOffHoursPolicyToProto(proto);
+  EXPECT_TRUE(off_hours_proto->guest_mode_enabled().guest_mode_enabled());
+  EXPECT_TRUE(off_hours_proto->allow_new_users().allow_new_users());
+  EXPECT_FALSE(off_hours_proto->camera_enabled().camera_enabled());
+}
+
+}  // namespace off_hours
+}  // namespace policy

chrome/browser/chromeos/policy/off_hours/off_hours_proto_parser_unittest.cc

+// Copyright 2017 The Chromium 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 "chrome/browser/chromeos/policy/off_hours/off_hours_proto_parser.h"
+
+#include <utility>
+
+#include "base/logging.h"
+#include "base/memory/ptr_util.h"
+#include "base/values.h"
+#include "chrome/browser/chromeos/policy/off_hours/off_hours_interval.h"
+#include "chrome/browser/chromeos/policy/off_hours/weekly_time.h"
+#include "chrome/browser/chromeos/policy/proto/chrome_device_policy.pb.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace em = enterprise_management;
+
+namespace policy {
+namespace off_hours {
+
+namespace {
+
+constexpr em::WeeklyTimeProto_DayOfWeek kWeekdays[] = {
+    em::WeeklyTimeProto::DAY_OF_WEEK_UNSPECIFIED,
+    em::WeeklyTimeProto::MONDAY,
+    em::WeeklyTimeProto::TUESDAY,
+    em::WeeklyTimeProto::WEDNESDAY,
+    em::WeeklyTimeProto::THURSDAY,
+    em::WeeklyTimeProto::FRIDAY,
+    em::WeeklyTimeProto::SATURDAY,
+    em::WeeklyTimeProto::SUNDAY};
+
+constexpr base::TimeDelta kHour = base::TimeDelta::FromHours(1);
+
+const char kUtcTimezone[] = "UTC";
+
+const int kDeviceAllowNewUsersTag = 3;
+const int kDeviceGuestModeEnabledTag = 8;
+
+const std::vector<int> kDefaultIgnoredPolicies = {kDeviceAllowNewUsersTag,
+                                                  kDeviceGuestModeEnabledTag};
+
+struct OffHoursPolicy {
+  std::string timezone;
+  std::vector<OffHoursInterval> intervals;
+  std::vector<int> ignored_policy_proto_tags;
+
+  OffHoursPolicy(const std::string& timezone,
+                 const std::vector<OffHoursInterval>& intervals,
+                 const std::vector<int>& ignored_policy_proto_tags)
+      : timezone(timezone),
+        intervals(intervals),
+        ignored_policy_proto_tags(ignored_policy_proto_tags) {}
+};
+
+em::DeviceOffHoursIntervalProto ConvertOffHoursIntervalToProto(
+    const OffHoursInterval& off_hours_interval) {
+  em::DeviceOffHoursIntervalProto interval_proto;
+  em::WeeklyTimeProto* start = interval_proto.mutable_start();
+  em::WeeklyTimeProto* end = interval_proto.mutable_end();
+  start->set_day_of_week(kWeekdays[off_hours_interval.start().day_of_week()]);
+  start->set_time(off_hours_interval.start().milliseconds());
+  end->set_day_of_week(kWeekdays[off_hours_interval.end().day_of_week()]);
+  end->set_time(off_hours_interval.end().milliseconds());
+  return interval_proto;
+}
+
+void RemoveOffHoursPolicyFromProto(em::ChromeDeviceSettingsProto* proto) {
+  proto->clear_device_off_hours();
+}
+
+void SetOffHoursPolicyToProto(em::ChromeDeviceSettingsProto* proto,
+                              const OffHoursPolicy& off_hours_policy) {
+  RemoveOffHoursPolicyFromProto(proto);
+  auto* off_hours = proto->mutable_device_off_hours();
+  for (auto interval : off_hours_policy.intervals) {
+    auto interval_proto = ConvertOffHoursIntervalToProto(interval);
+    auto* cur = off_hours->add_intervals();
+    *cur = interval_proto;
+  }
+  off_hours->set_timezone(off_hours_policy.timezone);
+  for (auto p : off_hours_policy.ignored_policy_proto_tags) {
+    off_hours->add_ignored_policy_proto_tags(p);
+  }
+}
+
+}  // namespace
+
+class ConvertOffHoursProtoToValueTest : public testing::Test {
+ protected:
+  ConvertOffHoursProtoToValueTest() {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ConvertOffHoursProtoToValueTest);
+};
+
+TEST_F(ConvertOffHoursProtoToValueTest, Test) {
+  WeeklyTime start = WeeklyTime(1, kHour.InMilliseconds());
+  WeeklyTime end = WeeklyTime(3, kHour.InMilliseconds() * 2);
+  std::vector<OffHoursInterval> intervals = {OffHoursInterval(start, end)};
+
+  em::ChromeDeviceSettingsProto proto;
+  SetOffHoursPolicyToProto(
+      &proto, OffHoursPolicy(kUtcTimezone, intervals, kDefaultIgnoredPolicies));
+
+  std::unique_ptr<base::DictionaryValue> off_hours_value =
+      policy::off_hours::ConvertOffHoursProtoToValue(proto.device_off_hours());
+
+  base::DictionaryValue off_hours_expected;
+  off_hours_expected.SetString("timezone", kUtcTimezone);
+  auto intervals_value = base::MakeUnique<base::ListValue>();
+  for (const auto& interval : intervals)
+    intervals_value->Append(interval.ToValue());
+  off_hours_expected.SetList("intervals", std::move(intervals_value));
+  auto ignored_policies_value = base::MakeUnique<base::ListValue>();
+  for (const auto& policy : kDefaultIgnoredPolicies)
+    ignored_policies_value->GetList().emplace_back(policy);
+  off_hours_expected.SetList("ignored_policy_proto_tags",
+                             std::move(ignored_policies_value));
+
+  EXPECT_EQ(*off_hours_value, off_hours_expected);
+}
+
+}  // namespace off_hours
+}  // namespace policy

chrome/browser/chromeos/policy/off_hours/time_utils.cc

@@ -20,7 +20,9 @@ namespace {
 constexpr base::TimeDelta kWeek = base::TimeDelta::FromDays(7);
 }  // namespace
 
-bool GetOffsetFromTimezoneToGmt(const std::string& timezone, int* offset) {
+bool GetOffsetFromTimezoneToGmt(const std::string& timezone,
+                                base::Clock* clock,
+                                int* offset) {
   auto zone = base::WrapUnique(
       icu::TimeZone::createTimeZone(icu::UnicodeString::fromUTF8(timezone)));
   if (*zone == icu::TimeZone::getUnknown()) {
@@ -39,6 +41,14 @@ bool GetOffsetFromTimezoneToGmt(const std::string& timezone, int* offset) {
     LOG(ERROR) << "Gregorian calendar error = " << u_errorName(status);
     return false;
   }
+  UDate cur_date = static_cast<UDate>(clock->Now().ToDoubleT() *
+                                      base::Time::kMillisecondsPerSecond);
+  status = U_ZERO_ERROR;
+  gregorian_calendar->setTime(cur_date, status);
+  if (U_FAILURE(status)) {
+    LOG(ERROR) << "Gregorian calendar set time error = " << u_errorName(status);
+    return false;
+  }
   status = U_ZERO_ERROR;
   UBool day_light = gregorian_calendar->inDaylightTime(status);
   if (U_FAILURE(status)) {
@@ -52,11 +62,13 @@ bool GetOffsetFromTimezoneToGmt(const std::string& timezone, int* offset) {
   return true;
 }
 
-std::vector<OffHoursInterval> ConvertIntervalsToGmt(
-    const std::vector<OffHoursInterval>& intervals,
+std::vector<off_hours::OffHoursInterval> ConvertIntervalsToGmt(
+    const std::vector<off_hours::OffHoursInterval>& intervals,
+    base::Clock* clock,
     const std::string& timezone) {
   int gmt_offset = 0;
-  bool no_offset_error = GetOffsetFromTimezoneToGmt(timezone, &gmt_offset);
+  bool no_offset_error =
+      GetOffsetFromTimezoneToGmt(timezone, clock, &gmt_offset);
   if (!no_offset_error)
     return {};
   std::vector<OffHoursInterval> gmt_intervals;

chrome/browser/chromeos/policy/off_hours/time_utils.h

@@ -15,13 +15,16 @@ namespace policy {
 namespace off_hours {
 
 // Put time in milliseconds which is added to local time to get GMT time to
-// |offset| considering current daylight time. Return true if there was no
+// |offset| considering daylight from |clock|. Return true if there was no
 // error.
-bool GetOffsetFromTimezoneToGmt(const std::string& timezone, int* offset);
+bool GetOffsetFromTimezoneToGmt(const std::string& timezone,
+                                base::Clock* clock,
+                                int* offset);
 
 // Convert time intervals from |timezone| to GMT timezone.
 std::vector<OffHoursInterval> ConvertIntervalsToGmt(
     const std::vector<OffHoursInterval>& intervals,
+    base::Clock* clock,
     const std::string& timezone);
 
 // Return duration till next "OffHours" time interval.

chrome/browser/chromeos/policy/off_hours/weekly_time.cc

@@ -57,9 +57,9 @@ WeeklyTime WeeklyTime::AddMilliseconds(int milliseconds) const {
 }
 
 // static
-WeeklyTime WeeklyTime::GetCurrentWeeklyTime() {
+WeeklyTime WeeklyTime::GetCurrentWeeklyTime(base::Clock* clock) {
   base::Time::Exploded exploded;
-  base::Time::Now().UTCExplode(&exploded);
+  clock->Now().UTCExplode(&exploded);
   int day_of_week = exploded.day_of_week;
   // Exploded contains 0-based day of week (0 = Sunday, etc.)
   if (day_of_week == 0)

chrome/browser/chromeos/policy/off_hours/weekly_time.h

@@ -7,6 +7,7 @@
 
 #include <memory>
 
+#include "base/time/clock.h"
 #include "base/time/time.h"
 #include "base/values.h"
 
@@ -40,7 +41,7 @@ class WeeklyTime {
   WeeklyTime AddMilliseconds(int milliseconds) const;
 
   // Return current time in WeeklyTime structure.
-  static WeeklyTime GetCurrentWeeklyTime();
+  static WeeklyTime GetCurrentWeeklyTime(base::Clock* clock);
 
  private:
   // Number of weekday (1 = Monday, 2 = Tuesday, etc.)

chrome/browser/chromeos/policy/off_hours/weekly_time_unittest.cc

@@ -11,9 +11,8 @@
 #include "base/values.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-namespace chromeos {
-
-using policy::off_hours::WeeklyTime;
+namespace policy {
+namespace off_hours {
 
 namespace {
 
@@ -199,4 +198,5 @@ INSTANTIATE_TEST_CASE_P(
                                     kThursday,
                                     15 * kMinutesInHour + 24)));
 
-}  // namespace chromeos
+}  // namespace off_hours
+}  // namespace policy