Move download item separator rendering to background rendering.

Before the separator was rendered as a border, which had all sorts of
odd interactions with the focus ring (including being rendered at the
wrong time, causing it to not refresh immediately after the focus ring
disappears - see associated bug).

Now it is rendered as part of the background, which means it is behind
all other elements. This provides the same effect with less code and is
robust to focus ring changes.

Bug: 1049546
Change-Id: I472067bd28569fac8ef00a6d6b6335d5935b6e0a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2294342Reviewed-by: Connie Wan <connily@chromium.org>
Commit-Queue: Dana Fried <dfried@chromium.org>
Cr-Commit-Position: refs/heads/master@{#788787}

chrome/browser/ui/views/download/download_item_view.cc

@@ -139,32 +139,6 @@ constexpr int kFileIconSize = 24;
 // The offset from the file icon to the danger icon.
 constexpr int kDangerIconOffset = 8;
 
-// The separator is drawn as a border. It's one dp wide.
-class SeparatorBorder : public views::Border {
- public:
-  explicit SeparatorBorder(SkColor separator_color)
-      : views::Border(separator_color) {}
-  ~SeparatorBorder() override {}
-
-  void Paint(const views::View& view, gfx::Canvas* canvas) override {
-    // The FocusRing replaces the separator border when we have focus.
-    if (view.HasFocus())
-      return;
-    int end_x = base::i18n::IsRTL() ? 0 : view.width() - 1;
-    canvas->DrawLine(gfx::Point(end_x, kTopBottomPadding),
-                     gfx::Point(end_x, view.height() - kTopBottomPadding),
-                     color());
-  }
-
-  gfx::Insets GetInsets() const override { return gfx::Insets(0, 0, 0, 1); }
-
-  gfx::Size GetMinimumSize() const override {
-    return gfx::Size(1, 2 * kTopBottomPadding + 1);
-  }
-
-  DISALLOW_COPY_AND_ASSIGN(SeparatorBorder);
-};
-
 // A stub subclass of Button that has no visuals.
 class TransparentButton : public views::Button {
  public:
@@ -691,7 +665,7 @@ gfx::Size DownloadItemView::CalculatePreferredSize() const {
                                    2 * kMinimumVerticalPadding + child_height));
 }
 
-void DownloadItemView::OnPaint(gfx::Canvas* canvas) {
+void DownloadItemView::OnPaintBackground(gfx::Canvas* canvas) {
   // Make sure to draw |this| opaquely. Since the toolbar color can be partially
   // transparent, start with a black backdrop (which is the default initialized
   // color for opaque canvases).
@@ -699,6 +673,19 @@ void DownloadItemView::OnPaint(gfx::Canvas* canvas) {
   canvas->DrawColor(
       GetThemeProvider()->GetColor(ThemeProperties::COLOR_DOWNLOAD_SHELF));
 
+  // Draw the separator as part of the background. It will be covered by the
+  // focus ring when the view has focus.
+  const int end_x = base::i18n::IsRTL() ? 0 : width() - 1;
+  const SkColor separator_color = GetThemeProvider()->GetColor(
+      ThemeProperties::COLOR_TOOLBAR_VERTICAL_SEPARATOR);
+  canvas->DrawLine(gfx::Point(end_x, kTopBottomPadding),
+                   gfx::Point(end_x, height() - kTopBottomPadding),
+                   separator_color);
+}
+
+void DownloadItemView::OnPaint(gfx::Canvas* canvas) {
+  OnPaintBackground(canvas);
+
   DrawIcon(canvas);
   OnPaintBorder(canvas);
 }
@@ -860,10 +847,6 @@ void DownloadItemView::UpdateColorsFromTheme() {
   if (!GetThemeProvider())
     return;
 
-  open_button_->SetBorder(
-      std::make_unique<SeparatorBorder>(GetThemeProvider()->GetColor(
-          ThemeProperties::COLOR_TOOLBAR_VERTICAL_SEPARATOR)));
-
   file_name_label_->SetTextStyle(GetEnabled() ? views::style::STYLE_PRIMARY
                                               : views::style::STYLE_DISABLED);
   if (model_->GetDangerType() ==

chrome/browser/ui/views/download/download_item_view.h

@@ -105,6 +105,7 @@ class DownloadItemView : public views::View,
  protected:
   // views::View:
   gfx::Size CalculatePreferredSize() const override;
+  void OnPaintBackground(gfx::Canvas* canvas) override;
   void OnPaint(gfx::Canvas* canvas) override;
   void OnThemeChanged() override;