Try to fix layout of scroll views in TrayDetailsViews.

ash/common/system/tray/tray_details_view.cc

Index: ash/common/system/tray/tray_details_view.cc
diff --git a/ash/common/system/tray/tray_details_view.cc b/ash/common/system/tray/tray_details_view.cc
index 3f775780e63e175d6731dc05b50a7ec1a1a16e5e..6e90a5a7b11158238e59a94ec910a192681b44c7 100644
--- a/ash/common/system/tray/tray_details_view.cc
+++ b/ash/common/system/tray/tray_details_view.cc
@@ -252,13 +252,14 @@ class ScrollBorder : public views::Border {
 
 TrayDetailsView::TrayDetailsView(SystemTrayItem* owner)
     : owner_(owner),
+      box_layout_(new views::BoxLayout(views::BoxLayout::kVertical, 0, 0, 0)),
       title_row_(nullptr),
       scroller_(nullptr),
       scroll_content_(nullptr),
       progress_bar_(nullptr),
       scroll_border_(nullptr),
       back_button_(nullptr) {
-  SetLayoutManager(new views::BoxLayout(views::BoxLayout::kVertical, 0, 0, 0));
+  SetLayoutManager(box_layout_);
   set_background(views::Background::CreateSolidBackground(kBackgroundColor));
 }
 
@@ -317,10 +318,15 @@ void TrayDetailsView::CreateScrollableList() {
   scroller_->SetContentsView(scroll_content_);
 
   // Note: |scroller_| takes ownership of |scroll_border_|.
-  scroll_border_ = new ScrollBorder;
-  scroller_->SetBorder(std::unique_ptr<views::Border>(scroll_border_));
+  if (!UseMd()) {
+    // In MD, the scroller is always the last thing, so this border is
+    // unnecessary and reserves extra space we don't want.
+    scroll_border_ = new ScrollBorder;
+    scroller_->SetBorder(std::unique_ptr<views::Border>(scroll_border_));
+  }
 
   AddChildView(scroller_);
+  box_layout_->SetFlexForView(scroller_, 1);
 }
 
 void TrayDetailsView::AddScrollSeparator() {
@@ -383,34 +389,33 @@ void TrayDetailsView::TransitionToDefaultView() {
 }
 
 void TrayDetailsView::Layout() {
-  if (bounds().IsEmpty()) {
+  if (UseMd()) {
+    views::View::Layout();
+    if (scroller_ && !scroller_->is_bounded())
+      scroller_->ClipHeightTo(0, scroller_->height());

[tdanderson, 2016/11/11 20:09:55]

It seems this only ever be reached once during the lifetime of |scroller_|,
i.e., after the first call to ClipHeightTo() then is_bounded() will always
return true, so no further calls to ClipHeightTo() will be made in subsequent
layout passes. Wouldn't this be problematic as the height of the scroller
continues to grow?

[Evan Stade, 2016/11/11 22:07:24]

you mean if more rows are added, or if the scroller itself gets allotted a
different height? It's fine if rows are added or removed. I don't think that
detailed views ever change in height, but yes that would be a problem if it did
occur (hence my comment about fragility). The hard-coded-ness of the height here
is perhaps a clue as to why someone felt the need to invent a "FixedSized"
scroll view.

Note that we essentially /have/ to call ClipHeightTo at some point because so
much of ScrollView only works if it is_bounded() (not so sure why, but perhaps
for good reason?).

+  }
+
+  if (UseMd() || bounds().IsEmpty()) {
     views::View::Layout();
     return;
   }
 
   if (scroller_) {
-    if (UseMd()) {
-      gfx::Size scroller_size = scroller()->GetPreferredSize();
-      gfx::Size pref_size = GetPreferredSize();
-      scroller()->ClipHeightTo(
-          0, scroller_size.height() - (pref_size.height() - height()));
-    } else {
-      scroller_->set_fixed_size(gfx::Size());
-      gfx::Size size = GetPreferredSize();
-
-      // Set the scroller to fill the space above the bottom row, so that the
-      // bottom row of the detailed view will always stay just above the title
-      // row.
-      gfx::Size scroller_size = scroll_content_->GetPreferredSize();
-      scroller_->set_fixed_size(
-          gfx::Size(width() + scroller_->GetScrollBarWidth(),
-                    scroller_size.height() - (size.height() - height())));
-    }
+    scroller_->set_fixed_size(gfx::Size());
+    gfx::Size size = GetPreferredSize();
+
+    // Set the scroller to fill the space above the bottom row, so that the
+    // bottom row of the detailed view will always stay just above the title
+    // row.
+    gfx::Size scroller_size = scroll_content_->GetPreferredSize();
+    scroller_->set_fixed_size(
+        gfx::Size(width() + scroller_->GetScrollBarWidth(),
+                  scroller_size.height() - (size.height() - height())));
   }
 
   views::View::Layout();
 
-  if (title_row_ && !UseMd()) {
+  if (title_row_) {
     // Always make sure the title row is bottom-aligned in non-MD.
     gfx::Rect fbounds = title_row_->bounds();
     fbounds.set_y(height() - title_row_->height());
@@ -418,6 +423,16 @@ void TrayDetailsView::Layout() {
   }
 }
 
+int TrayDetailsView::GetHeightForWidth(int width) const {
+  if (!UseMd() || bounds().IsEmpty())
+    return views::View::GetHeightForWidth(width);
+
+  // The height of the bubble that contains this detailed view is set to
+  // the preferred height of the default view, and that determines the
+  // initial height of |this|. Always request to stay the same height.
+  return height();
+}
+
 void TrayDetailsView::OnPaintBorder(gfx::Canvas* canvas) {
   if (scroll_border_) {
     int index = GetIndexOf(scroller_);