Show notification when app time limits change

Child user will see the notification with the details of time
limit change in the following cases:
* App did not have the limit and new limit was set.
* App had time limit set and it was removed.
* The value of time limit changed.
There are no notifications displayed for web apps, as they
share the limit with Chrome and notification will be shown
for Chrome.

Bug: 1052011
Test: AppTimeControllerTest
Change-Id: I662773969b4668c73bc3d6a9d572f8a11d147ae7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2065411Reviewed-by: Yilkal Abe <yilkal@chromium.org>
Commit-Queue: Aga Wronska <agawronska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743117}

chrome/app/chromeos_strings.grdp

@@ -10,21 +10,21 @@
     The application appears to be invalid.
   </message>
 
-  <!-- Per App Time Limit -->
+  <!-- Per-App Time Limits -->
   <message name="IDS_APP_TIME_LIMIT_APP_WILL_PAUSE_SYSTEM_NOTIFICATION_TITLE" desc="PerAppTimeLimit system notification title specifying that the application will pause soon.">
     <ph name="APP_NAME">$1 <ex>App x</ex></ph> will pause soon
   </message>
-
   <message name="IDS_APP_TIME_LIMIT_APP_WILL_PAUSE_SYSTEM_NOTIFICATION_MESSAGE" desc="PerAppTimeLimit system notification message specifying that the application will pause in the specified time.">
     <ph name="TIME">$1<ex> 5 minutes</ex></ph> left
   </message>
-
   <message name="IDS_APP_TIME_LIMIT_APP_TIME_LIMIT_SET_SYSTEM_NOTIFICATION_TITLE" desc="PerAppTimeLimit system notification title specifying that the application's time limit has been set by a parent.">
     Update from your parent
   </message>
-
   <message name="IDS_APP_TIME_LIMIT_APP_TIME_LIMIT_SET_SYSTEM_NOTIFICATION_MESSAGE" desc="PerAppTimeLimit system notification message specifying that the application's time limit has been set by a parent.">
-    <ph name="TIME">$1<ex> 1 hour 30 minutes</ex></ph> time limit set for <ph name="APP_NAME">$2<ex>App x</ex></ph>
+    <ph name="TIME">$1<ex> 1 hour 30 minutes</ex></ph> time limit set for <ph name="APP_NAME">$2<ex>Gmail</ex></ph>
+  </message>
+  <message name="IDS_APP_TIME_LIMIT_APP_TIME_LIMIT_REMOVED_SYSTEM_NOTIFICATION_MESSAGE" desc="PerAppTimeLimit system notification message specifying that the application's time limit has been removed by parent.">
+    Time limit removed for <ph name="APP_NAME">$1<ex>Gmail</ex></ph>
   </message>
 
   <!-- Wallpaper Manager -->

chrome/app/chromeos_strings_grdp/IDS_APP_TIME_LIMIT_APP_TIME_LIMIT_REMOVED_SYSTEM_NOTIFICATION_MESSAGE.png.sha1

+eb7ed0a8fdc230fbaada61eb9208584c5f6a7a91
\ No newline at end of file

chrome/app/chromeos_strings_grdp/IDS_APP_TIME_LIMIT_APP_TIME_LIMIT_SET_SYSTEM_NOTIFICATION_MESSAGE.png.sha1

+7899bffee570c333ed0993621d54aea186da3396
\ No newline at end of file

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

@@ -359,6 +359,7 @@ void AppActivityRegistry::SetAppLimit(
   // Limit 'data' are considered equal if only the |last_updated_| is different.
   // Update the limit to store new |last_updated_| value.
   bool did_change = !details.IsLimitEqual(app_limit);
+  ShowLimitUpdatedNotificationIfNeeded(app_id, details.limit, app_limit);
   details.limit = app_limit;
 
   // Limit 'data' is the same - no action needed.
@@ -671,6 +672,35 @@ void AppActivityRegistry::CheckTimeLimitForApp(const AppId& app_id) {
   }
 }
 
+void AppActivityRegistry::ShowLimitUpdatedNotificationIfNeeded(
+    const AppId& app_id,
+    const base::Optional<AppLimit>& old_limit,
+    const base::Optional<AppLimit>& new_limit) {
+  // Web app limit changes are covered by Chrome notification.
+  if (app_id.app_type() == apps::mojom::AppType::kWeb)
+    return;
+
+  const bool had_time_limit =
+      old_limit && old_limit->restriction() == AppRestriction::kTimeLimit;
+  const bool has_time_limit =
+      new_limit && new_limit->restriction() == AppRestriction::kTimeLimit;
+
+  // Time limit was removed.
+  if (!has_time_limit && had_time_limit) {
+    notification_delegate_->ShowAppTimeLimitNotification(
+        app_id, base::nullopt, AppNotification::kTimeLimitChanged);
+    return;
+  }
+
+  // Time limit was set or value changed.
+  if (has_time_limit && (!had_time_limit || old_limit->daily_limit() !=
+                                                new_limit->daily_limit())) {
+    notification_delegate_->ShowAppTimeLimitNotification(
+        app_id, new_limit->daily_limit(), AppNotification::kTimeLimitChanged);
+    return;
+  }
+}
+
 base::TimeDelta AppActivityRegistry::GetWebActiveRunningTime() const {
   base::TimeDelta active_running_time = base::TimeDelta::FromSeconds(0);
   for (const auto& app_info : activity_registry_) {

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

@@ -193,6 +193,13 @@ class AppActivityRegistry : public AppServiceWrapper::EventListener {
   // Checks the limit and shows notification if needed.
   void CheckTimeLimitForApp(const AppId& app_id);
 
+  // Shows notification about time limit updates for the app if there were
+  // relevant changes between |old_limit| and |new_limit|.
+  void ShowLimitUpdatedNotificationIfNeeded(
+      const AppId& app_id,
+      const base::Optional<AppLimit>& old_limit,
+      const base::Optional<AppLimit>& new_limit);
+
   base::TimeDelta GetWebActiveRunningTime() const;
 
   void WebTimeLimitReached(base::Time timestamp);

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

@@ -8,6 +8,7 @@
 #include <memory>
 #include <vector>
 
+#include "base/optional.h"
 #include "base/test/task_environment.h"
 #include "base/time/time.h"
 #include "chrome/browser/chromeos/child_accounts/time_limits/app_service_wrapper.h"
@@ -44,7 +45,7 @@ class AppTimeNotificationDelegateMock : public AppTimeNotificationDelegate {
 
   MOCK_METHOD3(ShowAppTimeLimitNotification,
                void(const chromeos::app_time::AppId&,
-                    base::TimeDelta,
+                    const base::Optional<base::TimeDelta>&,
                     chromeos::app_time::AppNotification));
 };
 

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

@@ -69,16 +69,15 @@ base::string16 GetTimeLimitMessage(base::TimeDelta time_limit, int cutoff) {
                                   time_limit);
 }
 
-base::string16 GetNotificationTitleFor(
-    const base::string16& app_name,
-    chromeos::app_time::AppNotification notification) {
+base::string16 GetNotificationTitleFor(const base::string16& app_name,
+                                       AppNotification notification) {
   switch (notification) {
-    case chromeos::app_time::AppNotification::kFiveMinutes:
-    case chromeos::app_time::AppNotification::kOneMinute:
+    case AppNotification::kFiveMinutes:
+    case AppNotification::kOneMinute:
       return l10n_util::GetStringFUTF16(
           IDS_APP_TIME_LIMIT_APP_WILL_PAUSE_SYSTEM_NOTIFICATION_TITLE,
           app_name);
-    case chromeos::app_time::AppNotification::kTimeLimitChanged:
+    case AppNotification::kTimeLimitChanged:
       return l10n_util::GetStringUTF16(
           IDS_APP_TIME_LIMIT_APP_TIME_LIMIT_SET_SYSTEM_NOTIFICATION_TITLE);
     default:
@@ -89,37 +88,41 @@ base::string16 GetNotificationTitleFor(
 
 base::string16 GetNotificationMessageFor(
     const base::string16& app_name,
-    chromeos::app_time::AppNotification notification,
-    base::TimeDelta time_limit) {
+    AppNotification notification,
+    base::Optional<base::TimeDelta> time_limit) {
   switch (notification) {
-    case chromeos::app_time::AppNotification::kFiveMinutes:
+    case AppNotification::kFiveMinutes:
       return l10n_util::GetStringFUTF16(
           IDS_APP_TIME_LIMIT_APP_WILL_PAUSE_SYSTEM_NOTIFICATION_MESSAGE,
           GetTimeLimitMessage(base::TimeDelta::FromMinutes(5), /* cutoff */ 1));
-    case chromeos::app_time::AppNotification::kOneMinute:
+    case AppNotification::kOneMinute:
       return l10n_util::GetStringFUTF16(
           IDS_APP_TIME_LIMIT_APP_WILL_PAUSE_SYSTEM_NOTIFICATION_MESSAGE,
           GetTimeLimitMessage(base::TimeDelta::FromMinutes(1), /* cutoff */ 1));
-    case chromeos::app_time::AppNotification::kTimeLimitChanged:
-      return l10n_util::GetStringFUTF16(
-          IDS_APP_TIME_LIMIT_APP_TIME_LIMIT_SET_SYSTEM_NOTIFICATION_MESSAGE,
-          GetTimeLimitMessage(time_limit, /* cutoff */ 3), app_name);
+    case AppNotification::kTimeLimitChanged:
+      return time_limit
+                 ? l10n_util::GetStringFUTF16(
+                       IDS_APP_TIME_LIMIT_APP_TIME_LIMIT_SET_SYSTEM_NOTIFICATION_MESSAGE,
+                       GetTimeLimitMessage(*time_limit, /* cutoff */ 3),
+                       app_name)
+                 : l10n_util::GetStringFUTF16(
+                       IDS_APP_TIME_LIMIT_APP_TIME_LIMIT_REMOVED_SYSTEM_NOTIFICATION_MESSAGE,
+                       app_name);
     default:
       NOTREACHED();
       return base::EmptyString16();
   }
 }
 
-std::string GetNotificationIdFor(
-    const std::string& app_name,
-    chromeos::app_time::AppNotification notification) {
+std::string GetNotificationIdFor(const std::string& app_name,
+                                 AppNotification notification) {
   std::string notification_id;
   switch (notification) {
-    case chromeos::app_time::AppNotification::kFiveMinutes:
-    case chromeos::app_time::AppNotification::kOneMinute:
+    case AppNotification::kFiveMinutes:
+    case AppNotification::kOneMinute:
       notification_id = kAppTimeLimitReachingNotificationId;
       break;
-    case chromeos::app_time::AppNotification::kTimeLimitChanged:
+    case AppNotification::kTimeLimitChanged:
       notification_id = kAppTimeLimitUpdateNotificationId;
       break;
     default:
@@ -131,14 +134,15 @@ std::string GetNotificationIdFor(
 }
 
 void ShowNotificationForApp(const std::string& app_name,
-                            chromeos::app_time::AppNotification notification,
-                            base::TimeDelta time_limit,
+                            AppNotification notification,
+                            base::Optional<base::TimeDelta> time_limit,
                             Profile* profile,
                             base::Optional<gfx::ImageSkia> icon) {
-  DCHECK(notification == chromeos::app_time::AppNotification::kFiveMinutes ||
-         notification == chromeos::app_time::AppNotification::kOneMinute ||
-         notification ==
-             chromeos::app_time::AppNotification::kTimeLimitChanged);
+  DCHECK(notification == AppNotification::kFiveMinutes ||
+         notification == AppNotification::kOneMinute ||
+         notification == AppNotification::kTimeLimitChanged);
+  DCHECK(notification == AppNotification::kTimeLimitChanged ||
+         time_limit.has_value());
 
   // Alright we have all the messages that we want.
   const base::string16 app_name_16 = base::UTF8ToUTF16(app_name);
@@ -342,26 +346,19 @@ void AppTimeController::TimeLimitsWhitelistPolicyUpdated(
 
 void AppTimeController::ShowAppTimeLimitNotification(
     const AppId& app_id,
-    base::TimeDelta time_limit,
+    const base::Optional<base::TimeDelta>& time_limit,
     AppNotification notification) {
-  switch (notification) {
-    case AppNotification::kFiveMinutes:
-    case AppNotification::kOneMinute:
-    case AppNotification::kTimeLimitChanged: {
-      std::string app_name = app_service_wrapper_->GetAppName(app_id);
-      int size_hint_in_dp = 48;
-      app_service_wrapper_->GetAppIcon(
-          app_id, size_hint_in_dp,
-          base::BindOnce(&ShowNotificationForApp, app_name, notification,
-                         time_limit, profile_));
-      return;
-    }
-    case AppNotification::kTimeLimitReached:
-      // TODO(1015658)
-      return;
-    default:
-      return;
-  }
+  DCHECK_NE(AppNotification::kUnknown, notification);
+
+  if (notification == AppNotification::kTimeLimitReached)
+    return;
+
+  const std::string app_name = app_service_wrapper_->GetAppName(app_id);
+  int size_hint_in_dp = 48;
+  app_service_wrapper_->GetAppIcon(
+      app_id, size_hint_in_dp,
+      base::BindOnce(&ShowNotificationForApp, app_name, notification,
+                     time_limit, profile_));
 }
 
 void AppTimeController::OnAppLimitReached(const AppId& app_id,

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

@@ -75,9 +75,10 @@ class AppTimeController : public SystemClockClient::Observer,
   void TimezoneChanged(const icu::TimeZone& timezone) override;
 
   // AppTimeNotificationDelegate:
-  void ShowAppTimeLimitNotification(const AppId& app_id,
-                                    base::TimeDelta time_limit,
-                                    AppNotification notification) override;
+  void ShowAppTimeLimitNotification(
+      const AppId& app_id,
+      const base::Optional<base::TimeDelta>& time_limit,
+      AppNotification notification) override;
 
   // AppActivityRegistry::AppStateObserver:
   void OnAppLimitReached(const AppId& app_id,

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

@@ -38,8 +38,8 @@ constexpr base::TimeDelta kOneHour = base::TimeDelta::FromHours(1);
 constexpr base::TimeDelta kZeroTime = base::TimeDelta::FromSeconds(0);
 constexpr char kApp1Name[] = "App1";
 constexpr char kApp2Name[] = "App2";
-const chromeos::app_time::AppId kApp1(apps::mojom::AppType::kArc, "1");
-const chromeos::app_time::AppId kApp2(apps::mojom::AppType::kArc, "2");
+const AppId kApp1(apps::mojom::AppType::kArc, "1");
+const AppId kApp2(apps::mojom::AppType::kArc, "2");
 
 }  // namespace
 
@@ -60,9 +60,10 @@ class AppTimeControllerTest : public testing::Test {
                             base::TimeDelta time_limit);
 
   void SimulateInstallArcApp(const AppId& app_id, const std::string& app_name);
-  bool HasNotificationFor(
-      const std::string& app_name,
-      chromeos::app_time::AppNotification notification) const;
+  bool HasNotificationFor(const std::string& app_name,
+                          AppNotification notification) const;
+  size_t GetNotificationsCount();
+  void DismissNotifications();
 
   AppTimeController::TestApi* test_api() { return test_api_.get(); }
   AppTimeController* controller() { return controller_.get(); }
@@ -160,14 +161,14 @@ void AppTimeControllerTest::SimulateInstallArcApp(const AppId& app_id,
 
 bool AppTimeControllerTest::HasNotificationFor(
     const std::string& app_name,
-    chromeos::app_time::AppNotification notification) const {
+    AppNotification notification) const {
   std::string notification_id;
   switch (notification) {
-    case chromeos::app_time::AppNotification::kFiveMinutes:
-    case chromeos::app_time::AppNotification::kOneMinute:
+    case AppNotification::kFiveMinutes:
+    case AppNotification::kOneMinute:
       notification_id = "time-limit-reaching-id-";
       break;
-    case chromeos::app_time::AppNotification::kTimeLimitChanged:
+    case AppNotification::kTimeLimitChanged:
       notification_id = "time-limit-updated-id-";
       break;
     default:
@@ -182,6 +183,17 @@ bool AppTimeControllerTest::HasNotificationFor(
   return message_center_notification.has_value();
 }
 
+size_t AppTimeControllerTest::GetNotificationsCount() {
+  return notification_tester_
+      .GetDisplayedNotificationsForType(NotificationHandler::Type::TRANSIENT)
+      .size();
+}
+
+void AppTimeControllerTest::DismissNotifications() {
+  notification_tester_.RemoveAllNotifications(
+      NotificationHandler::Type::TRANSIENT, true /* by_user */);
+}
+
 TEST_F(AppTimeControllerTest, EnableFeature) {
   EnablePerAppTimeLimits();
   EXPECT_TRUE(AppTimeController::ArePerAppTimeLimitsEnabled());
@@ -317,5 +329,46 @@ TEST_F(AppTimeControllerTest, TimeLimitNotification) {
   EXPECT_TRUE(HasNotificationFor(kApp1Name, AppNotification::kOneMinute));
 }
 
+TEST_F(AppTimeControllerTest, TimeLimitUpdatedNotification) {
+  AppActivityRegistry* registry = controller()->app_registry();
+
+  // Set new time limits.
+  const AppLimit limit1(AppRestriction::kTimeLimit,
+                        base::TimeDelta::FromMinutes(35), base::Time::Now());
+  const AppLimit limit2(AppRestriction::kTimeLimit,
+                        base::TimeDelta::FromMinutes(30), base::Time::Now());
+  registry->UpdateAppLimits({{kApp1, limit1}, {kApp2, limit2}});
+  task_environment().RunUntilIdle();
+
+  // Expect time limit changed notification for both apps.
+  EXPECT_EQ(2u, GetNotificationsCount());
+  EXPECT_TRUE(
+      HasNotificationFor(kApp1Name, AppNotification::kTimeLimitChanged));
+  EXPECT_TRUE(
+      HasNotificationFor(kApp2Name, AppNotification::kTimeLimitChanged));
+
+  DismissNotifications();
+
+  // Only update one time limit.
+  const AppLimit limit3(AppRestriction::kTimeLimit,
+                        base::TimeDelta::FromMinutes(10), base::Time::Now());
+  registry->UpdateAppLimits({{kApp1, limit1}, {kApp2, limit3}});
+  task_environment().RunUntilIdle();
+  EXPECT_EQ(1u, GetNotificationsCount());
+  EXPECT_TRUE(
+      HasNotificationFor(kApp2Name, AppNotification::kTimeLimitChanged));
+
+  DismissNotifications();
+
+  // Remove one time limit.
+  registry->UpdateAppLimits({{kApp2, limit3}});
+  task_environment().RunUntilIdle();
+  EXPECT_EQ(1u, GetNotificationsCount());
+  EXPECT_TRUE(
+      HasNotificationFor(kApp1Name, AppNotification::kTimeLimitChanged));
+
+  DismissNotifications();
+}
+
 }  // namespace app_time
 }  // namespace chromeos

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

@@ -5,6 +5,8 @@
 #ifndef CHROME_BROWSER_CHROMEOS_CHILD_ACCOUNTS_TIME_LIMITS_APP_TIME_NOTIFICATION_DELEGATE_H_
 #define CHROME_BROWSER_CHROMEOS_CHILD_ACCOUNTS_TIME_LIMITS_APP_TIME_NOTIFICATION_DELEGATE_H_
 
+#include "base/optional.h"
+
 namespace base {
 class TimeDelta;
 }  // namespace base
@@ -25,9 +27,10 @@ class AppTimeNotificationDelegate {
 
   virtual ~AppTimeNotificationDelegate() = default;
 
-  virtual void ShowAppTimeLimitNotification(const AppId& app_id,
-                                            base::TimeDelta time_limit,
-                                            AppNotification notification) = 0;
+  virtual void ShowAppTimeLimitNotification(
+      const AppId& app_id,
+      const base::Optional<base::TimeDelta>& time_limit,
+      AppNotification notification) = 0;
 };
 
 }  // namespace app_time