Don't shrink the message center height while open

ui/message_center/views/message_list_view.cc

 // Copyright (c) 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/command_line.h"
 #include "base/location.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "ui/gfx/animation/slide_animation.h"
 #include "ui/message_center/message_center_style.h"
@@ skipping 157 matching lines @@
 
 void MessageListView::ReorderChildLayers(ui::Layer* parent_layer) {
   // Reorder children to stack the last child layer at the top. Otherwise
   // upper notification may be hidden by the lower one.
   for (int i = 0; i < child_count(); ++i) {
     if (child_at(i)->layer())
       parent_layer->StackAtBottom(child_at(i)->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() {
   // Don't call DoUpdateIfPossible(), but let Layout() do the task without
   // animation. Reset will cause the change of the bubble size itself, and
   // animation from the old location will look weird.
   if (reposition_top_ >= 0) {
     has_deferred_task_ = false;
     // cancel cause OnBoundsAnimatorDone which deletes |deleted_when_done_|.
     animator_.Cancel();
     for (auto* view : deleting_views_)
       delete view;
     deleting_views_.clear();
     adding_views_.clear();
   }
 
   reposition_top_ = -1;
-  fixed_height_ = 0;
+
+  UpdateFixedHeight(fixed_height_, false);
 }
 
 void MessageListView::ClearAllClosableNotifications(
     const gfx::Rect& visible_scroll_rect) {
   for (int i = 0; i < child_count(); ++i) {
     // Safe cast since all views in MessageListView are MessageViews.
     MessageView* child = (MessageView*)child_at(i);
     if (!child->visible())
       continue;
     if (gfx::IntersectRects(child->bounds(), visible_scroll_rect).IsEmpty())
@@ skipping 74 matching lines @@
     return;
   }
 
   int new_height = GetHeightForWidth(child_area.width() + GetInsets().width());
   SetSize(gfx::Size(child_area.width() + GetInsets().width(), new_height));
 
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableMessageCenterAlwaysScrollUpUponNotificationRemoval))
     AnimateNotificationsBelowTarget();
   else
-    AnimateNotificationsAboveTarget();
+    AnimateNotifications();
 
   adding_views_.clear();
   deleting_views_.clear();
 
   if (!animator_.IsAnimating() && GetWidget())
     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();
   gfx::Rect child_area = GetContentsBounds();
   if (reposition_top_ >= 0) {
     for (int i = 0; i < child_count(); ++i) {
       views::View* child = child_at(i);
       if (child->y() >= reposition_top_) {
         // Find the target.
         target_index = i;
@@ skipping 43 matching lines @@
   DCHECK_EQ(heights.size(), deleting.size());
   // Calculate the vertical length between the top of message list and the top
   // of target. This is to shrink or expand the height of the message list
   // when the notifications above the target is changed.
   int vertical_gap_to_target_from_top = GetInsets().top();
   for (int i = 0; i < target_index; i++) {
     if (!deleting[i])
       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());
   // Layout the items above the target.
   int y = GetInsets().top();
   for (int i = 0; i < target_index; i++) {
     positions.push_back(y);
     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_;
   // Layout the target and the items below the target.
   for (int i = target_index; i < int(heights.size()); i++) {
     positions.push_back(y);
     if (!deleting[i])
       y += heights[i] + padding;
   }
   // 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();
   if (reposition_top_ >= 0) {
     // Find the target item.
     for (int i = 0; i < child_count(); ++i) {
       views::View* child = child_at(i);
       if (child->y() >= reposition_top_ &&
           deleting_views_.find(child) == deleting_views_.end()) {
         // Find the target.
         target_index = i;
         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) {
     std::vector<int> heights;
     std::vector<bool> deleting;
     heights.reserve(child_count());
     deleting.reserve(child_count());
     for (int i = 0; i < child_count(); i++) {
       views::View* child = child_at(i);
       heights.push_back(child->GetHeightForWidth(child_area.width()));
       deleting.push_back(deleting_views_.find(child) != deleting_views_.end());
     }
     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.
     int y = GetInsets().top();
     for (int i = 0; i < child_count(); ++i) {
       views::View* child = child_at(i);
       int height = child->GetHeightForWidth(child_area.width());
       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);
   }
 }
 
 bool MessageListView::AnimateChild(views::View* child,
                                    int top,
                                    int height,
                                    bool animate_on_move) {
   gfx::Rect child_area = GetContentsBounds();
   if (adding_views_.find(child) != adding_views_.end()) {
     child->SetBounds(child_area.right(), top, child_area.width(), height);
@@ skipping 36 matching lines @@
         base::TimeDelta::FromMilliseconds(
             kAnimateClearingNextNotificationDelayMS));
   }
 }
 
 void MessageListView::SetRepositionTargetForTest(const gfx::Rect& target_rect) {
   SetRepositionTarget(target_rect);
 }
 
 }  // namespace message_center