Move time limit notifications into own class

Creates a TimeLimitNotifier class to handle notification details for
bedtime and screen time limits, and adds tests.

Later, this can be used to hide the notification when it no longer
applies, probably by listening to some event.

Bug: 898000
Change-Id: Ie7844d6d0639815eac967062ae83b308346485d8
Reviewed-on: https://chromium-review.googlesource.com/c/1295182
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: Aga Wronska <agawronska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603740}

chrome/browser/chromeos/BUILD.gn

@@ -578,6 +578,8 @@ source_set("chromeos") {
     "child_accounts/screen_time_controller.h",
     "child_accounts/screen_time_controller_factory.cc",
     "child_accounts/screen_time_controller_factory.h",
+    "child_accounts/time_limit_notifier.cc",
+    "child_accounts/time_limit_notifier.h",
     "child_accounts/usage_time_limit_processor.cc",
     "child_accounts/usage_time_limit_processor.h",
     "chrome_browser_main_chromeos.cc",
@@ -2093,6 +2095,7 @@ source_set("unit_tests") {
     "authpolicy/auth_policy_credentials_manager_unittest.cc",
     "base/file_flusher_unittest.cc",
     "certificate_provider/certificate_provider_service_unittest.cc",
+    "child_accounts/time_limit_notifier_unittest.cc",
     "child_accounts/usage_time_limit_processor_unittest.cc",
     "crostini/crostini_manager_unittest.cc",
     "crostini/crostini_share_path_unittest.cc",

chrome/browser/chromeos/child_accounts/screen_time_controller.cc

@@ -4,42 +4,24 @@
 
 #include "chrome/browser/chromeos/child_accounts/screen_time_controller.h"
 
-#include "ash/public/cpp/vector_icons/vector_icons.h"
 #include "base/optional.h"
 #include "chrome/browser/chromeos/child_accounts/consumer_status_reporting_service.h"
 #include "chrome/browser/chromeos/child_accounts/consumer_status_reporting_service_factory.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
-#include "chrome/browser/notifications/notification_display_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/ash/login_screen_client.h"
 #include "chrome/common/pref_names.h"
-#include "chrome/grit/generated_resources.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/session_manager_client.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 #include "components/session_manager/core/session_manager.h"
 #include "content/public/browser/browser_context.h"
-#include "ui/base/l10n/l10n_util.h"
-#include "ui/base/l10n/time_format.h"
-#include "ui/message_center/public/cpp/notification.h"
 
 namespace chromeos {
 
 namespace {
 
-constexpr base::TimeDelta kWarningNotificationTimeout =
-    base::TimeDelta::FromMinutes(5);
-constexpr base::TimeDelta kExitNotificationTimeout =
-    base::TimeDelta::FromMinutes(1);
-
-// The notification id. All the time limit notifications share the same id so
-// that a subsequent notification can replace the previous one.
-constexpr char kTimeLimitNotificationId[] = "time-limit-notification";
-
-// The notifier id representing the app.
-constexpr char kTimeLimitNotifierId[] = "family-link";
-
 // Dictionary keys for prefs::kScreenTimeLastState.
 constexpr char kScreenStateLocked[] = "locked";
 constexpr char kScreenStateCurrentPolicyType[] = "active_policy";
@@ -61,7 +43,8 @@ void ScreenTimeController::RegisterProfilePrefs(PrefRegistrySimple* registry) {
 
 ScreenTimeController::ScreenTimeController(content::BrowserContext* context)
     : context_(context),
-      pref_service_(Profile::FromBrowserContext(context)->GetPrefs()) {
+      pref_service_(Profile::FromBrowserContext(context)->GetPrefs()),
+      time_limit_notifier_(context) {
   session_manager::SessionManager::Get()->AddObserver(this);
   system::TimezoneSettings::GetInstance()->AddObserver(this);
   pref_change_registrar_.Init(pref_service_);
@@ -114,13 +97,13 @@ void ScreenTimeController::CheckTimeLimit(const std::string& source) {
       ForceScreenLockByPolicy(state.next_unlock_time);
     }
   } else {
-    base::Optional<TimeLimitNotificationType> notification_type;
+    base::Optional<TimeLimitNotifier::LimitType> notification_type;
     switch (state.next_state_active_policy) {
       case usage_time_limit::ActivePolicies::kFixedLimit:
-        notification_type = kBedTime;
+        notification_type = TimeLimitNotifier::LimitType::kBedTime;
         break;
       case usage_time_limit::ActivePolicies::kUsageLimit:
-        notification_type = kScreenTime;
+        notification_type = TimeLimitNotifier::LimitType::kScreenTime;
         break;
       case usage_time_limit::ActivePolicies::kNoActivePolicy:
       case usage_time_limit::ActivePolicies::kOverride:
@@ -131,21 +114,11 @@ void ScreenTimeController::CheckTimeLimit(const std::string& source) {
 
     if (notification_type.has_value()) {
       // Schedule notification based on the remaining screen time until lock.
+      // TODO(crbug.com/898000): Dismiss a shown notification when it no longer
+      // applies.
       const base::TimeDelta remaining_time = state.next_state_change_time - now;
-      if (remaining_time >= kWarningNotificationTimeout) {
-        warning_notification_timer_.Start(
-            FROM_HERE, remaining_time - kWarningNotificationTimeout,
-            base::BindRepeating(
-                &ScreenTimeController::ShowNotification, base::Unretained(this),
-                notification_type.value(), kWarningNotificationTimeout));
-      }
-      if (remaining_time >= kExitNotificationTimeout) {
-        exit_notification_timer_.Start(
-            FROM_HERE, remaining_time - kExitNotificationTimeout,
-            base::BindRepeating(
-                &ScreenTimeController::ShowNotification, base::Unretained(this),
-                notification_type.value(), kExitNotificationTimeout));
-      }
+      time_limit_notifier_.MaybeScheduleNotifications(notification_type.value(),
+                                                      remaining_time);
     }
   }
 
@@ -190,32 +163,6 @@ void ScreenTimeController::UpdateTimeLimitsMessage(
       visible ? next_unlock_time : base::Optional<base::Time>());
 }
 
-void ScreenTimeController::ShowNotification(
-    ScreenTimeController::TimeLimitNotificationType type,
-    const base::TimeDelta& time_remaining) {
-  const base::string16 title = l10n_util::GetStringUTF16(
-      type == kScreenTime ? IDS_SCREEN_TIME_NOTIFICATION_TITLE
-                          : IDS_BED_TIME_NOTIFICATION_TITLE);
-  std::unique_ptr<message_center::Notification> notification =
-      message_center::Notification::CreateSystemNotification(
-          message_center::NOTIFICATION_TYPE_SIMPLE, kTimeLimitNotificationId,
-          title,
-          ui::TimeFormat::Simple(ui::TimeFormat::FORMAT_DURATION,
-                                 ui::TimeFormat::LENGTH_LONG, time_remaining),
-          l10n_util::GetStringUTF16(IDS_TIME_LIMIT_NOTIFICATION_DISPLAY_SOURCE),
-          GURL(),
-          message_center::NotifierId(
-              message_center::NotifierId::SYSTEM_COMPONENT,
-              kTimeLimitNotifierId),
-          message_center::RichNotificationData(),
-          new message_center::NotificationDelegate(),
-          ash::kNotificationSupervisedUserIcon,
-          message_center::SystemNotificationWarningLevel::NORMAL);
-  NotificationDisplayService::GetForProfile(
-      Profile::FromBrowserContext(context_))
-      ->Display(NotificationHandler::Type::TRANSIENT, *notification);
-}
-
 void ScreenTimeController::OnPolicyChanged() {
   CheckTimeLimit("OnPolicyChanged");
 }
@@ -227,8 +174,7 @@ void ScreenTimeController::ResetStateTimers() {
 
 void ScreenTimeController::ResetInSessionTimers() {
   VLOG(1) << "Stopping in-session timers";
-  warning_notification_timer_.Stop();
-  exit_notification_timer_.Stop();
+  time_limit_notifier_.UnscheduleNotifications();
 }
 
 void ScreenTimeController::SaveCurrentStateToPref(

chrome/browser/chromeos/child_accounts/screen_time_controller.h

@@ -7,6 +7,7 @@
 
 #include "base/time/time.h"
 #include "base/timer/timer.h"
+#include "chrome/browser/chromeos/child_accounts/time_limit_notifier.h"
 #include "chrome/browser/chromeos/child_accounts/usage_time_limit_processor.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/prefs/pref_change_registrar.h"
@@ -40,11 +41,6 @@ class ScreenTimeController : public KeyedService,
   base::TimeDelta GetScreenTimeDuration();
 
  private:
-  // The types of time limit notifications. |SCREEN_TIME| is used when the
-  // the screen time limit is about to be used up, and |BED_TIME| is used when
-  // the bed time is approaching.
-  enum TimeLimitNotificationType { kScreenTime, kBedTime };
-
   // Call time limit processor for new state.
   void CheckTimeLimit(const std::string& source);
 
@@ -62,10 +58,6 @@ class ScreenTimeController : public KeyedService,
   //                     when |visible| is true.
   void UpdateTimeLimitsMessage(bool visible, base::Time next_unlock_time);
 
-  // Show a notification indicating the remaining screen time.
-  void ShowNotification(ScreenTimeController::TimeLimitNotificationType type,
-                        const base::TimeDelta& time_remaining);
-
   // Called when the policy of time limits changes.
   void OnPolicyChanged();
 
@@ -89,14 +81,12 @@ class ScreenTimeController : public KeyedService,
   content::BrowserContext* context_;
   PrefService* pref_service_;
 
-  // Called to show warning and exit notifications.
-  base::OneShotTimer warning_notification_timer_;
-  base::OneShotTimer exit_notification_timer_;
-
-  // Timers that are called when lock screen state change event happens, ie,
-  // bedtime is over or the usage limit ends.
+  // Timer scheduled for when the next lock screen state change event is
+  // expected to happen, e.g. when bedtime is over or the usage limit ends.
   base::OneShotTimer next_state_timer_;
-  base::OneShotTimer reset_screen_time_timer_;
+
+  // Used to set up timers when a time limit is approaching.
+  TimeLimitNotifier time_limit_notifier_;
 
   PrefChangeRegistrar pref_change_registrar_;
 

chrome/browser/chromeos/child_accounts/time_limit_notifier.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/chromeos/child_accounts/time_limit_notifier.h"
+
+#include <memory>
+
+#include "ash/public/cpp/vector_icons/vector_icons.h"
+#include "base/memory/ref_counted.h"
+#include "chrome/browser/notifications/notification_display_service.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/grit/generated_resources.h"
+#include "content/public/browser/browser_context.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "ui/base/l10n/time_format.h"
+#include "ui/message_center/public/cpp/notification.h"
+#include "url/gurl.h"
+
+namespace chromeos {
+
+namespace {
+
+constexpr base::TimeDelta kWarningNotificationTimeout =
+    base::TimeDelta::FromMinutes(5);
+constexpr base::TimeDelta kExitNotificationTimeout =
+    base::TimeDelta::FromMinutes(1);
+
+// The notification id. All the time limit notifications share the same id so
+// that a subsequent notification can replace the previous one.
+constexpr char kTimeLimitNotificationId[] = "time-limit-notification";
+
+// The notifier id representing the app.
+constexpr char kTimeLimitNotifierId[] = "family-link";
+
+void ShowNotification(TimeLimitNotifier::LimitType limit_type,
+                      base::TimeDelta time_remaining,
+                      content::BrowserContext* context) {
+  const base::string16 title = l10n_util::GetStringUTF16(
+      limit_type == TimeLimitNotifier::LimitType::kScreenTime
+          ? IDS_SCREEN_TIME_NOTIFICATION_TITLE
+          : IDS_BED_TIME_NOTIFICATION_TITLE);
+  std::unique_ptr<message_center::Notification> notification =
+      message_center::Notification::CreateSystemNotification(
+          message_center::NOTIFICATION_TYPE_SIMPLE, kTimeLimitNotificationId,
+          title,
+          ui::TimeFormat::Simple(ui::TimeFormat::FORMAT_DURATION,
+                                 ui::TimeFormat::LENGTH_LONG, time_remaining),
+          l10n_util::GetStringUTF16(IDS_TIME_LIMIT_NOTIFICATION_DISPLAY_SOURCE),
+          GURL(),
+          message_center::NotifierId(
+              message_center::NotifierId::SYSTEM_COMPONENT,
+              kTimeLimitNotifierId),
+          message_center::RichNotificationData(),
+          base::MakeRefCounted<message_center::NotificationDelegate>(),
+          ash::kNotificationSupervisedUserIcon,
+          message_center::SystemNotificationWarningLevel::NORMAL);
+  NotificationDisplayService::GetForProfile(
+      Profile::FromBrowserContext(context))
+      ->Display(NotificationHandler::Type::TRANSIENT, *notification);
+}
+
+}  // namespace
+
+TimeLimitNotifier::TimeLimitNotifier(content::BrowserContext* context)
+    : TimeLimitNotifier(context, nullptr /* task_runner */) {}
+
+TimeLimitNotifier::~TimeLimitNotifier() = default;
+
+void TimeLimitNotifier::MaybeScheduleNotifications(
+    LimitType limit_type,
+    base::TimeDelta remaining_time) {
+  // Stop any previously set timers.
+  UnscheduleNotifications();
+
+  if (remaining_time >= kWarningNotificationTimeout) {
+    warning_notification_timer_.Start(
+        FROM_HERE, remaining_time - kWarningNotificationTimeout,
+        base::BindOnce(&ShowNotification, limit_type,
+                       kWarningNotificationTimeout, context_));
+  }
+  if (remaining_time >= kExitNotificationTimeout) {
+    exit_notification_timer_.Start(
+        FROM_HERE, remaining_time - kExitNotificationTimeout,
+        base::BindOnce(&ShowNotification, limit_type, kExitNotificationTimeout,
+                       context_));
+  }
+}
+
+void TimeLimitNotifier::UnscheduleNotifications() {
+  // TODO(crbug.com/897975): Stop() should be sufficient, but doesn't have the
+  // expected effect in tests.
+  warning_notification_timer_.AbandonAndStop();
+  exit_notification_timer_.AbandonAndStop();
+}
+
+TimeLimitNotifier::TimeLimitNotifier(
+    content::BrowserContext* context,
+    scoped_refptr<base::SequencedTaskRunner> task_runner)
+    : context_(context) {
+  if (task_runner.get()) {
+    warning_notification_timer_.SetTaskRunner(task_runner);
+    exit_notification_timer_.SetTaskRunner(task_runner);
+  }
+}
+
+}  // namespace chromeos

chrome/browser/chromeos/child_accounts/time_limit_notifier.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_CHROMEOS_CHILD_ACCOUNTS_TIME_LIMIT_NOTIFIER_H_
+#define CHROME_BROWSER_CHROMEOS_CHILD_ACCOUNTS_TIME_LIMIT_NOTIFIER_H_
+
+#include "base/macros.h"
+#include "base/memory/scoped_refptr.h"
+#include "base/time/time.h"
+#include "base/timer/timer.h"
+
+namespace base {
+class SequencedTaskRunner;
+}
+
+namespace content {
+class BrowserContext;
+}
+
+namespace chromeos {
+
+// Schedules warning notifications for screen time usage or bed time limits.
+class TimeLimitNotifier {
+ public:
+  // The types of time limits to notify for. kScreenTime is used when the
+  // the screen time limit is about to be used up, and kBedTime is used when
+  // the bed time is approaching.
+  enum class LimitType { kScreenTime, kBedTime };
+
+  explicit TimeLimitNotifier(content::BrowserContext* context);
+  ~TimeLimitNotifier();
+
+  // Schedules warning and/or exit notifications based on the time remaining.
+  void MaybeScheduleNotifications(LimitType limit_type,
+                                  base::TimeDelta remaining_time);
+
+  // Cancels any scheduled notification timers.
+  void UnscheduleNotifications();
+
+ private:
+  friend class TimeLimitNotifierTest;
+
+  // For tests, sets up the notification timers using the given task runner.
+  TimeLimitNotifier(content::BrowserContext* context,
+                    scoped_refptr<base::SequencedTaskRunner> task_runner);
+
+  content::BrowserContext* const context_;
+
+  // Called to show warning and exit notifications.
+  base::OneShotTimer warning_notification_timer_;
+  base::OneShotTimer exit_notification_timer_;
+
+  DISALLOW_COPY_AND_ASSIGN(TimeLimitNotifier);
+};
+
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_CHILD_ACCOUNTS_TIME_LIMIT_NOTIFIER_H_

chrome/browser/chromeos/child_accounts/time_limit_notifier_unittest.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/chromeos/child_accounts/time_limit_notifier.h"
+
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/test/test_mock_time_task_runner.h"
+#include "base/time/time.h"
+#include "chrome/browser/notifications/notification_display_service_tester.h"
+#include "chrome/test/base/testing_profile.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromeos {
+
+class TimeLimitNotifierTest : public testing::Test {
+ public:
+  TimeLimitNotifierTest()
+      : task_runner_(base::MakeRefCounted<base::TestMockTimeTaskRunner>()),
+        notification_tester_(&profile_),
+        notifier_(&profile_, task_runner_) {}
+  ~TimeLimitNotifierTest() override = default;
+
+ protected:
+  bool HasNotification() {
+    return notification_tester_.GetNotification("time-limit-notification")
+        .has_value();
+  }
+
+  void RemoveNotification() {
+    notification_tester_.RemoveAllNotifications(
+        NotificationHandler::Type::TRANSIENT, true /* by_user */);
+  }
+
+  scoped_refptr<base::TestMockTimeTaskRunner> task_runner_;
+  content::TestBrowserThreadBundle bundle_;
+  TestingProfile profile_;
+  NotificationDisplayServiceTester notification_tester_;
+  TimeLimitNotifier notifier_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TimeLimitNotifierTest);
+};
+
+TEST_F(TimeLimitNotifierTest, ShowNotifications) {
+  notifier_.MaybeScheduleNotifications(
+      TimeLimitNotifier::LimitType::kScreenTime,
+      base::TimeDelta::FromMinutes(20));
+
+  // Fast forward a bit, but not far enough to show a notification.
+  task_runner_->FastForwardBy(base::TimeDelta::FromMinutes(10));
+  EXPECT_FALSE(HasNotification());
+
+  // Fast forward to the 5-minute warning time.
+  task_runner_->FastForwardBy(base::TimeDelta::FromMinutes(5));
+  EXPECT_TRUE(HasNotification());
+
+  // Fast forward to the 1-minute warning time.
+  task_runner_->FastForwardBy(base::TimeDelta::FromMinutes(4));
+  EXPECT_TRUE(HasNotification());
+}
+
+TEST_F(TimeLimitNotifierTest, DismissNotification) {
+  notifier_.MaybeScheduleNotifications(TimeLimitNotifier::LimitType::kBedTime,
+                                       base::TimeDelta::FromMinutes(10));
+
+  // Fast forward to the 5-minute warning time.
+  task_runner_->FastForwardBy(base::TimeDelta::FromMinutes(5));
+  EXPECT_TRUE(HasNotification());
+  RemoveNotification();
+
+  // Fast forward one minute; the same notification is not reshown.
+  task_runner_->FastForwardBy(base::TimeDelta::FromMinutes(1));
+  EXPECT_FALSE(HasNotification());
+
+  // Fast forward to the 1-minute warning time.
+  task_runner_->FastForwardBy(base::TimeDelta::FromMinutes(3));
+  EXPECT_TRUE(HasNotification());
+}
+
+TEST_F(TimeLimitNotifierTest, OnlyExitNotification) {
+  notifier_.MaybeScheduleNotifications(
+      TimeLimitNotifier::LimitType::kScreenTime,
+      base::TimeDelta::FromMinutes(3));
+
+  // Fast forward a bit, but not far enough to show a notification.
+  task_runner_->FastForwardBy(base::TimeDelta::FromMinutes(1));
+  EXPECT_FALSE(HasNotification());
+
+  // Fast forward to the 1-minute warning time.
+  task_runner_->FastForwardBy(base::TimeDelta::FromMinutes(1));
+  EXPECT_TRUE(HasNotification());
+  RemoveNotification();
+
+  // Only one notification was shown.
+  task_runner_->FastForwardUntilNoTasksRemain();
+  EXPECT_FALSE(HasNotification());
+}
+
+TEST_F(TimeLimitNotifierTest, NoNotifications) {
+  notifier_.MaybeScheduleNotifications(TimeLimitNotifier::LimitType::kBedTime,
+                                       base::TimeDelta::FromSeconds(30));
+
+  task_runner_->FastForwardUntilNoTasksRemain();
+  EXPECT_FALSE(HasNotification());
+}
+
+TEST_F(TimeLimitNotifierTest, UnscheduleNotifications) {
+  notifier_.MaybeScheduleNotifications(
+      TimeLimitNotifier::LimitType::kScreenTime,
+      base::TimeDelta::FromMinutes(10));
+
+  // Fast forward to the 5-minute warning time.
+  task_runner_->FastForwardBy(base::TimeDelta::FromMinutes(5));
+  EXPECT_TRUE(HasNotification());
+  RemoveNotification();
+
+  // Stop the timers.
+  notifier_.UnscheduleNotifications();
+  task_runner_->FastForwardUntilNoTasksRemain();
+  EXPECT_FALSE(HasNotification());
+}
+
+TEST_F(TimeLimitNotifierTest, RescheduleNotifications) {
+  notifier_.MaybeScheduleNotifications(
+      TimeLimitNotifier::LimitType::kScreenTime,
+      base::TimeDelta::FromMinutes(20));
+
+  // Update the notifier with a new limit.
+  notifier_.MaybeScheduleNotifications(
+      TimeLimitNotifier::LimitType::kScreenTime,
+      base::TimeDelta::FromMinutes(30));
+
+  // Fast forward a bit, but not far enough to show a notification.
+  task_runner_->FastForwardBy(base::TimeDelta::FromMinutes(20));
+  EXPECT_FALSE(HasNotification());
+
+  // Fast forward to the 5-minute warning time.
+  task_runner_->FastForwardBy(base::TimeDelta::FromMinutes(5));
+  EXPECT_TRUE(HasNotification());
+  RemoveNotification();
+
+  // Fast forward to the 1-minute warning time.
+  task_runner_->FastForwardBy(base::TimeDelta::FromMinutes(4));
+  EXPECT_TRUE(HasNotification());
+}
+
+}  // namespace chromeos