Reland "Collapse older notification popups."

This is a reland of 4497e01a.

Change fron the original CL (PatchSet #1):
* Loop over existing toasts in a way that won't crash when a toast is
  removed mid-loop. SetExpanded may cause the toast to be removed.
  See the comment for detail.

Original change's description:
> Do not close widget in TrayBubbleView for notification center.
>
> This is a follow-up CL of https://crrev.com/c/816778 .
>
> There were three code paths to close notification center:
> * Only WebNotificationTray::HideMessageCenter is called.
>   (clicking on tray bell icon)
> + TrayBubbleWrapper::OnWindowActivated is called in addition.
>   (pressing search button)
> + TrayBubbleView::OnWidgetActivationChanged is called in addition.
>   (clicking on screenshot notification in notification center)
>
> On the last code path, Widget::Close() is called before
> WebNotificationTray::HideMessageCenter().
> HideMessageCenter() assumed the widget is still open, thus the crash
> https://crbug.com/792583#c14 was caused.
>
> The quick fix CL fixed HideMessageCenter(), but it is confusing that
> the widget can be closed through multiple code paths.
> This CL uses TrayBubbleView::InitParams::close_on_deactivate so that
> the TrayBubbleView does not close the widget.
>
> TEST=manual
> BUG=792583
>
> Change-Id: Ibff47189719043d6ec6830f50888c8323e63f44a
> Reviewed-on: https://chromium-review.googlesource.com/833429
> Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
> Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#528866}

TEST=MessagePopupCollectionTest.AddedAndRemovedAtSameTime
BUG=792583,804389

Change-Id: I79257b3c64e27926687f50284d7c3e2d03375c28
Reviewed-on: https://chromium-review.googlesource.com/886101
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532767}

ui/message_center/views/message_popup_collection.cc

@@ -129,6 +129,31 @@ void MessagePopupCollection::UpdateWidgets() {
       alignment_delegate_->IsPrimaryDisplayForNotification();
 #endif
 
+  // Check if the popups contain a new notification.
+  bool has_new_toasts = false;
+  for (auto* popup : popups) {
+    if (!FindToast(popup->id())) {
+      has_new_toasts = true;
+      break;
+    }
+  }
+
+  // If a new notification is found, collapse all existing notifications
+  // beforehand.
+  if (has_new_toasts) {
+    for (Toasts::const_iterator iter = toasts_.begin();
+         iter != toasts_.end();) {
+      // SetExpanded() may fire PreferredSizeChanged(), which may end up
+      // removing the toast in OnNotificationUpdated(). So we have to increment
+      // the iterator in a way that is safe even if the current iterator is
+      // invalidated during the loop.
+      MessageView* view = (*iter++)->message_view();
+      if (view->IsMouseHovered() || view->manually_expanded_or_collapsed())
+        continue;
+      view->SetExpanded(false);
+    }
+  }
+
   // Iterate in the reverse order to keep the oldest toasts on screen. Newer
   // items may be ignored if there are no room to place them.
   for (NotificationList::PopupNotifications::const_reverse_iterator iter =

ui/message_center/views/message_popup_collection_unittest.cc

@@ -753,5 +753,34 @@ TEST_F(MessagePopupCollectionTest, ChangingNotificationSize) {
   WaitForTransitionsDone();
 }
 
+// Regression test for https://crbug.com/804389 where notifications are added
+// and removed at the same time when UpdateWidgets is called.
+#if defined(OS_CHROMEOS)
+TEST_F(MessagePopupCollectionTest, AddedAndRemovedAtSameTime) {
+  collection()->IncrementDeferCounter();
+  std::vector<std::string> notification_ids;
+  for (size_t i = 0; i < kMaxVisiblePopupNotifications; ++i)
+    notification_ids.push_back(AddNotification());
+  collection()->DecrementDeferCounter();
+  WaitForTransitionsDone();
+
+  // Depending on the timing of ScopedNotificationsIterationLock, it is possible
+  // that a new notificaiton is added before the observer method of
+  // MarkSinglePopupAsShown are called.
+  // To reproduce the similar state in the unit test, it removes observer.
+  // TODO(tetsui): Remove this workaround with ScopedNotificationsIterationLock.
+  MessageCenter::Get()->RemoveObserver(collection());
+  for (auto& notification_id : notification_ids)
+    MessageCenter::Get()->MarkSinglePopupAsShown(notification_id, false);
+  MessageCenter::Get()->AddObserver(collection());
+
+  AddNotification();
+  WaitForTransitionsDone();
+
+  CloseAllToasts();
+  WaitForTransitionsDone();
+}
+#endif
+
 }  // namespace test
 }  // namespace message_center

ui/message_center/views/message_view.h

@@ -94,6 +94,10 @@ class MESSAGE_CENTER_EXPORT MessageView
   void set_scroller(views::ScrollView* scroller) { scroller_ = scroller; }
   std::string notification_id() const { return notification_id_; }
 
+  bool manually_expanded_or_collapsed() const {
+    return manually_expanded_or_collapsed_;
+  }
+
  protected:
   // Creates and add close button to view hierarchy when necessary. Derived
   // classes should call this after its view hierarchy is populated to ensure
@@ -107,6 +111,10 @@ class MESSAGE_CENTER_EXPORT MessageView
   views::View* background_view() { return background_view_; }
   views::ScrollView* scroller() { return scroller_; }
 
+  void set_manually_expanded_or_collapsed() {
+    manually_expanded_or_collapsed_ = true;
+  }
+
  private:
   std::string notification_id_;
   views::View* background_view_ = nullptr;  // Owned by views hierarchy.
@@ -117,6 +125,10 @@ class MESSAGE_CENTER_EXPORT MessageView
   // Flag if the notification is set to pinned or not.
   bool pinned_ = false;
 
+  // True if the notification is expanded/collapsed by user interaction.
+  // If true, MessagePopupCollection will not auto-collapse the notification.
+  bool manually_expanded_or_collapsed_ = false;
+
   std::unique_ptr<views::Painter> focus_painter_;
 
   views::SlideOutController slide_out_controller_;

ui/message_center/views/notification_view_md.cc

@@ -744,6 +744,7 @@ void NotificationViewMD::ButtonPressed(views::Button* sender,
   // |expand_button| can be focused by TAB.
   if (sender == header_row_) {
     if (IsExpandable()) {
+      set_manually_expanded_or_collapsed();
       ToggleExpanded();
       Layout();
       SchedulePaint();

ui/message_center/views/notification_view_md_unittest.cc

@@ -678,6 +678,20 @@ TEST_F(NotificationViewMDTest, ExpandLongMessage) {
   EXPECT_EQ(collapsed_height, notification_view()->message_view_->height());
   EXPECT_EQ(collapsed_preferred_height,
             notification_view()->GetPreferredSize().height());
+
+  // Test |manually_expanded_or_collapsed| being set when the toggle is done by
+  // user interaction.
+  EXPECT_FALSE(notification_view()->manually_expanded_or_collapsed());
+
+  // Construct a mouse click event 1 pixel inside the header.
+  gfx::Point done_cursor_location(1, 1);
+  views::View::ConvertPointToScreen(notification_view()->header_row_,
+                                    &done_cursor_location);
+  ui::test::EventGenerator generator(widget()->GetNativeWindow());
+  generator.MoveMouseTo(done_cursor_location);
+  generator.ClickLeftButton();
+
+  EXPECT_TRUE(notification_view()->manually_expanded_or_collapsed());
 }
 
 TEST_F(NotificationViewMDTest, TestAccentColor) {

ui/message_center/views/toast_contents_view.cc

@@ -83,6 +83,7 @@ ToastContentsView::~ToastContentsView() {
 
 void ToastContentsView::SetContents(MessageView* view,
                                     bool a11y_feedback_for_updates) {
+  message_view_ = view;
   bool already_has_contents = child_count() > 0;
   RemoveAllChildViews(true);
   AddChildView(view);

ui/message_center/views/toast_contents_view.h

@@ -80,6 +80,8 @@ class MESSAGE_CENTER_EXPORT ToastContentsView
 
   const std::string& id() const { return id_; }
 
+  MessageView* message_view() { return message_view_; }
+
   // Overridden from views::View:
   void OnMouseEntered(const ui::MouseEvent& event) override;
   void OnMouseExited(const ui::MouseEvent& event) override;
@@ -141,6 +143,9 @@ class MESSAGE_CENTER_EXPORT ToastContentsView
   gfx::Point origin_;
   gfx::Size preferred_size_;
 
+  // Weak reference to the MessageView.
+  MessageView* message_view_ = nullptr;
+
   DISALLOW_COPY_AND_ASSIGN(ToastContentsView);
 };