Don't show notification at the beginning of each session.

Notifications should be shown only when app limit has changed. To
know if app limit has changed at the beginning of each session, check
if the new limit's update is after the latest seen before. If so, show
notification.

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

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

@@ -146,6 +146,7 @@ void AppActivityRegistry::RegisterProfilePrefs(PrefRegistrySimple* registry) {
   registry->RegisterListPref(prefs::kPerAppTimeLimitsAppActivities);
   registry->RegisterInt64Pref(prefs::kPerAppTimeLimitsLastSuccessfulReportTime,
                               0);
+  registry->RegisterInt64Pref(prefs::kPerAppTimeLimitsLatestLimitUpdateTime, 0);
 }
 
 AppActivityRegistry::AppActivityRegistry(
@@ -403,6 +404,7 @@ void AppActivityRegistry::OnSuccessfullyReported(base::Time timestamp) {
 
 void AppActivityRegistry::UpdateAppLimits(
     const std::map<AppId, AppLimit>& app_limits) {
+  base::Time latest_update = latest_app_limit_update_;
   for (auto& entry : activity_registry_) {
     const AppId& app_id = entry.first;
     base::Optional<AppLimit> new_limit;
@@ -421,7 +423,17 @@ void AppActivityRegistry::UpdateAppLimits(
     }
 
     SetAppLimit(app_id, new_limit);
+
+    if (new_limit && new_limit->last_updated() > latest_update)
+      latest_update = new_limit->last_updated();
   }
+
+  latest_app_limit_update_ = latest_update;
+
+  // Update the latest app limit update.
+  profile_->GetPrefs()->SetInt64(
+      prefs::kPerAppTimeLimitsLatestLimitUpdateTime,
+      latest_app_limit_update_.ToDeltaSinceWindowsEpoch().InMicroseconds());
 }
 
 void AppActivityRegistry::SetAppLimit(
@@ -841,6 +853,12 @@ void AppActivityRegistry::ShowLimitUpdatedNotificationIfNeeded(
   if (app_id.app_type() == apps::mojom::AppType::kWeb)
     return;
 
+  // Don't show notification if the time limit's update was older than the
+  // latest update.
+  if (new_limit && new_limit->last_updated() <= latest_app_limit_update_) {
+    return;
+  }
+
   const bool had_time_limit =
       old_limit && old_limit->restriction() == AppRestriction::kTimeLimit;
   const bool has_time_limit =
@@ -893,6 +911,19 @@ void AppActivityRegistry::WebTimeLimitReached(base::Time timestamp) {
 }
 
 void AppActivityRegistry::InitializeRegistryFromPref() {
+  PrefService* pref_service = profile_->GetPrefs();
+  DCHECK(pref_service);
+
+  int64_t last_limits_updates =
+      pref_service->GetInt64(prefs::kPerAppTimeLimitsLatestLimitUpdateTime);
+
+  latest_app_limit_update_ = base::Time::FromDeltaSinceWindowsEpoch(
+      base::TimeDelta::FromMicroseconds(last_limits_updates));
+
+  InitializeAppActivities();
+}
+
+void AppActivityRegistry::InitializeAppActivities() {
   PrefService* pref_service = profile_->GetPrefs();
   const base::Value* value =
       pref_service->GetList(prefs::kPerAppTimeLimitsAppActivities);

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

@@ -227,6 +227,7 @@ class AppActivityRegistry : public AppServiceWrapper::EventListener {
   // Installed applications, their AppStates and their running active times will
   // be restored.
   void InitializeRegistryFromPref();
+  void InitializeAppActivities();
 
   // Updates |AppActivity::active_times_| to include the current activity up to
   // |timestamp| then creates the most up to date instance of PersistedAppInfo.
@@ -256,6 +257,9 @@ class AppActivityRegistry : public AppServiceWrapper::EventListener {
 
   // Repeating timer to trigger saving app activity to pref service.
   base::RepeatingTimer save_data_to_pref_service_;
+
+  // This records the timestamp of the latest set app limit.
+  base::Time latest_app_limit_update_;
 };
 
 }  // namespace app_time

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

@@ -844,5 +844,66 @@ TEST_F(AppActivityRegistryTest, OverrideLimitReachedState) {
   registry().OnAppActive(kApp2, GetWindowForApp(kApp2), base::Time::Now());
 }
 
+TEST_F(AppActivityRegistryTest, AvoidReduntantNotifications) {
+  const base::TimeDelta delta = base::TimeDelta::FromMinutes(5);
+  AppLimit chrome_limit(AppRestriction::kTimeLimit,
+                        base::TimeDelta::FromMinutes(30), base::Time::Now());
+  AppLimit app1_limit(AppRestriction::kTimeLimit,
+                      base::TimeDelta::FromMinutes(5),
+                      base::Time::Now() + delta);
+  std::map<AppId, AppLimit> app_limits = {{GetChromeAppId(), chrome_limit},
+                                          {kApp1, app1_limit}};
+  EXPECT_CALL(notification_delegate_mock(),
+              ShowAppTimeLimitNotification(
+                  GetChromeAppId(), chrome_limit.daily_limit(),
+                  chromeos::app_time::AppNotification::kTimeLimitChanged))
+      .Times(1);
+
+  EXPECT_CALL(notification_delegate_mock(),
+              ShowAppTimeLimitNotification(
+                  kApp1, app1_limit.daily_limit(),
+                  chromeos::app_time::AppNotification::kTimeLimitChanged))
+      .Times(1);
+
+  registry().UpdateAppLimits(app_limits);
+  registry().SaveAppActivity();
+
+  // Reinitialized the registry. We don't expect redundant time limit updatese
+  // will result in notifications.
+  ReInitializeRegistry();
+
+  EXPECT_CALL(notification_delegate_mock(),
+              ShowAppTimeLimitNotification(
+                  GetChromeAppId(), testing::_,
+                  chromeos::app_time::AppNotification::kTimeLimitChanged))
+      .Times(0);
+  EXPECT_CALL(notification_delegate_mock(),
+              ShowAppTimeLimitNotification(
+                  kApp1, testing::_,
+                  chromeos::app_time::AppNotification::kTimeLimitChanged))
+      .Times(0);
+
+  registry().UpdateAppLimits(app_limits);
+
+  // Update the limit for Chrome.
+  AppLimit new_chrome_limit(AppRestriction::kTimeLimit,
+                            base::TimeDelta::FromMinutes(15),
+                            base::Time::Now() + 2 * delta);
+  app_limits.at(GetChromeAppId()) = new_chrome_limit;
+
+  // Expect that there will be a notification for Chrome but not for kApp1.
+  EXPECT_CALL(notification_delegate_mock(),
+              ShowAppTimeLimitNotification(
+                  GetChromeAppId(), new_chrome_limit.daily_limit(),
+                  chromeos::app_time::AppNotification::kTimeLimitChanged))
+      .Times(1);
+  EXPECT_CALL(notification_delegate_mock(),
+              ShowAppTimeLimitNotification(
+                  kApp1, testing::_,
+                  chromeos::app_time::AppNotification::kTimeLimitChanged))
+      .Times(0);
+  registry().UpdateAppLimits(app_limits);
+}
+
 }  // namespace app_time
 }  // namespace chromeos

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

@@ -369,8 +369,10 @@ TEST_F(AppTimeControllerTest, TimeLimitUpdatedNotification) {
   DismissNotifications();
 
   // Only update one time limit.
+  const base::TimeDelta delta = base::TimeDelta::FromMinutes(1);
   const AppLimit limit3(AppRestriction::kTimeLimit,
-                        base::TimeDelta::FromMinutes(10), base::Time::Now());
+                        base::TimeDelta::FromMinutes(10),
+                        base::Time::Now() + delta);
   registry->UpdateAppLimits({{kApp1, limit1}, {kApp2, limit3}});
   task_environment().RunUntilIdle();
   EXPECT_EQ(1u, GetNotificationsCount());

chrome/common/pref_names.cc

@@ -964,6 +964,10 @@ const char kPerAppTimeLimitsLastResetTime[] =
 const char kPerAppTimeLimitsLastSuccessfulReportTime[] =
     "child_user.per_app_time_limits.last_successful_report_time";
 
+// Int64 to specify the latest AppLimit update timestamp from.
+const char kPerAppTimeLimitsLatestLimitUpdateTime[] =
+    "child_user.per_app_time_limits.latest_limit_update_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

@@ -313,6 +313,7 @@ extern const char kParentAccessCodeConfig[];
 extern const char kPerAppTimeLimitsAppActivities[];
 extern const char kPerAppTimeLimitsLastResetTime[];
 extern const char kPerAppTimeLimitsLastSuccessfulReportTime[];
+extern const char kPerAppTimeLimitsLatestLimitUpdateTime[];
 extern const char kPerAppTimeLimitsPolicy[];
 extern const char kPerAppTimeLimitsWhitelistPolicy[];
 extern const char kDeviceWallpaperImageFilePath[];