desktop media: use views TableView for tab list

This change makes DesktopMediaPickerViews use a views TableView
for the list of tabs to cast, which brings the behavior of this
control (especially wrt accessibility) significantly closer to
the platform-native behavior and the behavior of other Views UI.

Specifically this change:
1) Introduce a new View class named DesktopMediaTabList which
   uses a TableView to display a DesktopMediaList;
2) Adds a method to DesktopMediaListController to construct a
   DesktopMediaTabList;
3) Adds two interfaces to DesktopMediaListController:
   SourceListListener, which listens for changes to a
   DesktopMediaList, and ListView, which exposes the selection
   state of a visual source list;
4) Has DesktopMediaListView (the "old" list view class)
   implement both these interfaces;
5) Has DesktopMediaTabList implement both these interfaces;
6) Adds support to DesktopMediaPickerViewsTestApi for handling
   either DesktopMediaListView or DesktopMediaTabList;
7) Has DesktopMediaPickerViews use a DesktopMediaTabList (via
   DesktopMediaListController) when appropriate;
8) Reduces the minimum size of TabDesktopMediaList's thumbnail
   from 20x20 to 16x16, since TableView requires a 16x16 image;

To test this change:
  From unit_tests run DesktopMediaPickerViewsTest.*
  From browser_tests run DesktopMediaPickerViewsBrowserTest.*
    and WebRtcDesktopCaptureBrowserTest.*

Future work:

Replacing DesktopMediaListView with another class, specialized
for displaying a grid of SourceViews but using standard Views
layout primitives, and excising the custom ::Layout and ::Paint
methods.

Bug: 726005
Change-Id: Iaa82711b2ed89ab3f47d6522219252dd290135be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1555042
Auto-Submit: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Thomas Guilbert <tguilbert@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#649129}

chrome/browser/media/webrtc/tab_desktop_media_list.cc

@@ -19,6 +19,7 @@
 #include "media/base/video_util.h"
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "third_party/skia/include/core/SkImage.h"
+#include "ui/gfx/favicon_size.h"
 #include "ui/gfx/image/image.h"
 
 using content::BrowserThread;
@@ -28,8 +29,8 @@ namespace {
 
 gfx::ImageSkia CreateEnclosedFaviconImage(gfx::Size size,
                                           const gfx::ImageSkia& favicon) {
-  DCHECK_GE(size.width(), 20);
-  DCHECK_GE(size.height(), 20);
+  DCHECK_GE(size.width(), gfx::kFaviconSize);
+  DCHECK_GE(size.height(), gfx::kFaviconSize);
 
   // Create a bitmap.
   SkBitmap result;

chrome/browser/ui/BUILD.gn

@@ -2503,6 +2503,8 @@ jumbo_split_static_library("ui") {
       "views/desktop_capture/desktop_media_picker_views.h",
       "views/desktop_capture/desktop_media_source_view.cc",
       "views/desktop_capture/desktop_media_source_view.h",
+      "views/desktop_capture/desktop_media_tab_list.cc",
+      "views/desktop_capture/desktop_media_tab_list.h",
       "views/device_chooser_content_view.cc",
       "views/device_chooser_content_view.h",
       "views/download/download_danger_prompt_views.cc",

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

@@ -5,10 +5,12 @@
 #include "chrome/browser/ui/views/desktop_capture/desktop_media_list_controller.h"
 
 #include "base/command_line.h"
+#include "base/numerics/safe_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task/post_task.h"
 #include "chrome/browser/ui/views/desktop_capture/desktop_media_list_view.h"
 #include "chrome/browser/ui/views/desktop_capture/desktop_media_picker_views.h"
+#include "chrome/browser/ui/views/desktop_capture/desktop_media_tab_list.h"
 #include "chrome/common/chrome_switches.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
@@ -33,6 +35,16 @@ std::unique_ptr<views::View> DesktopMediaListController::CreateView(
   return view;
 }
 
+std::unique_ptr<views::View> DesktopMediaListController::CreateTabListView(
+    const base::string16& accessible_name) {
+  DCHECK(!view_);
+
+  auto view = std::make_unique<DesktopMediaTabList>(this, accessible_name);
+  view_ = view.get();
+  view_observer_.Add(view_);
+  return view;
+}
+
 void DesktopMediaListController::StartUpdating(
     content::DesktopMediaID dialog_window_id) {
   media_list_->SetViewDialogWindowId(dialog_window_id);
@@ -46,10 +58,7 @@ void DesktopMediaListController::FocusView() {
 
 base::Optional<content::DesktopMediaID>
 DesktopMediaListController::GetSelection() const {
-  if (!view_ || !view_->GetSelection())
-    return base::nullopt;
-  return base::Optional<content::DesktopMediaID>(
-      view_->GetSelection()->source_id());
+  return view_ ? view_->GetSelection() : base::nullopt;
 }
 
 void DesktopMediaListController::OnSourceListLayoutChanged() {
@@ -69,8 +78,12 @@ void DesktopMediaListController::AcceptSpecificSource(
   dialog_->AcceptSpecificSource(source);
 }
 
+size_t DesktopMediaListController::GetSourceCount() const {
+  return base::checked_cast<size_t>(media_list_->GetSourceCount());
+}
+
 const DesktopMediaList::Source& DesktopMediaListController::GetSource(
-    int index) const {
+    size_t index) const {
   return media_list_->GetSource(index);
 }
 
@@ -84,8 +97,10 @@ views::View* DesktopMediaListController::GetViewForInitialFocus() {
 
 void DesktopMediaListController::OnSourceAdded(DesktopMediaList* list,
                                                int index) {
-  if (view_)
-    view_->OnSourceAdded(index);
+  if (view_) {
+    view_->GetSourceListListener()->OnSourceAdded(
+        base::checked_cast<size_t>(index));
+  }
 
   std::string autoselect_source =
       base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
@@ -103,26 +118,35 @@ void DesktopMediaListController::OnSourceAdded(DesktopMediaList* list,
 
 void DesktopMediaListController::OnSourceRemoved(DesktopMediaList* list,
                                                  int index) {
-  if (view_)
-    view_->OnSourceRemoved(index);
+  if (view_) {
+    view_->GetSourceListListener()->OnSourceRemoved(
+        base::checked_cast<size_t>(index));
+  }
 }
 
 void DesktopMediaListController::OnSourceMoved(DesktopMediaList* list,
                                                int old_index,
                                                int new_index) {
-  if (view_)
-    view_->OnSourceMoved(old_index, new_index);
+  if (view_) {
+    view_->GetSourceListListener()->OnSourceMoved(
+        base::checked_cast<size_t>(old_index),
+        base::checked_cast<size_t>(new_index));
+  }
 }
 void DesktopMediaListController::OnSourceNameChanged(DesktopMediaList* list,
                                                      int index) {
-  if (view_)
-    view_->OnSourceNameChanged(index);
+  if (view_) {
+    view_->GetSourceListListener()->OnSourceNameChanged(
+        base::checked_cast<size_t>(index));
+  }
 }
 void DesktopMediaListController::OnSourceThumbnailChanged(
     DesktopMediaList* list,
     int index) {
-  if (view_)
-    view_->OnSourceThumbnailChanged(index);
+  if (view_) {
+    view_->GetSourceListListener()->OnSourceThumbnailChanged(
+        base::checked_cast<size_t>(index));
+  }
 }
 
 void DesktopMediaListController::OnViewIsDeleting(views::View* view) {

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

@@ -15,19 +15,46 @@
 #include "ui/views/view.h"
 #include "ui/views/view_observer.h"
 
-class DesktopMediaListView;
 class DesktopMediaPickerDialogView;
 
-// This class is the controller for a DesktopMediaListView. It is responsible
-// for:
+// This class is the controller for a View that displays a DesktopMediaList. It
+// is responsible for:
 //   * Observing a DesktopMediaList
-//   * Updating its internal view (currently a DesktopMediaListView) when that
-//     DesktopMediaList changes
-//   * Providing access to the state of its internal view to the dialog
-//   * Proxying between its internal view's callbacks and the dialog's callbacks
+//   * Updating its controlled view when that DesktopMediaList changes
+//   * Providing access to the state of its controlled view to the dialog
+//   * Proxying between its controlled view's callbacks and the dialog's
+//     callbacks
 class DesktopMediaListController : public DesktopMediaListObserver,
                                    public views::ViewObserver {
  public:
+  // The interface implemented by a controlled view or one of its helper classes
+  // to listen for updates to the source list.
+  class SourceListListener {
+   public:
+    virtual void OnSourceAdded(size_t index) = 0;
+    virtual void OnSourceRemoved(size_t index) = 0;
+    virtual void OnSourceMoved(size_t old_index, size_t new_index) = 0;
+    virtual void OnSourceNameChanged(size_t index) = 0;
+    virtual void OnSourceThumbnailChanged(size_t index) = 0;
+  };
+
+  // The abstract interface implemented by any view controlled by this
+  // controller.
+  class ListView : public views::View {
+   public:
+    // Returns the DesktopMediaID of the selected element of this list, or
+    // nullopt if no element is selected.
+    virtual base::Optional<content::DesktopMediaID> GetSelection() = 0;
+
+    // Returns the SourceListListener to use to notify this ListView of changes
+    // to the backing DesktopMediaList.
+    virtual SourceListListener* GetSourceListListener() = 0;
+
+   protected:
+    ListView() = default;
+    ~ListView() override = default;
+  };
+
   DesktopMediaListController(DesktopMediaPickerDialogView* dialog,
                              std::unique_ptr<DesktopMediaList> media_list);
   ~DesktopMediaListController() override;
@@ -40,6 +67,9 @@ class DesktopMediaListController : public DesktopMediaListObserver,
       DesktopMediaSourceViewStyle single_style,
       const base::string16& accessible_name);
 
+  std::unique_ptr<views::View> CreateTabListView(
+      const base::string16& accessible_name);
+
   // Starts observing the DesktopMediaList given earlier, ignoring any entries
   // whose id matches dialog_window_id.
   void StartUpdating(content::DesktopMediaID dialog_window_id);
@@ -63,9 +93,10 @@ class DesktopMediaListController : public DesktopMediaListObserver,
   // controller's source list is the one being shown.
   views::View* GetViewForInitialFocus();
 
-  // These two methods are used by the view (or its subviews) to query and
+  // These methods are used by the view (or its subviews) to query and
   // update the underlying DesktopMediaList.
-  const DesktopMediaList::Source& GetSource(int index) const;
+  size_t GetSourceCount() const;
+  const DesktopMediaList::Source& GetSource(size_t index) const;
   void SetThumbnailSize(const gfx::Size& size);
 
  private:
@@ -91,7 +122,7 @@ class DesktopMediaListController : public DesktopMediaListObserver,
 
   DesktopMediaPickerDialogView* dialog_;
   std::unique_ptr<DesktopMediaList> media_list_;
-  DesktopMediaListView* view_{nullptr};
+  ListView* view_ = nullptr;
   ScopedObserver<views::View, views::ViewObserver> view_observer_{this};
 
   base::WeakPtrFactory<DesktopMediaListController> weak_factory_{this};

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

@@ -46,6 +46,12 @@ gfx::ImageSkia LoadDefaultIcon(aura::Window* window) {
 }
 #endif
 
+DesktopMediaSourceView* AsDesktopMediaSourceView(views::View* view) {
+  DCHECK_EQ(DesktopMediaSourceView::kDesktopMediaSourceViewClassName,
+            view->GetClassName());
+  return static_cast<DesktopMediaSourceView*>(view);
+}
+
 }  // namespace
 
 DesktopMediaListView::DesktopMediaListView(
@@ -73,15 +79,6 @@ void DesktopMediaListView::OnDoubleClick() {
   controller_->AcceptSource();
 }
 
-DesktopMediaSourceView* DesktopMediaListView::GetSelection() {
-  for (int i = 0; i < child_count(); ++i) {
-    DesktopMediaSourceView* source_view = GetChild(i);
-    if (source_view->is_selected())
-      return source_view;
-  }
-  return nullptr;
-}
-
 gfx::Size DesktopMediaListView::CalculatePreferredSize() const {
   int total_rows =
       (child_count() + active_style_->columns - 1) / active_style_->columns;
@@ -128,7 +125,7 @@ bool DesktopMediaListView::OnKeyPressed(const ui::KeyEvent& event) {
   if (position_increment == 0)
     return false;
 
-  views::View* selected = GetSelection();
+  views::View* selected = GetSelectedView();
   views::View* new_selected = nullptr;
 
   if (selected) {
@@ -147,7 +144,18 @@ bool DesktopMediaListView::OnKeyPressed(const ui::KeyEvent& event) {
   return true;
 }
 
-void DesktopMediaListView::OnSourceAdded(int index) {
+base::Optional<content::DesktopMediaID> DesktopMediaListView::GetSelection() {
+  DesktopMediaSourceView* view = GetSelectedView();
+  return view ? base::Optional<content::DesktopMediaID>(view->source_id())
+              : base::nullopt;
+}
+
+DesktopMediaListController::SourceListListener*
+DesktopMediaListView::GetSourceListListener() {
+  return this;
+}
+
+void DesktopMediaListView::OnSourceAdded(size_t index) {
   const DesktopMediaList::Source& source = controller_->GetSource(index);
 
   // We are going to have a second item, apply the generic style.
@@ -180,8 +188,8 @@ void DesktopMediaListView::OnSourceAdded(int index) {
   PreferredSizeChanged();
 }
 
-void DesktopMediaListView::OnSourceRemoved(int index) {
-  DesktopMediaSourceView* view = GetChild(index);
+void DesktopMediaListView::OnSourceRemoved(size_t index) {
+  DesktopMediaSourceView* view = AsDesktopMediaSourceView(children()[index]);
   DCHECK(view);
 
   bool was_selected = view->is_selected();
@@ -201,20 +209,22 @@ void DesktopMediaListView::OnSourceRemoved(int index) {
   PreferredSizeChanged();
 }
 
-void DesktopMediaListView::OnSourceMoved(int old_index, int new_index) {
+void DesktopMediaListView::OnSourceMoved(size_t old_index, size_t new_index) {
   ReorderChildView(child_at(old_index), new_index);
   PreferredSizeChanged();
 }
 
-void DesktopMediaListView::OnSourceNameChanged(int index) {
+void DesktopMediaListView::OnSourceNameChanged(size_t index) {
   const DesktopMediaList::Source& source = controller_->GetSource(index);
-  DesktopMediaSourceView* source_view = GetChild(index);
+  DesktopMediaSourceView* source_view =
+      AsDesktopMediaSourceView(children()[index]);
   source_view->SetName(source.name);
 }
 
-void DesktopMediaListView::OnSourceThumbnailChanged(int index) {
+void DesktopMediaListView::OnSourceThumbnailChanged(size_t index) {
   const DesktopMediaList::Source& source = controller_->GetSource(index);
-  DesktopMediaSourceView* source_view = GetChild(index);
+  DesktopMediaSourceView* source_view =
+      AsDesktopMediaSourceView(children()[index]);
   source_view->SetThumbnail(source.thumbnail);
 }
 
@@ -224,16 +234,15 @@ void DesktopMediaListView::SetStyle(DesktopMediaSourceViewStyle* style) {
       style->image_rect.width() - 2 * style->selection_border_thickness,
       style->image_rect.height() - 2 * style->selection_border_thickness));
 
-  for (int i = 0; i < child_count(); i++) {
-    GetChild(i)->SetStyle(*active_style_);
-  }
+  for (auto* child : children())
+    AsDesktopMediaSourceView(child)->SetStyle(*active_style_);
 }
 
-DesktopMediaSourceView* DesktopMediaListView::GetChild(int index) {
-  views::View* child = child_at(index);
-  DCHECK_EQ(DesktopMediaSourceView::kDesktopMediaSourceViewClassName,
-            child->GetClassName());
-  return static_cast<DesktopMediaSourceView*>(child);
+DesktopMediaSourceView* DesktopMediaListView::GetSelectedView() {
+  const auto i = std::find_if(
+      children().cbegin(), children().cend(),
+      [](View* v) { return AsDesktopMediaSourceView(v)->is_selected(); });
+  return (i == children().cend()) ? nullptr : AsDesktopMediaSourceView(*i);
 }
 
 void DesktopMediaListView::GetAccessibleNodeData(ui::AXNodeData* node_data) {

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

@@ -8,6 +8,7 @@
 #include <memory>
 
 #include "chrome/browser/media/webrtc/desktop_media_list.h"
+#include "chrome/browser/ui/views/desktop_capture/desktop_media_list_controller.h"
 #include "chrome/browser/ui/views/desktop_capture/desktop_media_source_view.h"
 #include "content/public/browser/desktop_media_id.h"
 #include "ui/views/view.h"
@@ -16,7 +17,9 @@ class DesktopMediaListController;
 
 // View that shows a list of desktop media sources available from
 // DesktopMediaList.
-class DesktopMediaListView : public views::View {
+class DesktopMediaListView
+    : public DesktopMediaListController::ListView,
+      public DesktopMediaListController::SourceListListener {
  public:
   DesktopMediaListView(DesktopMediaListController* controller,
                        DesktopMediaSourceViewStyle generic_style,
@@ -31,27 +34,29 @@ class DesktopMediaListView : public views::View {
   // Called by DesktopMediaSourceView when a source has been double-clicked.
   void OnDoubleClick();
 
-  // Returns currently selected source.
-  DesktopMediaSourceView* GetSelection();
-
-  // views::View overrides.
+  // views::View:
   gfx::Size CalculatePreferredSize() const override;
   void Layout() override;
   bool OnKeyPressed(const ui::KeyEvent& event) override;
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
 
-  void OnSourceAdded(int index);
-  void OnSourceRemoved(int index);
-  void OnSourceMoved(int old_index, int new_index);
-  void OnSourceNameChanged(int index);
-  void OnSourceThumbnailChanged(int index);
+  // DesktopMediaListController::ListView:
+  base::Optional<content::DesktopMediaID> GetSelection() override;
+  DesktopMediaListController::SourceListListener* GetSourceListListener()
+      override;
+
+  // DesktopMediaListController::SourceListListener:
+  void OnSourceAdded(size_t index) override;
+  void OnSourceRemoved(size_t index) override;
+  void OnSourceMoved(size_t old_index, size_t new_index) override;
+  void OnSourceNameChanged(size_t index) override;
+  void OnSourceThumbnailChanged(size_t index) override;
 
  private:
   // Change the source style of this list on the fly.
   void SetStyle(DesktopMediaSourceViewStyle* style);
 
-  // Helper for child_at().
-  DesktopMediaSourceView* GetChild(int index);
+  DesktopMediaSourceView* GetSelectedView();
 
   DesktopMediaListController* controller_;
 

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

@@ -155,31 +155,14 @@ DesktopMediaPickerDialogView::DesktopMediaPickerDialogView(
       case DesktopMediaID::TYPE_WEB_CONTENTS: {
         source_types_.push_back(DesktopMediaID::TYPE_WEB_CONTENTS);
 
-        const DesktopMediaSourceViewStyle kTabStyle(
-            1,                                     // columns
-            gfx::Size(600, 30),                    // item_size
-            gfx::Rect(),                           // icon_rect
-            gfx::Rect(46, 0, 490, 30),             // label_rect
-            gfx::HorizontalAlignment::ALIGN_LEFT,  // text_alignment
-            gfx::Rect(10, 2, 26, 26),              // image_rect
-            1,                                     // selection_border_thickness
-            0);                                    // focus_rectangle_inset
-
-        views::ScrollView* tab_scroll_view =
-            views::ScrollView::CreateScrollViewWithBorder();
-        base::string16 tab_title_text =
+        base::string16 title =
             l10n_util::GetStringUTF16(IDS_DESKTOP_MEDIA_PICKER_SOURCE_TYPE_TAB);
         auto list_controller = std::make_unique<DesktopMediaListController>(
             this, std::move(source_list));
-        tab_scroll_view->SetContents(
-            list_controller->CreateView(kTabStyle, kTabStyle, tab_title_text));
+        pane_->AddTab(title,
+                      list_controller->CreateTabListView(title).release());
         list_controllers_.push_back(std::move(list_controller));
 
-        tab_scroll_view->ClipHeightTo(kTabStyle.item_size.height() * 10,
-                                      kTabStyle.item_size.height() * 10);
-        tab_scroll_view->set_hide_horizontal_scrollbar(true);
-
-        pane_->AddTab(tab_title_text, tab_scroll_view);
         pane_->set_listener(this);
         break;
       }

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

@@ -39,13 +39,13 @@ class DesktopMediaPickerDialogView : public views::DialogDelegateView,
   void OnSourceListLayoutChanged();
   void SelectTab(content::DesktopMediaID::Type source_type);
 
-  // views::TabbedPaneListener overrides.
+  // views::TabbedPaneListener:
   void TabSelectedAt(int index) override;
 
-  // views::View overrides.
+  // views::View:
   gfx::Size CalculatePreferredSize() const override;
 
-  // views::DialogDelegateView overrides.
+  // views::DialogDelegateView:
   ui::ModalType GetModalType() const override;
   base::string16 GetWindowTitle() const override;
   bool IsDialogButtonEnabled(ui::DialogButton button) const override;
@@ -89,7 +89,7 @@ class DesktopMediaPickerViews : public DesktopMediaPicker {
 
   void NotifyDialogResult(content::DesktopMediaID source);
 
-  // DesktopMediaPicker overrides.
+  // DesktopMediaPicker:
   void Show(const DesktopMediaPicker::Params& params,
             std::vector<std::unique_ptr<DesktopMediaList>> source_lists,
             const DoneCallback& done_callback) override;

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

@@ -7,16 +7,30 @@
 #include "chrome/browser/ui/views/desktop_capture/desktop_media_list_controller.h"
 #include "chrome/browser/ui/views/desktop_capture/desktop_media_list_view.h"
 #include "chrome/browser/ui/views/desktop_capture/desktop_media_picker_views.h"
+#include "chrome/browser/ui/views/desktop_capture/desktop_media_tab_list.h"
 
 #include "ui/events/base_event_utils.h"
 #include "ui/views/controls/button/checkbox.h"
 #include "ui/views/controls/tabbed_pane/tabbed_pane.h"
+#include "ui/views/controls/table/table_view.h"
+
+namespace {
+
+bool IsDesktopMediaTabList(views::View* view) {
+  return !strcmp(view->GetClassName(), "DesktopMediaTabList");
+}
+
+}  // namespace
 
 DesktopMediaPickerViewsTestApi::DesktopMediaPickerViewsTestApi() = default;
 DesktopMediaPickerViewsTestApi::~DesktopMediaPickerViewsTestApi() = default;
 
-void DesktopMediaPickerViewsTestApi::FocusSourceAtIndex(int index) {
-  GetSourceAtIndex(index)->RequestFocus();
+void DesktopMediaPickerViewsTestApi::FocusSourceAtIndex(size_t index) {
+  views::View* source_view = GetSourceAtIndex(index);
+  if (source_view)
+    source_view->RequestFocus();
+  else
+    GetTableView()->Select(index);
 }
 
 void DesktopMediaPickerViewsTestApi::FocusAudioCheckbox() {
@@ -24,21 +38,33 @@ void DesktopMediaPickerViewsTestApi::FocusAudioCheckbox() {
 }
 
 void DesktopMediaPickerViewsTestApi::PressMouseOnSourceAtIndex(
-    int index,
+    size_t index,
     bool double_click) {
   int flags = ui::EF_LEFT_MOUSE_BUTTON;
   if (double_click)
     flags |= ui::EF_IS_DOUBLE_CLICK;
-  GetSourceAtIndex(index)->OnMousePressed(
-      ui::MouseEvent(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
-                     ui::EventTimeForNow(), flags, ui::EF_LEFT_MOUSE_BUTTON));
+  views::View* source_view = GetSourceAtIndex(index);
+  if (source_view) {
+    source_view->OnMousePressed(
+        ui::MouseEvent(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
+                       ui::EventTimeForNow(), flags, ui::EF_LEFT_MOUSE_BUTTON));
+  } else {
+    // There's no source view to target, and trying to target a specific source
+    // within a larger view would be breakage-prone; just ask the TableView to
+    // to select.
+    GetTableView()->Select(index);
+    if (double_click)
+      picker_->dialog_->GetSelectedController()->AcceptSource();
+  }
 }
 
-void DesktopMediaPickerViewsTestApi::DoubleTapSourceAtIndex(int index) {
+void DesktopMediaPickerViewsTestApi::DoubleTapSourceAtIndex(size_t index) {
   ui::GestureEventDetails details(ui::ET_GESTURE_TAP);
   details.set_tap_count(2);
   ui::GestureEvent double_tap(10, 10, 0, base::TimeTicks(), details);
-  GetSourceAtIndex(index)->OnGestureEvent(&double_tap);
+  views::View* source_view = GetSourceAtIndex(index);
+  DCHECK(source_view);
+  source_view->OnGestureEvent(&double_tap);
 }
 
 void DesktopMediaPickerViewsTestApi::SelectTabForSourceType(
@@ -50,14 +76,19 @@ void DesktopMediaPickerViewsTestApi::SelectTabForSourceType(
   picker_->dialog_->pane_->SelectTabAt(std::distance(source_types.cbegin(), i));
 }
 
-int DesktopMediaPickerViewsTestApi::GetSelectedSourceId() const {
+base::Optional<int> DesktopMediaPickerViewsTestApi::GetSelectedSourceId()
+    const {
   DesktopMediaListController* controller =
       picker_->dialog_->GetSelectedController();
   base::Optional<content::DesktopMediaID> source = controller->GetSelection();
-  return source.has_value() ? source.value().id : -1;
+  return source.has_value() ? base::Optional<int>(source.value().id)
+                            : base::nullopt;
 }
 
-bool DesktopMediaPickerViewsTestApi::HasSourceAtIndex(int index) const {
+bool DesktopMediaPickerViewsTestApi::HasSourceAtIndex(size_t index) const {
+  const views::TableView* table = GetTableView();
+  if (table)
+    return base::checked_cast<size_t>(table->RowCount()) > index;
   return bool{GetSourceAtIndex(index)};
 }
 
@@ -70,12 +101,30 @@ views::Checkbox* DesktopMediaPickerViewsTestApi::GetAudioShareCheckbox() {
 }
 
 const views::View* DesktopMediaPickerViewsTestApi::GetSourceAtIndex(
-    int index) const {
+    size_t index) const {
+  views::View* list = picker_->dialog_->GetSelectedController()->view_;
+  if (IsDesktopMediaTabList(list) || index >= list->children().size())
+    return nullptr;
+  return list->children()[index];
+}
+
+views::View* DesktopMediaPickerViewsTestApi::GetSourceAtIndex(size_t index) {
+  views::View* list = picker_->dialog_->GetSelectedController()->view_;
+  if (IsDesktopMediaTabList(list) || index >= list->children().size())
+    return nullptr;
+  return list->children()[index];
+}
+
+const views::TableView* DesktopMediaPickerViewsTestApi::GetTableView() const {
   views::View* list = picker_->dialog_->GetSelectedController()->view_;
-  return (index < list->child_count()) ? list->child_at(index) : nullptr;
+  return IsDesktopMediaTabList(list)
+             ? static_cast<DesktopMediaTabList*>(list)->child_
+             : nullptr;
 }
 
-views::View* DesktopMediaPickerViewsTestApi::GetSourceAtIndex(int index) {
+views::TableView* DesktopMediaPickerViewsTestApi::GetTableView() {
   views::View* list = picker_->dialog_->GetSelectedController()->view_;
-  return (index < list->child_count()) ? list->child_at(index) : nullptr;
+  return IsDesktopMediaTabList(list)
+             ? static_cast<DesktopMediaTabList*>(list)->child_
+             : nullptr;
 }

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

@@ -11,6 +11,7 @@ class DesktopMediaPickerViews;
 
 namespace views {
 class Checkbox;
+class TableView;
 class View;
 }  // namespace views
 
@@ -26,19 +27,21 @@ class DesktopMediaPickerViewsTestApi {
   void set_picker(DesktopMediaPickerViews* picker) { picker_ = picker; }
 
   void FocusAudioCheckbox();
-  void PressMouseOnSourceAtIndex(int index, bool double_click = false);
+  void PressMouseOnSourceAtIndex(size_t index, bool double_click = false);
   void SelectTabForSourceType(content::DesktopMediaID::Type source_type);
   views::Checkbox* GetAudioShareCheckbox();
 
-  bool HasSourceAtIndex(int index) const;
-  void FocusSourceAtIndex(int index);
-  void DoubleTapSourceAtIndex(int index);
-  int GetSelectedSourceId() const;
+  bool HasSourceAtIndex(size_t index) const;
+  void FocusSourceAtIndex(size_t index);
+  void DoubleTapSourceAtIndex(size_t index);
+  base::Optional<int> GetSelectedSourceId() const;
   views::View* GetSelectedListView();
 
  private:
-  const views::View* GetSourceAtIndex(int index) const;
-  views::View* GetSourceAtIndex(int index);
+  const views::View* GetSourceAtIndex(size_t index) const;
+  views::View* GetSourceAtIndex(size_t index);
+  const views::TableView* GetTableView() const;
+  views::TableView* GetTableView();
 
   DesktopMediaPickerViews* picker_;
 };

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

@@ -136,13 +136,15 @@ TEST_F(DesktopMediaPickerViewsTest, SelectMediaSourceViewOnSingleClick) {
         DesktopMediaID(source_type, 20));
 
     // By default, nothing should be selected.
-    EXPECT_EQ(-1, test_api_.GetSelectedSourceId());
+    EXPECT_FALSE(test_api_.GetSelectedSourceId().has_value());
 
     test_api_.PressMouseOnSourceAtIndex(0);
-    EXPECT_EQ(10, test_api_.GetSelectedSourceId());
+    ASSERT_TRUE(test_api_.GetSelectedSourceId().has_value());
+    EXPECT_EQ(10, test_api_.GetSelectedSourceId().value());
 
     test_api_.PressMouseOnSourceAtIndex(1);
-    EXPECT_EQ(20, test_api_.GetSelectedSourceId());
+    ASSERT_TRUE(test_api_.GetSelectedSourceId().has_value());
+    EXPECT_EQ(20, test_api_.GetSelectedSourceId().value());
   }
 }
 
@@ -208,13 +210,16 @@ TEST_F(DesktopMediaPickerViewsTest, FocusMediaSourceViewToSelect) {
         DesktopMediaID(source_type, 20));
 
     test_api_.FocusSourceAtIndex(0);
-    EXPECT_EQ(10, test_api_.GetSelectedSourceId());
+    ASSERT_TRUE(test_api_.GetSelectedSourceId().has_value());
+    EXPECT_EQ(10, test_api_.GetSelectedSourceId().value());
 
     test_api_.FocusAudioCheckbox();
-    EXPECT_EQ(10, test_api_.GetSelectedSourceId());
+    ASSERT_TRUE(test_api_.GetSelectedSourceId().has_value());
+    EXPECT_EQ(10, test_api_.GetSelectedSourceId().value());
 
     test_api_.FocusSourceAtIndex(1);
-    EXPECT_EQ(20, test_api_.GetSelectedSourceId());
+    ASSERT_TRUE(test_api_.GetSelectedSourceId().has_value());
+    EXPECT_EQ(20, test_api_.GetSelectedSourceId().value());
   }
 }
 

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

+// Copyright 2019 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 "chrome/browser/ui/views/desktop_capture/desktop_media_tab_list.h"
+
+#include "base/numerics/safe_conversions.h"
+#include "ui/gfx/favicon_size.h"
+#include "ui/views/accessibility/view_accessibility.h"
+#include "ui/views/border.h"
+#include "ui/views/controls/scroll_view.h"
+#include "ui/views/controls/table/table_view.h"
+#include "ui/views/controls/table/table_view_observer.h"
+#include "ui/views/layout/fill_layout.h"
+#include "ui/views/view.h"
+
+namespace {
+
+// ui::TableModel that wraps a DesktopMediaListController and listens for
+// updates from it.
+class TabListModel : public ui::TableModel,
+                     public DesktopMediaListController::SourceListListener {
+ public:
+  explicit TabListModel(DesktopMediaListController* controller);
+
+  // ui::TableModel:
+  int RowCount() override;
+  base::string16 GetText(int row, int column) override;
+  gfx::ImageSkia GetIcon(int row) override;
+  void SetObserver(ui::TableModelObserver* observer) override;
+
+  // DesktopMediaListController::SourceListListener:
+  void OnSourceAdded(size_t index) override;
+  void OnSourceRemoved(size_t index) override;
+  void OnSourceMoved(size_t old_index, size_t new_index) override;
+  void OnSourceNameChanged(size_t index) override;
+  void OnSourceThumbnailChanged(size_t index) override;
+
+ private:
+  TabListModel(const TabListModel&) = delete;
+  TabListModel operator=(const TabListModel&) = delete;
+
+  DesktopMediaListController* controller_;
+  ui::TableModelObserver* observer_ = nullptr;
+};
+
+TabListModel::TabListModel(DesktopMediaListController* controller)
+    : controller_(controller) {}
+
+int TabListModel::RowCount() {
+  return base::checked_cast<int>(controller_->GetSourceCount());
+}
+
+base::string16 TabListModel::GetText(int row, int column) {
+  return controller_->GetSource(row).name;
+}
+
+gfx::ImageSkia TabListModel::GetIcon(int row) {
+  return controller_->GetSource(row).thumbnail;
+}
+
+void TabListModel::SetObserver(ui::TableModelObserver* observer) {
+  observer_ = observer;
+}
+
+void TabListModel::OnSourceAdded(size_t index) {
+  observer_->OnItemsAdded(index, 1);
+}
+
+void TabListModel::OnSourceRemoved(size_t index) {
+  observer_->OnItemsRemoved(index, 1);
+}
+
+void TabListModel::OnSourceMoved(size_t old_index, size_t new_index) {
+  observer_->OnItemsMoved(old_index, 1, new_index);
+}
+
+void TabListModel::OnSourceNameChanged(size_t index) {
+  observer_->OnItemsChanged(index, 1);
+}
+
+void TabListModel::OnSourceThumbnailChanged(size_t index) {
+  observer_->OnItemsChanged(index, 1);
+}
+
+// TableViewObserver implementation that bridges between the actual TableView
+// listing tabs and the DesktopMediaTabList.
+class TabListViewObserver : public views::TableViewObserver {
+ public:
+  explicit TabListViewObserver(DesktopMediaListController* controller);
+
+  void OnSelectionChanged() override;
+  void OnDoubleClick() override;
+  void OnKeyDown(ui::KeyboardCode virtual_keycode) override;
+
+ private:
+  TabListViewObserver(const TabListViewObserver&) = delete;
+  TabListViewObserver operator=(const TabListViewObserver&) = delete;
+
+  DesktopMediaListController* controller_;
+};
+
+TabListViewObserver::TabListViewObserver(DesktopMediaListController* controller)
+    : controller_(controller) {}
+
+void TabListViewObserver::OnSelectionChanged() {
+  controller_->OnSourceSelectionChanged();
+}
+
+void TabListViewObserver::OnDoubleClick() {
+  controller_->AcceptSource();
+}
+
+void TabListViewObserver::OnKeyDown(ui::KeyboardCode virtual_keycode) {
+  if (virtual_keycode == ui::VKEY_RETURN)
+    controller_->AcceptSource();
+}
+
+}  // namespace
+
+DesktopMediaTabList::DesktopMediaTabList(DesktopMediaListController* controller,
+                                         const base::string16& accessible_name)
+    : controller_(controller) {
+  // The thumbnail size isn't allowed to be smaller than gfx::kFaviconSize by
+  // the underlying media list. TableView requires that the icon size be exactly
+  // ui::TableModel::kIconSize; if it's not, rendering of the TableView breaks.
+  // This DCHECK enforces that kIconSize is an acceptable size for the source
+  // list.
+  DCHECK_GE(ui::TableModel::kIconSize, gfx::kFaviconSize);
+
+  controller_->SetThumbnailSize(
+      gfx::Size(ui::TableModel::kIconSize, ui::TableModel::kIconSize));
+
+  SetLayoutManager(std::make_unique<views::FillLayout>());
+
+  model_ = std::make_unique<TabListModel>(controller_);
+  view_observer_ = std::make_unique<TabListViewObserver>(controller_);
+
+  auto child = std::make_unique<views::TableView>(
+      model_.get(), std::vector<ui::TableColumn>(1), views::ICON_AND_TEXT,
+      true);
+  child->set_observer(view_observer_.get());
+  child->GetViewAccessibility().OverrideName(accessible_name);
+  child->SetBorder(views::CreateSolidBorder(1, SK_ColorBLACK));
+  child_ = child.get();
+
+  AddChildView(views::TableView::CreateScrollViewWithTable(std::move(child)));
+}
+
+DesktopMediaTabList::~DesktopMediaTabList() {
+  child_->SetModel(nullptr);
+}
+
+const char* DesktopMediaTabList::GetClassName() const {
+  return "DesktopMediaTabList";
+}
+
+base::Optional<content::DesktopMediaID> DesktopMediaTabList::GetSelection() {
+  int row = child_->FirstSelectedRow();
+  if (row == -1)
+    return base::nullopt;
+  return controller_->GetSource(row).id;
+}
+
+DesktopMediaListController::SourceListListener*
+DesktopMediaTabList::GetSourceListListener() {
+  return model_.get();
+}

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

+// Copyright 2019 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.
+
+#ifndef CHROME_BROWSER_UI_VIEWS_DESKTOP_CAPTURE_DESKTOP_MEDIA_TAB_LIST_H_
+#define CHROME_BROWSER_UI_VIEWS_DESKTOP_CAPTURE_DESKTOP_MEDIA_TAB_LIST_H_
+
+#include "chrome/browser/ui/views/desktop_capture/desktop_media_list_controller.h"
+
+namespace views {
+class TableView;
+class View;
+}  // namespace views
+
+namespace {
+class TabListModel;
+class TabListViewObserver;
+}  // namespace
+
+// This class is one of the possible ListViews a DesktopMediaListController can
+// control. It displays a table of sources, one per line, with their "thumbnail"
+// scaled down and displayed as an icon on that line of the table. It is used to
+// display a list of tabs which are possible cast sources.
+//
+// Internally, this class has two helper classes:
+// * TabListModel, which is a ui::TableModel that proxies for DesktopMediaList -
+//   it fetches data from the DesktopMediaList to populate the model, and
+//   observes the DesktopMediaList to update the TableModel.
+// * TabListViewObserver, which is a TableViewObserver that notifies the
+//   controller when the user takes an action on the TableView.
+//
+// Since TableView really wants to be the child of a ScrollView, this class's
+// internal view hierarchy actually looks like:
+//   DesktopMediaTabList
+//     ScrollView
+//       [ScrollView internal helper Views]
+//         TableView
+class DesktopMediaTabList : public DesktopMediaListController::ListView {
+ public:
+  DesktopMediaTabList(DesktopMediaListController* controller,
+                      const base::string16& accessible_name);
+  ~DesktopMediaTabList() override;
+
+  // views::View:
+  const char* GetClassName() const override;
+
+  // DesktopMediaListController::ListView:
+  base::Optional<content::DesktopMediaID> GetSelection() override;
+  DesktopMediaListController::SourceListListener* GetSourceListListener()
+      override;
+
+ private:
+  friend class DesktopMediaPickerViewsTestApi;
+
+  DesktopMediaTabList(const DesktopMediaTabList&) = delete;
+  DesktopMediaTabList operator=(const DesktopMediaTabList&) = delete;
+
+  DesktopMediaListController* controller_;
+  std::unique_ptr<TabListModel> model_;
+  std::unique_ptr<TabListViewObserver> view_observer_;
+  views::TableView* child_ = nullptr;
+};
+
+#endif  // CHROME_BROWSER_UI_VIEWS_DESKTOP_CAPTURE_DESKTOP_MEDIA_TAB_LIST_H_