Save last reset time in AppTimeController.

This cl saves the last reset time in AppTimeController in
user pref. The value is saved every time the
|AppTimeController:: reset_timer_| fires; the timer fires once
every 24 hours.

Last reset time value is read from the PrefService when
AppTimeController is constructed. It is used to check if the reset time
has been crossed while the device was off.

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

chrome/browser/chromeos/BUILD.gn

@@ -2732,6 +2732,8 @@ source_set("unit_tests") {
     "child_accounts/time_limits/app_activity_registry_unittest.cc",
     "child_accounts/time_limits/app_service_wrapper_unittest.cc",
     "child_accounts/time_limits/app_time_controller_unittest.cc",
+    "child_accounts/time_limits/app_time_limits_policy_builder.cc",
+    "child_accounts/time_limits/app_time_limits_policy_builder.h",
     "child_accounts/time_limits/app_time_limits_whitelist_policy_test_utils.cc",
     "child_accounts/time_limits/app_time_limits_whitelist_policy_test_utils.h",
     "child_accounts/time_limits/app_time_test_utils.cc",

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

@@ -291,6 +291,23 @@ AppState AppActivityRegistry::GetAppState(const AppId& app_id) const {
   return activity_registry_.at(app_id).activity.app_state();
 }
 
+std::vector<PauseAppInfo> AppActivityRegistry::GetPausedApps(
+    bool show_pause_dialog) const {
+  std::vector<PauseAppInfo> paused_apps;
+  for (const auto& info : activity_registry_) {
+    const AppId& app_id = info.first;
+    const AppDetails& details = info.second;
+    if (GetAppState(app_id) == AppState::kLimitReached) {
+      DCHECK(details.limit.has_value());
+      DCHECK(details.limit->daily_limit().has_value());
+      paused_apps.push_back(PauseAppInfo(
+          app_id, details.limit->daily_limit().value(), show_pause_dialog));
+    }
+  }
+
+  return paused_apps;
+}
+
 AppActivityReportInterface::ReportParams
 AppActivityRegistry::GenerateAppActivityReport(
     enterprise_management::ChildStatusReportRequest* report) const {

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

@@ -118,6 +118,9 @@ class AppActivityRegistry : public AppServiceWrapper::EventListener {
 
   AppState GetAppState(const AppId& app_id) const;
 
+  // Returns the vector of paused applications.
+  std::vector<PauseAppInfo> GetPausedApps(bool show_pause_dialog) const;
+
   // Populates |report| with collected app activity. Returns whether any data
   // were reported.
   AppActivityReportInterface::ReportParams GenerateAppActivityReport(

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

@@ -61,6 +61,14 @@ std::string AppServiceIdFromAppId(const AppId& app_id, Profile* profile) {
              : app_id.app_id();
 }
 
+apps::PauseData PauseAppInfoToPauseData(const PauseAppInfo& pause_info) {
+  apps::PauseData details;
+  details.should_show_pause_dialog = pause_info.show_pause_dialog;
+  details.hours = pause_info.daily_limit.InHours();
+  details.minutes = pause_info.daily_limit.InMinutes() % 60;
+  return details;
+}
+
 }  // namespace
 
 AppServiceWrapper::AppServiceWrapper(Profile* profile) : profile_(profile) {
@@ -70,17 +78,18 @@ AppServiceWrapper::AppServiceWrapper(Profile* profile) : profile_(profile) {
 
 AppServiceWrapper::~AppServiceWrapper() = default;
 
-void AppServiceWrapper::PauseApp(const AppId& app_id,
-                                 base::TimeDelta daily_limit,
-                                 bool show_dialog) {
-  apps::PauseData details;
-  details.should_show_pause_dialog = show_dialog;
-  details.hours = daily_limit.InHours();
-  details.minutes =
-      (daily_limit - base::TimeDelta::FromHours(details.hours)).InMinutes();
-
+void AppServiceWrapper::PauseApp(const PauseAppInfo& pause_app) {
   const std::map<std::string, apps::PauseData> apps{
-      {GetAppServiceId(app_id), std::move(details)}};
+      {GetAppServiceId(pause_app.app_id), PauseAppInfoToPauseData(pause_app)}};
+  GetAppProxy()->PauseApps(apps);
+}
+
+void AppServiceWrapper::PauseApps(
+    const std::vector<PauseAppInfo>& paused_apps) {
+  std::map<std::string, apps::PauseData> apps;
+  for (const auto& entry : paused_apps) {
+    apps[GetAppServiceId(entry.app_id)] = PauseAppInfoToPauseData(entry);
+  }
   GetAppProxy()->PauseApps(apps);
 }
 

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

@@ -30,6 +30,7 @@ namespace chromeos {
 namespace app_time {
 
 class AppId;
+struct PauseAppInfo;
 
 // Wrapper around AppService.
 // Provides abstraction layer for Per-App Time Limits (PATL). Takes care of
@@ -82,14 +83,14 @@ class AppServiceWrapper : public apps::AppRegistryCache::Observer,
   AppServiceWrapper& operator=(const AppServiceWrapper&) = delete;
   ~AppServiceWrapper() override;
 
-  // Pauses the app identified by |app_id|.
-  // Uses |daily_limit| to communicate applied time restriction to the user by
-  // showing the dialog. After this is called user will not be able to launch
-  // the app and the visual effect will be applied to the icon. |show_dialog|
-  // indicates whether the user should be notified with a dialog.
-  void PauseApp(const AppId& app_id,
-                base::TimeDelta daily_limit,
-                bool show_dialog);
+  // Pauses the app identified by |PauseAppInfo::app_id|.
+  // Uses |PauseAppInfo::daily_limit| to communicate applied time restriction to
+  // the user by showing the dialog. After this is called user will not be able
+  // to launch the app and the visual effect will be applied to the icon.
+  // |PauseAppInfo::show_pause_dialog| indicates whether the user should be
+  // notified with a dialog.
+  void PauseApp(const PauseAppInfo& pause_app);
+  void PauseApps(const std::vector<PauseAppInfo>& paused_apps);
 
   // Resets time restriction from the app identified with |app_id|. After this
   // is called user will be able to use the app again and the visual effect

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

@@ -197,7 +197,7 @@ base::Time AppTimeController::TestApi::GetNextResetTime() const {
 }
 
 base::Time AppTimeController::TestApi::GetLastResetTime() const {
-  return controller_->last_limits_reset_time_.value();
+  return controller_->last_limits_reset_time_;
 }
 
 AppActivityRegistry* AppTimeController::TestApi::app_registry() {
@@ -215,6 +215,7 @@ bool AppTimeController::IsAppActivityReportingEnabled() {
 
 // static
 void AppTimeController::RegisterProfilePrefs(PrefRegistrySimple* registry) {
+  registry->RegisterInt64Pref(prefs::kPerAppTimeLimitsLastResetTime, 0);
   registry->RegisterDictionaryPref(prefs::kPerAppTimeLimitsPolicy);
   registry->RegisterDictionaryPref(prefs::kPerAppTimeLimitsWhitelistPolicy);
 }
@@ -234,20 +235,16 @@ AppTimeController::AppTimeController(Profile* profile)
   if (WebTimeLimitEnforcer::IsEnabled())
     web_time_enforcer_ = std::make_unique<WebTimeLimitEnforcer>(this);
 
-  app_registry_->AddAppStateObserver(this);
-
   PrefService* pref_service = profile->GetPrefs();
   RegisterProfilePrefObservers(pref_service);
+  TimeLimitsWhitelistPolicyUpdated(prefs::kPerAppTimeLimitsWhitelistPolicy);
+  TimeLimitsPolicyUpdated(prefs::kPerAppTimeLimitsPolicy);
 
-  // TODO: Update the following from user pref.instead of setting it to Now().
-  SetLastResetTime(base::Time::Now());
-
-  if (HasTimeCrossedResetBoundary()) {
-    OnResetTimeReached();
-  } else {
-    ScheduleForTimeLimitReset();
-  }
+  // Restore the last reset time. If reset time has have been crossed, triggers
+  // AppActivityRegistry to clear up the running active times of applications.
+  RestoreLastResetTime();
 
+  // Start observing system clock client and time zone settings.
   auto* system_clock_client = SystemClockClient::Get();
   // SystemClockClient may not be initialized in some tests.
   if (system_clock_client)
@@ -257,8 +254,13 @@ AppTimeController::AppTimeController(Profile* profile)
   if (time_zone_settings)
     time_zone_settings->AddObserver(this);
 
-  // Update app limits and reset time from policy.
-  TimeLimitsPolicyUpdated(prefs::kPerAppTimeLimitsPolicy);
+  // At this point application states should have been restored. Get the paused
+  // applications and notify app service. Don't show dialog for the paused apps.
+  app_service_wrapper_->PauseApps(
+      app_registry_->GetPausedApps(/* show_pause_dialog */ false));
+
+  // Start observing |app_registry_|
+  app_registry_->AddAppStateObserver(this);
 }
 
 AppTimeController::~AppTimeController() {
@@ -365,7 +367,7 @@ void AppTimeController::ShowAppTimeLimitNotification(
 void AppTimeController::OnAppLimitReached(const AppId& app_id,
                                           base::TimeDelta time_limit,
                                           bool was_active) {
-  app_service_wrapper_->PauseApp(app_id, time_limit, was_active);
+  app_service_wrapper_->PauseApp(PauseAppInfo(app_id, time_limit, was_active));
 }
 
 void AppTimeController::OnAppLimitRemoved(const AppId& app_id) {
@@ -432,23 +434,40 @@ void AppTimeController::OnResetTimeReached() {
   ScheduleForTimeLimitReset();
 }
 
+void AppTimeController::RestoreLastResetTime() {
+  PrefService* pref_service = profile_->GetPrefs();
+  int64_t reset_time =
+      pref_service->GetInt64(prefs::kPerAppTimeLimitsLastResetTime);
+
+  if (reset_time == 0) {
+    SetLastResetTime(base::Time::Now());
+  } else {
+    last_limits_reset_time_ = base::Time::FromDeltaSinceWindowsEpoch(
+        base::TimeDelta::FromMicroseconds(reset_time));
+  }
+
+  if (HasTimeCrossedResetBoundary()) {
+    OnResetTimeReached();
+  } else {
+    ScheduleForTimeLimitReset();
+  }
+}
+
 void AppTimeController::SetLastResetTime(base::Time timestamp) {
   last_limits_reset_time_ = timestamp;
-  // TODO(crbug.com/1015658) : |last_limits_reset_time_| should be persisted
-  // across sessions.
+
+  PrefService* service = profile_->GetPrefs();
+  DCHECK(service);
+  service->SetInt64(prefs::kPerAppTimeLimitsLastResetTime,
+                    timestamp.ToDeltaSinceWindowsEpoch().InMicroseconds());
+  service->CommitPendingWrite();
 }
 
 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;
+  return now < last_limits_reset_time_ || now >= kDay + last_limits_reset_time_;
 }
 
 }  // namespace app_time

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

@@ -7,7 +7,6 @@
 
 #include <memory>
 
-#include "base/optional.h"
 #include "base/time/default_tick_clock.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
@@ -116,6 +115,8 @@ class AppTimeController : public SystemClockClient::Observer,
 
   void ScheduleForTimeLimitReset();
   void OnResetTimeReached();
+
+  void RestoreLastResetTime();
   void SetLastResetTime(base::Time timestamp);
 
   // Called when the system time or timezone may have changed.
@@ -129,7 +130,7 @@ class AppTimeController : public SystemClockClient::Observer,
   base::TimeDelta limits_reset_time_ = base::TimeDelta::FromHours(6);
 
   // The last time when |reset_timer_| fired.
-  base::Optional<base::Time> last_limits_reset_time_;
+  base::Time last_limits_reset_time_;
 
   // Timer scheduled for the next reset of app time limits.
   // Only set when |reset_time_| is

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

@@ -10,18 +10,23 @@
 #include "base/test/scoped_feature_list.h"
 #include "base/test/task_environment.h"
 #include "base/time/time.h"
+#include "base/values.h"
 #include "chrome/browser/apps/app_service/app_service_test.h"
 #include "chrome/browser/chromeos/child_accounts/time_limits/app_activity_registry.h"
+#include "chrome/browser/chromeos/child_accounts/time_limits/app_time_limits_policy_builder.h"
 #include "chrome/browser/chromeos/child_accounts/time_limits/app_time_test_utils.h"
 #include "chrome/browser/chromeos/child_accounts/time_limits/app_types.h"
 #include "chrome/browser/notifications/notification_display_service_tester.h"
 #include "chrome/browser/ui/app_list/arc/arc_app_test.h"
 #include "chrome/common/chrome_features.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chromeos/dbus/system_clock/system_clock_client.h"
 #include "chromeos/settings/timezone_settings.h"
 #include "components/arc/mojom/app.mojom.h"
 #include "components/arc/test/fake_app_instance.h"
+#include "components/prefs/pref_service.h"
+#include "components/prefs/scoped_user_pref_update.h"
 #include "content/public/test/browser_task_environment.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/message_center/public/cpp/notification.h"
@@ -65,6 +70,9 @@ class AppTimeControllerTest : public testing::Test {
   size_t GetNotificationsCount();
   void DismissNotifications();
 
+  void DeleteController();
+  void InstantiateController();
+
   AppTimeController::TestApi* test_api() { return test_api_.get(); }
   AppTimeController* controller() { return controller_.get(); }
 
@@ -82,6 +90,8 @@ class AppTimeControllerTest : public testing::Test {
 
   apps::AppServiceTest& app_service_test() { return app_service_test_; }
 
+  Profile& profile() { return profile_; }
+
  private:
   content::BrowserTaskEnvironment task_environment_{
       base::test::TaskEnvironment::TimeSource::MOCK_TIME};
@@ -112,8 +122,7 @@ void AppTimeControllerTest::SetUp() {
       arc::FakeAppInstance::IconResponseType::ICON_RESPONSE_SKIP);
   task_environment_.RunUntilIdle();
 
-  controller_ = std::make_unique<AppTimeController>(&profile_);
-  test_api_ = std::make_unique<AppTimeController::TestApi>(controller_.get());
+  InstantiateController();
   SimulateInstallArcApp(kApp1, kApp1Name);
   SimulateInstallArcApp(kApp2, kApp2Name);
 }
@@ -194,6 +203,16 @@ void AppTimeControllerTest::DismissNotifications() {
       NotificationHandler::Type::TRANSIENT, true /* by_user */);
 }
 
+void AppTimeControllerTest::DeleteController() {
+  controller_.reset();
+  test_api_.reset();
+}
+
+void AppTimeControllerTest::InstantiateController() {
+  controller_ = std::make_unique<AppTimeController>(&profile_);
+  test_api_ = std::make_unique<AppTimeController::TestApi>(controller_.get());
+}
+
 TEST_F(AppTimeControllerTest, EnableFeature) {
   EnablePerAppTimeLimits();
   EXPECT_TRUE(AppTimeController::ArePerAppTimeLimitsEnabled());
@@ -370,5 +389,77 @@ TEST_F(AppTimeControllerTest, TimeLimitUpdatedNotification) {
   DismissNotifications();
 }
 
+TEST_F(AppTimeControllerTest, RestoreLastResetTime) {
+  {
+    AppTimeLimitsPolicyBuilder builder;
+    builder.AddAppLimit(kApp1, AppLimit(AppRestriction::kTimeLimit,
+                                        kOneHour * 2, base::Time::Now()));
+    builder.AddAppLimit(kApp2, AppLimit(AppRestriction::kTimeLimit,
+                                        kOneHour / 2, base::Time::Now()));
+    builder.SetResetTime(6, 0);
+    DictionaryPrefUpdate update(profile().GetPrefs(),
+                                prefs::kPerAppTimeLimitsPolicy);
+    base::Value* value = update.Get();
+    *value = builder.value().Clone();
+  }
+
+  // If there was no valid last reset time stored in user pref,
+  // AppTimeController sets it to base::Time::Now().
+  base::Time last_reset_time = base::Time::Now();
+  EXPECT_EQ(test_api()->GetLastResetTime(), last_reset_time);
+
+  controller()->app_registry()->OnAppActive(kApp1, nullptr, last_reset_time);
+  controller()->app_registry()->OnAppActive(kApp2, nullptr, last_reset_time);
+  task_environment().FastForwardBy(kOneHour);
+
+  controller()->app_registry()->OnAppInactive(kApp1, nullptr,
+                                              base::Time::Now());
+  EXPECT_EQ(controller()->app_registry()->GetAppState(kApp1),
+            AppState::kAvailable);
+  EXPECT_EQ(controller()->app_registry()->GetAppState(kApp2),
+            AppState::kLimitReached);
+
+  AppActivityRegistry::TestApi(controller()->app_registry()).SaveAppActivity();
+
+  DeleteController();
+
+  // Don't change last reset time. Ensure that application state's are not
+  // cleared.
+  InstantiateController();
+
+  // Make sure that AppTimeController doesn't always take base::Time::Now() for
+  // its last reset time.
+  EXPECT_EQ(test_api()->GetLastResetTime(), last_reset_time);
+
+  EXPECT_EQ(controller()->app_registry()->GetAppState(kApp1),
+            AppState::kAvailable);
+  EXPECT_EQ(controller()->app_registry()->GetAppState(kApp2),
+            AppState::kLimitReached);
+  EXPECT_EQ(controller()->app_registry()->GetActiveTime(kApp1), kOneHour);
+  EXPECT_EQ(controller()->app_registry()->GetActiveTime(kApp2), kOneHour / 2);
+
+  DeleteController();
+
+  // Now let's update the last reset time so that it is 24 hours before
+  // |last_reset_time|.
+  last_reset_time = last_reset_time - kDay;
+  profile().GetPrefs()->SetInt64(
+      prefs::kPerAppTimeLimitsLastResetTime,
+      last_reset_time.ToDeltaSinceWindowsEpoch().InMicroseconds());
+
+  InstantiateController();
+
+  // AppTimeController will realize that the reset boundary has been crossed.
+  // Therefore, it will trigger reset and update the last reset time to now.
+  EXPECT_EQ(test_api()->GetLastResetTime(), base::Time::Now());
+
+  EXPECT_EQ(controller()->app_registry()->GetAppState(kApp1),
+            AppState::kAvailable);
+  EXPECT_EQ(controller()->app_registry()->GetAppState(kApp2),
+            AppState::kAvailable);
+  EXPECT_EQ(controller()->app_registry()->GetActiveTime(kApp1), kZeroTime);
+  EXPECT_EQ(controller()->app_registry()->GetActiveTime(kApp2), kZeroTime);
+}
+
 }  // namespace app_time
 }  // namespace chromeos

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

@@ -87,6 +87,11 @@ std::ostream& operator<<(std::ostream& out, const AppId& id) {
              << "]";
 }
 
+PauseAppInfo::PauseAppInfo(const AppId& app,
+                           base::TimeDelta limit,
+                           bool show_dialog)
+    : app_id(app), daily_limit(limit), show_pause_dialog(show_dialog) {}
+
 AppLimit::AppLimit(AppRestriction restriction,
                    base::Optional<base::TimeDelta> daily_limit,
                    base::Time last_updated)

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

@@ -100,6 +100,14 @@ class AppId {
   std::string app_id_;
 };
 
+struct PauseAppInfo {
+  PauseAppInfo(const AppId& app, base::TimeDelta limit, bool show_dialog);
+
+  AppId app_id;
+  base::TimeDelta daily_limit;
+  bool show_pause_dialog = true;
+};
+
 // Represents restriction that can be applied to an installed app.
 class AppLimit {
  public:

chrome/common/pref_names.cc

@@ -944,6 +944,10 @@ const char kParentAccessCodeConfig[] = "child_user.parent_access_code.config";
 const char kPerAppTimeLimitsAppActivities[] =
     "child_user.per_app_time_limits.app_activities";
 
+// Int64 to specify the last timestamp the AppActivityRegistry was reset.
+const char kPerAppTimeLimitsLastResetTime[] =
+    "child_user.per_app_time_limits.last_reset_time";
+
 // Dictionary pref containing the per-app time limits configuration for child
 // user. Controlled by PerAppTimeLimits policy.
 const char kPerAppTimeLimitsPolicy[] = "child_user.per_app_time_limits.policy";

chrome/common/pref_names.h

@@ -308,6 +308,7 @@ extern const char kNetworkFileSharesPreconfiguredShares[];
 extern const char kMostRecentlyUsedNetworkFileShareURL[];
 extern const char kParentAccessCodeConfig[];
 extern const char kPerAppTimeLimitsAppActivities[];
+extern const char kPerAppTimeLimitsLastResetTime[];
 extern const char kPerAppTimeLimitsPolicy[];
 extern const char kPerAppTimeLimitsWhitelistPolicy[];
 extern const char kDeviceWallpaperImageFilePath[];