Use a single views::MenuConfig instance for each platform

Use a single views::MenuConfig instance for each platform

Most noticeably, always use the GTK-style menu layout on Desktop Linux.

Previously, the MenuConfig used by a particular MenuItemView could be
affected by that View's Widget's GetNativeTheme().

This once allowed a menu to decide whether to use a ChromeOS-style menu
or a native Desktop-style menu config on the same platform. But that's
no longer needed, and makes trouble with the "Classic Theme" in Desktop
Linux. (e.g. because menus are not always in a Widget when they are
configured, so the native theme is hard to determine.)

Using a single instance simplifies things and fixes a regression where
the Ash-style was being used on Desktop Linux, resulting in menus
without a border and unexpectedly large item padding. Regression due to
the combination of r344651, r345479, and r360168.

Note this means --open-ash on Desktop Linux, and Metro mode on Windows,
will use non-Ash menus. See screenshots at http://crbug.com/544550#c13

BUG=415953, 544550
Committed: https://crrev.com/5d23f8c18f1d9fbf1655ece40b31d59dea443c60
Cr-Commit-Position: refs/heads/master@{#363924}

Committed: https://crrev.com/b8fc4b06aafee080c1df6766ac4a4b2528895f39
Cr-Commit-Position: refs/heads/master@{#364577}

[tapted, 2015-11-27 03:09:44]

Description was changed from

==========
Always use the GTK-style menu layout on Desktop Linux.

With r360168, Views menus are now consistently using GetNativeTheme()
for theming. However, on Linux,
 - the singleton returned by MenuConfig::instance() is only initialized once,
 - View::GetNativeTheme() depends on the Widget the view is contained in, and
 - MenuItemViews are not always added to a Widget when they require a
MenuConfig.

This causes the MenuConfig initialization to depend on particular
initialization paths, and makes it unsuitable for switching between
Ash-style and GTK-style on Desktop Linux.

Using the Ash-style is OK for most things on Desktop Linux. However, with
the "Classic Theme" set in chrome://settings, and on Window managers that
do not draw window shadows, the menus look funny. This is partly due to
r344651 r345479, which moved the Ash-style, 0-px menu border into
MenuConfig. Meaning that, as well as having CrOS-style padding, menus in
Desktop Linux with the Classic Theme now also have no border at all.

To simplify things, always use GTK-style menu layout on Desktop Linux.

Note this means --open-ash on Desktop Linux will use non-Ash menus.

BUG=415953, 544550
==========

to

==========
Always use the GTK-style menu layout on Desktop Linux.

With r360168, Views menus are now consistently using GetNativeTheme()
for theming. However, on Linux,
 - the singleton returned by MenuConfig::instance() is only initialized once,
 - View::GetNativeTheme() depends on the Widget the view is contained in, and
 - MenuItemViews are not always added to a Widget when they require a
MenuConfig.

This causes the MenuConfig initialization to depend on particular
initialization paths, and makes it unsuitable for switching between
Ash-style and GTK-style on Desktop Linux.

Using the Ash-style is OK for most things on Desktop Linux. However, with
the "Classic Theme" set in chrome://settings, and on Window managers that
do not draw window shadows, the menus look funny. This is partly due to
r344651 r345479, which moved the Ash-style, 0-px menu border into
MenuConfig. Meaning that, as well as having CrOS-style padding, menus in
Desktop Linux with the Classic Theme now also have no border at all.

To simplify things, always use GTK-style menu layout on Desktop Linux.

Note this means --open-ash on Desktop Linux will use non-Ash menus.

BUG=415953, 544550
==========

[tapted, 2015-11-27 03:09:44]

tapted@chromium.org changed reviewers:
+ estade@chromium.org, sky@chromium.org

[tapted, 2015-11-27 03:16:24]

Hi sky and Evan, please take a look

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

https://codereview.chromium.org/1477253002/diff/20001/ui/views/controls/menu/...
File ui/views/controls/menu/menu_config_aura.cc (right):

https://codereview.chromium.org/1477253002/diff/20001/ui/views/controls/menu/...
ui/views/controls/menu/menu_config_aura.cc:42: corner_radius = 2;
Won't this change effect windows too?

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

based on the outcome of the email thread, it seems like we should be good to
remove the NativeTheme argument to MenuConfig (only holdout would be ash
windows, but that's already semi broken and it's deprecated)

https://codereview.chromium.org/1477253002/diff/20001/ui/views/controls/menu/...
File ui/views/controls/menu/menu_config_aura.cc (right):

https://codereview.chromium.org/1477253002/diff/20001/ui/views/controls/menu/...
ui/views/controls/menu/menu_config_aura.cc:17: // On Desktop Linux, always use
the GTK config (MenuConfig member initializers),
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).

https://codereview.chromium.org/1477253002/diff/20001/ui/views/controls/menu/...
ui/views/controls/menu/menu_config_aura.cc:26: void MenuConfig::InitAura(const
ui::NativeTheme* theme) {
|theme| isn't used; remove it?

[tapted, 2015-12-01 04:24:35]

Patchset #3 (id:40001) has been deleted

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

https://codereview.chromium.org/1477253002/diff/20001/ui/views/controls/menu/...
File ui/views/controls/menu/menu_config_aura.cc (right):

https://codereview.chromium.org/1477253002/diff/20001/ui/views/controls/menu/...
ui/views/controls/menu/menu_config_aura.cc:17: // On Desktop Linux, always use
the GTK config (MenuConfig member initializers),
On 2015/11/30 18:44:34, Evan Stade wrote:
> 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).

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".

https://codereview.chromium.org/1477253002/diff/20001/ui/views/controls/menu/...
ui/views/controls/menu/menu_config_aura.cc:26: void MenuConfig::InitAura(const
ui::NativeTheme* theme) {
On 2015/11/30 18:44:34, Evan Stade wrote:
> |theme| isn't used; remove it?

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

https://codereview.chromium.org/1477253002/diff/20001/ui/views/controls/menu/...
ui/views/controls/menu/menu_config_aura.cc:42: corner_radius = 2;
On 2015/11/30 16:54:33, sky wrote:
> Won't this change effect windows too?

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).

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

https://codereview.chromium.org/1477253002/diff/20001/ui/views/controls/menu/...
File ui/views/controls/menu/menu_config_aura.cc (right):

https://codereview.chromium.org/1477253002/diff/20001/ui/views/controls/menu/...
ui/views/controls/menu/menu_config_aura.cc:26: void MenuConfig::InitAura(const
ui::NativeTheme* theme) {
On 2015/12/01 07:35:09, tapted wrote:
> On 2015/11/30 18:44:34, Evan Stade wrote:
> > |theme| isn't used; remove it?
> 
> Done (removed from InitAura - removing from Init() as well is probably better
in
> isolation since it will affect metro mode more seriously).

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.

[tapted, 2015-12-04 06:09:16]

Description was changed from

==========
Always use the GTK-style menu layout on Desktop Linux.

With r360168, Views menus are now consistently using GetNativeTheme()
for theming. However, on Linux,
 - the singleton returned by MenuConfig::instance() is only initialized once,
 - View::GetNativeTheme() depends on the Widget the view is contained in, and
 - MenuItemViews are not always added to a Widget when they require a
MenuConfig.

This causes the MenuConfig initialization to depend on particular
initialization paths, and makes it unsuitable for switching between
Ash-style and GTK-style on Desktop Linux.

Using the Ash-style is OK for most things on Desktop Linux. However, with
the "Classic Theme" set in chrome://settings, and on Window managers that
do not draw window shadows, the menus look funny. This is partly due to
r344651 r345479, which moved the Ash-style, 0-px menu border into
MenuConfig. Meaning that, as well as having CrOS-style padding, menus in
Desktop Linux with the Classic Theme now also have no border at all.

To simplify things, always use GTK-style menu layout on Desktop Linux.

Note this means --open-ash on Desktop Linux will use non-Ash menus.

BUG=415953, 544550
==========

to

==========
Use a single views::MenuConfig instance for each platform

Most noticeably, always use the GTK-style menu layout on Desktop Linux.

Previously, the MenuConfig used by a particular MenuItemView could be
affected by that View's Widget's GetNativeTheme().

This once allowed a menu to decide whether to use a ChromeOS-style menu
or a native Desktop-style menu config on the same platform. But that's
no longer needed, and makes trouble with the "Classic Theme" in Desktop
Linux. (e.g. because menus are not always in a Widget when they are
configured and on Linux, so the native theme is hard to determine.

Using a single instance simplifies things and fixes a regression where
the Ash-style was being used on Desktop Linux, resulting in menus
without a border and unexpectedly large item padding. Regression due to
the combination of r344651, r345479, and r360168.

Note this means --open-ash on Desktop Linux, and Metro mode on Windows,
will use non-Ash menus. See screenshots at http://crbug.com/544550#c13

BUG=415953, 544550
==========

[tapted, 2015-12-07 00:03:45]

On 2015/12/01 18:07:50, Evan Stade wrote:
>
https://codereview.chromium.org/1477253002/diff/20001/ui/views/controls/menu/...
> File ui/views/controls/menu/menu_config_aura.cc (right):
> 
>
https://codereview.chromium.org/1477253002/diff/20001/ui/views/controls/menu/...
> ui/views/controls/menu/menu_config_aura.cc:26: void MenuConfig::InitAura(const
> ui::NativeTheme* theme) {
> On 2015/12/01 07:35:09, tapted wrote:
> > On 2015/11/30 18:44:34, Evan Stade wrote:
> > > |theme| isn't used; remove it?
> > 
> > Done (removed from InitAura - removing from Init() as well is probably
better
> in
> > isolation since it will affect metro mode more seriously).
> 
> 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.

like a bandaid.. "right off"! https://youtu.be/2RVtTluuS1A

I pulled out a few hairs with it - PTAL.

[tapted, 2015-12-07 00:05:25]

Description was changed from

==========
Use a single views::MenuConfig instance for each platform

Most noticeably, always use the GTK-style menu layout on Desktop Linux.

Previously, the MenuConfig used by a particular MenuItemView could be
affected by that View's Widget's GetNativeTheme().

This once allowed a menu to decide whether to use a ChromeOS-style menu
or a native Desktop-style menu config on the same platform. But that's
no longer needed, and makes trouble with the "Classic Theme" in Desktop
Linux. (e.g. because menus are not always in a Widget when they are
configured and on Linux, so the native theme is hard to determine.

Using a single instance simplifies things and fixes a regression where
the Ash-style was being used on Desktop Linux, resulting in menus
without a border and unexpectedly large item padding. Regression due to
the combination of r344651, r345479, and r360168.

Note this means --open-ash on Desktop Linux, and Metro mode on Windows,
will use non-Ash menus. See screenshots at http://crbug.com/544550#c13

BUG=415953, 544550
==========

to

==========
Use a single views::MenuConfig instance for each platform

Most noticeably, always use the GTK-style menu layout on Desktop Linux.

Previously, the MenuConfig used by a particular MenuItemView could be
affected by that View's Widget's GetNativeTheme().

This once allowed a menu to decide whether to use a ChromeOS-style menu
or a native Desktop-style menu config on the same platform. But that's
no longer needed, and makes trouble with the "Classic Theme" in Desktop
Linux. (e.g. because menus are not always in a Widget when they are
configured, so the native theme is hard to determine.)

Using a single instance simplifies things and fixes a regression where
the Ash-style was being used on Desktop Linux, resulting in menus
without a border and unexpectedly large item padding. Regression due to
the combination of r344651, r345479, and r360168.

Note this means --open-ash on Desktop Linux, and Metro mode on Windows,
will use non-Ash menus. See screenshots at http://crbug.com/544550#c13

BUG=415953, 544550
==========

[Evan Stade, 2015-12-07 19:51:42]

excellent. Scott is the owner here but from my perspective this lgtm

https://codereview.chromium.org/1477253002/diff/120001/ui/views/controls/menu...
File ui/views/controls/menu/menu_config.cc (right):

https://codereview.chromium.org/1477253002/diff/120001/ui/views/controls/menu...
ui/views/controls/menu/menu_config.cc:13: menu_vertical_border_size(3),
I think it's fairly confusing to have this common initializer block. It's nigh
impossible to tell which parts of this actually matter to which platforms. Can
we move this to each platform file and only set the defaults that actually
matter (i.e. aren't overridden in init)?

You/someone could also do this as a follow-up if you don't want to make it part
of this CL.

https://codereview.chromium.org/1477253002/diff/120001/ui/views/controls/menu...
File ui/views/controls/menu/menu_item_view.cc (right):

https://codereview.chromium.org/1477253002/diff/120001/ui/views/controls/menu...
ui/views/controls/menu/menu_item_view.cc:64: return MenuConfig::instance();
inline this instead of having a helper?

[sky, 2015-12-07 20:16:14]

LGTM

[tapted, 2015-12-08 11:49:23]

https://codereview.chromium.org/1477253002/diff/120001/ui/views/controls/menu...
File ui/views/controls/menu/menu_config.cc (right):

https://codereview.chromium.org/1477253002/diff/120001/ui/views/controls/menu...
ui/views/controls/menu/menu_config.cc:13: menu_vertical_border_size(3),
On 2015/12/07 19:51:42, Evan Stade wrote:
> I think it's fairly confusing to have this common initializer block. It's nigh
> impossible to tell which parts of this actually matter to which platforms. Can
> we move this to each platform file and only set the defaults that actually
> matter (i.e. aren't overridden in init)?
> 
> You/someone could also do this as a follow-up if you don't want to make it
part
> of this CL.

I think a follow-up is best - since it will need a bit of scrutiny to ensure the
values are moved over correctly.

There might be some nicer things to try too, like `bzero`ing the struct in the
constructor, or using a static initializer once the FontList is moved out.

https://codereview.chromium.org/1477253002/diff/120001/ui/views/controls/menu...
File ui/views/controls/menu/menu_item_view.cc (right):

https://codereview.chromium.org/1477253002/diff/120001/ui/views/controls/menu...
ui/views/controls/menu/menu_item_view.cc:64: return MenuConfig::instance();
On 2015/12/07 19:51:42, Evan Stade wrote:
> inline this instead of having a helper?

Done. I thought it would be a lot of churn, but I tried it out and it's not bad
(net result: a couple more lines removed)

[tapted, 2015-12-08 11:49:30]

The CQ bit was checked by tapted@chromium.org

[tapted, 2015-12-08 11:49:31]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org,
estade@chromium.org
Link to the patchset: https://codereview.chromium.org/1477253002/#ps140001
(title: "remove helper")

[commit-bot: I haz the power, 2015-12-08 11:49:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1477253002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1477253002/140001

[commit-bot: I haz the power, 2015-12-08 12:21:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-08 12:21:23]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Evan Stade, 2015-12-08 20:06:36]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2015-12-08 20:07:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1477253002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1477253002/140001

[commit-bot: I haz the power, 2015-12-09 02:25:11]

Description was changed from

==========
Use a single views::MenuConfig instance for each platform

Most noticeably, always use the GTK-style menu layout on Desktop Linux.

Previously, the MenuConfig used by a particular MenuItemView could be
affected by that View's Widget's GetNativeTheme().

This once allowed a menu to decide whether to use a ChromeOS-style menu
or a native Desktop-style menu config on the same platform. But that's
no longer needed, and makes trouble with the "Classic Theme" in Desktop
Linux. (e.g. because menus are not always in a Widget when they are
configured, so the native theme is hard to determine.)

Using a single instance simplifies things and fixes a regression where
the Ash-style was being used on Desktop Linux, resulting in menus
without a border and unexpectedly large item padding. Regression due to
the combination of r344651, r345479, and r360168.

Note this means --open-ash on Desktop Linux, and Metro mode on Windows,
will use non-Ash menus. See screenshots at http://crbug.com/544550#c13

BUG=415953, 544550
==========

to

==========
Use a single views::MenuConfig instance for each platform

Most noticeably, always use the GTK-style menu layout on Desktop Linux.

Previously, the MenuConfig used by a particular MenuItemView could be
affected by that View's Widget's GetNativeTheme().

This once allowed a menu to decide whether to use a ChromeOS-style menu
or a native Desktop-style menu config on the same platform. But that's
no longer needed, and makes trouble with the "Classic Theme" in Desktop
Linux. (e.g. because menus are not always in a Widget when they are
configured, so the native theme is hard to determine.)

Using a single instance simplifies things and fixes a regression where
the Ash-style was being used on Desktop Linux, resulting in menus
without a border and unexpectedly large item padding. Regression due to
the combination of r344651, r345479, and r360168.

Note this means --open-ash on Desktop Linux, and Metro mode on Windows,
will use non-Ash menus. See screenshots at http://crbug.com/544550#c13

BUG=415953, 544550
==========

[commit-bot: I haz the power, 2015-12-09 02:25:12]

Committed patchset #7 (id:140001)

[commit-bot: I haz the power, 2015-12-09 02:26:05]

Description was changed from

==========
Use a single views::MenuConfig instance for each platform

Most noticeably, always use the GTK-style menu layout on Desktop Linux.

Previously, the MenuConfig used by a particular MenuItemView could be
affected by that View's Widget's GetNativeTheme().

This once allowed a menu to decide whether to use a ChromeOS-style menu
or a native Desktop-style menu config on the same platform. But that's
no longer needed, and makes trouble with the "Classic Theme" in Desktop
Linux. (e.g. because menus are not always in a Widget when they are
configured, so the native theme is hard to determine.)

Using a single instance simplifies things and fixes a regression where
the Ash-style was being used on Desktop Linux, resulting in menus
without a border and unexpectedly large item padding. Regression due to
the combination of r344651, r345479, and r360168.

Note this means --open-ash on Desktop Linux, and Metro mode on Windows,
will use non-Ash menus. See screenshots at http://crbug.com/544550#c13

BUG=415953, 544550
==========

to

==========
Use a single views::MenuConfig instance for each platform

Most noticeably, always use the GTK-style menu layout on Desktop Linux.

Previously, the MenuConfig used by a particular MenuItemView could be
affected by that View's Widget's GetNativeTheme().

This once allowed a menu to decide whether to use a ChromeOS-style menu
or a native Desktop-style menu config on the same platform. But that's
no longer needed, and makes trouble with the "Classic Theme" in Desktop
Linux. (e.g. because menus are not always in a Widget when they are
configured, so the native theme is hard to determine.)

Using a single instance simplifies things and fixes a regression where
the Ash-style was being used on Desktop Linux, resulting in menus
without a border and unexpectedly large item padding. Regression due to
the combination of r344651, r345479, and r360168.

Note this means --open-ash on Desktop Linux, and Metro mode on Windows,
will use non-Ash menus. See screenshots at http://crbug.com/544550#c13

BUG=415953, 544550

Committed: https://crrev.com/5d23f8c18f1d9fbf1655ece40b31d59dea443c60
Cr-Commit-Position: refs/heads/master@{#363924}
==========

[commit-bot: I haz the power, 2015-12-09 02:26:06]

Patchset 7 (id:??) landed as
https://crrev.com/5d23f8c18f1d9fbf1655ece40b31d59dea443c60
Cr-Commit-Position: refs/heads/master@{#363924}

[johnme, 2015-12-10 15:08:45]

A revert of this CL (patchset #7 id:140001) has been created in
https://codereview.chromium.org/1514693006/ by johnme@chromium.org.

The reason for reverting is: This broke compilation on
https://build.chromium.org/p/chromium.android/builders/Android%20Aura%20Build...
(the patch landed in an earlier build, but was masked by another compile failure
:-|):

../../ui/views/controls/menu/menu_config.cc:45: error: undefined reference to
'views::MenuConfig::Init()'.

[tapted, 2015-12-10 22:52:50]

Description was changed from

==========
Use a single views::MenuConfig instance for each platform

Most noticeably, always use the GTK-style menu layout on Desktop Linux.

Previously, the MenuConfig used by a particular MenuItemView could be
affected by that View's Widget's GetNativeTheme().

This once allowed a menu to decide whether to use a ChromeOS-style menu
or a native Desktop-style menu config on the same platform. But that's
no longer needed, and makes trouble with the "Classic Theme" in Desktop
Linux. (e.g. because menus are not always in a Widget when they are
configured, so the native theme is hard to determine.)

Using a single instance simplifies things and fixes a regression where
the Ash-style was being used on Desktop Linux, resulting in menus
without a border and unexpectedly large item padding. Regression due to
the combination of r344651, r345479, and r360168.

Note this means --open-ash on Desktop Linux, and Metro mode on Windows,
will use non-Ash menus. See screenshots at http://crbug.com/544550#c13

BUG=415953, 544550

Committed: https://crrev.com/5d23f8c18f1d9fbf1655ece40b31d59dea443c60
Cr-Commit-Position: refs/heads/master@{#363924}
==========

to

==========
Use a single views::MenuConfig instance for each platform

Most noticeably, always use the GTK-style menu layout on Desktop Linux.

Previously, the MenuConfig used by a particular MenuItemView could be
affected by that View's Widget's GetNativeTheme().

This once allowed a menu to decide whether to use a ChromeOS-style menu
or a native Desktop-style menu config on the same platform. But that's
no longer needed, and makes trouble with the "Classic Theme" in Desktop
Linux. (e.g. because menus are not always in a Widget when they are
configured, so the native theme is hard to determine.)

Using a single instance simplifies things and fixes a regression where
the Ash-style was being used on Desktop Linux, resulting in menus
without a border and unexpectedly large item padding. Regression due to
the combination of r344651, r345479, and r360168.

Note this means --open-ash on Desktop Linux, and Metro mode on Windows,
will use non-Ash menus. See screenshots at http://crbug.com/544550#c13

BUG=415953, 544550

Committed: https://crrev.com/5d23f8c18f1d9fbf1655ece40b31d59dea443c60
Cr-Commit-Position: refs/heads/master@{#363924}
==========

[tapted, 2015-12-11 00:16:36]

On 2015/12/10 15:08:45, johnme wrote:
> A revert of this CL (patchset #7 id:140001) has been created in
> https://codereview.chromium.org/1514693006/ by mailto:johnme@chromium.org.
> 
> The reason for reverting is: This broke compilation on
>
https://build.chromium.org/p/chromium.android/builders/Android%20Aura%20Build...

views on android! Mind: blown. These are exciting times :)

Added menu_config_android.cc and double-checked with the android_aura_compile
trybot - all checks out. It's a simple fix (and I think there are CLs blocked on
this) - happy to address any concerns you raise in a follow-up. Thanks!

[tapted, 2015-12-11 00:17:23]

The CQ bit was checked by tapted@chromium.org

[tapted, 2015-12-11 00:17:24]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org,
estade@chromium.org
Link to the patchset: https://codereview.chromium.org/1477253002/#ps180001
(title: "Add menu_config_android")

[commit-bot: I haz the power, 2015-12-11 00:20:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1477253002/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1477253002/180001

[Evan Stade, 2015-12-11 00:22:03]

not an owner but lgtm again

[sky, 2015-12-11 00:41:14]

SLGTM

[commit-bot: I haz the power, 2015-12-11 01:51:58]

Description was changed from

==========
Use a single views::MenuConfig instance for each platform

Most noticeably, always use the GTK-style menu layout on Desktop Linux.

Previously, the MenuConfig used by a particular MenuItemView could be
affected by that View's Widget's GetNativeTheme().

This once allowed a menu to decide whether to use a ChromeOS-style menu
or a native Desktop-style menu config on the same platform. But that's
no longer needed, and makes trouble with the "Classic Theme" in Desktop
Linux. (e.g. because menus are not always in a Widget when they are
configured, so the native theme is hard to determine.)

Using a single instance simplifies things and fixes a regression where
the Ash-style was being used on Desktop Linux, resulting in menus
without a border and unexpectedly large item padding. Regression due to
the combination of r344651, r345479, and r360168.

Note this means --open-ash on Desktop Linux, and Metro mode on Windows,
will use non-Ash menus. See screenshots at http://crbug.com/544550#c13

BUG=415953, 544550

Committed: https://crrev.com/5d23f8c18f1d9fbf1655ece40b31d59dea443c60
Cr-Commit-Position: refs/heads/master@{#363924}
==========

to

==========
Use a single views::MenuConfig instance for each platform

Most noticeably, always use the GTK-style menu layout on Desktop Linux.

Previously, the MenuConfig used by a particular MenuItemView could be
affected by that View's Widget's GetNativeTheme().

This once allowed a menu to decide whether to use a ChromeOS-style menu
or a native Desktop-style menu config on the same platform. But that's
no longer needed, and makes trouble with the "Classic Theme" in Desktop
Linux. (e.g. because menus are not always in a Widget when they are
configured, so the native theme is hard to determine.)

Using a single instance simplifies things and fixes a regression where
the Ash-style was being used on Desktop Linux, resulting in menus
without a border and unexpectedly large item padding. Regression due to
the combination of r344651, r345479, and r360168.

Note this means --open-ash on Desktop Linux, and Metro mode on Windows,
will use non-Ash menus. See screenshots at http://crbug.com/544550#c13

BUG=415953, 544550

Committed: https://crrev.com/5d23f8c18f1d9fbf1655ece40b31d59dea443c60
Cr-Commit-Position: refs/heads/master@{#363924}
==========

[commit-bot: I haz the power, 2015-12-11 01:51:58]

Committed patchset #9 (id:180001)

[commit-bot: I haz the power, 2015-12-11 01:52:41]

Description was changed from

==========
Use a single views::MenuConfig instance for each platform

Most noticeably, always use the GTK-style menu layout on Desktop Linux.

Previously, the MenuConfig used by a particular MenuItemView could be
affected by that View's Widget's GetNativeTheme().

This once allowed a menu to decide whether to use a ChromeOS-style menu
or a native Desktop-style menu config on the same platform. But that's
no longer needed, and makes trouble with the "Classic Theme" in Desktop
Linux. (e.g. because menus are not always in a Widget when they are
configured, so the native theme is hard to determine.)

Using a single instance simplifies things and fixes a regression where
the Ash-style was being used on Desktop Linux, resulting in menus
without a border and unexpectedly large item padding. Regression due to
the combination of r344651, r345479, and r360168.

Note this means --open-ash on Desktop Linux, and Metro mode on Windows,
will use non-Ash menus. See screenshots at http://crbug.com/544550#c13

BUG=415953, 544550

Committed: https://crrev.com/5d23f8c18f1d9fbf1655ece40b31d59dea443c60
Cr-Commit-Position: refs/heads/master@{#363924}
==========

to

==========
Use a single views::MenuConfig instance for each platform

Most noticeably, always use the GTK-style menu layout on Desktop Linux.

Previously, the MenuConfig used by a particular MenuItemView could be
affected by that View's Widget's GetNativeTheme().

This once allowed a menu to decide whether to use a ChromeOS-style menu
or a native Desktop-style menu config on the same platform. But that's
no longer needed, and makes trouble with the "Classic Theme" in Desktop
Linux. (e.g. because menus are not always in a Widget when they are
configured, so the native theme is hard to determine.)

Using a single instance simplifies things and fixes a regression where
the Ash-style was being used on Desktop Linux, resulting in menus
without a border and unexpectedly large item padding. Regression due to
the combination of r344651, r345479, and r360168.

Note this means --open-ash on Desktop Linux, and Metro mode on Windows,
will use non-Ash menus. See screenshots at http://crbug.com/544550#c13

BUG=415953, 544550

Committed: https://crrev.com/5d23f8c18f1d9fbf1655ece40b31d59dea443c60
Cr-Commit-Position: refs/heads/master@{#363924}

Committed: https://crrev.com/b8fc4b06aafee080c1df6766ac4a4b2528895f39
Cr-Commit-Position: refs/heads/master@{#364577}
==========

[commit-bot: I haz the power, 2015-12-11 01:52:42]

Patchset 9 (id:??) landed as
https://crrev.com/b8fc4b06aafee080c1df6766ac4a4b2528895f39
Cr-Commit-Position: refs/heads/master@{#364577}