Night Light: Persist manual status changes during automatic schedules

When an automatic schedule is being used, users can toggle the status
of Night Light manually from the system tray or the system settings.

This manual setting used to get reverted, and the schedule used to get
reapplied in the following cases:
- Switching users.
- Resume from suspend.
- Receiving a new geoposition.

This CL makes it possible to remember manual toggles, and reapply
them if possible.

BUG=999397
TEST=Manually, added new tests.

Change-Id: Idb1b76cf24c95e0833a65542f0b0ffa341195c3e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2049950
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#741569}

ash/system/night_light/night_light_controller_impl.cc

@@ -670,6 +670,10 @@ void NightLightControllerImpl::SetCurrentGeoposition(
 
   is_current_geoposition_from_cache_ = false;
   StoreCachedGeoposition(position);
+
+  const base::Time previous_sunset = delegate_->GetSunsetTime();
+  const base::Time previous_sunrise = delegate_->GetSunriseTime();
+
   if (!delegate_->SetGeoposition(position)) {
     VLOG(1) << "Not refreshing since geoposition hasn't changed";
     return;
@@ -678,15 +682,29 @@ void NightLightControllerImpl::SetCurrentGeoposition(
   // If the schedule type is sunset to sunrise or custom, a potential change in
   // the geoposition might mean timezone change as well as a change in the start
   // and end times. In these cases, we must trigger a refresh.
-  if (GetScheduleType() != ScheduleType::kNone)
-    Refresh(true /* did_schedule_change */);
+  if (GetScheduleType() == ScheduleType::kNone)
+    return;
+
+  // We only keep manual toggles if the change in geoposition results in an hour
+  // or more in either sunset or sunrise times. A one-hour threshold is used
+  // here as an indication of a possible timezone change, and this case, manual
+  // toggles should be ignored.
+  constexpr base::TimeDelta kOneHourDuration = base::TimeDelta::FromHours(1);
+  const bool keep_manual_toggles_during_schedules =
+      (delegate_->GetSunsetTime() - previous_sunset).magnitude() <
+          kOneHourDuration &&
+      (delegate_->GetSunriseTime() - previous_sunrise).magnitude() <
+          kOneHourDuration;
+
+  Refresh(/*did_schedule_change=*/true, keep_manual_toggles_during_schedules);
 }
 
 void NightLightControllerImpl::SuspendDone(
     const base::TimeDelta& sleep_duration) {
   // Time changes while the device is suspended. We need to refresh the schedule
   // upon device resume to know what the status should be now.
-  Refresh(true /* did_schedule_change */);
+  Refresh(/*did_schedule_change=*/true,
+          /*keep_manual_toggles_during_schedules=*/true);
 }
 
 void NightLightControllerImpl::Close(bool by_user) {
@@ -756,6 +774,26 @@ NightLightControllerImpl::GetAutoNightLightNotificationForTesting() const {
       kNotificationId);
 }
 
+bool NightLightControllerImpl::MaybeRestoreSchedule() {
+  DCHECK(active_user_pref_service_);
+  DCHECK_NE(GetScheduleType(), ScheduleType::kNone);
+
+  auto iter = per_user_schedule_target_state_.find(active_user_pref_service_);
+  if (iter == per_user_schedule_target_state_.end())
+    return false;
+
+  ScheduleTargetState& target_state = iter->second;
+  // It may be that the device was suspended for a very long time that the
+  // target time is no longer valid.
+  if (target_state.target_time <= delegate_->GetNow())
+    return false;
+
+  VLOG(1) << "Restoring a previous schedule.";
+  DCHECK_NE(GetEnabled(), target_state.target_status);
+  ScheduleNextToggle(target_state.target_time - delegate_->GetNow());
+  return true;
+}
+
 bool NightLightControllerImpl::UserHasEverChangedSchedule() const {
   return active_user_pref_service_ &&
          active_user_pref_service_->HasPrefPath(prefs::kNightLightScheduleType);
@@ -865,12 +903,14 @@ void NightLightControllerImpl::RefreshDisplaysTemperature(
 }
 
 void NightLightControllerImpl::ReapplyColorTemperatures() {
+  DCHECK(temperature_animation_);
   const float target_temperature = GetEnabled() ? GetColorTemperature() : 0.0f;
-  if (temperature_animation_ && temperature_animation_->is_animating()) {
+  if (temperature_animation_->is_animating()) {
     // Do not interrupt an on-going animation towards the same target value.
     if (temperature_animation_->target_temperature() == target_temperature)
       return;
 
+    NOTREACHED();
     temperature_animation_->Stop();
   }
 
@@ -921,7 +961,8 @@ void NightLightControllerImpl::InitFromUserPrefs() {
   LoadCachedGeopositionIfNeeded();
   if (GetAmbientColorEnabled())
     UpdateAmbientRgbScalingFactors();
-  Refresh(true /* did_schedule_change */);
+  Refresh(/*did_schedule_change=*/true,
+          /*keep_manual_toggles_during_schedules=*/true);
   NotifyStatusChanged();
   NotifyClientWithScheduleChange();
   is_first_user_init_ = false;
@@ -952,7 +993,8 @@ void NightLightControllerImpl::OnEnabledPrefChanged() {
     ShowAutoNightLightNotification();
   }
 
-  Refresh(false /* did_schedule_change */);
+  Refresh(/*did_schedule_change=*/false,
+          /*keep_manual_toggles_during_schedules=*/false);
   NotifyStatusChanged();
 }
 
@@ -978,47 +1020,57 @@ void NightLightControllerImpl::OnScheduleTypePrefChanged() {
   VLOG(1) << "Schedule type changed. New type: " << GetScheduleType() << ".";
   DCHECK(active_user_pref_service_);
   NotifyClientWithScheduleChange();
-  Refresh(true /* did_schedule_change */);
+  Refresh(/*did_schedule_change=*/true,
+          /*keep_manual_toggles_during_schedules=*/false);
 
   UMA_HISTOGRAM_ENUMERATION("Ash.NightLight.ScheduleType", GetScheduleType());
 }
 
 void NightLightControllerImpl::OnCustomSchedulePrefsChanged() {
   DCHECK(active_user_pref_service_);
-  Refresh(true /* did_schedule_change */);
+  Refresh(/*did_schedule_change=*/true,
+          /*keep_manual_toggles_during_schedules=*/false);
 }
 
-void NightLightControllerImpl::Refresh(bool did_schedule_change) {
-  RefreshDisplaysTemperature(GetColorTemperature());
-
-  const ScheduleType type = GetScheduleType();
-  switch (type) {
+void NightLightControllerImpl::Refresh(
+    bool did_schedule_change,
+    bool keep_manual_toggles_during_schedules) {
+  switch (GetScheduleType()) {
     case ScheduleType::kNone:
       timer_.Stop();
+      RefreshDisplaysTemperature(GetColorTemperature());
       return;
 
     case ScheduleType::kSunsetToSunrise:
       RefreshScheduleTimer(delegate_->GetSunsetTime(),
-                           delegate_->GetSunriseTime(), did_schedule_change);
+                           delegate_->GetSunriseTime(), did_schedule_change,
+                           keep_manual_toggles_during_schedules);
       return;
 
     case ScheduleType::kCustom:
-      RefreshScheduleTimer(GetCustomStartTime().ToTimeToday(),
-                           GetCustomEndTime().ToTimeToday(),
-                           did_schedule_change);
+      RefreshScheduleTimer(
+          GetCustomStartTime().ToTimeToday(), GetCustomEndTime().ToTimeToday(),
+          did_schedule_change, keep_manual_toggles_during_schedules);
       return;
   }
 }
 
-void NightLightControllerImpl::RefreshScheduleTimer(base::Time start_time,
-                                                    base::Time end_time,
-                                                    bool did_schedule_change) {
+void NightLightControllerImpl::RefreshScheduleTimer(
+    base::Time start_time,
+    base::Time end_time,
+    bool did_schedule_change,
+    bool keep_manual_toggles_during_schedules) {
   if (GetScheduleType() == ScheduleType::kNone) {
     NOTREACHED();
     timer_.Stop();
     return;
   }
 
+  if (keep_manual_toggles_during_schedules && MaybeRestoreSchedule()) {
+    RefreshDisplaysTemperature(GetColorTemperature());
+    return;
+  }
+
   // NOTE: Users can set any weird combinations.
   const base::Time now = delegate_->GetNow();
   if (end_time <= start_time) {
@@ -1124,13 +1176,21 @@ void NightLightControllerImpl::RefreshScheduleTimer(base::Time start_time,
   // status to be toggled according to the time that corresponds with the
   // opposite status of the current one.
   ScheduleNextToggle(GetEnabled() ? end_time - now : start_time - now);
+  RefreshDisplaysTemperature(GetColorTemperature());
 }
 
 void NightLightControllerImpl::ScheduleNextToggle(base::TimeDelta delay) {
+  DCHECK(active_user_pref_service_);
+
   const bool new_status = !GetEnabled();
+  const base::Time target_time = delegate_->GetNow() + delay;
+
+  per_user_schedule_target_state_[active_user_pref_service_] =
+      ScheduleTargetState{target_time, new_status};
+
   VLOG(1) << "Setting Night Light to toggle to "
           << (new_status ? "enabled" : "disabled") << " at "
-          << base::TimeFormatTimeOfDay(delegate_->GetNow() + delay);
+          << base::TimeFormatTimeOfDay(target_time);
   timer_.Start(FROM_HERE, delay,
                base::BindOnce(&NightLightControllerImpl::SetEnabled,
                               base::Unretained(this), new_status,

ash/system/night_light/night_light_controller_impl.h

@@ -12,6 +12,7 @@
 #include "ash/public/cpp/night_light_controller.h"
 #include "ash/session/session_observer.h"
 #include "ash/system/night_light/time_of_day.h"
+#include "base/containers/flat_map.h"
 #include "base/memory/weak_ptr.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
@@ -193,6 +194,10 @@ class ASH_EXPORT NightLightControllerImpl
   message_center::Notification* GetAutoNightLightNotificationForTesting() const;
 
  private:
+  // Attempts restoring a previously stored schedule for the current user if
+  // possible and returns true if so, false otherwise.
+  bool MaybeRestoreSchedule();
+
   // Returns true if the user has ever changed the schedule type, which means we
   // must respect the user's choice and let it overwrite Auto Night Light.
   bool UserHasEverChangedSchedule() const;
@@ -256,17 +261,23 @@ class ASH_EXPORT NightLightControllerImpl
   // parameters. |did_schedule_change| is true when Refresh() is called as a
   // result of a change in one of the schedule related prefs, and false
   // otherwise.
-  void Refresh(bool did_schedule_change);
+  // If |keep_manual_toggles_during_schedules| is true, refreshing the schedule
+  // will not override a previous user's decision to toggle the NightLight
+  // status while the schedule is being used.
+  void Refresh(bool did_schedule_change,
+               bool keep_manual_toggles_during_schedules);
 
   // Given the desired start and end times that determine the time interval
   // during which NightLight will be ON, depending on the time of "now", it
   // refreshes the |timer_| to either schedule the future start or end of
   // NightLight mode, as well as update the current status if needed.
-  // For |did_schedule_change|, see Refresh() above.
+  // For |did_schedule_change| and |keep_manual_toggles_during_schedules|, see
+  // Refresh() above.
   // This function should never be called if the schedule type is |kNone|.
   void RefreshScheduleTimer(base::Time start_time,
                             base::Time end_time,
-                            bool did_schedule_change);
+                            bool did_schedule_change,
+                            bool keep_manual_toggles_during_schedules);
 
   // Schedule the upcoming next toggle of NightLight mode. This is used for the
   // automatic status changes of NightLight which always use an
@@ -286,6 +297,18 @@ class ASH_EXPORT NightLightControllerImpl
 
   std::unique_ptr<ColorTemperatureAnimation> temperature_animation_;
 
+  // Tracks the upcoming NightLight state changes per each user due to automatic
+  // schedules. This can be used to restore a manually toggled status while the
+  // schedule is being used. See MaybeRestoreSchedule().
+  struct ScheduleTargetState {
+    // The time at which NightLight will switch to |target_status| defined
+    // below.
+    base::Time target_time;
+    bool target_status;
+  };
+  base::flat_map<PrefService*, ScheduleTargetState>
+      per_user_schedule_target_state_;
+
   // The timer that schedules the start and end of NightLight when the schedule
   // type is either kSunsetToSunrise or kCustom.
   base::OneShotTimer timer_;

ash/system/night_light/night_light_controller_unittest.cc

@@ -17,6 +17,7 @@
 #include "ash/session/test_session_controller_client.h"
 #include "ash/shell.h"
 #include "ash/system/night_light/night_light_controller_impl.h"
+#include "ash/system/night_light/time_of_day.h"
 #include "ash/test/ash_test_base.h"
 #include "ash/test/ash_test_helper.h"
 #include "ash/test_shell_delegate.h"
@@ -28,6 +29,7 @@
 #include "base/optional.h"
 #include "base/strings/pattern.h"
 #include "base/test/scoped_feature_list.h"
+#include "base/time/time.h"
 #include "components/prefs/pref_service.h"
 #include "ui/compositor/layer.h"
 #include "ui/display/fake/fake_display_snapshot.h"
@@ -45,6 +47,25 @@ namespace {
 constexpr char kUser1Email[] = "user1@nightlight";
 constexpr char kUser2Email[] = "user2@nightlight";
 
+enum AmPm { kAM, kPM };
+
+// Convenience function for constructing a TimeOfDay object for exact hours
+// during the day. |hour| is between 1 and 12.
+TimeOfDay MakeTimeOfDay(int hour, AmPm am_pm) {
+  DCHECK_GE(hour, 1);
+  DCHECK_LE(hour, 12);
+
+  if (am_pm == kAM) {
+    hour %= 12;
+  } else {
+    if (hour != 12)
+      hour += 12;
+    hour %= 24;
+  }
+
+  return TimeOfDay(hour * 60);
+}
+
 NightLightControllerImpl* GetController() {
   return Shell::Get()->night_light_controller();
 }
@@ -131,6 +152,7 @@ class TestDelegate : public NightLightControllerImpl::Delegate {
   TestDelegate() = default;
   ~TestDelegate() override = default;
 
+  void SetFakeNow(base::Time time) { fake_now_ = time; }
   void SetFakeNow(TimeOfDay time) { fake_now_ = time.ToTimeToday(); }
   void SetFakeSunset(TimeOfDay time) { fake_sunset_ = time.ToTimeToday(); }
   void SetFakeSunrise(TimeOfDay time) { fake_sunrise_ = time.ToTimeToday(); }
@@ -1112,6 +1134,157 @@ TEST_F(NightLightTest, TestNightLightAndAmbientColorInteraction) {
   EXPECT_LT(night_light_and_ambient_rgb.z(), night_light_rgb.z());
 }
 
+// Tests that manual changes to NightLight status while a schedule is being used
+// will be remembered and reapplied across user switches.
+TEST_F(NightLightTest, MultiUserManualStatusToggleWithSchedules) {
+  // Setup user 1 to use a custom schedule from 3pm till 8pm, and user 2 to use
+  // a sunset-to-sunrise schedule from 5pm till 4am.
+  //
+  //
+  //          |<--- User 1 NL on --->|
+  //          |                      |
+  // <--------+--------+-------------+----------------------------+----------->
+  //         3pm      5pm           8pm                          4am
+  //                   |                                          |
+  //                   |<-------------- User 2 NL on ------------>|
+  //
+  // Test cases at:
+  //
+  // <---+---------+------------+------------+----------------------------+--->
+  //    2pm       4pm         7pm           10pm                         9am
+  //
+
+  delegate()->SetFakeNow(MakeTimeOfDay(2, kPM));
+  delegate()->SetFakeSunset(MakeTimeOfDay(5, kPM));
+  delegate()->SetFakeSunrise(MakeTimeOfDay(4, kAM));
+
+  constexpr float kUser1Temperature = 0.6f;
+  constexpr float kUser2Temperature = 0.8f;
+
+  NightLightControllerImpl* controller = GetController();
+  controller->SetCustomStartTime(MakeTimeOfDay(3, kPM));
+  controller->SetCustomEndTime(MakeTimeOfDay(8, kPM));
+  controller->SetScheduleType(NightLightController::ScheduleType::kCustom);
+  controller->SetColorTemperature(kUser1Temperature);
+  SwitchActiveUser(kUser2Email);
+  controller->SetScheduleType(
+      NightLightController::ScheduleType::kSunsetToSunrise);
+  controller->SetColorTemperature(kUser2Temperature);
+  SwitchActiveUser(kUser1Email);
+
+  struct {
+    base::Time fake_now;
+    bool user_1_expected_status;
+    bool user_2_expected_status;
+  } kTestCases[] = {
+      {MakeTimeOfDay(2, kPM).ToTimeToday(), false, false},
+      {MakeTimeOfDay(4, kPM).ToTimeToday(), true, false},
+      {MakeTimeOfDay(7, kPM).ToTimeToday(), true, true},
+      {MakeTimeOfDay(10, kPM).ToTimeToday(), false, true},
+      {MakeTimeOfDay(9, kAM).ToTimeToday() +
+           base::TimeDelta::FromDays(1),  // 9:00 AM tomorrow.
+       false, false},
+  };
+
+  // Verifies that NightLight status is |expected_status| and the given
+  // |user_temperature| is applied only when NightLight is expected to be
+  // enabled.
+  auto verify_night_light_state = [controller](bool expected_status,
+                                               float user_temperature) {
+    EXPECT_EQ(expected_status, controller->GetEnabled());
+    TestCompositorsTemperature(expected_status ? user_temperature : 0.0f);
+  };
+
+  bool user_1_previous_status = false;
+  for (const auto& test_case : kTestCases) {
+    // Each test case begins when user_1 is active.
+    SCOPED_TRACE(TimeOfDay::FromTime(test_case.fake_now).ToString());
+
+    const bool user_1_toggled_status = !test_case.user_1_expected_status;
+    const bool user_2_toggled_status = !test_case.user_2_expected_status;
+
+    // Apply the test's case fake time, and fire the timer if there's a change
+    // expected in NightLight's status.
+    delegate()->SetFakeNow(test_case.fake_now);
+    if (user_1_previous_status != test_case.user_1_expected_status)
+      controller->timer()->FireNow();
+    user_1_previous_status = test_case.user_1_expected_status;
+
+    // The untoggled states for both users should match the expected ones
+    // according to their schedules.
+    verify_night_light_state(test_case.user_1_expected_status,
+                             kUser1Temperature);
+    SwitchActiveUser(kUser2Email);
+    verify_night_light_state(test_case.user_2_expected_status,
+                             kUser2Temperature);
+
+    // Manually toggle NightLight for user_2 and expect that it will be
+    // remembered when we switch to user_1 and back.
+    controller->Toggle();
+    verify_night_light_state(user_2_toggled_status, kUser2Temperature);
+    SwitchActiveUser(kUser1Email);
+    verify_night_light_state(test_case.user_1_expected_status,
+                             kUser1Temperature);
+    SwitchActiveUser(kUser2Email);
+    verify_night_light_state(user_2_toggled_status, kUser2Temperature);
+
+    // Toggle it for user_1 as well, and expect it will be remembered and won't
+    // affect the already toggled state for user_2.
+    SwitchActiveUser(kUser1Email);
+    verify_night_light_state(test_case.user_1_expected_status,
+                             kUser1Temperature);
+    controller->Toggle();
+    verify_night_light_state(user_1_toggled_status, kUser1Temperature);
+    SwitchActiveUser(kUser2Email);
+    verify_night_light_state(user_2_toggled_status, kUser2Temperature);
+
+    // Toggle both users back to their original states in preparation for the
+    // next test case.
+    controller->Toggle();
+    verify_night_light_state(test_case.user_2_expected_status,
+                             kUser2Temperature);
+    SwitchActiveUser(kUser1Email);
+    verify_night_light_state(user_1_toggled_status, kUser1Temperature);
+    controller->Toggle();
+    verify_night_light_state(test_case.user_1_expected_status,
+                             kUser1Temperature);
+  }
+}
+
+TEST_F(NightLightTest, ManualStatusToggleCanPersistAfterResumeFromSuspend) {
+  delegate()->SetFakeNow(MakeTimeOfDay(11, kAM));
+  NightLightControllerImpl* controller = GetController();
+  controller->SetCustomStartTime(MakeTimeOfDay(3, kPM));
+  controller->SetCustomEndTime(MakeTimeOfDay(8, kPM));
+  controller->SetScheduleType(NightLightController::ScheduleType::kCustom);
+  EXPECT_FALSE(controller->GetEnabled());
+
+  // Toggle the status manually and expect that NightLight is scheduled to
+  // turn back off at 8:00 PM.
+  controller->Toggle();
+  EXPECT_TRUE(controller->GetEnabled());
+  EXPECT_TRUE(controller->timer()->IsRunning());
+  EXPECT_EQ(base::TimeDelta::FromHours(9),
+            controller->timer()->GetCurrentDelay());
+
+  // Simulate suspend and then resume at 2:00 PM (which is outside the user's
+  // custom schedule). However, the manual toggle to on should be kept.
+  delegate()->SetFakeNow(MakeTimeOfDay(2, kPM));
+  controller->SuspendDone(base::TimeDelta{});
+  EXPECT_TRUE(controller->GetEnabled());
+
+  // Suspend again and resume at 5:00 PM (which is within the user's custom
+  // schedule). The schedule should be applied normally.
+  delegate()->SetFakeNow(MakeTimeOfDay(5, kPM));
+  controller->SuspendDone(base::TimeDelta{});
+  EXPECT_TRUE(controller->GetEnabled());
+
+  // Suspend and resume at 9:00 PM and expect NightLight to be off.
+  delegate()->SetFakeNow(MakeTimeOfDay(9, kPM));
+  controller->SuspendDone(base::TimeDelta{});
+  EXPECT_FALSE(controller->GetEnabled());
+}
+
 // Fixture for testing behavior of Night Light when displays support hardware
 // CRTC matrices.
 class NightLightCrtcTest : public NightLightTest {