Pls mark the bug as fixed if CL is landed in trunk and nothing else is pending. Thank you.

Well, the good news is that the menus now aren't aligned in either mode.

I'll have a look at this maybe for M70.

The bug turned out to be here in MenuController::CalculateMenuBounds():

    int border_size = menu_config.CornerRadiusForMenu(this);
    if (!border_size)
      border_size = menu_config.menu_vertical_border_size;
    menu_bounds.set_y(item_loc.y() - border_size);

This is wrong for menus that have an outer border in addition to the corner radius. The easiest fix is to simply ask the existing menu what its top inset is instead of recomputing it :)

Fix: https://chromium-review.googlesource.com/c/chromium/src/+/1231736

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

commit 8b1391794f25f6e7363771a43c1988b3c5b8e5a5
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Wed Sep 19 13:03:46 2018

views: use menu border inset to position submenus

Using the border inset directly, instead of trying to recompute it from the
MenuConfig, avoids positioning bugs when the submenu will have a border applied
to it.

Bug:  834934 
Change-Id: Ic6c23f7dd5e34b643724c346bb89dac038dd58e4
Reviewed-on: https://chromium-review.googlesource.com/1231736
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592377}
[modify] https://crrev.com/8b1391794f25f6e7363771a43c1988b3c5b8e5a5/ui/views/controls/menu/menu_controller.cc

Hey ellyjones@, I checked latest Snapshot (#592398) and it seems that your change breaks the Normal and the Increase Contrast Mode now :( 

Screenshots are attached. You can see in both screenshots that the submenu is too low in both modes now.

Deleted: Normal.png 40.1 KB

	Normal.png 40.1 KB View Download

Deleted: Increase Contrast.png 49.8 KB

	Increase Contrast.png 49.8 KB View Download

#11: That's... odd...

It looks fixed to me locally at latest canary (71.0.3556.0). Can you test with 71.0.3556.0?

Yes, in latest canary (71.0.3556.0) it is looking fixed to me in both modes. But from https://chromium.googlesource.com/chromium/src/+log/71.0.3555.0..71.0.3556.0?pretty=fuller&n=10000 your CL is not in that range (cut at #592301).

It is now broken in latest Canary Version 71.0.3557.0.

Looks like it wasn't an issue any more in 71.0.3556.0 (maybe fixed with another CL in the meantime) and your CL wasn't necessary any longer. WDYT?

Hey ellyjones@: Okay i did a bisect and found the following result:

This issue seemed to fixed before you landed your CL with the following Following bisect range https://chromium.googlesource.com/chromium/src/+log/a769a161d7246aaaf0be066389b32dfb398c7798..968ae52004a9471722ca7c0d0368d8c87bb87dec

Probably with https://chromium-review.googlesource.com/c/chromium/src/+/1159532

Maybe it helps :)

Oh interesting, okay. Lemme check that locally and if so I'll just revert my change.

Okay, thanks. It looks like that the mentioned CL in c#15 removed the borders around the menus. Maybe that (regression) fixed this issue :) 

Hi ellyjones@: Looks like the CL mentioned in C#15 has been reverted (for now???).

So it looks fixed now with your change (at the moment) :)

Deleted: Bildschirmfoto 2018-09-25 um 19.16.52.png 63.3 KB

	Bildschirmfoto 2018-09-25 um 19.16.52.png 63.3 KB View Download

Deleted: Bildschirmfoto 2018-09-25 um 19.16.30.png 80.2 KB

	Bildschirmfoto 2018-09-25 um 19.16.30.png 80.2 KB View Download

cc kylixrd@ - there's some interaction between my change in this bug and yours in issue 837782.

***Mass UI Triage***
As per dev comments.

This is Fixed for me on current trunk and has been for a while.