Use NativeTheme to draw vertical menu separators.

chrome/browser/ui/views/toolbar/app_menu.cc

Index: chrome/browser/ui/views/toolbar/app_menu.cc
diff --git a/chrome/browser/ui/views/toolbar/app_menu.cc b/chrome/browser/ui/views/toolbar/app_menu.cc
index 0309da713eaf67d859522da394a333acbe45e620..759a7a65e4bb6c77f1c09d8636bb6bbe91199101 100644
--- a/chrome/browser/ui/views/toolbar/app_menu.cc
+++ b/chrome/browser/ui/views/toolbar/app_menu.cc
@@ -154,8 +154,6 @@ class InMenuButtonBackground : public views::Background {
   // Overridden from views::Background.
   void Paint(gfx::Canvas* canvas, View* view) const override {
     CustomButton* button = CustomButton::AsCustomButton(view);
-    views::Button::ButtonState state =
-        button ? button->state() : views::Button::STATE_NORMAL;
     int h = view->height();
 
     // Draw leading border if desired.
@@ -165,32 +163,24 @@ class InMenuButtonBackground : public views::Background {
       // already, so we end up flipping exactly once.
       gfx::ScopedRTLFlipCanvas scoped_canvas(
           canvas, view->width(), !view->flip_canvas_on_paint_for_rtl_ui());
-      canvas->FillRect(gfx::Rect(0, 0, 1, h),
-                       BorderColor(view, views::Button::STATE_NORMAL));
+      ui::NativeTheme::ExtraParams params;
+      gfx::Rect separator_bounds = gfx::Rect(0, 0, 1, h);

[tapted, 2017/04/11 02:48:06]

The `1` (i.e. width) on this line should be
MenuConfig::instance().separator_thickness.

[Evan Stade, 2017/04/11 17:53:12]

Done.

+      params.menu_separator.paint_rect = &separator_bounds;
+      params.menu_separator.type = ui::VERTICAL_SEPARATOR;
+      view->GetNativeTheme()->Paint(
+          canvas->sk_canvas(), ui::NativeTheme::kMenuPopupSeparator,
+          ui::NativeTheme::kNormal, separator_bounds, params);
       bounds.Inset(gfx::Insets(0, 1, 0, 0));

[tapted, 2017/04/11 02:48:06]

This `1` should be separator_thickness too, so that the background doesn't paint
over the separator.

[Evan Stade, 2017/04/11 17:53:12]

Ick. This is reserving space in DIP, but we may only want to paint a single
pixel for the separator (<1 dip). So it seems like we should paint the bg first
and the separator on top. Only this doesn't work on GTK because the background
isn't a solid color but itself has a border.

I've taken your suggestion for now but we'll have to keep working on this.

[tapted, 2017/04/12 00:31:17]

Acknowledged.

     }
 
     // Fill in background for state.
     bounds.set_x(view->GetMirroredXForRect(bounds));
+    views::Button::ButtonState state =
+        button ? button->state() : views::Button::STATE_NORMAL;
     DrawBackground(canvas, view, bounds, state);
   }
 
  private:
-  static SkColor BorderColor(View* view, views::Button::ButtonState state) {

[Evan Stade, 2017/04/10 21:49:33]

note that this function was only ever called with state == STATE_NORMAL so the
other two colors were never actually used.

-    ui::NativeTheme* theme = view->GetNativeTheme();
-    switch (state) {
-      case views::Button::STATE_HOVERED:
-        return theme->GetSystemColor(
-            ui::NativeTheme::kColorId_HoverMenuButtonBorderColor);
-      case views::Button::STATE_PRESSED:
-        return theme->GetSystemColor(
-            ui::NativeTheme::kColorId_FocusedMenuButtonBorderColor);
-      default:
-        return theme->GetSystemColor(
-            ui::NativeTheme::kColorId_EnabledMenuButtonBorderColor);
-    }
-  }
-
   static SkColor BackgroundColor(const View* view,
                                  views::Button::ButtonState state) {
     const ui::NativeTheme* theme = view->GetNativeTheme();