desktopmedia: slightly fix focus

This change:
1) Makes DesktopMediaListView un-focusable; it gives no useful
   indication of focus and provides no behavior when focused so it
   primarily serves to confuse. Note that it does have an OnKeyPressed
   handler, but that is invoked while its child views (the sources)
   are focused so that behavior remains.
2) Makes DesktopMediaSourceView use the menu selected foreground color
   rather than the focused border color to indicate selection. Using
   the latter led to the appearance of a focus ring on the selected
   element even when the involved source view does not have focus.
3) Makes DesktopMediaSourceView use a focus ring to indicate focus
   rather than an old-style dashed focus rectangle.
4) Removes DesktopMediaPickerViewsTest.ListViewHasInitialFocus since
   the list view is no longer supposed to have focus initially
   (or ever)

Bug: 982226
Change-Id: I0118313883eec60c2685662e9cc9c672bccd18b0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1811758
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#698462}

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

@@ -70,8 +70,6 @@ DesktopMediaListView::DesktopMediaListView(
       active_style_(&single_style_),
       accessible_name_(accessible_name) {
   SetStyle(&single_style_);
-
-  SetFocusBehavior(FocusBehavior::ALWAYS);
 }
 
 DesktopMediaListView::~DesktopMediaListView() {}
@@ -234,9 +232,7 @@ void DesktopMediaListView::OnSourceThumbnailChanged(size_t index) {
 
 void DesktopMediaListView::SetStyle(DesktopMediaSourceViewStyle* style) {
   active_style_ = style;
-  controller_->SetThumbnailSize(gfx::Size(
-      style->image_rect.width() - 2 * style->selection_border_thickness,
-      style->image_rect.height() - 2 * style->selection_border_thickness));
+  controller_->SetThumbnailSize(style->image_rect.size());
 
   for (auto* child : children())
     AsDesktopMediaSourceView(child)->SetStyle(*active_style_);

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

@@ -92,7 +92,6 @@ DesktopMediaPickerDialogView::DesktopMediaPickerDialogView(
             gfx::Rect(),                             // label_rect
             gfx::HorizontalAlignment::ALIGN_CENTER,  // text_alignment
             gfx::Rect(20, 20, 320, 240),             // image_rect
-            4,   // selection_border_thickness
             5);  // focus_rectangle_inset
 
         const DesktopMediaSourceViewStyle kGenericScreenStyle(
@@ -102,7 +101,6 @@ DesktopMediaPickerDialogView::DesktopMediaPickerDialogView(
             gfx::Rect(15, 165, 240, 40),             // label_rect
             gfx::HorizontalAlignment::ALIGN_CENTER,  // text_alignment
             gfx::Rect(15, 15, 240, 150),             // image_rect
-            2,   // selection_border_thickness
             5);  // focus_rectangle_inset
 
         std::unique_ptr<views::ScrollView> screen_scroll_view =
@@ -134,7 +132,6 @@ DesktopMediaPickerDialogView::DesktopMediaPickerDialogView(
             gfx::Rect(32, 110, 138, 40),           // label_rect
             gfx::HorizontalAlignment::ALIGN_LEFT,  // text_alignment
             gfx::Rect(8, 8, 164, 104),             // image_rect
-            2,                                     // selection_border_thickness
             5);                                    // focus_rectangle_inset
 
         std::unique_ptr<views::ScrollView> window_scroll_view =

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

@@ -256,11 +256,6 @@ TEST_F(DesktopMediaPickerViewsTest, OkButtonDisabledWhenNoSelection) {
   }
 }
 
-// Verifies that the MediaListView gets the initial focus.
-TEST_F(DesktopMediaPickerViewsTest, ListViewHasInitialFocus) {
-  EXPECT_TRUE(test_api_.GetSelectedListView()->HasFocus());
-}
-
 // Verifies the visible status of audio checkbox.
 TEST_F(DesktopMediaPickerViewsTest, AudioCheckboxState) {
   bool expect_value = false;

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

@@ -11,7 +11,8 @@
 #include "ui/accessibility/ax_node_data.h"
 #include "ui/gfx/canvas.h"
 #include "ui/native_theme/native_theme.h"
-#include "ui/views/border.h"
+#include "ui/views/background.h"
+#include "ui/views/controls/focus_ring.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/label.h"
 
@@ -27,7 +28,6 @@ DesktopMediaSourceViewStyle::DesktopMediaSourceViewStyle(
     const gfx::Rect& label_rect,
     gfx::HorizontalAlignment text_alignment,
     const gfx::Rect& image_rect,
-    int selection_border_thickness,
     int focus_rectangle_inset)
     : columns(columns),
       item_size(item_size),
@@ -35,7 +35,6 @@ DesktopMediaSourceViewStyle::DesktopMediaSourceViewStyle(
       label_rect(label_rect),
       text_alignment(text_alignment),
       image_rect(image_rect),
-      selection_border_thickness(selection_border_thickness),
       focus_rectangle_inset(focus_rectangle_inset) {}
 
 DesktopMediaSourceView::DesktopMediaSourceView(
@@ -45,9 +44,6 @@ DesktopMediaSourceView::DesktopMediaSourceView(
     : parent_(parent),
       source_id_(source_id),
       style_(style),
-      icon_view_(new views::ImageView()),
-      image_view_(new views::ImageView()),
-      label_(new views::Label()),
       selected_(false) {
   AddChildView(icon_view_);
   AddChildView(image_view_);
@@ -94,15 +90,14 @@ void DesktopMediaSourceView::SetSelected(bool selected) {
       }
     }
 
-    const SkColor border_color = GetNativeTheme()->GetSystemColor(
-        ui::NativeTheme::kColorId_FocusedBorderColor);
-    image_view_->SetBorder(views::CreateSolidBorder(
-        style_.selection_border_thickness, border_color));
+    image_view_->SetBackground(
+        views::CreateSolidBackground(GetNativeTheme()->GetSystemColor(
+            ui::NativeTheme::kColorId_FocusedMenuItemBackgroundColor)));
     label_->SetFontList(label_->font_list().Derive(0, gfx::Font::NORMAL,
                                                    gfx::Font::Weight::BOLD));
     parent_->OnSelectionChanged();
   } else {
-    image_view_->SetBorder(views::NullBorder());
+    image_view_->SetBackground(nullptr);
     label_->SetFontList(label_->font_list().Derive(0, gfx::Font::NORMAL,
                                                    gfx::Font::Weight::NORMAL));
   }
@@ -117,12 +112,6 @@ const char* DesktopMediaSourceView::GetClassName() const {
 void DesktopMediaSourceView::SetStyle(DesktopMediaSourceViewStyle style) {
   style_ = style;
   image_view_->SetBoundsRect(style_.image_rect);
-  if (selected_) {
-    const SkColor border_color = GetNativeTheme()->GetSystemColor(
-        ui::NativeTheme::kColorId_FocusedBorderColor);
-    image_view_->SetBorder(views::CreateSolidBorder(
-        style_.selection_border_thickness, border_color));
-  }
   icon_view_->SetBoundsRect(style_.icon_rect);
   icon_view_->SetImageSize(style_.icon_rect.size());
   label_->SetBoundsRect(style_.label_rect);
@@ -150,27 +139,10 @@ bool DesktopMediaSourceView::IsGroupFocusTraversable() const {
   return false;
 }
 
-void DesktopMediaSourceView::OnPaint(gfx::Canvas* canvas) {
-  View::OnPaint(canvas);
-  if (HasFocus()) {
-    gfx::Rect bounds(GetLocalBounds());
-    bounds.Inset(style_.focus_rectangle_inset, style_.focus_rectangle_inset);
-    canvas->DrawFocusRect(bounds);
-  }
-}
-
 void DesktopMediaSourceView::OnFocus() {
   View::OnFocus();
   SetSelected(true);
   ScrollRectToVisible(gfx::Rect(size()));
-  // We paint differently when focused.
-  SchedulePaint();
-}
-
-void DesktopMediaSourceView::OnBlur() {
-  View::OnBlur();
-  // We paint differently when focused.
-  SchedulePaint();
 }
 
 bool DesktopMediaSourceView::OnMousePressed(const ui::MouseEvent& event) {

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

@@ -7,6 +7,9 @@
 
 #include "content/public/browser/desktop_media_id.h"
 #include "ui/gfx/text_constants.h"
+#include "ui/views/controls/focus_ring.h"
+#include "ui/views/controls/image_view.h"
+#include "ui/views/controls/label.h"
 #include "ui/views/view.h"
 
 namespace views {
@@ -25,7 +28,6 @@ struct DesktopMediaSourceViewStyle {
                               const gfx::Rect& label_rect,
                               gfx::HorizontalAlignment text_alignment,
                               const gfx::Rect& image_rect,
-                              int selection_border_thickness,
                               int focus_rectangle_inset);
 
   // This parameter controls how many source items can be displayed in a row.
@@ -42,10 +44,6 @@ struct DesktopMediaSourceViewStyle {
   gfx::HorizontalAlignment text_alignment;
   gfx::Rect image_rect;
 
-  // When a source item is selected, we paint the border to show it. This
-  // parameter controls how thick the border would be.
-  int selection_border_thickness;
-
   // When a source item is focused, we paint dotted line. This parameter
   // controls the distance between dotted line and the source view boundary.
   int focus_rectangle_inset;
@@ -78,9 +76,7 @@ class DesktopMediaSourceView : public views::View {
   const char* GetClassName() const override;
   views::View* GetSelectedViewForGroup(int group) override;
   bool IsGroupFocusTraversable() const override;
-  void OnPaint(gfx::Canvas* canvas) override;
   void OnFocus() override;
-  void OnBlur() override;
   bool OnMousePressed(const ui::MouseEvent& event) override;
   void OnGestureEvent(ui::GestureEvent* event) override;
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
@@ -100,9 +96,12 @@ class DesktopMediaSourceView : public views::View {
   content::DesktopMediaID source_id_;
 
   DesktopMediaSourceViewStyle style_;
-  views::ImageView* icon_view_;
-  views::ImageView* image_view_;
-  views::Label* label_;
+  views::ImageView* icon_view_ = new views::ImageView;
+  views::ImageView* image_view_ = new views::ImageView;
+  views::Label* label_ = new views::Label;
+
+  std::unique_ptr<views::FocusRing> focus_ring_ =
+      views::FocusRing::Install(this);
 
   bool selected_;