Update Notification Center behavior for Lockscreen

This CL updates the session_state_notification_blocker to
hide non-system notifications from the notification center
when the screen is locked. We still allow system priority
notifications to be shown.

We still factor in hidden notifications in the counter shown
in the tray. This is done by ignoring the
session_state_notification_blocker when getting the count.

Note: Use PS1 as base to hide unrelated formatting changes.

Bug: b:264925225

Change-Id: Id2448b79ed7fb9b0aa768527b82afd104441208f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4189968
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Commit-Queue: Ahmed Mehfooz <amehfooz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1097557}

ash/system/message_center/message_center_controller.h

@@ -62,6 +62,10 @@ class ASH_EXPORT MessageCenterController
     return inactive_user_notification_blocker_.get();
   }
 
+  SessionStateNotificationBlocker* session_state_notification_blocker() {
+    return session_state_notification_blocker_.get();
+  }
+
   PhoneHubNotificationController* phone_hub_notification_controller() {
     return phone_hub_notification_controller_.get();
   }

ash/system/message_center/message_center_utils.cc

@@ -7,8 +7,11 @@
 #include "ash/public/cpp/metrics_util.h"
 #include "ash/public/cpp/vm_camera_mic_constants.h"
 #include "ash/root_window_controller.h"
+#include "ash/session/session_controller_impl.h"
 #include "ash/shell.h"
+#include "ash/system/message_center/message_center_controller.h"
 #include "ash/system/message_center/notification_grouping_controller.h"
+#include "ash/system/message_center/session_state_notification_blocker.h"
 #include "ash/system/status_area_widget.h"
 #include "ash/system/unified/unified_system_tray.h"
 #include "base/metrics/histogram_functions.h"
@@ -27,8 +30,9 @@ namespace {
 void ReportAnimationSmoothness(const std::string& animation_histogram_name,
                                int smoothness) {
   // Record animation smoothness if `animation_histogram_name` is given.
-  if (!animation_histogram_name.empty())
+  if (!animation_histogram_name.empty()) {
     base::UmaHistogramPercentage(animation_histogram_name, smoothness);
+  }
 }
 
 }  // namespace
@@ -39,10 +43,12 @@ namespace message_center_utils {
 
 bool CompareNotifications(message_center::Notification* n1,
                           message_center::Notification* n2) {
-  if (n1->pinned() && !n2->pinned())
+  if (n1->pinned() && !n2->pinned()) {
     return true;
-  if (!n1->pinned() && n2->pinned())
+  }
+  if (!n1->pinned() && n2->pinned()) {
     return false;
+  }
   return message_center::CompareTimestampSerial()(n1, n2);
 }
 
@@ -61,18 +67,33 @@ std::vector<message_center::Notification*> GetSortedNotificationsWithOwnView() {
 
 size_t GetNotificationCount() {
   size_t count = 0;
+
+  // We need to ignore the `session_state_notification_blocker` when getting the
+  // notification count on the lock screen. This is because we want the counter
+  // to show the total number of available notifications including notifications
+  // that are hidden by the blocker.
+  const message_center::NotificationBlocker* blocker_to_ignore =
+      Shell::Get()->session_controller()->IsScreenLocked()
+          ? Shell::Get()
+                ->message_center_controller()
+                ->session_state_notification_blocker()
+          : nullptr;
+
   for (message_center::Notification* notification :
-       message_center::MessageCenter::Get()->GetVisibleNotifications()) {
+       message_center::MessageCenter::Get()
+           ->GetVisibleNotificationsWithoutBlocker(blocker_to_ignore)) {
     const std::string& notifier = notification->notifier_id().id;
     // Don't count these notifications since we have `CameraMicTrayItemView` to
     // show indicators on the systray.
-    if (notifier == kVmCameraMicNotifierId)
+    if (notifier == kVmCameraMicNotifierId) {
       continue;
+    }
 
     // Don't count group child notifications since they're contained in a single
     // parent view.
-    if (notification->group_child())
+    if (notification->group_child()) {
       continue;
+    }
 
     ++count;
   }
@@ -83,8 +104,10 @@ message_center::NotificationViewController*
 GetActiveNotificationViewControllerForDisplay(int64_t display_id) {
   RootWindowController* root_window_controller =
       Shell::GetRootWindowControllerWithDisplayId(display_id);
-  if (!root_window_controller || !root_window_controller->GetStatusAreaWidget())
+  if (!root_window_controller ||
+      !root_window_controller->GetStatusAreaWidget()) {
     return nullptr;
+  }
 
   return root_window_controller->GetStatusAreaWidget()
       ->unified_system_tray()

ash/system/message_center/session_state_notification_blocker.cc

@@ -4,6 +4,7 @@
 
 #include "ash/system/message_center/session_state_notification_blocker.h"
 
+#include "ash/constants/ash_features.h"
 #include "ash/public/cpp/message_center/oobe_notification_constants.h"
 #include "ash/session/session_controller_impl.h"
 #include "ash/shell.h"
@@ -12,6 +13,7 @@
 #include "base/containers/contains.h"
 #include "ui/message_center/message_center.h"
 #include "ui/message_center/public/cpp/notification.h"
+#include "ui/message_center/public/cpp/notification_types.h"
 #include "ui/message_center/public/cpp/notifier_id.h"
 
 using session_manager::SessionState;
@@ -40,6 +42,11 @@ bool CalculateShouldShowNotification() {
     return false;
   }
 
+  // Do not show notifications on the lockscreen with QsRevamp.
+  if (features::IsQsRevampEnabled() && state == SessionState::LOCKED) {
+    return false;
+  }
+
   return true;
 }
 
@@ -117,18 +124,29 @@ bool SessionStateNotificationBlocker::ShouldShowNotification(
   }
 
   // Never show notifications in kiosk mode.
-  if (Shell::Get()->session_controller()->IsRunningInAppMode())
+  if (Shell::Get()->session_controller()->IsRunningInAppMode()) {
     return false;
+  }
 
+  const SessionState session_state =
+      Shell::Get()->session_controller()->GetSessionState();
   // Do not show the "Do not disturb" notification if there is no active
   // session.
   if (notification.id() ==
           DoNotDisturbNotificationController::kDoNotDisturbNotificationId &&
-      Shell::Get()->session_controller()->GetSessionState() !=
-          SessionState::ACTIVE) {
+      session_state != SessionState::ACTIVE) {
     return false;
   }
 
+  // Only allow System priority notifications to be shown on the lock screen. We
+  // need to provide an exception here since by default we're blocking all
+  // notifications when the screen is locked.
+  if (notification.priority() ==
+          message_center::NotificationPriority::SYSTEM_PRIORITY &&
+      session_state == SessionState::LOCKED) {
+    return true;
+  }
+
   if (IsAllowedDuringOOBE(notification.id())) {
     return true;
   }
@@ -162,8 +180,9 @@ bool SessionStateNotificationBlocker::ShouldShowNotificationAsPopup(
 }
 
 void SessionStateNotificationBlocker::OnFirstSessionStarted() {
-  if (!g_use_login_delay_for_test)
+  if (!g_use_login_delay_for_test) {
     return;
+  }
   login_delay_timer_.Start(FROM_HERE, kLoginNotificationDelay, this,
                            &SessionStateNotificationBlocker::OnLoginTimerEnded);
 }

ash/system/message_center/session_state_notification_blocker_unittest.cc

@@ -45,16 +45,16 @@ class SessionStateNotificationBlockerTest
   // tests::AshTestBase overrides:
   void SetUp() override {
     scoped_feature_list_ = std::make_unique<base::test::ScopedFeatureList>();
-    scoped_feature_list_->InitWithFeatureState(features::kNotificationsRefresh,
-                                               IsNotificationsRefreshEnabled());
+    scoped_feature_list_->InitWithFeatureState(features::kQsRevamp,
+                                               IsQsRevampEnabled());
 
     NoSessionAshTestBase::SetUp();
     blocker_ = std::make_unique<SessionStateNotificationBlocker>(
         message_center::MessageCenter::Get());
     blocker_->AddObserver(this);
   }
 
-  bool IsNotificationsRefreshEnabled() const { return GetParam(); }
+  bool IsQsRevampEnabled() const { return GetParam(); }
 
   void TearDown() override {
     blocker_->RemoveObserver(this);
@@ -180,7 +180,18 @@ TEST_P(SessionStateNotificationBlockerTest, BaseTest) {
   SetLockedState(true);
   EXPECT_EQ(1, GetStateChangedCountAndReset());
   EXPECT_FALSE(ShouldShowNotificationAsPopup(notifier_id));
-  EXPECT_TRUE(ShouldShowNotification(notifier_id));
+  if (IsQsRevampEnabled()) {
+    // Only system notifications are allowed to be shown in the notification
+    // center in the lock screen when QsRevamp is enabled.
+    EXPECT_FALSE(ShouldShowNotification(notifier_id));
+
+    message_center::NotifierId system_notifier_id(
+        message_center::NotifierType::SYSTEM_COMPONENT, kNotifierSystemPriority,
+        NotificationCatalogName::kTestCatalogName);
+    EXPECT_TRUE(ShouldShowNotification(system_notifier_id));
+  } else {
+    EXPECT_TRUE(ShouldShowNotification(notifier_id));
+  }
 
   // Unlock.
   SetLockedState(false);

ash/system/notification_center/notification_center_bubble_unittest.cc

@@ -210,4 +210,18 @@ TEST_F(NotificationCenterBubbleTest,
                   ->IsExpanded());
 }
 
+TEST_F(NotificationCenterBubbleTest, LockScreenNotificationVisibility) {
+  std::string system_id, id;
+  system_id = test_api()->AddSystemNotification();
+  id = test_api()->AddNotification();
+
+  GetSessionControllerClient()->LockScreen();
+
+  test_api()->ToggleBubble();
+
+  EXPECT_FALSE(test_api()->GetNotificationViewForId(id));
+  EXPECT_TRUE(test_api()->GetNotificationViewForId(system_id));
+  EXPECT_TRUE(test_api()->GetNotificationViewForId(system_id)->GetVisible());
+}
+
 }  // namespace ash

ash/system/notification_center/notification_center_test_api.cc

@@ -26,6 +26,7 @@
 #include "ui/events/test/event_generator.h"
 #include "ui/message_center/message_center.h"
 #include "ui/message_center/public/cpp/message_center_constants.h"
+#include "ui/message_center/public/cpp/notification_types.h"
 #include "ui/message_center/public/cpp/notifier_id.h"
 #include "ui/message_center/views/message_popup_view.h"
 
@@ -70,11 +71,15 @@ std::string NotificationCenterTestApi::AddCustomNotification(
     const ui::ImageModel& icon,
     const std::u16string& display_source,
     const GURL& url,
-    const message_center::NotifierId& notifier_id) {
+    const message_center::NotifierId& notifier_id,
+    const message_center::NotificationPriority priority) {
   const std::string id = GenerateNotificationId();
 
-  message_center::MessageCenter::Get()->AddNotification(CreateNotification(
-      id, title, message, icon, display_source, url, notifier_id));
+  auto notification = CreateNotification(id, title, message, icon,
+                                         display_source, url, notifier_id);
+  notification->set_priority(priority);
+  message_center::MessageCenter::Get()->AddNotification(
+      std::move(notification));
   return id;
 }
 
@@ -95,6 +100,17 @@ std::string NotificationCenterTestApi::AddNotificationWithSourceUrl(
   return id;
 }
 
+std::string NotificationCenterTestApi::AddSystemNotification() {
+  message_center::NotifierId notifier_id;
+  notifier_id.type = message_center::NotifierType::SYSTEM_COMPONENT;
+
+  return AddCustomNotification(
+      /*title=*/u"test_title",
+      /*message=*/u"test_message", ui::ImageModel(), base::EmptyString16(),
+      GURL(), notifier_id,
+      message_center::NotificationPriority::SYSTEM_PRIORITY);
+}
+
 void NotificationCenterTestApi::RemoveNotification(const std::string& id) {
   message_center::MessageCenter::Get()->RemoveNotification(id,
                                                            /*by_user=*/true);

ash/system/notification_center/notification_center_test_api.h

@@ -10,6 +10,7 @@
 
 #include "base/strings/string_util.h"
 #include "ui/base/models/image_model.h"
+#include "ui/message_center/public/cpp/notification_types.h"
 #include "ui/message_center/public/cpp/notifier_id.h"
 
 class GURL;
@@ -61,7 +62,9 @@ class NotificationCenterTestApi {
       const std::u16string& display_source = base::EmptyString16(),
       const GURL& url = GURL(),
       const message_center::NotifierId& notifier_id =
-          message_center::NotifierId());
+          message_center::NotifierId(),
+      const message_center::NotificationPriority =
+          message_center::NotificationPriority::DEFAULT_PRIORITY);
 
   // Adds a notification and returns the associated id.
   std::string AddNotification();
@@ -70,6 +73,10 @@ class NotificationCenterTestApi {
   // the provided url as a string. Useful for testing notification grouping.
   std::string AddNotificationWithSourceUrl(const std::string& url);
 
+  // Adds a notification with the system component notifier and system priority
+  // level.
+  std::string AddSystemNotification();
+
   // Removes the notification associated with the provided id.
   void RemoveNotification(const std::string& id);
 

ash/system/unified/notification_counter_view.cc

@@ -4,6 +4,7 @@
 
 #include "ash/system/unified/notification_counter_view.h"
 
+#include "ash/constants/ash_features.h"
 #include "ash/resources/vector_icons/vector_icons.h"
 #include "ash/session/session_controller_impl.h"
 #include "ash/shell.h"
@@ -49,8 +50,9 @@ const gfx::FontList& GetNumberIconFontList() {
 }
 
 ui::ColorId SeparatorIconColorId(session_manager::SessionState state) {
-  if (state == session_manager::SessionState::OOBE)
+  if (state == session_manager::SessionState::OOBE) {
     return ui::kColorAshIconInOobe;
+  }
   return ui::kColorAshSystemUIMenuSeparator;
 }
 
@@ -62,7 +64,8 @@ bool ShouldShowCounterView() {
   return !message_center::MessageCenter::Get()->IsQuietMode() &&
          session_controller->ShouldShowNotificationTray() &&
          (!session_controller->IsScreenLocked() ||
-          AshMessageCenterLockScreenController::IsEnabled());
+          AshMessageCenterLockScreenController::IsEnabled() ||
+          features::IsQsRevampEnabled());
 }
 
 class NumberIconImageSource : public gfx::CanvasImageSource {

ash/system/unified/notification_counter_view_unittest.cc

@@ -189,4 +189,21 @@ TEST_P(NotificationCounterViewTest, DoNotDisturbIconVisibility) {
   EXPECT_TRUE(GetDoNotDisturbIconView()->GetVisible());
 }
 
+TEST_P(NotificationCounterViewTest, LockScreenCounter) {
+  // This behavior is only applicable when QsRevamp is enabled.
+  if (!IsQsRevampEnabled()) {
+    return;
+  }
+
+  for (size_t i = 0; i < kTrayNotificationMaxCount; i++) {
+    AddNotification(base::NumberToString(i));
+  }
+
+  // Make sure we show the full count of notifications on the lock screen.
+  BlockUserSession(BLOCKED_BY_LOCK_SCREEN);
+  EXPECT_TRUE(GetNotificationCounterView()->GetVisible());
+  EXPECT_EQ(static_cast<int>(kTrayNotificationMaxCount),
+            GetNotificationCounterView()->count_for_display_for_testing());
+}
+
 }  // namespace ash