media picker: show tabs in tab list

DesktopMediaPickerDialogViews (DMPDV) sizes itself based on the
preferred size of its root view, which ultimately depends via
TabbedPane on the preferred sizes of DesktopMediaListView (DLMV)
or DesktopMediaTabList (DMTL). DMLV computes its preferred size
based on the number of sources in its list. DMTL did not compute
its preferred size at all, but used FillLayout, which meant its
actual preferred size was the maximum preferred size of any of
its children. As it happened, its only child is a ScrollView, which
always has a preferred size of (0,0) when not bounded via
ClipHeightTo(). That meant that if the DMPDV *only* contained a
DMTL, its preferred size was not large enough to actually show any
of the DMTL, leading to the linked bug.

The fix uses a couple of empirical layout constants :) specifically,
I chose minimum and maximum numbers of rows to show in the DMTL such
that it looks okay with both 1 tab and 100 tabs.

Testing this change is fiddly:
1) Make an official build, OR:
    a) Make an unofficial build
    b) Load //chrome/browser/resources/hangouts_service as an unpacked
       extension (using developer mode in chrome://extensions), ignoring
       the JS error you get
2) Go to https://meet.google.com and start a new meeting
3) In the bottom-right, Present > Chrome Tab

This will create a DMPDV that only has a DMTL, no DMLVs.

Bug: 965408
Change-Id: I1de28690c579dc2a09fd232d7a3927dee8a70c9a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1626010
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Auto-Submit: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#662639}

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

@@ -294,4 +294,52 @@ TEST_F(DesktopMediaPickerViewsTest, DoneWithAudioShare) {
   GetPickerDialogView()->GetDialogClientView()->AcceptWindow();
   base::RunLoop().RunUntilIdle();
 }
+
+// This test validates that a DesktopMediaPickerViews that only has a tab list
+// has a reasonable default size, and chooses reasonable bounds on its own size
+// when the source list grows. Specifically, DesktopMediaPickerViews should
+// never be "too small" (ie: it should always be sized as though it contains at
+// least m sources, for some small fixed m) and never be "too large" (ie: it
+// should only grow to show at most n sources, for some fixed n > m). This unit
+// test checks for three properties of a dialog containing k sources:
+//   1) Adding another source such that k < m does not change the dialog's size
+//   2) Adding another source when k > n does not change the dialog's size
+//   3) Adding a source when m <= k <= n does change the dialog's size
+TEST_F(DesktopMediaPickerViewsTest, TabListHasReasonableSize) {
+  auto AddTabSource = [&]() {
+    media_lists_[DesktopMediaID::TYPE_WEB_CONTENTS]->AddSourceByFullMediaID(
+        DesktopMediaID(DesktopMediaID::TYPE_WEB_CONTENTS, 0));
+  };
+
+  auto GetDialogHeight = [&]() {
+    return GetPickerDialogView()
+        ->GetDialogClientView()
+        ->GetPreferredSize()
+        .height();
+  };
+
+  // The dialog's height should not change when doing from zero sources to two
+  // sources.
+  int old_size = GetDialogHeight();
+  AddTabSource();
+  AddTabSource();
+  EXPECT_EQ(GetDialogHeight(), old_size);
+
+  // The dialog's height should change when going from two to twelve, though.
+  for (int i = 0; i < 10; i++)
+    AddTabSource();
+  EXPECT_GT(GetDialogHeight(), old_size);
+
+  for (int i = 0; i < 50; i++)
+    AddTabSource();
+
+  // And then it shouldn't change when going from a large number of sources (in
+  // this case 62) to a larger number, because the ScrollView should scroll
+  // large numbers of sources.
+  old_size = GetDialogHeight();
+  for (int i = 0; i < 50; i++)
+    AddTabSource();
+  EXPECT_EQ(GetDialogHeight(), old_size);
+}
+
 }  // namespace views

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

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/ui/views/desktop_capture/desktop_media_tab_list.h"
 
+#include "base/numerics/ranges.h"
 #include "base/numerics/safe_conversions.h"
 #include "ui/gfx/favicon_size.h"
 #include "ui/views/accessibility/view_accessibility.h"
@@ -155,6 +156,15 @@ const char* DesktopMediaTabList::GetClassName() const {
   return "DesktopMediaTabList";
 }
 
+gfx::Size DesktopMediaTabList::CalculatePreferredSize() const {
+  int rows = model_->RowCount();
+  // Empirical constants! Don't show too many rows at a time if the user has
+  // hundreds of tabs, but don't show too few if there's only one tab because
+  // the UI then looks squished.
+  rows = base::ClampToRange(rows, 4, 10);
+  return gfx::Size(0, rows * child_->row_height());
+}
+
 base::Optional<content::DesktopMediaID> DesktopMediaTabList::GetSelection() {
   int row = child_->FirstSelectedRow();
   if (row == -1)

chrome/browser/ui/views/desktop_capture/desktop_media_tab_list.h

@@ -43,6 +43,7 @@ class DesktopMediaTabList : public DesktopMediaListController::ListView {
 
   // views::View:
   const char* GetClassName() const override;
+  gfx::Size CalculatePreferredSize() const override;
 
   // DesktopMediaListController::ListView:
   base::Optional<content::DesktopMediaID> GetSelection() override;