Night Light: Fix a bug in how the schedule is refreshed.

- There was a bug when the start and end times are inverted
  (i.e. start time is greater than end time as a time of day
   today).
- The schedule should refresh when device resumes from sleep
  since time will have changed while the device was suspended.

BUG=768902
TEST=Added new tests

Change-Id: I105085e36e9a68db97506e69286b471b8a738731
Reviewed-on: https://chromium-review.googlesource.com/821413Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523807}

ash/system/night_light/night_light_controller.cc

@@ -17,6 +17,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/numerics/ranges.h"
 #include "base/time/time.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 #include "third_party/icu/source/i18n/astro.h"
@@ -196,9 +197,13 @@ NightLightController::NightLightController()
       binding_(this) {
   Shell::Get()->session_controller()->AddObserver(this);
   Shell::Get()->window_tree_host_manager()->AddObserver(this);
+  chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->AddObserver(
+      this);
 }
 
 NightLightController::~NightLightController() {
+  chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->RemoveObserver(
+      this);
   Shell::Get()->window_tree_host_manager()->RemoveObserver(this);
   Shell::Get()->session_controller()->RemoveObserver(this);
 }
@@ -377,6 +382,12 @@ void NightLightController::SetClient(mojom::NightLightClientPtr client) {
   }
 }
 
+void NightLightController::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 */);
+}
+
 void NightLightController::SetDelegateForTesting(
     std::unique_ptr<Delegate> delegate) {
   delegate_ = std::move(delegate);
@@ -502,6 +513,7 @@ void NightLightController::RefreshScheduleTimer(base::Time start_time,
   }
 
   // NOTE: Users can set any weird combinations.
+  const base::Time now = delegate_->GetNow();
   if (end_time <= start_time) {
     // Example:
     // Start: 9:00 PM, End: 6:00 AM.
@@ -511,9 +523,31 @@ void NightLightController::RefreshScheduleTimer(base::Time start_time,
     //        |                    |
     //       end                 start
     //
-    // From our perspective, the end time is always a day later.
+    // Note that the above times are times of day (today). It is important to
+    // know where "now" is with respect to these times to decide how to adjust
+    // them.
+    if (end_time >= now) {
+      // If the end time (today) is greater than the time now, this means "now"
+      // is within the NightLight schedule, and the start time is actually
+      // yesterday. The above timeline is interpreted as:
+      //
+      //   21:00 (-1day)              6:00
+      // <----- + ----------- + ------ + ----->
+      //        |             |        |
+      //      start          now      end
+      //
+      start_time -= base::TimeDelta::FromDays(1);
+    } else {
+      // Two possibilities here:
+      // - Either "now" is greater than the end time, but less than start time.
+      //   This means NightLight is outside the schedule, waiting for the next
+      //   start time. The end time is actually a day later.
+      // - Or "now" is greater than both the start and end times. This means
+      //   NightLight is within the schedule, waiting to turn off at the next
+      //   end time, which is also a day later.
       end_time += base::TimeDelta::FromDays(1);
     }
+  }
 
   DCHECK_GE(end_time, start_time);
 
@@ -522,7 +556,6 @@ void NightLightController::RefreshScheduleTimer(base::Time start_time,
   bool enable_now = false;
 
   // Where are we now with respect to the start and end times?
-  const base::Time now = delegate_->GetNow();
   if (now < start_time) {
     // Example:
     // Start: 6:00 PM today, End: 6:00 AM tomorrow, Now: 4:00 PM.

ash/system/night_light/night_light_controller.h

@@ -15,6 +15,7 @@
 #include "base/observer_list.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
+#include "chromeos/dbus/power_manager_client.h"
 #include "components/prefs/pref_change_registrar.h"
 #include "mojo/public/cpp/bindings/binding.h"
 
@@ -27,9 +28,11 @@ class ColorTemperatureAnimation;
 
 // Controls the NightLight feature that adjusts the color temperature of the
 // screen.
-class ASH_EXPORT NightLightController : public mojom::NightLightController,
+class ASH_EXPORT NightLightController
+    : public mojom::NightLightController,
       public WindowTreeHostManager::Observer,
-                                        public SessionObserver {
+      public SessionObserver,
+      public chromeos::PowerManagerClient::Observer {
  public:
   using ScheduleType = mojom::NightLightController::ScheduleType;
 
@@ -125,6 +128,9 @@ class ASH_EXPORT NightLightController : public mojom::NightLightController,
   void SetCurrentGeoposition(mojom::SimpleGeopositionPtr position) override;
   void SetClient(mojom::NightLightClientPtr client) override;
 
+  // chromeos::PowerManagerClient::Observer:
+  void SuspendDone(const base::TimeDelta& sleep_duration) override;
+
   void SetDelegateForTesting(std::unique_ptr<Delegate> delegate);
 
  private:

ash/system/night_light/night_light_controller_unittest.cc

@@ -615,6 +615,128 @@ TEST_F(NightLightTest, TestSunsetSunriseGeoposition) {
             controller->timer().GetCurrentDelay());
 }
 
+// Tests that on device resume from sleep, the NightLight status is updated
+// correctly if the time has changed meanwhile.
+TEST_F(NightLightTest, TestCustomScheduleOnResume) {
+  NightLightController* controller = GetController();
+  // Now is 4:00 PM.
+  delegate()->SetFakeNow(TimeOfDay(16 * 60));
+  SetNightLightEnabled(false);
+  // Start time is at 6:00 PM and end time is at 10:00 PM. NightLight should be
+  // off.
+  //      16:00         18:00         22:00
+  // <----- + ----------- + ----------- + ----->
+  //        |             |             |
+  //       now          start          end
+  //
+  controller->SetColorTemperature(0.4f);
+  controller->SetCustomStartTime(TimeOfDay(18 * 60));
+  controller->SetCustomEndTime(TimeOfDay(22 * 60));
+  controller->SetScheduleType(NightLightController::ScheduleType::kCustom);
+
+  EXPECT_FALSE(controller->GetEnabled());
+  TestCompositorsTemperature(0.0f);
+  EXPECT_TRUE(controller->timer().IsRunning());
+  // NightLight should start in 2 hours.
+  EXPECT_EQ(base::TimeDelta::FromHours(2),
+            controller->timer().GetCurrentDelay());
+
+  // Now simulate that the device was suspended for 3 hours, and the time now
+  // is 7:00 PM when the devices was resumed. Expect that NightLight turns on.
+  delegate()->SetFakeNow(TimeOfDay(19 * 60));
+  controller->SuspendDone(base::TimeDelta::Max());
+
+  EXPECT_TRUE(controller->GetEnabled());
+  TestCompositorsTemperature(0.4f);
+  EXPECT_TRUE(controller->timer().IsRunning());
+  // NightLight should end in 3 hours.
+  EXPECT_EQ(base::TimeDelta::FromHours(3),
+            controller->timer().GetCurrentDelay());
+}
+
+// The following tests ensure that the NightLight schedule is correctly
+// refreshed when the start and end times are inverted (i.e. the "start time" as
+// a time of day today is in the future with respect to the "end time" also as a
+// time of day today).
+//
+// Case 1: "Now" is less than both "end" and "start".
+TEST_F(NightLightTest, TestCustomScheduleInvertedStartAndEndTimesCase1) {
+  NightLightController* controller = GetController();
+  // Now is 4:00 AM.
+  delegate()->SetFakeNow(TimeOfDay(4 * 60));
+  SetNightLightEnabled(false);
+  // Start time is at 9:00 PM and end time is at 6:00 AM. "Now" is less than
+  // both. NightLight should be on.
+  //       4:00          6:00         21:00
+  // <----- + ----------- + ----------- + ----->
+  //        |             |             |
+  //       now           end          start
+  //
+  controller->SetColorTemperature(0.4f);
+  controller->SetCustomStartTime(TimeOfDay(21 * 60));
+  controller->SetCustomEndTime(TimeOfDay(6 * 60));
+  controller->SetScheduleType(NightLightController::ScheduleType::kCustom);
+
+  EXPECT_TRUE(controller->GetEnabled());
+  TestCompositorsTemperature(0.4f);
+  EXPECT_TRUE(controller->timer().IsRunning());
+  // NightLight should end in two hours.
+  EXPECT_EQ(base::TimeDelta::FromHours(2),
+            controller->timer().GetCurrentDelay());
+}
+
+// Case 2: "Now" is between "end" and "start".
+TEST_F(NightLightTest, TestCustomScheduleInvertedStartAndEndTimesCase2) {
+  NightLightController* controller = GetController();
+  // Now is 6:00 AM.
+  delegate()->SetFakeNow(TimeOfDay(6 * 60));
+  SetNightLightEnabled(false);
+  // Start time is at 9:00 PM and end time is at 4:00 AM. "Now" is between both.
+  // NightLight should be off.
+  //       4:00          6:00         21:00
+  // <----- + ----------- + ----------- + ----->
+  //        |             |             |
+  //       end           now          start
+  //
+  controller->SetColorTemperature(0.4f);
+  controller->SetCustomStartTime(TimeOfDay(21 * 60));
+  controller->SetCustomEndTime(TimeOfDay(4 * 60));
+  controller->SetScheduleType(NightLightController::ScheduleType::kCustom);
+
+  EXPECT_FALSE(controller->GetEnabled());
+  TestCompositorsTemperature(0.0f);
+  EXPECT_TRUE(controller->timer().IsRunning());
+  // NightLight should start in 15 hours.
+  EXPECT_EQ(base::TimeDelta::FromHours(15),
+            controller->timer().GetCurrentDelay());
+}
+
+// Case 3: "Now" is greater than both "start" and "end".
+TEST_F(NightLightTest, TestCustomScheduleInvertedStartAndEndTimesCase3) {
+  NightLightController* controller = GetController();
+  // Now is 11:00 PM.
+  delegate()->SetFakeNow(TimeOfDay(23 * 60));
+  SetNightLightEnabled(false);
+  // Start time is at 9:00 PM and end time is at 4:00 AM. "Now" is greater than
+  // both. NightLight should be on.
+  //       4:00         21:00         23:00
+  // <----- + ----------- + ----------- + ----->
+  //        |             |             |
+  //       end          start          now
+  //
+  controller->SetColorTemperature(0.4f);
+  controller->SetCustomStartTime(TimeOfDay(21 * 60));
+  controller->SetCustomEndTime(TimeOfDay(4 * 60));
+  controller->SetScheduleType(NightLightController::ScheduleType::kCustom);
+
+  EXPECT_TRUE(controller->GetEnabled());
+  TestCompositorsTemperature(0.4f);
+  EXPECT_TRUE(controller->timer().IsRunning());
+  // NightLight should end in 5 hours.
+  EXPECT_EQ(base::TimeDelta::FromHours(5),
+            controller->timer().GetCurrentDelay());
+}
+
 }  // namespace
 
 }  // namespace ash