Align the Mac Omnibox items vertically again.

Align the Mac Omnibox items vertically again.

The recent refactoring didn't quite work out and it needs to be
reverted.

BUG=617144

[Mikus, 2016-06-10 07:57:52]

Description was changed from

==========
Align the Mac Omnibox items vertically again.

The recent refactoring didn't quite work out and it needs to be
reverted.

BUG=617144
==========

to

==========
Align the Mac Omnibox items vertically again.

The recent refactoring didn't quite work out and it needs to be
reverted.

BUG=617144
==========

[Mikus, 2016-06-10 07:57:52]

mboc@opera.com changed reviewers:
+ tapted@chromium.org

[tapted, 2016-06-14 00:08:36]

What's the actual bug? If there's something going on that isn't understood, we
can't just plaster over it with inefficient code.

[tapted, 2016-06-14 05:51:34]

So really we should lock the baseline in the popup - I'm not convinced that some
OS font change will preserve the baseline.

https://codereview.chromium.org/2057043002/diff/1/chrome/browser/ui/cocoa/omn...
File chrome/browser/ui/cocoa/omnibox/omnibox_popup_matrix.mm (right):

https://codereview.chromium.org/2057043002/diff/1/chrome/browser/ui/cocoa/omn...
chrome/browser/ui/cocoa/omnibox/omnibox_popup_matrix.mm:155: [layoutManager
defaultLineHeightForFont:OmniboxViewMac::GetLargeFont(
defaultLineHeightForFont has a rounding bug - see
https://codereview.chromium.org/1252893002 -- but I don't think that's it.

https://codereview.chromium.org/2057043002/diff/1/chrome/browser/ui/cocoa/omn...
File chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm (left):

https://codereview.chromium.org/2057043002/diff/1/chrome/browser/ui/cocoa/omn...
chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm:1097: return rb
So the minimal fix seems to be to do

  // Request a bold font, then make it larger. ResourceBundle will do the
  // opposite which makes a large system normal font a non-system bold font.
  // That gives a different baseline to making the non-system bold font larger.
  // And while the omnibox locks the baseline in ApplyTextStyle(),
  // OmniboxPopupCellData does not.
  return rb
      .GetFontWithDelta(0, gfx::Font::NORMAL, gfx::Font::Weight::BOLD)
      .Derive(kOmniboxNormalFontSizeDelta, gfx::Font::NORMAL,
              gfx::Font::Weight::BOLD)
      .GetNativeFont();

[joleksy, 2016-06-15 13:28:23]

On 2016/06/14 05:51:34, tapted wrote:
> So really we should lock the baseline in the popup - I'm not convinced that
some
> OS font change will preserve the baseline.
> 
>
https://codereview.chromium.org/2057043002/diff/1/chrome/browser/ui/cocoa/omn...
> File chrome/browser/ui/cocoa/omnibox/omnibox_popup_matrix.mm (right):
> 
>
https://codereview.chromium.org/2057043002/diff/1/chrome/browser/ui/cocoa/omn...
> chrome/browser/ui/cocoa/omnibox/omnibox_popup_matrix.mm:155: [layoutManager
> defaultLineHeightForFont:OmniboxViewMac::GetLargeFont(
> defaultLineHeightForFont has a rounding bug - see
> https://codereview.chromium.org/1252893002 -- but I don't think that's it.
> 
>
https://codereview.chromium.org/2057043002/diff/1/chrome/browser/ui/cocoa/omn...
> File chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm (left):
> 
>
https://codereview.chromium.org/2057043002/diff/1/chrome/browser/ui/cocoa/omn...
> chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm:1097: return rb
> So the minimal fix seems to be to do
> 
>   // Request a bold font, then make it larger. ResourceBundle will do the
>   // opposite which makes a large system normal font a non-system bold font.
>   // That gives a different baseline to making the non-system bold font
larger.
>   // And while the omnibox locks the baseline in ApplyTextStyle(),
>   // OmniboxPopupCellData does not.
>   return rb
>       .GetFontWithDelta(0, gfx::Font::NORMAL, gfx::Font::Weight::BOLD)
>       .Derive(kOmniboxNormalFontSizeDelta, gfx::Font::NORMAL,
>               gfx::Font::Weight::BOLD)
>       .GetNativeFont();

New review with suggested change created:
https://codereview.chromium.org/2068163002/