Bug Fix: Desktop Picker Window Shows Unnecessary Scrollbar

chrome/browser/ui/views/desktop_capture/desktop_media_list_view.cc

Index: chrome/browser/ui/views/desktop_capture/desktop_media_list_view.cc
diff --git a/chrome/browser/ui/views/desktop_capture/desktop_media_list_view.cc b/chrome/browser/ui/views/desktop_capture/desktop_media_list_view.cc
index 8b2a367de1e2e256e85bdd630f521386c055220e..5f59f0e90ff3e34ad1f01ec2213b9f29ba2ec48c 100644
--- a/chrome/browser/ui/views/desktop_capture/desktop_media_list_view.cc
+++ b/chrome/browser/ui/views/desktop_capture/desktop_media_list_view.cc
@@ -118,6 +118,9 @@ void DesktopMediaListView::Layout() {
 
     x += active_style_->item_size.width();
   }
+
+  SetSize(gfx::Size(active_style_->columns * active_style_->item_size.width(),

[msw, 2016/09/02 00:35:29]

Layout() is typically called in response to size changes; it's not common (and
potentially dangerous?) to change the view size during layout, afaik. (ie. this
SetSize() should re-trigger Layout() until your SetSize() argument matches the
current View::size(), which seems potentially dangerous)

It's more likely that the preferred size is slightly incorrect (missing some
padding or border or scrollbar size or something?) or isn't correctly updated
before layout. Here are some questions that should help determine where the
problem lies:

1) What is the difference between the current size() here and the size you're
attempting to set?
2) What is the last value returned from GetPreferredSize? (should match size()
here)
3) When is Layout called relative to AddChildViewAt/PreferredSizeChanged calls
in OnSourceAdded?

[qiangchen, 2016/09/02 16:19:52]

See my comment#5 in the bug for a thorough cause. In short
1. The Preferred Size does not play a role at all from my testing.
2. The size will always be set just by its parent, just before layout() gets
called. 
3. If we do not SetSize in layout then scroll bar will always be visible.

[sky, 2016/09/02 18:38:30]

I agree this seems weird, but ScrollView has weird semantics and in general
views contained in a ScrollView set their bounds in layout. We should really
make ScrollView behave better in this regard, but until them I'm fine with this.
One comment though, it's pretty typical to go make sure the contents view of a
scrollview is at least as wide/high as the parent (see TableView::Layout). Are
you sure you don't want that? The reason for that is so you get whatever
background you may have on the contents view filling the bounds.

[qiangchen, 2016/09/02 22:10:57]

Done.

As our logic guarantees no horizontal overflow, and thus the width will be just
parent_content_size.width().

+                    y + active_style_->item_size.height()));
 }
 
 bool DesktopMediaListView::OnKeyPressed(const ui::KeyEvent& event) {