Try to fix layout of scroll views in TrayDetailsViews.

ash/common/system/tray/tray_details_view.cc

 // Copyright (c) 2012 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 "ash/common/system/tray/tray_details_view.h"
 
 #include "ash/common/ash_view_ids.h"
 #include "ash/common/material_design/material_design_controller.h"
 #include "ash/common/system/tray/fixed_sized_scroll_view.h"
 #include "ash/common/system/tray/system_tray.h"
@@ skipping 234 matching lines @@
 
   gfx::Size GetMinimumSize() const override { return gfx::Size(0, 1); }
 
   bool visible_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(ScrollBorder);
 };
 
 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));
 }
 
 TrayDetailsView::~TrayDetailsView() {}
 
 void TrayDetailsView::OnViewClicked(views::View* sender) {
   if (!UseMd() && title_row_ && sender == title_row_->content()) {
     TransitionToDefaultView();
     return;
   }
@@ skipping 38 matching lines @@
   Layout();
 }
 
 void TrayDetailsView::CreateScrollableList() {
   DCHECK(!scroller_);
   scroll_content_ = new ScrollContentsView();
   scroller_ = new FixedSizedScrollView;
   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() {
   DCHECK(scroll_content_);
   // Do not draw the separator if it is the very first item
   // in the scrollable list.
   if (scroll_content_->has_children())
     scroll_content_->AddChildView(new ScrollSeparator);
 }
 
@@ skipping 42 matching lines @@
     if (title_row_ && title_row_->content() &&
         title_row_->content()->HasFocus()) {
       owner->set_restore_focus(true);
     }
   }
   owner->system_tray()->ShowDefaultView(BUBBLE_USE_EXISTING);
   owner->set_restore_focus(false);
 }
 
 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();
+    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())));
-    }
+    // 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());
     title_row_->SetBoundsRect(fbounds);
   }
 }
 
+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_);
     if (index < child_count() - 1 && child_at(index + 1) != title_row_)
       scroll_border_->set_visible(true);
     else
       scroll_border_->set_visible(false);
   }
 
   views::View::OnPaintBorder(canvas);
 }
 
 }  // namespace ash