Fix TrayItemView behavior for multi-display case

This CL fixes an issue where a `TrayItemView` on a non-primary display
could stay visible when it is supposed to be hidden after locking and
then unlocking the device.

Fixed: b:284139989
Change-Id: Ifdff885cfc608acb7095e761fb2b0f3ca58ef308
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4571173
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Commit-Queue: Elliot Tuck <etuck@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1149934}

ash/system/notification_center/notification_center_test_api.cc

@@ -138,9 +138,16 @@ bool NotificationCenterTestApi::IsBubbleShown() {
 }
 
 bool NotificationCenterTestApi::IsPinnedIconShown() {
-  return notification_center_tray_->notification_icons_controller_->tray_items()
-      .back()
-      ->GetVisible();
+  return IsPinnedIconShownOnDisplay(primary_display_id_);
+}
+
+bool NotificationCenterTestApi::IsPinnedIconShownOnDisplay(int64_t display_id) {
+  auto* notification_center_tray = GetTrayOnDisplay(display_id);
+  CHECK(notification_center_tray);
+  auto tray_items =
+      notification_center_tray->notification_icons_controller_->tray_items();
+  CHECK(!tray_items.empty());
+  return tray_items.back()->GetVisible();
 }
 
 bool NotificationCenterTestApi::IsPopupShown(const std::string& id) {
@@ -188,7 +195,19 @@ message_center::MessagePopupView* NotificationCenterTestApi::GetPopupViewForId(
 }
 
 NotificationCenterTray* NotificationCenterTestApi::GetTray() {
-  return notification_center_tray_;
+  return features::IsQsRevampEnabled() ? notification_center_tray_ : nullptr;
+}
+
+NotificationCenterTray* NotificationCenterTestApi::GetTrayOnDisplay(
+    int64_t display_id) {
+  auto* root_window_controller =
+      Shell::Get()->GetRootWindowControllerWithDisplayId(display_id);
+  if (!root_window_controller || !features::IsQsRevampEnabled()) {
+    return nullptr;
+  }
+  return root_window_controller->shelf()
+      ->status_area_widget()
+      ->notification_center_tray();
 }
 
 views::Widget* NotificationCenterTestApi::GetWidget() {

ash/system/notification_center/notification_center_test_api.h

@@ -93,9 +93,15 @@ class NotificationCenterTestApi {
   // Returns true if `NotificationCenterBubble` is shown, false otherwise.
   bool IsBubbleShown();
 
-  // Returns true if a pinned icons is shown in the `NotificationCenterTray`.
+  // Returns true if a pinned icon is shown in the primary display's
+  // `NotificationCenterTray`.
   bool IsPinnedIconShown();
 
+  // Returns true if a pinned icon is shown in the `NotificationCenterTray`
+  // associated with the display having an id of `display_id`. `CHECK()`s that
+  // there exists a notification center tray associated with that display.
+  bool IsPinnedIconShownOnDisplay(int64_t display_id);
+
   // Returns true if a popup associated with the provided `id` exists, false
   // otherwise.
   bool IsPopupShown(const std::string& id);
@@ -122,9 +128,15 @@ class NotificationCenterTestApi {
   // nullptr otherwise.
   message_center::MessagePopupView* GetPopupViewForId(const std::string& id);
 
-  // Returns the `NotificationCenterTray` in the shelf.
+  // Returns the `NotificationCenterTray` for the primary display, or nullptr if
+  // QS revamp is not enabled.
   NotificationCenterTray* GetTray();
 
+  // Returns the `NotificationCenterTray` associated with the display having an
+  // id of `display_id`, or nullptr if there is no display with that id. Also
+  // returns nullptr if QS revamp is not enabled.
+  NotificationCenterTray* GetTrayOnDisplay(int64_t display_id);
+
   // Returns the widget that owns the `TrayBubbleView` for the notification
   // center.
   views::Widget* GetWidget();

ash/system/notification_center/notification_center_tray_unittest.cc

@@ -16,12 +16,15 @@
 #include "ash/system/status_area_widget.h"
 #include "ash/system/status_area_widget_test_helper.h"
 #include "ash/system/tray/tray_constants.h"
+#include "ash/system/unified/notification_counter_view.h"
 #include "ash/system/unified/unified_system_tray.h"
 #include "ash/test/ash_test_base.h"
 #include "base/command_line.h"
 #include "base/run_loop.h"
 #include "base/test/scoped_feature_list.h"
+#include "chromeos/ash/components/login/auth/auth_events_recorder.h"
 #include "ui/base/accelerators/accelerator.h"
+#include "ui/compositor/scoped_animation_duration_scale_mode.h"
 
 namespace ash {
 
@@ -289,6 +292,52 @@ TEST_F(NotificationCenterTrayTest, FocusRing) {
             test_api()->GetTray()->size() + kTrayBackgroundFocusPadding.size());
 }
 
+// Tests that `NotificationCounterView` is not still visible on secondary
+// display after logging in with a pinned notification present. This covers
+// b/284139989.
+TEST_F(NotificationCenterTrayTest,
+       NotificationCounterVisibilityForMultiDisplay) {
+  // The behavior under test relies on `TrayItemView` animations being
+  // scheduled, but `TrayItemView` animations are bypassed when the animation
+  // duration scale mode is set to ZERO_DURATION. Hence, set the animation
+  // duration scale mode to something else for this test.
+  ui::ScopedAnimationDurationScaleMode test_duration_mode(
+      ui::ScopedAnimationDurationScaleMode::NON_ZERO_DURATION);
+
+  // This test relies on the lock screen actually being created (and creating
+  // the lock screen requires the existence of an `AuthEventsRecorder`).
+  std::unique_ptr<AuthEventsRecorder> auth_events_recorder =
+      AuthEventsRecorder::CreateForTesting();
+  GetSessionControllerClient()->set_show_lock_screen_views(true);
+
+  // Create two displays.
+  UpdateDisplay("800x799,800x799");
+  auto secondary_display_id = display_manager()->GetDisplayAt(1).id();
+  auto* secondary_notification_center_tray =
+      test_api()->GetTrayOnDisplay(secondary_display_id);
+  auto* secondary_notification_counter_view =
+      secondary_notification_center_tray->notification_icons_controller()
+          ->notification_counter_view();
+
+  // Add a pinned notification.
+  test_api()->AddPinnedNotification();
+
+  // Verify that the secondary display's notification center tray shows an icon
+  // for the pinned notification and not the `NotificationCounterView`.
+  ASSERT_TRUE(test_api()->IsPinnedIconShownOnDisplay(secondary_display_id));
+  ASSERT_FALSE(secondary_notification_counter_view->GetVisible());
+
+  // Go to the lock screen.
+  GetSessionControllerClient()->LockScreen();
+
+  // Log back in.
+  GetSessionControllerClient()->UnlockScreen();
+
+  // Verify that the `NotificationCounterView` on the secondary display is not
+  // visible.
+  EXPECT_FALSE(secondary_notification_counter_view->GetVisible());
+}
+
 // Test suite for the notification center when `kPrivacyIndicators` is enabled.
 class NotificationCenterTrayPrivacyIndicatorsTest : public AshTestBase {
  public:

ash/system/status_area_animation_controller.cc

@@ -89,11 +89,20 @@ void StatusAreaAnimationController::PerformAnimation(bool visible) {
     views::AnimationBuilder()
         .SetPreemptionStrategy(ui::LayerAnimator::PreemptionStrategy::
                                    IMMEDIATELY_ANIMATE_TO_NEW_TARGET)
+        .OnScheduled(base::BindOnce(
+            [](base::WeakPtr<StatusAreaAnimationController> ptr) {
+              if (!ptr) {
+                return;
+              }
+              ptr->is_hide_animation_scheduled_ = true;
+            },
+            weak_factory_.GetWeakPtr()))
         .OnAborted(base::BindOnce(
             [](base::WeakPtr<StatusAreaAnimationController> ptr) {
               if (!ptr) {
                 return;
               }
+              ptr->is_hide_animation_scheduled_ = false;
               ptr->notification_center_tray_->OnAnimationAborted();
               ptr->ImmediatelyUpdateTrayItemVisibilities();
             },
@@ -103,6 +112,7 @@ void StatusAreaAnimationController::PerformAnimation(bool visible) {
               if (!ptr) {
                 return;
               }
+              ptr->is_hide_animation_scheduled_ = false;
               ptr->notification_center_tray_->OnAnimationEnded();
               ptr->ImmediatelyUpdateTrayItemVisibilities();
             },

ash/system/status_area_animation_controller.h

@@ -28,6 +28,9 @@ class ASH_EXPORT StatusAreaAnimationController
       const StatusAreaAnimationController&) = delete;
   ~StatusAreaAnimationController() override;
 
+  // Returns true if the "hide" animation is scheduled to run, false otherwise.
+  bool is_hide_animation_scheduled() { return is_hide_animation_scheduled_; }
+
  private:
   // Starts running the visibility animation sequence. This will be the "show"
   // animation sequence if `visible` is true, otherwise it will be the "hide"
@@ -65,6 +68,9 @@ class ASH_EXPORT StatusAreaAnimationController
       notification_center_tray_item_animation_enablers_;
   raw_ptr<NotificationCenterTray, ExperimentalAsh> notification_center_tray_;
 
+  // Whether the "hide" animation is scheduled to be run.
+  bool is_hide_animation_scheduled_ = false;
+
   base::WeakPtrFactory<StatusAreaAnimationController> weak_factory_{this};
 };
 

ash/system/status_area_widget.h

@@ -199,6 +199,10 @@ class ASH_EXPORT StatusAreaWidget : public SessionObserver,
     collapse_state_ = state;
   }
 
+  StatusAreaAnimationController* animation_controller() {
+    return animation_controller_.get();
+  }
+
  private:
   friend class MediaTrayTest;
   friend class TrayBackgroundViewTest;

ash/system/tray/tray_item_view.cc

@@ -7,6 +7,7 @@
 #include "ash/constants/ash_features.h"
 #include "ash/public/cpp/shelf_types.h"
 #include "ash/shelf/shelf.h"
+#include "ash/system/status_area_animation_controller.h"
 #include "ash/system/tray/tray_constants.h"
 #include "base/metrics/histogram_functions.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
@@ -173,13 +174,16 @@ void TrayItemView::PerformVisibilityAnimation(bool visible) {
 
   // Immediately progress to the end of the animation if animation is disabled.
   if (!ShouldVisibilityChangeBeAnimated()) {
-    // Tray items need to stay visible during the notification center tray's
-    // hide animation, so don't do anything here.
+    // Tray items need to stay visible if the notification center tray's hide
+    // animation is going to run, so don't hide the tray item here.
     // `StatusAreaAnimationController` will call `ImmediatelyUpdateVisibility()`
     // once the hide animation is over to ensure that all tray items are given a
     // chance to properly update their visibilities. Only applicable when the
     // QS revamp is enabled.
-    if (features::IsQsRevampEnabled() && !target_visible_) {
+    if (features::IsQsRevampEnabled() && !target_visible_ &&
+        shelf_->status_area_widget()
+            ->animation_controller()
+            ->is_hide_animation_scheduled()) {
       return;
     }
     animation_->SetSlideDuration(base::TimeDelta());