Don't shrink the message center height while open

ui/message_center/views/message_list_view.cc

Index: ui/message_center/views/message_list_view.cc
diff --git a/ui/message_center/views/message_list_view.cc b/ui/message_center/views/message_list_view.cc
index 92d5d7fd7edfaf7b8c0bc48958df7fdd1bade6b3..f50126ea0d43298b051b32a9f3598cc75bc71c9a 100644
--- a/ui/message_center/views/message_list_view.cc
+++ b/ui/message_center/views/message_list_view.cc
@@ -175,9 +175,31 @@ void MessageListView::ReorderChildLayers(ui::Layer* parent_layer) {
   }
 }
 
+void MessageListView::UpdateFixedHeight(int requested_height,
+                                        bool prevent_scroll) {
+  int previous_fixed_height = fixed_height_;
+  int min_height;
+  if (scroller_) {
+    gfx::Rect visible_rect = scroller_->GetVisibleRect();
+
+    // When the |prevent_scroll| flag is set, we use the bottom position of the
+    // visible rect. It's to keep the current visible window, in other words,
+    // not to be scrolled, when the visible rect has a blank area at the bottom.
+    // Otherwize, we use the height of the visible rect. It's to make the height
+    // smaller as possible.
+    min_height = prevent_scroll ? visible_rect.bottom() : visible_rect.height();
+  } else {
+    min_height = fixed_height_;
+  }
+  fixed_height_ = std::max(min_height, requested_height);
+
+  if (previous_fixed_height != fixed_height_) {
+    PreferredSizeChanged();
+  }
+}
+
 void MessageListView::SetRepositionTarget(const gfx::Rect& target) {
   reposition_top_ = std::max(target.y(), 0);
-  fixed_height_ = GetHeightForWidth(width());
 }
 
 void MessageListView::ResetRepositionSession() {
@@ -195,7 +217,8 @@ void MessageListView::ResetRepositionSession() {
   }
 
   reposition_top_ = -1;
-  fixed_height_ = 0;
+
+  UpdateFixedHeight(fixed_height_, false);
 }
 
 void MessageListView::ClearAllClosableNotifications(
@@ -290,7 +313,7 @@ void MessageListView::DoUpdateIfPossible() {
           switches::kEnableMessageCenterAlwaysScrollUpUponNotificationRemoval))
     AnimateNotificationsBelowTarget();
   else
-    AnimateNotificationsAboveTarget();
+    AnimateNotifications();
 
   adding_views_.clear();
   deleting_views_.clear();
@@ -299,6 +322,7 @@ void MessageListView::DoUpdateIfPossible() {
     GetWidget()->SynthesizeMouseMoveEvent();
 }
 
+// TODO(yoshiki): Remove this method. It is no longer maintained.
 void MessageListView::AnimateNotificationsBelowTarget() {
   int target_index = -1;
   int padding = kMarginBetweenItems - MessageView::GetShadowInsets().bottom();
@@ -362,11 +386,13 @@ std::vector<int> MessageListView::ComputeRepositionOffsets(
       vertical_gap_to_target_from_top += heights[i] + padding;
   }
 
-  // If the calculated length is changed from |repositon_top_|, it means that
-  // some of items above the target are updated and their height are changed.
+  // If the calculated length is expanded from |repositon_top_|, it means that
+  // some of items above the target are updated and their height gets longer.

[Eliot Courtney, 2017/04/12 06:16:18]

nit: gets longer => increased

[yoshiki, 2017/04/14 04:37:06]

Done.

   // Adjust the vertical length above the target.
-  fixed_height_ -= reposition_top_ - vertical_gap_to_target_from_top;
-  reposition_top_ = vertical_gap_to_target_from_top;
+  if (vertical_gap_to_target_from_top > reposition_top_) {
+    fixed_height_ += vertical_gap_to_target_from_top - reposition_top_;
+    reposition_top_ = vertical_gap_to_target_from_top;
+  }
 
   std::vector<int> positions;
   positions.reserve(heights.size());
@@ -377,7 +403,8 @@ std::vector<int> MessageListView::ComputeRepositionOffsets(
     if (!deleting[i])
       y += heights[i] + padding;
   }
-  DCHECK_EQ(y, reposition_top_);
+  DCHECK_EQ(y, vertical_gap_to_target_from_top);
+  DCHECK_LE(y, reposition_top_);
 
   // Match the top with |reposition_top_|.
   y = reposition_top_;
@@ -390,12 +417,12 @@ std::vector<int> MessageListView::ComputeRepositionOffsets(
   // If the target view, or any views below it expand they might exceed

[Eliot Courtney, 2017/04/12 06:16:18]

This comment feels like it might need updating to reflect its relationship with
the new UpdateFixedHeight call.

[yoshiki, 2017/04/14 04:37:06]

Done.

   // |fixed_height_|. Rather than letting them push out the bottom and be
   // clipped, instead increase |fixed_height_|.
-  fixed_height_ = std::max(fixed_height_, y - padding + GetInsets().bottom());
+  UpdateFixedHeight(y - padding + GetInsets().bottom(), true);
 
   return positions;
 }
 
-void MessageListView::AnimateNotificationsAboveTarget() {
+void MessageListView::AnimateNotifications() {
   int target_index = -1;
   int padding = kMarginBetweenItems - MessageView::GetShadowInsets().bottom();
   gfx::Rect child_area = GetContentsBounds();
@@ -410,15 +437,6 @@ void MessageListView::AnimateNotificationsAboveTarget() {
         break;
       }
     }
-    // If no items are below |reposition_top_|, use the last item as the target.

[Eliot Courtney, 2017/04/12 06:16:18]

Is this no longer necessary? How come?

[yoshiki, 2017/04/14 04:37:06]

Now animating direction changed from downward to upward, so we don't need to set
the last item as the target.

-    if (target_index == -1) {
-      target_index = child_count() - 1;
-      for (; target_index != -1; target_index--) {
-        views::View* target_view = child_at(target_index);
-        if (deleting_views_.find(target_view) == deleting_views_.end())
-          break;
-      }
-    }
   }
 
   if (target_index != -1) {
@@ -434,7 +452,9 @@ void MessageListView::AnimateNotificationsAboveTarget() {
     std::vector<int> ys =
         ComputeRepositionOffsets(heights, deleting, target_index, padding);
     for (int i = 0; i < child_count(); ++i) {
-      AnimateChild(child_at(i), ys[i], heights[i], true /* animate_on_move */);
+      bool above_target = (i < target_index);
+      AnimateChild(child_at(i), ys[i], heights[i],
+                   !above_target /* animate_on_move */);
     }
   } else {
     // Layout all the items.
@@ -445,7 +465,8 @@ void MessageListView::AnimateNotificationsAboveTarget() {
       if (AnimateChild(child, y, height, true))
         y += height + padding;
     }
-    fixed_height_ = y - padding + GetInsets().bottom();
+    int new_height = y - padding + GetInsets().bottom();
+    UpdateFixedHeight(new_height, false);
   }
 }