Skip to content

Commit

Permalink
Use new scheduling_utils in ScheduledFeature.
Browse files Browse the repository at this point in the history
This simply integrates the new function into ScheduledFeature with
absolutely no caller-facing changes (note ScheduledFeature's unit
tests all pass without any changes). In the next CL, new APIs will
be added to ScheduledFeature that allows callers to observe the
current SunsetToSunriseCheckpoint. It's split up this way to make
the reviewing easier.

Bug: b:266612167
Change-Id: If19e92cdf53e44d14e57408c505159d62391a998
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4279570
Commit-Queue: Eric Sum <esum@google.com>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1109018}
  • Loading branch information
esum26 authored and Chromium LUCI CQ committed Feb 23, 2023
1 parent e11a5d0 commit 6147161
Show file tree
Hide file tree
Showing 5 changed files with 155 additions and 150 deletions.
15 changes: 9 additions & 6 deletions ash/system/scheduled_feature/schedule_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#include "base/check.h"
#include "base/logging.h"
#include "base/notreached.h"
#include "base/time/clock.h"

namespace ash::schedule_utils {

Expand Down Expand Up @@ -62,9 +61,7 @@ std::vector<Slot> BuildSchedule(base::Time sunrise_time,

// Shift `sunset_time` such that
// `sunrise_time` <= `sunset_time` < `sunrise_time + kOneDay`.
const base::TimeDelta amount_to_advance_sunset =
(sunrise_time - sunset_time).CeilToMultiple(kOneDay);
sunset_time += amount_to_advance_sunset;
sunset_time = ShiftWithinOneDayFrom(sunrise_time, sunset_time);

const base::TimeDelta daylight_duration = sunset_time - sunrise_time;
DCHECK_GE(daylight_duration, base::TimeDelta());
Expand Down Expand Up @@ -107,8 +104,7 @@ Slot GetNextSlot(const size_t current_idx, const std::vector<Slot>& schedule) {

Position GetCurrentPosition(const base::Time sunrise_time,
const base::Time sunset_time,
const base::Clock* custom_clock) {
const base::Time now = custom_clock ? custom_clock->Now() : base::Time::Now();
const base::Time now) {
const std::vector<Slot> schedule =
BuildSchedule(sunrise_time, sunset_time, now);
DCHECK(!schedule.empty());
Expand All @@ -129,4 +125,11 @@ Position GetCurrentPosition(const base::Time sunrise_time,
return Position();
}

base::Time ShiftWithinOneDayFrom(const base::Time origin,
const base::Time time_in) {
const base::TimeDelta amount_to_advance_time_in =
(origin - time_in).CeilToMultiple(kOneDay);
return time_in + amount_to_advance_time_in;
}

} // namespace ash::schedule_utils
20 changes: 10 additions & 10 deletions ash/system/scheduled_feature/schedule_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@
#include "ash/public/cpp/schedule_enums.h"
#include "base/time/time.h"

namespace base {
class Clock;
} // namespace base

namespace ash::schedule_utils {

struct ASH_EXPORT Position {
Expand All @@ -27,12 +23,16 @@ struct ASH_EXPORT Position {
// Returns the current position in the schedule using local `sunrise_time`
// and `sunset_time`. The date of the provided sunrise/sunset times are
// irrelevant; their corresponding times of day are extracted and used
// internally. This uses system time to get "now", or the `custom_clock` if
// specified.
ASH_EXPORT Position
GetCurrentPosition(const base::Time sunrise_time,
const base::Time sunset_time,
const base::Clock* custom_clock = nullptr);
// internally.
ASH_EXPORT Position GetCurrentPosition(const base::Time sunrise_time,
const base::Time sunset_time,
const base::Time now);

// Shifts `time_in` by a whole number of days such that it's < 1 day from the
// `origin`:
// `origin` <= output < `origin` + 24 hours
ASH_EXPORT base::Time ShiftWithinOneDayFrom(const base::Time origin,
const base::Time time_in);

} // namespace ash::schedule_utils

Expand Down
91 changes: 63 additions & 28 deletions ash/system/scheduled_feature/schedule_utils_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,44 +58,44 @@ TEST_F(ScheduleUtilsTest, DetectsAllCheckpoints) {
const base::Time sunrise_time = BuildTime("06:00:00");
const base::Time sunset_time = BuildTime("18:00:00");
AdvanceClockTo("05:59:59");
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, &clock_),
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kSunset,
SunsetToSunriseCheckpoint::kSunrise, base::Seconds(1)));

AdvanceClockTo("06:00:00");
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, &clock_),
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kSunrise,
SunsetToSunriseCheckpoint::kMorning, base::Hours(4)));

AdvanceClockTo("09:59:59");
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, &clock_),
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kSunrise,
SunsetToSunriseCheckpoint::kMorning, base::Seconds(1)));

AdvanceClockTo("10:00:00");
EXPECT_THAT(
GetCurrentPosition(sunrise_time, sunset_time, &clock_),
GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kMorning,
SunsetToSunriseCheckpoint::kLateAfternoon, base::Hours(6)));

AdvanceClockTo("15:59:59");
EXPECT_THAT(
GetCurrentPosition(sunrise_time, sunset_time, &clock_),
GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kMorning,
SunsetToSunriseCheckpoint::kLateAfternoon, base::Seconds(1)));

AdvanceClockTo("16:00:00");
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, &clock_),
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kLateAfternoon,
SunsetToSunriseCheckpoint::kSunset, base::Hours(2)));

AdvanceClockTo("17:59:59");
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, &clock_),
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kLateAfternoon,
SunsetToSunriseCheckpoint::kSunset, base::Seconds(1)));

AdvanceClockTo("18:00:00");
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, &clock_),
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kSunset,
SunsetToSunriseCheckpoint::kSunrise, base::Hours(12)));
}
Expand All @@ -109,23 +109,23 @@ TEST_F(ScheduleUtilsTest, SunsetJustAfterSunrise) {
const base::Time sunset_time = BuildTime("13:30:00");
AdvanceClockTo("12:00:00");
EXPECT_THAT(
GetCurrentPosition(sunrise_time, sunset_time, &clock_),
GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kSunrise,
SunsetToSunriseCheckpoint::kMorning, base::Minutes(30)));

AdvanceClockTo("12:30:00");
EXPECT_THAT(
GetCurrentPosition(sunrise_time, sunset_time, &clock_),
GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kMorning,
SunsetToSunriseCheckpoint::kLateAfternoon, base::Minutes(45)));

AdvanceClockTo("13:15:00");
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, &clock_),
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kLateAfternoon,
SunsetToSunriseCheckpoint::kSunset, base::Minutes(15)));

AdvanceClockTo("13:30:00");
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, &clock_),
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kSunset,
SunsetToSunriseCheckpoint::kSunrise,
base::Days(1) - base::Minutes(90)));
Expand All @@ -139,25 +139,25 @@ TEST_F(ScheduleUtilsTest, SunriseJustAfterSunset) {
const base::Time sunrise_time = BuildTime("14:00:00");
const base::Time sunset_time = BuildTime("12:00:00");
AdvanceClockTo("14:00:00");
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, &clock_),
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kSunrise,
SunsetToSunriseCheckpoint::kMorning,
base::Hours(7) + base::Minutes(20)));

AdvanceClockTo("21:20:00");
EXPECT_THAT(
GetCurrentPosition(sunrise_time, sunset_time, &clock_),
GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kMorning,
SunsetToSunriseCheckpoint::kLateAfternoon, base::Hours(11)));

AdvanceClockTo("08:20:00");
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, &clock_),
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kLateAfternoon,
SunsetToSunriseCheckpoint::kSunset,
base::Hours(3) + base::Minutes(40)));

AdvanceClockTo("12:00:00");
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, &clock_),
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kSunset,
SunsetToSunriseCheckpoint::kSunrise, base::Hours(2)));
}
Expand All @@ -171,20 +171,20 @@ TEST_F(ScheduleUtilsTest, ShiftsScheduleToMatchDate) {
const base::Time sunset_time = BuildTime("18:00:00");
const auto test_all_checkpoints = [this, sunrise_time, sunset_time]() {
AdvanceClockTo("08:00:00");
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, &clock_),
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kSunrise,
SunsetToSunriseCheckpoint::kMorning, base::Hours(2)));
AdvanceClockTo("12:00:00");
EXPECT_THAT(
GetCurrentPosition(sunrise_time, sunset_time, &clock_),
GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kMorning,
SunsetToSunriseCheckpoint::kLateAfternoon, base::Hours(4)));
AdvanceClockTo("17:00:00");
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, &clock_),
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kLateAfternoon,
SunsetToSunriseCheckpoint::kSunset, base::Hours(1)));
AdvanceClockTo("00:00:00");
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, &clock_),
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kSunset,
SunsetToSunriseCheckpoint::kSunrise, base::Hours(6)));
};
Expand All @@ -203,21 +203,21 @@ TEST_F(ScheduleUtilsTest, MinimalSpaceFromSunriseToSunset) {
base::Time sunset_time = sunrise_time + base::Microseconds(6);
AdvanceClockTo("00:00:00");
EXPECT_THAT(
GetCurrentPosition(sunrise_time, sunset_time, &clock_),
GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kSunrise,
SunsetToSunriseCheckpoint::kMorning, base::Microseconds(2)));
clock_.Advance(base::Microseconds(2));
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, &clock_),
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kMorning,
SunsetToSunriseCheckpoint::kLateAfternoon,
base::Microseconds(3)));
clock_.Advance(base::Microseconds(3));
EXPECT_THAT(
GetCurrentPosition(sunrise_time, sunset_time, &clock_),
GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kLateAfternoon,
SunsetToSunriseCheckpoint::kSunset, base::Microseconds(1)));
clock_.Advance(base::Microseconds(1));
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, &clock_),
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kSunset, _, _));
}

Expand All @@ -229,13 +229,14 @@ TEST_F(ScheduleUtilsTest, SunriseExactlyEqualsSunset) {
const base::Time sunset_time = sunrise_time;
AdvanceClockTo("00:00:00");
Position current_position =
GetCurrentPosition(sunrise_time, sunset_time, &clock_);
GetCurrentPosition(sunrise_time, sunset_time, clock_.Now());
EXPECT_EQ(current_position.current_checkpoint,
current_position.next_checkpoint);
EXPECT_EQ(current_position.time_until_next_checkpoint, base::Days(1));

AdvanceClockTo("12:00:00");
current_position = GetCurrentPosition(sunrise_time, sunset_time, &clock_);
current_position =
GetCurrentPosition(sunrise_time, sunset_time, clock_.Now());
EXPECT_EQ(current_position.current_checkpoint,
current_position.next_checkpoint);
EXPECT_EQ(current_position.time_until_next_checkpoint, base::Hours(12));
Expand All @@ -250,15 +251,49 @@ TEST_F(ScheduleUtilsTest, NoSpaceForMorningAndLateAfternoon) {
const base::Time sunset_time = sunrise_time + base::Microseconds(1);
AdvanceClockTo("00:00:00");
EXPECT_THAT(
GetCurrentPosition(sunrise_time, sunset_time, &clock_),
GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kSunrise,
SunsetToSunriseCheckpoint::kSunset, base::Microseconds(1)));
clock_.Advance(base::Microseconds(1));
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, &clock_),
EXPECT_THAT(GetCurrentPosition(sunrise_time, sunset_time, clock_.Now()),
FieldsAre(SunsetToSunriseCheckpoint::kSunset,
SunsetToSunriseCheckpoint::kSunrise,
base::Days(1) - base::Microseconds(1)));
}

TEST_F(ScheduleUtilsTest, GetTimeUntilNextEvent) {
const base::Time origin = clock_.Now();
// Event time is same as now.
EXPECT_EQ(ShiftWithinOneDayFrom(origin, origin), origin);

// Event time is ahead of now.
EXPECT_EQ(ShiftWithinOneDayFrom(origin, origin + base::Hours(6)),
origin + base::Hours(6));
EXPECT_EQ(ShiftWithinOneDayFrom(origin, origin + base::Hours(18)),
origin + base::Hours(18));
EXPECT_EQ(ShiftWithinOneDayFrom(origin, origin + base::Days(1)), origin);
EXPECT_EQ(
ShiftWithinOneDayFrom(origin, origin + base::Days(1) + base::Hours(6)),
origin + base::Hours(6));
EXPECT_EQ(
ShiftWithinOneDayFrom(origin, origin + base::Days(1) + base::Hours(18)),
origin + base::Hours(18));
EXPECT_EQ(ShiftWithinOneDayFrom(origin, origin + base::Days(2)), origin);

// Event time is behind now.
EXPECT_EQ(ShiftWithinOneDayFrom(origin, origin - base::Hours(6)),
origin + base::Hours(18));
EXPECT_EQ(ShiftWithinOneDayFrom(origin, origin - base::Hours(18)),
origin + base::Hours(6));
EXPECT_EQ(ShiftWithinOneDayFrom(origin, origin - base::Days(1)), origin);
EXPECT_EQ(
ShiftWithinOneDayFrom(origin, origin - base::Days(1) - base::Hours(6)),
origin + base::Hours(18));
EXPECT_EQ(
ShiftWithinOneDayFrom(origin, origin - base::Days(1) - base::Hours(18)),
origin + base::Hours(6));
EXPECT_EQ(ShiftWithinOneDayFrom(origin, origin - base::Days(2)), origin);
}

} // namespace
} // namespace ash::schedule_utils

0 comments on commit 6147161

Please sign in to comment.