Remove junk while auto-expansion of notifications

We recently re-implemented auto-expansion of notification
(crbug.com/810656), but it sometimes caused some junkness
while animation when it was auto-expanded. That was because
auto-expanded is done in the final phase of layouting.

This CL moves the auto-expansion to earlier phase:
- (on add): just before the view is added to the list
- (on update): just before the view is updated in the list
- (on remove): just after the view is removed from the list

Bug: 810656
Test: manual
Change-Id: I015f45f74eef37f89f11edc1b3b346d18771862c
Reviewed-on: https://chromium-review.googlesource.com/958757
Commit-Queue: Yoshiki Iguchi <yoshiki@chromium.org>
Reviewed-by: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543990}

ash/message_center/message_center_view_unittest.cc

@@ -413,7 +413,7 @@ void MessageCenterViewTest::RemoveDefaultNotifications() {
 }
 
 void MessageCenterViewTest::WaitForAnimationToFinish() {
-  if (GetAnimator()->IsAnimating()) {
+  while (GetAnimator()->IsAnimating()) {
     run_loop_ = std::make_unique<base::RunLoop>();
     run_loop_->Run();
   }

ash/message_center/message_list_view.cc

@@ -93,6 +93,9 @@ void MessageListView::AddNotificationAt(MessageView* view, int index) {
     ++real_index;
   }
 
+  if (real_index == 0)
+    ExpandSpecifiedNotificationAndCollapseOthers(view);
+
   AddChildViewAt(view, real_index);
   if (GetContentsBounds().IsEmpty())
     return;
@@ -133,6 +136,10 @@ void MessageListView::RemoveNotification(MessageView* view) {
     }
     DoUpdateIfPossible();
   }
+
+  int index = GetIndexOf(view);
+  if (index == 0)
+    ExpandTopNotificationAndCollapseOthers();
 }
 
 void MessageListView::UpdateNotification(MessageView* view,
@@ -144,6 +151,9 @@ void MessageListView::UpdateNotification(MessageView* view,
   int index = GetIndexOf(view);
   DCHECK_LE(0, index);  // GetIndexOf is negative if not a child.
 
+  if (index == 0)
+    ExpandSpecifiedNotificationAndCollapseOthers(view);
+
   animator_.StopAnimatingView(view);
   if (deleting_views_.find(view) != deleting_views_.end())
     deleting_views_.erase(view);
@@ -410,8 +420,6 @@ void MessageListView::DoUpdateIfPossible() {
     return;
   }
 
-  ExpandTopNotification();
-
   AnimateNotifications();
 
   // Should calculate and set new size after calling AnimateNotifications()
@@ -426,33 +434,44 @@ void MessageListView::DoUpdateIfPossible() {
     GetWidget()->SynthesizeMouseMoveEvent();
 }
 
-void MessageListView::ExpandTopNotification() {
-  bool is_top = true;
+void MessageListView::ExpandSpecifiedNotificationAndCollapseOthers(
+    message_center::MessageView* target_view) {
+  if (!target_view->IsManuallyExpandedOrCollapsed() &&
+      target_view->IsAutoExpandingAllowed()) {
+    target_view->SetExpanded(true);
+  }
+
   for (int i = 0; i < child_count(); ++i) {
     MessageView* view = static_cast<MessageView*>(child_at(i));
+    // Target view is already processed above.
+    if (target_view == view)
+      continue;
+    // Skip if the view is invalid.
     if (!IsValidChild(view))
       continue;
-
+    // We don't touch if the view has been manually expanded or collapsed.
     if (view->IsManuallyExpandedOrCollapsed())
       continue;
 
-    // For top notification:
-    // - IsAutoExpandingAllowed() == true:
-    //   Expands the notification at top if its expand status is never manually
-    //   changed if it's allowed.
-    // - IsAutoExpandingAllowed() == false:
-    //   Collapse explicitly if the notification is not allowed to expand.
-    //   This is necessary for the case that the notification is promoted to
-    //   BUNDLED type and gets not-allowed to expand.
-    // For other notifications:
-    // - Should be collapsed.
-    view->SetExpanded(is_top && view->IsAutoExpandingAllowed());
-
-    // The non first notifications is not top.
-    is_top = false;
+    // Otherwise, collapse the notification.
+    view->SetExpanded(false);
   }
 }
 
+void MessageListView::ExpandTopNotificationAndCollapseOthers() {
+  MessageView* top_notification = nullptr;
+  for (int i = 0; i < child_count(); ++i) {
+    MessageView* view = static_cast<MessageView*>(child_at(i));
+    if (!IsValidChild(view))
+      continue;
+    top_notification = view;
+    break;
+  }
+
+  if (top_notification != nullptr)
+    ExpandSpecifiedNotificationAndCollapseOthers(top_notification);
+}
+
 std::vector<int> MessageListView::ComputeRepositionOffsets(
     const std::vector<int>& heights,
     const std::vector<bool>& deleting,

ash/message_center/message_list_view.h

@@ -88,7 +88,14 @@ class ASH_EXPORT MessageListView : public views::View,
 
   bool IsValidChild(const views::View* child) const;
   void DoUpdateIfPossible();
-  void ExpandTopNotification();
+
+  // For given notification, expand it if it is allowed to be expanded and is
+  // never manually expanded:
+  // For other notifications, collapse if it's never manually expanded.
+  void ExpandSpecifiedNotificationAndCollapseOthers(
+      message_center::MessageView* target_view);
+
+  void ExpandTopNotificationAndCollapseOthers();
 
   // Animates all notifications to align with the top of the last closed
   // notification.