Relatedly: 

 crbug.com/837780  – corner radii on combo box menus
 crbug.com/829347  – macviews implementation

Seems like this is P2 rather than P3.

kylixrd: can you give a brief description of what's required to turn this on? Is there any reason it wasn't turned on earlier that we should be aware of?

I found src/ui/views/controls/menu/menu_config* and see this in _mac.mm:

if (ui::MaterialDesignController::IsSecondaryUiMaterial())
  InitMaterialMenuConfig(this);

Maybe something like InitMaterialMenuConfig should be done for the other platforms - but is nothing like that already done elsewhere as part of MD implementation?  This might not be a simple matter of throwing a switch?

Sending this over to kylixrd@

bettes@, is this what we really want to do? The menus as configured by InitMaterialMenuConfig() don't look very good.

Current Windows menu on left, Material menu on right.

Deleted: MaterialMenus.png 17.6 KB

	MaterialMenus.png 17.6 KB View Download

This is worse... turning on the border also turns on the corner radii. However the shadows don't paint correctly on the top left and bottom right.

Deleted: MaterialMenuWithBorder.png 8.7 KB

	MaterialMenuWithBorder.png 8.7 KB View Download

Look on Mac with CL 1159532 applied.

Deleted: Screen Shot 2018-08-02 at 11.10.43 AM.png 39.6 KB

	Screen Shot 2018-08-02 at 11.10.43 AM.png 39.6 KB View Download

It's probably best to have ENG own this. Marking as Available. 

Re c9 and 10: It might be a bit (or a lot) jarring at first, but we should evaluate this as a whole before passing too much judgement: 
- proper corner radius
- ensuring shadows paint to corners 
- proper line height
- proper menu width*


*from ellyjones: the width of menus is the greater of 320 or the width required to show their contents except for combobox menus. 

The current mac menu is >320, but ideally it's 320. There's nothing in it's contents to suggest otherwise to me. 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9c5131ad426872a1c931a6e3a27bbd4c469b7721

commit 9c5131ad426872a1c931a6e3a27bbd4c469b7721
Author: Allen Bauer <kylixrd@chromium.org>
Date: Wed Sep 12 17:08:01 2018

Under refresh, use the same menu options for Windows/Linux as under Mac

Use the BubbleBorder for the menus since the drop-shadow under Windows
doesn't track the shape of the menu. We also have much more control over
the shape and depth of the drop-shadow.

Bug: 837782
Change-Id: I91b9340251c2cd0b60e3b3f66d450a4791113007
Reviewed-on: https://chromium-review.googlesource.com/1159532
Commit-Queue: Allen Bauer <kylixrd@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590731}
[modify] https://crrev.com/9c5131ad426872a1c931a6e3a27bbd4c469b7721/chrome/browser/ui/window_sizer/window_sizer_ash_uitest.cc
[modify] https://crrev.com/9c5131ad426872a1c931a6e3a27bbd4c469b7721/ui/views/controls/menu/menu_config.cc
[modify] https://crrev.com/9c5131ad426872a1c931a6e3a27bbd4c469b7721/ui/views/controls/menu/menu_config.h
[modify] https://crrev.com/9c5131ad426872a1c931a6e3a27bbd4c469b7721/ui/views/controls/menu/menu_config_chromeos.cc
[modify] https://crrev.com/9c5131ad426872a1c931a6e3a27bbd4c469b7721/ui/views/controls/menu/menu_config_linux.cc
[modify] https://crrev.com/9c5131ad426872a1c931a6e3a27bbd4c469b7721/ui/views/controls/menu/menu_config_mac.mm
[modify] https://crrev.com/9c5131ad426872a1c931a6e3a27bbd4c469b7721/ui/views/controls/menu/menu_config_win.cc
[modify] https://crrev.com/9c5131ad426872a1c931a6e3a27bbd4c469b7721/ui/views/controls/menu/menu_controller.cc
[modify] https://crrev.com/9c5131ad426872a1c931a6e3a27bbd4c469b7721/ui/views/controls/menu/menu_controller_unittest.cc
[modify] https://crrev.com/9c5131ad426872a1c931a6e3a27bbd4c469b7721/ui/views/controls/menu/menu_item_view_unittest.cc
[modify] https://crrev.com/9c5131ad426872a1c931a6e3a27bbd4c469b7721/ui/views/controls/menu/menu_scroll_view_container.cc
[modify] https://crrev.com/9c5131ad426872a1c931a6e3a27bbd4c469b7721/ui/views/controls/menu/menu_scroll_view_container.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f741847f8f4d1cf317b62e26fc8b1a54df9bb568

commit f741847f8f4d1cf317b62e26fc8b1a54df9bb568
Author: Adithya Srinivasan <adithyas@chromium.org>
Date: Thu Sep 13 15:47:40 2018

Revert "Under refresh, use the same menu options for Windows/Linux as under Mac"

This reverts commit 9c5131ad426872a1c931a6e3a27bbd4c469b7721.

Reason for revert: Speculative revert, might be causing flakiness in ookmarkBarViewTest24.ContextMenusKeyboardEscape. See  https://crbug.com/883600 .
 
Original change's description:
> Under refresh, use the same menu options for Windows/Linux as under Mac
> 
> Use the BubbleBorder for the menus since the drop-shadow under Windows
> doesn't track the shape of the menu. We also have much more control over
> the shape and depth of the drop-shadow.
> 
> Bug: 837782
> Change-Id: I91b9340251c2cd0b60e3b3f66d450a4791113007
> Reviewed-on: https://chromium-review.googlesource.com/1159532
> Commit-Queue: Allen Bauer <kylixrd@chromium.org>
> Reviewed-by: Evan Stade <estade@chromium.org>
> Reviewed-by: Michael Wasserman <msw@chromium.org>
> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#590731}

TBR=ellyjones@chromium.org,msw@chromium.org,estade@chromium.org,kylixrd@chromium.org

Change-Id: Ic4cc5a69d1cbd48041ae4c3305846efe414e0511
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 837782
Reviewed-on: https://chromium-review.googlesource.com/1224950
Reviewed-by: Adithya Srinivasan <adithyas@chromium.org>
Commit-Queue: Adithya Srinivasan <adithyas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591027}
[modify] https://crrev.com/f741847f8f4d1cf317b62e26fc8b1a54df9bb568/chrome/browser/ui/window_sizer/window_sizer_ash_uitest.cc
[modify] https://crrev.com/f741847f8f4d1cf317b62e26fc8b1a54df9bb568/ui/views/controls/menu/menu_config.cc
[modify] https://crrev.com/f741847f8f4d1cf317b62e26fc8b1a54df9bb568/ui/views/controls/menu/menu_config.h
[modify] https://crrev.com/f741847f8f4d1cf317b62e26fc8b1a54df9bb568/ui/views/controls/menu/menu_config_chromeos.cc
[modify] https://crrev.com/f741847f8f4d1cf317b62e26fc8b1a54df9bb568/ui/views/controls/menu/menu_config_linux.cc
[modify] https://crrev.com/f741847f8f4d1cf317b62e26fc8b1a54df9bb568/ui/views/controls/menu/menu_config_mac.mm
[modify] https://crrev.com/f741847f8f4d1cf317b62e26fc8b1a54df9bb568/ui/views/controls/menu/menu_config_win.cc
[modify] https://crrev.com/f741847f8f4d1cf317b62e26fc8b1a54df9bb568/ui/views/controls/menu/menu_controller.cc
[modify] https://crrev.com/f741847f8f4d1cf317b62e26fc8b1a54df9bb568/ui/views/controls/menu/menu_controller_unittest.cc
[modify] https://crrev.com/f741847f8f4d1cf317b62e26fc8b1a54df9bb568/ui/views/controls/menu/menu_item_view_unittest.cc
[modify] https://crrev.com/f741847f8f4d1cf317b62e26fc8b1a54df9bb568/ui/views/controls/menu/menu_scroll_view_container.cc
[modify] https://crrev.com/f741847f8f4d1cf317b62e26fc8b1a54df9bb568/ui/views/controls/menu/menu_scroll_view_container.h

I think the CL in #26 made it to canary and changed the drop shadow of menus on Mac. IMO they feel a bit out-of-place since there's very little visual separation between the menu and the underlying window.

Yes, confirming c#29 from rsesek@.

Please find attached two screenshots (actual vs expected).

Deleted: expected_with_shadow.png 43.4 KB

	expected_with_shadow.png 43.4 KB View Download

Deleted: actual_no_shadow.png 41.8 KB

	actual_no_shadow.png 41.8 KB View Download

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1922e955fae5e266603059a97356ff8af058f049

commit 1922e955fae5e266603059a97356ff8af058f049
Author: Allen Bauer <kylixrd@chromium.org>
Date: Tue Sep 18 19:49:36 2018

Reland "Under refresh, use the same menu options for Windows/Linux as under Mac"

This is a reland of 9c5131ad426872a1c931a6e3a27bbd4c469b7721

Cannot reproduce the flake. Relanding for more information.

Original change's description:
> Under refresh, use the same menu options for Windows/Linux as under Mac
>
> Use the BubbleBorder for the menus since the drop-shadow under Windows
> doesn't track the shape of the menu. We also have much more control over
> the shape and depth of the drop-shadow.
>
> Bug: 837782
> Change-Id: I91b9340251c2cd0b60e3b3f66d450a4791113007
> Reviewed-on: https://chromium-review.googlesource.com/1159532
> Commit-Queue: Allen Bauer <kylixrd@chromium.org>
> Reviewed-by: Evan Stade <estade@chromium.org>
> Reviewed-by: Michael Wasserman <msw@chromium.org>
> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#590731}

TBR=ellyjones@chromium.org
TBR=msw@chromium.org
TBR=estade@chromium.org

Bug: 837782
Change-Id: I80a07fb2d716c9372d18a8d48df3e7e00ff286ab
Reviewed-on: https://chromium-review.googlesource.com/1229173
Commit-Queue: Allen Bauer <kylixrd@chromium.org>
Reviewed-by: Allen Bauer <kylixrd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592156}
[modify] https://crrev.com/1922e955fae5e266603059a97356ff8af058f049/chrome/browser/ui/window_sizer/window_sizer_ash_uitest.cc
[modify] https://crrev.com/1922e955fae5e266603059a97356ff8af058f049/ui/views/controls/menu/menu_config.cc
[modify] https://crrev.com/1922e955fae5e266603059a97356ff8af058f049/ui/views/controls/menu/menu_config.h
[modify] https://crrev.com/1922e955fae5e266603059a97356ff8af058f049/ui/views/controls/menu/menu_config_chromeos.cc
[modify] https://crrev.com/1922e955fae5e266603059a97356ff8af058f049/ui/views/controls/menu/menu_config_linux.cc
[modify] https://crrev.com/1922e955fae5e266603059a97356ff8af058f049/ui/views/controls/menu/menu_config_mac.mm
[modify] https://crrev.com/1922e955fae5e266603059a97356ff8af058f049/ui/views/controls/menu/menu_config_win.cc
[modify] https://crrev.com/1922e955fae5e266603059a97356ff8af058f049/ui/views/controls/menu/menu_controller.cc
[modify] https://crrev.com/1922e955fae5e266603059a97356ff8af058f049/ui/views/controls/menu/menu_controller_unittest.cc
[modify] https://crrev.com/1922e955fae5e266603059a97356ff8af058f049/ui/views/controls/menu/menu_item_view_unittest.cc
[modify] https://crrev.com/1922e955fae5e266603059a97356ff8af058f049/ui/views/controls/menu/menu_scroll_view_container.cc
[modify] https://crrev.com/1922e955fae5e266603059a97356ff8af058f049/ui/views/controls/menu/menu_scroll_view_container.h

RE: #30

Assigning to bettes@ for comment regarding the shadow depth. Please advise on what the proper spec should be.

https://docs.google.com/presentation/d/1EO7TOpIMJ7QHjaTVw9St-q6naKwtXX2TwzMirG5EsKY/edit?pli=1#slide=id.g36ad6489af_0_34

The padding for context menus for ChromeOS App icons (in the shelf and launcher) were broken, bisected to the Reland. 

This is because we need |touchable_anchor_offset| to be 8 in ChromeOS when the menu is touchable (controller->use_touchable_layout()).

Since InitMaterialMenuConfig() cannot get the controller, we should find a better way to set this variable.

This change also breaks the shadows for the touchable app menus.

+abodenha after talking to sgabriel about other times when blanket modifications have broken chromeOS components in ways not predicted.

In this case, the change modifies variables that are only (to my knowledge, because I created the variables) used for ChromeOS touchable app menus (menus for app icons in the launcher and shelf).

Sgabriel@ said you were talking about other times where MD changes have effected CrOS in ways not intended. We thought you should see this too.

Thanks for looping me in.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/869ed2c00969dc8feefa8525eb7e56b0c9097bb5

commit 869ed2c00969dc8feefa8525eb7e56b0c9097bb5
Author: Thomas Anderson <thomasanderson@chromium.org>
Date: Mon Sep 24 19:55:22 2018

Revert "Reland "Under refresh, use the same menu options for Windows/Linux as under Mac""

This reverts commit 1922e955fae5e266603059a97356ff8af058f049.

Reason for revert: Causing  bug 887424 

Original change's description:
> Reland "Under refresh, use the same menu options for Windows/Linux as under Mac"
>
> This is a reland of 9c5131ad426872a1c931a6e3a27bbd4c469b7721
>
> Cannot reproduce the flake. Relanding for more information.
>
> Original change's description:
> > Under refresh, use the same menu options for Windows/Linux as under Mac
> >
> > Use the BubbleBorder for the menus since the drop-shadow under Windows
> > doesn't track the shape of the menu. We also have much more control over
> > the shape and depth of the drop-shadow.
> >
> > Bug: 837782
> > Change-Id: I91b9340251c2cd0b60e3b3f66d450a4791113007
> > Reviewed-on: https://chromium-review.googlesource.com/1159532
> > Commit-Queue: Allen Bauer <kylixrd@chromium.org>
> > Reviewed-by: Evan Stade <estade@chromium.org>
> > Reviewed-by: Michael Wasserman <msw@chromium.org>
> > Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#590731}
>
> TBR=ellyjones@chromium.org
> TBR=msw@chromium.org
> TBR=estade@chromium.org
>
> Bug: 837782
> Change-Id: I80a07fb2d716c9372d18a8d48df3e7e00ff286ab
> Reviewed-on: https://chromium-review.googlesource.com/1229173
> Commit-Queue: Allen Bauer <kylixrd@chromium.org>
> Reviewed-by: Allen Bauer <kylixrd@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#592156}

TBR=ellyjones@chromium.org,msw@chromium.org,estade@chromium.org,kylixrd@chromium.org
NOTRY=true
NOPRESUBMIT=true

Bug: 837782
Change-Id: Ie834df815bd1fb1d0f49d7ddc84adecf75ea351e
Reviewed-on: https://chromium-review.googlesource.com/1240733
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593651}
[modify] https://crrev.com/869ed2c00969dc8feefa8525eb7e56b0c9097bb5/chrome/browser/ui/window_sizer/window_sizer_ash_uitest.cc
[modify] https://crrev.com/869ed2c00969dc8feefa8525eb7e56b0c9097bb5/ui/views/controls/menu/menu_config.cc
[modify] https://crrev.com/869ed2c00969dc8feefa8525eb7e56b0c9097bb5/ui/views/controls/menu/menu_config.h
[modify] https://crrev.com/869ed2c00969dc8feefa8525eb7e56b0c9097bb5/ui/views/controls/menu/menu_config_chromeos.cc
[modify] https://crrev.com/869ed2c00969dc8feefa8525eb7e56b0c9097bb5/ui/views/controls/menu/menu_config_linux.cc
[modify] https://crrev.com/869ed2c00969dc8feefa8525eb7e56b0c9097bb5/ui/views/controls/menu/menu_config_mac.mm
[modify] https://crrev.com/869ed2c00969dc8feefa8525eb7e56b0c9097bb5/ui/views/controls/menu/menu_config_win.cc
[modify] https://crrev.com/869ed2c00969dc8feefa8525eb7e56b0c9097bb5/ui/views/controls/menu/menu_controller.cc
[modify] https://crrev.com/869ed2c00969dc8feefa8525eb7e56b0c9097bb5/ui/views/controls/menu/menu_controller_unittest.cc
[modify] https://crrev.com/869ed2c00969dc8feefa8525eb7e56b0c9097bb5/ui/views/controls/menu/menu_item_view_unittest.cc
[modify] https://crrev.com/869ed2c00969dc8feefa8525eb7e56b0c9097bb5/ui/views/controls/menu/menu_scroll_view_container.cc
[modify] https://crrev.com/869ed2c00969dc8feefa8525eb7e56b0c9097bb5/ui/views/controls/menu/menu_scroll_view_container.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cc2d59606974db2ab66d96b206a56b0bc5e1f362

commit cc2d59606974db2ab66d96b206a56b0bc5e1f362
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Mon Sep 24 20:07:17 2018

[Merge to 3559] Revert "Reland "Under refresh, use the same menu options for Windows/Linux as under Mac""

> This reverts commit 1922e955fae5e266603059a97356ff8af058f049.
>
> Reason for revert: Causing  bug 887424 
>
> Original change's description:
> > Reland "Under refresh, use the same menu options for Windows/Linux as under Mac"
> >
> > This is a reland of 9c5131ad426872a1c931a6e3a27bbd4c469b7721
> >
> > Cannot reproduce the flake. Relanding for more information.
> >
> > Original change's description:
> > > Under refresh, use the same menu options for Windows/Linux as under Mac
> > >
> > > Use the BubbleBorder for the menus since the drop-shadow under Windows
> > > doesn't track the shape of the menu. We also have much more control over
> > > the shape and depth of the drop-shadow.
> > >
> > > Bug: 837782
> > > Change-Id: I91b9340251c2cd0b60e3b3f66d450a4791113007
> > > Reviewed-on: https://chromium-review.googlesource.com/1159532
> > > Commit-Queue: Allen Bauer <kylixrd@chromium.org>
> > > Reviewed-by: Evan Stade <estade@chromium.org>
> > > Reviewed-by: Michael Wasserman <msw@chromium.org>
> > > Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#590731}
> >
> > TBR=ellyjones@chromium.org
> > TBR=msw@chromium.org
> > TBR=estade@chromium.org
> >
> > Bug: 837782
> > Change-Id: I80a07fb2d716c9372d18a8d48df3e7e00ff286ab
> > Reviewed-on: https://chromium-review.googlesource.com/1229173
> > Commit-Queue: Allen Bauer <kylixrd@chromium.org>
> > Reviewed-by: Allen Bauer <kylixrd@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#592156}
>
> TBR=ellyjones@chromium.org,msw@chromium.org,estade@chromium.org,kylixrd@chromium.org
> NOTRY=true
> NOPRESUBMIT=true
>
> Bug: 837782
> Change-Id: Ie834df815bd1fb1d0f49d7ddc84adecf75ea351e
> Reviewed-on: https://chromium-review.googlesource.com/1240733
> Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
> Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#593651}

TBR=ellyjones@chromium.org,msw@chromium.org,estade@chromium.org,kylixrd@chromium.org
BUG= 887424 
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true

Change-Id: If61535acd638674e6ae4d3e0f7d6850be9e6f8cc
Reviewed-on: https://chromium-review.googlesource.com/1241234
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/branch-heads/3559@{#9}
Cr-Branched-From: b522cfef32adcfdedf00a6e03362101db42bd80a-refs/heads/master@{#593399}
[modify] https://crrev.com/cc2d59606974db2ab66d96b206a56b0bc5e1f362/chrome/browser/ui/window_sizer/window_sizer_ash_uitest.cc
[modify] https://crrev.com/cc2d59606974db2ab66d96b206a56b0bc5e1f362/ui/views/controls/menu/menu_config.cc
[modify] https://crrev.com/cc2d59606974db2ab66d96b206a56b0bc5e1f362/ui/views/controls/menu/menu_config.h
[modify] https://crrev.com/cc2d59606974db2ab66d96b206a56b0bc5e1f362/ui/views/controls/menu/menu_config_chromeos.cc
[modify] https://crrev.com/cc2d59606974db2ab66d96b206a56b0bc5e1f362/ui/views/controls/menu/menu_config_linux.cc
[modify] https://crrev.com/cc2d59606974db2ab66d96b206a56b0bc5e1f362/ui/views/controls/menu/menu_config_mac.mm
[modify] https://crrev.com/cc2d59606974db2ab66d96b206a56b0bc5e1f362/ui/views/controls/menu/menu_config_win.cc
[modify] https://crrev.com/cc2d59606974db2ab66d96b206a56b0bc5e1f362/ui/views/controls/menu/menu_controller.cc
[modify] https://crrev.com/cc2d59606974db2ab66d96b206a56b0bc5e1f362/ui/views/controls/menu/menu_controller_unittest.cc
[modify] https://crrev.com/cc2d59606974db2ab66d96b206a56b0bc5e1f362/ui/views/controls/menu/menu_item_view_unittest.cc
[modify] https://crrev.com/cc2d59606974db2ab66d96b206a56b0bc5e1f362/ui/views/controls/menu/menu_scroll_view_container.cc
[modify] https://crrev.com/cc2d59606974db2ab66d96b206a56b0bc5e1f362/ui/views/controls/menu/menu_scroll_view_container.h

Reassigning to kylixrd@ who's working on this.