Observe Time or Timezone changes in PerAppTimeLimit.

This CL observes time or timezone changes in per app
time limits. If the time or timezone changes result in
crossing 24 hours + the last saved reset time, we reset the
running active times.

If the time or timezone changes result in |base::Time::Now()|
being before the last reset time, we reset the running active
times.

Bug: 1034551
Change-Id: Iac0d3fcd1c41e1ad33f36a5997a958b31584d03e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2008280Reviewed-by: Aga Wronska <agawronska@chromium.org>
Commit-Queue: Yilkal Abe <yilkal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737101}

chrome/browser/chromeos/child_accounts/time_limits/app_time_controller.cc

@@ -25,11 +25,29 @@
 namespace chromeos {
 namespace app_time {
 
+namespace {
+
+constexpr base::TimeDelta kDay = base::TimeDelta::FromHours(24);
+
+}  // namespace
+
 AppTimeController::TestApi::TestApi(AppTimeController* controller)
     : controller_(controller) {}
 
 AppTimeController::TestApi::~TestApi() = default;
 
+void AppTimeController::TestApi::SetLastResetTime(base::Time time) {
+  controller_->SetLastResetTime(time);
+}
+
+base::Time AppTimeController::TestApi::GetNextResetTime() const {
+  return controller_->GetNextResetTime();
+}
+
+base::Time AppTimeController::TestApi::GetLastResetTime() const {
+  return controller_->last_limits_reset_time_.value();
+}
+
 AppActivityRegistry* AppTimeController::TestApi::app_registry() {
   return controller_->app_registry_.get();
 }
@@ -67,10 +85,32 @@ AppTimeController::AppTimeController(Profile* profile)
 
   // TODO: Update the following from user pref.instead of setting it to Now().
   SetLastResetTime(base::Time::Now());
-  ScheduleForTimeLimitReset();
+
+  if (HasTimeCrossedResetBoundary()) {
+    OnResetTimeReached();
+  } else {
+    ScheduleForTimeLimitReset();
+  }
+
+  auto* system_clock_client = SystemClockClient::Get();
+  // SystemClockClient may not be initialized in some tests.
+  if (system_clock_client)
+    system_clock_client->AddObserver(this);
+
+  auto* time_zone_settings = system::TimezoneSettings::GetInstance();
+  if (time_zone_settings)
+    time_zone_settings->AddObserver(this);
 }
 
-AppTimeController::~AppTimeController() = default;
+AppTimeController::~AppTimeController() {
+  auto* time_zone_settings = system::TimezoneSettings::GetInstance();
+  if (time_zone_settings)
+    time_zone_settings->RemoveObserver(this);
+
+  auto* system_clock_client = SystemClockClient::Get();
+  if (system_clock_client)
+    system_clock_client->RemoveObserver(this);
+}
 
 bool AppTimeController::IsExtensionWhitelisted(
     const std::string& extension_id) const {
@@ -79,6 +119,17 @@ bool AppTimeController::IsExtensionWhitelisted(
   return true;
 }
 
+void AppTimeController::SystemClockUpdated() {
+  if (HasTimeCrossedResetBoundary())
+    OnResetTimeReached();
+}
+
+void AppTimeController::TimezoneChanged(const icu::TimeZone& timezone) {
+  // Timezone changes may not require us to reset information,
+  // however, they may require updating the scheduled reset time.
+  ScheduleForTimeLimitReset();
+}
+
 void AppTimeController::RegisterProfilePrefObservers(
     PrefService* pref_service) {
   pref_registrar_ = std::make_unique<PrefChangeRegistrar>();
@@ -183,5 +234,18 @@ void AppTimeController::SetLastResetTime(base::Time timestamp) {
   // across sessions.
 }
 
+bool AppTimeController::HasTimeCrossedResetBoundary() const {
+  // last_limits_reset_time_ doesn't have a value yet. This may be because
+  // it has not yet been populated yet (it has not been restored yet).
+  if (!last_limits_reset_time_.has_value())
+    return false;
+
+  // Time after system time or timezone changed.
+  base::Time now = base::Time::Now();
+  base::Time last_reset = last_limits_reset_time_.value();
+
+  return now < last_reset || now >= kDay + last_reset;
+}
+
 }  // namespace app_time
 }  // namespace chromeos

chrome/browser/chromeos/child_accounts/time_limits/app_time_controller.h

@@ -12,6 +12,8 @@
 #include "base/time/time.h"
 #include "base/timer/timer.h"
 #include "chrome/browser/chromeos/child_accounts/time_limits/app_time_notification_delegate.h"
+#include "chromeos/dbus/system_clock/system_clock_client.h"
+#include "chromeos/settings/timezone_settings.h"
 
 class Profile;
 class PrefRegistrySimple;
@@ -31,7 +33,9 @@ class AppServiceWrapper;
 class WebTimeLimitEnforcer;
 
 // Coordinates per-app time limit for child user.
-class AppTimeController : public AppTimeNotificationDelegate {
+class AppTimeController : public SystemClockClient::Observer,
+                          public system::TimezoneSettings::Observer,
+                          public AppTimeNotificationDelegate {
  public:
   // Used for tests to get internal implementation details.
   class TestApi {
@@ -39,6 +43,10 @@ class AppTimeController : public AppTimeNotificationDelegate {
     explicit TestApi(AppTimeController* controller);
     ~TestApi();
 
+    void SetLastResetTime(base::Time time);
+    base::Time GetNextResetTime() const;
+    base::Time GetLastResetTime() const;
+
     AppActivityRegistry* app_registry();
 
    private:
@@ -58,14 +66,20 @@ class AppTimeController : public AppTimeNotificationDelegate {
 
   bool IsExtensionWhitelisted(const std::string& extension_id) const;
 
-  const WebTimeLimitEnforcer* web_time_enforcer() const {
-    return web_time_enforcer_.get();
-  }
+  // SystemClockClient::Observer:
+  void SystemClockUpdated() override;
+
+  // system::TimezoneSetting::Observer:
+  void TimezoneChanged(const icu::TimeZone& timezone) override;
 
   // AppTimeNotificationDelegate:
   void ShowAppTimeLimitNotification(const AppId& app_id,
                                     AppNotification notification) override;
 
+  const WebTimeLimitEnforcer* web_time_enforcer() const {
+    return web_time_enforcer_.get();
+  }
+
   WebTimeLimitEnforcer* web_time_enforcer() { return web_time_enforcer_.get(); }
 
   const AppActivityRegistry* app_registry() const {
@@ -85,8 +99,11 @@ class AppTimeController : public AppTimeNotificationDelegate {
   void OnResetTimeReached();
   void SetLastResetTime(base::Time timestamp);
 
+  // Called when the system time or timezone may have changed.
+  bool HasTimeCrossedResetBoundary() const;
+
   // The time of the day when app time limits should be reset.
-  // Defaults to 6am.
+  // Defaults to 6am local time.
   base::TimeDelta limits_reset_time_ = base::TimeDelta::FromHours(6);
 
   // The last time when |reset_timer_| fired.

chrome/browser/chromeos/child_accounts/time_limits/app_time_controller_unittest.cc

@@ -4,13 +4,34 @@
 
 #include "chrome/browser/chromeos/child_accounts/time_limits/app_time_controller.h"
 
+#include "base/strings/utf_string_conversions.h"
 #include "base/test/scoped_feature_list.h"
+#include "base/test/task_environment.h"
+#include "base/time/time.h"
+#include "chrome/browser/chromeos/child_accounts/time_limits/app_activity_registry.h"
+#include "chrome/browser/chromeos/child_accounts/time_limits/app_types.h"
 #include "chrome/common/chrome_features.h"
+#include "chrome/test/base/testing_profile.h"
+#include "chromeos/dbus/system_clock/system_clock_client.h"
+#include "chromeos/settings/timezone_settings.h"
+#include "content/public/test/browser_task_environment.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace chromeos {
 namespace app_time {
 
+namespace {
+
+constexpr char kStartTime[] = "1 Jan 2020 00:00:00 GMT";
+constexpr base::TimeDelta kDay = base::TimeDelta::FromHours(24);
+constexpr base::TimeDelta kSixHours = base::TimeDelta::FromHours(6);
+constexpr base::TimeDelta kOneHour = base::TimeDelta::FromHours(1);
+constexpr base::TimeDelta kZeroTime = base::TimeDelta::FromSeconds(0);
+const chromeos::app_time::AppId kApp1(apps::mojom::AppType::kArc, "1");
+const chromeos::app_time::AppId kApp2(apps::mojom::AppType::kArc, "2");
+
+}  // namespace
+
 class AppTimeControllerTest : public testing::Test {
  protected:
   AppTimeControllerTest() = default;
@@ -18,18 +39,180 @@ class AppTimeControllerTest : public testing::Test {
   AppTimeControllerTest& operator=(const AppTimeControllerTest&) = delete;
   ~AppTimeControllerTest() override = default;
 
-  void EnablePerAppTimeLimits() {
-    scoped_feature_list_.InitAndEnableFeature(features::kPerAppTimeLimits);
+  void SetUp() override;
+  void TearDown() override;
+
+  void EnablePerAppTimeLimits();
+
+  void CreateActivityForApp(const AppId& app_id,
+                            base::TimeDelta active_time,
+                            base::TimeDelta time_limit);
+
+  AppTimeController::TestApi* test_api() { return test_api_.get(); }
+  AppTimeController* controller() { return controller_.get(); }
+  content::BrowserTaskEnvironment& task_environment() {
+    return task_environment_;
+  }
+  SystemClockClient::TestInterface* system_clock_client_test() {
+    return SystemClockClient::Get()->GetTestInterface();
   }
 
  private:
+  content::BrowserTaskEnvironment task_environment_{
+      base::test::TaskEnvironment::TimeSource::MOCK_TIME};
+  TestingProfile profile_;
+  std::unique_ptr<AppTimeController> controller_;
+  std::unique_ptr<AppTimeController::TestApi> test_api_;
   base::test::ScopedFeatureList scoped_feature_list_;
 };
 
+void AppTimeControllerTest::SetUp() {
+  SystemClockClient::InitializeFake();
+  testing::Test::SetUp();
+
+  // The tests are going to start at local midnight on January 1.
+  base::Time time;
+  ASSERT_TRUE(base::Time::FromString(kStartTime, &time));
+  base::Time local_midnight = time.LocalMidnight();
+  base::TimeDelta forward_by = local_midnight - base::Time::Now();
+  task_environment_.FastForwardBy(forward_by);
+
+  controller_ = std::make_unique<AppTimeController>(&profile_);
+  test_api_ = std::make_unique<AppTimeController::TestApi>(controller_.get());
+}
+
+void AppTimeControllerTest::TearDown() {
+  test_api_.reset();
+  controller_.reset();
+  SystemClockClient::Shutdown();
+  testing::Test::TearDown();
+}
+
+void AppTimeControllerTest::EnablePerAppTimeLimits() {
+  scoped_feature_list_.InitAndEnableFeature(features::kPerAppTimeLimits);
+}
+
+void AppTimeControllerTest::CreateActivityForApp(const AppId& app_id,
+                                                 base::TimeDelta time_active,
+                                                 base::TimeDelta time_limit) {
+  AppActivityRegistry* registry = controller_->app_registry();
+  registry->OnAppInstalled(app_id);
+  registry->OnAppAvailable(app_id);
+  registry->SetAppTimeLimitForTest(app_id, time_limit, base::Time::Now());
+
+  // AppActivityRegistry uses |window| to uniquely identify between different
+  // instances of the same active application. Since this test is just trying to
+  // mock one instance of an application, using nullptr is good enough.
+  registry->OnAppActive(app_id, /* window */ nullptr, base::Time::Now());
+  task_environment_.FastForwardBy(time_active);
+  if (time_active < time_limit) {
+    registry->OnAppInactive(app_id, /* window */ nullptr, base::Time::Now());
+  }
+}
+
 TEST_F(AppTimeControllerTest, EnableFeature) {
   EnablePerAppTimeLimits();
   EXPECT_TRUE(AppTimeController::ArePerAppTimeLimitsEnabled());
 }
 
+TEST_F(AppTimeControllerTest, GetNextResetTime) {
+  base::Time start_time = base::Time::Now();
+
+  base::Time next_reset_time = test_api()->GetNextResetTime();
+  base::Time local_midnight = next_reset_time.LocalMidnight();
+  EXPECT_EQ(kSixHours, next_reset_time - local_midnight);
+
+  EXPECT_TRUE(next_reset_time >= start_time);
+  EXPECT_TRUE(next_reset_time <= start_time + kDay);
+}
+
+TEST_F(AppTimeControllerTest, ResetTimeReached) {
+  base::Time start_time = base::Time::Now();
+
+  // Assert that we start at midnight.
+  ASSERT_EQ(start_time, start_time.LocalMidnight());
+
+  // This App will not reach its time limit. Advances time by 1 hour.
+  CreateActivityForApp(kApp1, kOneHour, kOneHour * 2);
+
+  // This app will reach its time limit. Advances time by 1 hour.
+  CreateActivityForApp(kApp2, kOneHour, kOneHour / 2);
+
+  EXPECT_EQ(controller()->app_registry()->GetActiveTime(kApp1), kOneHour);
+  EXPECT_EQ(controller()->app_registry()->GetActiveTime(kApp2), kOneHour / 2);
+  EXPECT_EQ(controller()->app_registry()->GetAppState(kApp1),
+            AppState::kAvailable);
+  EXPECT_EQ(controller()->app_registry()->GetAppState(kApp2),
+            AppState::kLimitReached);
+
+  // The default reset time is 6 hours after local midnight. Fast forward by 4
+  // hours to reach it. FastForwardBy triggers the reset timer.
+  task_environment().FastForwardBy(base::TimeDelta::FromHours(4));
+
+  // Make sure that there is no activity
+  EXPECT_EQ(controller()->app_registry()->GetActiveTime(kApp1), kZeroTime);
+  EXPECT_EQ(controller()->app_registry()->GetActiveTime(kApp2), kZeroTime);
+  EXPECT_EQ(controller()->app_registry()->GetAppState(kApp1),
+            AppState::kAvailable);
+  EXPECT_EQ(controller()->app_registry()->GetAppState(kApp2),
+            AppState::kAvailable);
+}
+
+TEST_F(AppTimeControllerTest, SystemTimeChangedFastForwardByTwoDays) {
+  CreateActivityForApp(kApp1, kOneHour, kOneHour * 2);
+  CreateActivityForApp(kApp2, kOneHour, kOneHour / 2);
+
+  // Advance system time with two days. TaskEnvironment::AdvanceClock doesn't
+  // run the tasks that have been posted. This allows us to simulate the system
+  // time changing to two days ahead without triggering the reset timer.
+  task_environment().AdvanceClock(2 * kDay);
+
+  // Since the reset timer has not been triggered the application activities are
+  // instact.
+  EXPECT_EQ(controller()->app_registry()->GetActiveTime(kApp1), kOneHour);
+  EXPECT_EQ(controller()->app_registry()->GetActiveTime(kApp2), kOneHour / 2);
+  EXPECT_EQ(controller()->app_registry()->GetAppState(kApp1),
+            AppState::kAvailable);
+  EXPECT_EQ(controller()->app_registry()->GetAppState(kApp2),
+            AppState::kLimitReached);
+
+  // Notify AppTimeController that system time has changed. This triggers reset.
+  system_clock_client_test()->NotifyObserversSystemClockUpdated();
+
+  // Make sure that there is no activity
+  EXPECT_EQ(controller()->app_registry()->GetActiveTime(kApp1), kZeroTime);
+  EXPECT_EQ(controller()->app_registry()->GetActiveTime(kApp2), kZeroTime);
+  EXPECT_EQ(controller()->app_registry()->GetAppState(kApp1),
+            AppState::kAvailable);
+  EXPECT_EQ(controller()->app_registry()->GetAppState(kApp2),
+            AppState::kAvailable);
+}
+
+TEST_F(AppTimeControllerTest, SystemTimeChangedGoingBackwards) {
+  CreateActivityForApp(kApp1, kOneHour, kOneHour * 2);
+  CreateActivityForApp(kApp2, kOneHour, kOneHour / 2);
+
+  EXPECT_EQ(controller()->app_registry()->GetActiveTime(kApp1), kOneHour);
+  EXPECT_EQ(controller()->app_registry()->GetActiveTime(kApp2), kOneHour / 2);
+  EXPECT_EQ(controller()->app_registry()->GetAppState(kApp1),
+            AppState::kAvailable);
+  EXPECT_EQ(controller()->app_registry()->GetAppState(kApp2),
+            AppState::kLimitReached);
+
+  // Simulate time has gone back by setting the last reset time to be in the
+  // future.
+  base::Time last_reset_time = test_api()->GetLastResetTime();
+  test_api()->SetLastResetTime(last_reset_time + 2 * kDay);
+  system_clock_client_test()->NotifyObserversSystemClockUpdated();
+
+  // Make sure that there is no activity
+  EXPECT_EQ(controller()->app_registry()->GetActiveTime(kApp1), kZeroTime);
+  EXPECT_EQ(controller()->app_registry()->GetActiveTime(kApp2), kZeroTime);
+  EXPECT_EQ(controller()->app_registry()->GetAppState(kApp1),
+            AppState::kAvailable);
+  EXPECT_EQ(controller()->app_registry()->GetAppState(kApp2),
+            AppState::kAvailable);
+}
+
 }  // namespace app_time
 }  // namespace chromeos