Use a single views::MenuConfig instance for each platform

ui/views/controls/menu/menu_config_aura.cc

 // Copyright 2014 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 "ui/views/controls/menu/menu_config.h"
 
-#include "ui/base/layout.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/image/image_skia.h"
-#include "ui/native_theme/native_theme_aura.h"
 #include "ui/resources/grit/ui_resources.h"
 #include "ui/views/controls/menu/menu_image_util.h"
-#include "ui/views/resources/grit/views_resources.h"
 
 namespace views {
 
-namespace {
-#if defined(OS_CHROMEOS)
-static const int kMenuCornerRadiusForAura = 2;
-#else
-static const int kMenuCornerRadiusForAura = 0;
-#endif
-}  // namespace
-
 #if !defined(OS_WIN)
 void MenuConfig::Init(const ui::NativeTheme* theme) {
-  if (theme == ui::NativeThemeAura::instance())
-    InitAura(theme);
+// On Desktop Linux, always use the GTK config (MenuConfig member initializers),

[Evan Stade, 2015/11/30 18:44:34]

are you sure the MenuConfig member initializers have anything to do with GTK? I
thought they were picked to match Windows (although menu_config_win overwrites
some of them).

[tapted, 2015/12/01 07:35:09]

Hm - that was the way I was reasoning about it, since it was
Gtk2UI::GetNativeTheme() returning an instance of NativeThemeGtk2::instance()
rather than ui::NativeThemeAura::instance() that was causing the InitAura call
to be skipped on Desktop Linux. But the condition causing that is gone now so
it's a bit confusing to mention GTK.

Changed this to say "always use the same config".

+// even for "Classic mode" on Linux (chrome://settings/#themes-reset).
+// Note this causes --open-ash on Linux to use the Desktop Linux style as well.
+#if defined(OS_CHROMEOS)
+  InitAura(theme);
+#endif
 }
-#endif
+#endif  // OS_WIN
 
 void MenuConfig::InitAura(const ui::NativeTheme* theme) {

[Evan Stade, 2015/11/30 18:44:34]

|theme| isn't used; remove it?

[tapted, 2015/12/01 07:35:09]

Done (removed from InitAura - removing from Init() as well is probably better in
isolation since it will affect metro mode more seriously).

[Evan Stade, 2015/12/01 18:07:50]

I think it's ok to peel the bandaid off all at once. Metro mode is deprecated
and being removed very soon, plus menus are already half broken in metro mode
and it's been decided they're not worth fixing.

   submenu_horizontal_inset = 1;
   arrow_to_edge_padding = 20;
   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
   arrow_width = rb.GetImageNamed(IDR_MENU_HIERARCHY_ARROW).Width();
   gfx::ImageSkia check = GetMenuCheckImage(false);
   check_height = check.height();
   item_min_height = 29;
   separator_spacing_height = 7;
   separator_lower_height = 8;
   separator_upper_height = 8;
   label_to_arrow_padding = 20;
   label_to_minor_text_padding = 20;
   always_use_icon_to_label_padding = true;
   align_arrow_and_shortcut = true;
   offset_context_menus = true;
-  corner_radius = kMenuCornerRadiusForAura;
+  corner_radius = 2;

[sky, 2015/11/30 16:54:33]

Won't this change effect windows too?

[tapted, 2015/12/01 07:35:09]

Ah, yes - it would change menus in Ash/Metro, which wasn't the intention. I've
reverted this bit.

(I'm pretty sure this config doesn't currently "leak" outside of metro mode on
Windows [otherwise the use_outer_border line below would be causing problems..],
so the situation could be simplified further OS_WIN, but probably not just yet).

 
   // In Ash, the border is provided by the shadow.
   use_outer_border = false;
 }
 
 #if !defined(OS_WIN)
 // static
 const MenuConfig& MenuConfig::instance(const ui::NativeTheme* theme) {
-  CR_DEFINE_STATIC_LOCAL(MenuConfig, instance,
-                         (theme ? theme : ui::NativeThemeAura::instance()));
+  CR_DEFINE_STATIC_LOCAL(MenuConfig, instance, (theme));
   return instance;
 }
 #endif
 
 }  // namespace views