Update control button focus in MessageView.

On Chrome OS, notification control buttons (close & settings) should
change its visibility depending on its focus.
It's handled in MessageCenterView by iterating through MesageViews of
MessageListView, but it's awkward and can be handled in MessageView.

It eliminates duplicated logic of MessageCenterView and
UnifiedMessageCenterView. Also it restores focus support in popups.

TEST=manual(non-ARC and ARC notifications)
BUG=none

Change-Id: Iec9612d219f4dce5e16d713952ad4d5003a760a3
Reviewed-on: https://chromium-review.googlesource.com/1220853Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592293}

ash/message_center/message_center_bubble.cc

@@ -113,7 +113,6 @@ void MessageCenterBubble::InitializeContents(
   message_center_view_ = new MessageCenterView(message_center_, max_height_);
   bubble_view_->AddChildView(new ContentsView(this, message_center_view_));
   message_center_view_->SetMaxHeight(max_height_);
-  message_center_view_->Init();
   // Resize the content of the bubble view to the given bubble size. This is
   // necessary in case of the bubble border forcing a bigger size then the
   // |new_bubble_view| actually wants. See crbug.com/169390.

ash/message_center/message_center_view.cc

@@ -249,15 +249,6 @@ MessageCenterView::~MessageCenterView() {
 
   if (!is_closing_)
     message_center_->RemoveObserver(this);
-
-  if (focus_manager_)
-    focus_manager_->RemoveFocusChangeListener(this);
-}
-
-void MessageCenterView::Init() {
-  focus_manager_ = GetFocusManager();
-  if (focus_manager_)
-    focus_manager_->AddFocusChangeListener(this);
 }
 
 void MessageCenterView::SetNotifications(
@@ -336,26 +327,6 @@ void MessageCenterView::SetIsClosing(bool is_closing) {
     message_center_->AddObserver(this);
 }
 
-void MessageCenterView::OnDidChangeFocus(views::View* before,
-                                         views::View* now) {
-  // Update the button visibility when the focus state is changed.
-  size_t count = message_list_view_->GetNotificationCount();
-  for (size_t i = 0; i < count; ++i) {
-    MessageView* view = message_list_view_->GetNotificationAt(i);
-    // ControlButtonsView is not in the same view hierarchy on ARC++
-    // notifications, so check it separately.
-    if (view->Contains(before) || view->Contains(now) ||
-        (view->GetControlButtonsView() &&
-         (view->GetControlButtonsView()->Contains(before) ||
-          view->GetControlButtonsView()->Contains(now)))) {
-      view->UpdateControlButtonsVisibility();
-    }
-
-    // Ensure that a notification is not removed or added during iteration.
-    DCHECK_EQ(count, message_list_view_->GetNotificationCount());
-  }
-}
-
 void MessageCenterView::UpdateScrollerShadowVisibility() {
   // |scroller_shadow_| is visible only if |scroller_| is not all scrolled.
   scroller_shadow_->SetVisible(scroller_->contents()->height() +

ash/message_center/message_center_view.h

@@ -44,15 +44,12 @@ class ASH_EXPORT MessageCenterView
       public SessionObserver,
       public MessageListView::Observer,
       public gfx::AnimationDelegate,
-      public views::FocusChangeListener,
       public views::ViewObserver {
  public:
   MessageCenterView(message_center::MessageCenter* message_center,
                     int max_height);
   ~MessageCenterView() override;
 
-  void Init();
-
   void SetNotifications(
       const message_center::NotificationList::Notifications& notifications);
 
@@ -68,10 +65,6 @@ class ASH_EXPORT MessageCenterView
 
   void SetMaxHeight(int max_height) { max_height_ = max_height; }
 
-  // Overridden from views::FocusChangeListener
-  void OnWillChangeFocus(views::View* before, views::View* now) override {}
-  void OnDidChangeFocus(views::View* before, views::View* now) override;
-
   void UpdateScrollerShadowVisibility();
 
   static const size_t kMaxVisibleNotifications;
@@ -181,8 +174,6 @@ class ASH_EXPORT MessageCenterView
   // Current view mode. During animation, it is the target mode.
   Mode mode_ = Mode::NO_NOTIFICATIONS;
 
-  views::FocusManager* focus_manager_ = nullptr;
-
   ScopedSessionObserver session_observer_{this};
 
   DISALLOW_COPY_AND_ASSIGN(MessageCenterView);

ash/system/unified/unified_message_center_view.cc

@@ -139,11 +139,17 @@ UnifiedMessageCenterView::UnifiedMessageCenterView(
 
 UnifiedMessageCenterView::~UnifiedMessageCenterView() {
   message_center_->RemoveObserver(this);
-  if (focus_manager_)
-    focus_manager_->RemoveFocusChangeListener(this);
+  RemovedFromWidget();
+}
+
+void UnifiedMessageCenterView::RemovedFromWidget() {
+  if (!focus_manager_)
+    return;
+  focus_manager_->RemoveFocusChangeListener(this);
+  focus_manager_ = nullptr;
 }
 
-void UnifiedMessageCenterView::Init() {
+void UnifiedMessageCenterView::AddedToWidget() {
   focus_manager_ = GetFocusManager();
   if (focus_manager_)
     focus_manager_->AddFocusChangeListener(this);
@@ -278,22 +284,6 @@ void UnifiedMessageCenterView::OnWillChangeFocus(views::View* before,
 
 void UnifiedMessageCenterView::OnDidChangeFocus(views::View* before,
                                                 views::View* now) {
-  // Update the button visibility when the focus state is changed.
-  size_t count = message_list_view_->GetNotificationCount();
-  for (size_t i = 0; i < count; ++i) {
-    MessageView* view = message_list_view_->GetNotificationAt(i);
-    // ControlButtonsView is not in the same view hierarchy on ARC++
-    // notifications, so check it separately.
-    if (view->Contains(before) || view->Contains(now) ||
-        (view->GetControlButtonsView() &&
-         (view->GetControlButtonsView()->Contains(before) ||
-          view->GetControlButtonsView()->Contains(now)))) {
-      view->UpdateControlButtonsVisibility();
-    }
-
-    // Ensure that a notification is not removed or added during iteration.
-    DCHECK_EQ(count, message_list_view_->GetNotificationCount());
-  }
   OnMessageCenterScrolled();
 }
 

ash/system/unified/unified_message_center_view.h

@@ -24,12 +24,6 @@ class MessageCenter;
 
 }  // namespace message_center
 
-namespace views {
-
-class FocusManager;
-
-}  // namespace views
-
 namespace ash {
 
 class UnifiedSystemTrayController;
@@ -67,9 +61,6 @@ class ASH_EXPORT UnifiedMessageCenterView
                            message_center::MessageCenter* message_center);
   ~UnifiedMessageCenterView() override;
 
-  // Initialize focus listener.
-  void Init();
-
   // Set the maximum height that the view can take.
   void SetMaxHeight(int max_height);
 
@@ -85,6 +76,8 @@ class ASH_EXPORT UnifiedMessageCenterView
   // views::View:
   void Layout() override;
   gfx::Size CalculatePreferredSize() const override;
+  void RemovedFromWidget() override;
+  void AddedToWidget() override;
 
   // message_center::MessageCenterObserver:
   void OnNotificationAdded(const std::string& id) override;

ash/system/unified/unified_system_tray_bubble.cc

@@ -112,7 +112,6 @@ UnifiedSystemTrayBubble::UnifiedSystemTrayBubble(UnifiedSystemTray* tray,
 
   TrayBackgroundView::InitializeBubbleAnimations(bubble_widget_);
   bubble_view_->InitializeAndShowBubble();
-  unified_view_->Init();
 
   if (app_list::features::IsBackgroundBlurEnabled()) {
     bubble_widget_->client_view()->layer()->SetBackgroundBlur(

ash/system/unified/unified_system_tray_view.cc

@@ -273,10 +273,6 @@ UnifiedSystemTrayView::UnifiedSystemTrayView(
 
 UnifiedSystemTrayView::~UnifiedSystemTrayView() = default;
 
-void UnifiedSystemTrayView::Init() {
-  message_center_view_->Init();
-}
-
 void UnifiedSystemTrayView::SetMaxHeight(int max_height) {
   message_center_view_->SetMaxHeight(max_height);
 }

ash/system/unified/unified_system_tray_view.h

@@ -54,9 +54,6 @@ class ASH_EXPORT UnifiedSystemTrayView : public views::View,
                         bool initially_expanded);
   ~UnifiedSystemTrayView() override;
 
-  // Initialize after the view is attached to the widget.
-  void Init();
-
   // Set the maximum height that the view can take.
   void SetMaxHeight(int max_height);
 

ui/message_center/views/message_view.cc

@@ -74,7 +74,9 @@ MessageView::MessageView(const Notification& notification)
   UpdateCornerRadius(0, 0);
 }
 
-MessageView::~MessageView() {}
+MessageView::~MessageView() {
+  RemovedFromWidget();
+}
 
 void MessageView::UpdateWithNotification(const Notification& notification) {
   pinned_ = notification.pinned();
@@ -267,6 +269,19 @@ void MessageView::OnGestureEvent(ui::GestureEvent* event) {
   event->SetHandled();
 }
 
+void MessageView::RemovedFromWidget() {
+  if (!focus_manager_)
+    return;
+  focus_manager_->RemoveFocusChangeListener(this);
+  focus_manager_ = nullptr;
+}
+
+void MessageView::AddedToWidget() {
+  focus_manager_ = GetFocusManager();
+  if (focus_manager_)
+    focus_manager_->AddFocusChangeListener(this);
+}
+
 ui::Layer* MessageView::GetSlideOutLayer() {
   return is_nested_ ? layer() : GetWidget()->GetLayer();
 }
@@ -286,6 +301,16 @@ void MessageView::OnSlideOut() {
                                            true /* by_user */);
 }
 
+void MessageView::OnWillChangeFocus(views::View* before, views::View* now) {}
+
+void MessageView::OnDidChangeFocus(views::View* before, views::View* now) {
+  if (Contains(before) || Contains(now) ||
+      (GetControlButtonsView() && (GetControlButtonsView()->Contains(before) ||
+                                   GetControlButtonsView()->Contains(now)))) {
+    UpdateControlButtonsVisibility();
+  }
+}
+
 SlideOutController::SlideMode MessageView::CalculateSlideMode() const {
   if (disable_slide_)
     return SlideOutController::SlideMode::NO_SLIDE;

ui/message_center/views/message_view.h

@@ -19,6 +19,7 @@
 #include "ui/message_center/public/cpp/notification_delegate.h"
 #include "ui/message_center/views/slide_out_controller.h"
 #include "ui/views/animation/ink_drop_host_view.h"
+#include "ui/views/focus/focus_manager.h"
 #include "ui/views/view.h"
 
 namespace views {
@@ -38,7 +39,8 @@ class NotificationControlButtonsView;
 // An base class for a notification entry. Contains background and other
 // elements shared by derived notification views.
 class MESSAGE_CENTER_EXPORT MessageView : public views::InkDropHostView,
-                                          public SlideOutController::Delegate {
+                                          public SlideOutController::Delegate,
+                                          public views::FocusChangeListener {
  public:
   static const char kViewClassName[];
 
@@ -78,7 +80,6 @@ class MESSAGE_CENTER_EXPORT MessageView : public views::InkDropHostView,
   void RequestFocusOnCloseButton();
 
   virtual NotificationControlButtonsView* GetControlButtonsView() const = 0;
-  virtual void UpdateControlButtonsVisibility() = 0;
 
   virtual void SetExpanded(bool expanded);
   virtual bool IsExpanded() const;
@@ -114,6 +115,8 @@ class MESSAGE_CENTER_EXPORT MessageView : public views::InkDropHostView,
   void OnFocus() override;
   void OnBlur() override;
   void OnGestureEvent(ui::GestureEvent* event) override;
+  void RemovedFromWidget() override;
+  void AddedToWidget() override;
   const char* GetClassName() const final;
 
   // message_center::SlideOutController::Delegate
@@ -121,6 +124,10 @@ class MESSAGE_CENTER_EXPORT MessageView : public views::InkDropHostView,
   void OnSlideChanged() override;
   void OnSlideOut() override;
 
+  // views::FocusChangeListener:
+  void OnWillChangeFocus(views::View* before, views::View* now) override;
+  void OnDidChangeFocus(views::View* before, views::View* now) override;
+
   void AddSlideObserver(SlideObserver* observer);
 
   Mode GetMode() const;
@@ -145,6 +152,8 @@ class MESSAGE_CENTER_EXPORT MessageView : public views::InkDropHostView,
   // it is on top of other views.
   void CreateOrUpdateCloseButtonView(const Notification& notification);
 
+  virtual void UpdateControlButtonsVisibility() = 0;
+
   // Changes the background color and schedules a paint.
   virtual void SetDrawBackgroundAsActive(bool active);
 
@@ -182,6 +191,8 @@ class MESSAGE_CENTER_EXPORT MessageView : public views::InkDropHostView,
   // True if the slide is disabled forcibly.
   bool disable_slide_ = false;
 
+  views::FocusManager* focus_manager_ = nullptr;
+
   DISALLOW_COPY_AND_ASSIGN(MessageView);
 };
 

ui/message_center/views/slidable_message_view.h

@@ -29,14 +29,6 @@ class MESSAGE_CENTER_EXPORT SlidableMessageView
   void OnSettingsButtonPressed(const ui::Event& event) override;
   void OnSnoozeButtonPressed(const ui::Event& event) override;
 
-  NotificationControlButtonsView* GetControlButtonsView() {
-    return message_view_->GetControlButtonsView();
-  }
-
-  void UpdateControlButtonsVisibility() {
-    return message_view_->UpdateControlButtonsVisibility();
-  }
-
   void SetExpanded(bool expanded) {
     return message_view_->SetExpanded(expanded);
   }
@@ -87,4 +79,4 @@ class MESSAGE_CENTER_EXPORT SlidableMessageView
 
 }  // namespace message_center
 
-#endif  // UI_MESSAGE_CENTER_VIEWS_SLIDABLE_MESSAGE_VIEW_H_q
+#endif  // UI_MESSAGE_CENTER_VIEWS_SLIDABLE_MESSAGE_VIEW_H_
\ No newline at end of file