[Win, Linux] Always display the avatar button text at the same size.

chrome/browser/ui/views/profiles/new_avatar_button.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 "chrome/browser/ui/views/profiles/new_avatar_button.h"
 
 #include "base/win/windows_version.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/profiles/profiles_state.h"
 #include "chrome/browser/ui/browser.h"
 #include "grit/theme_resources.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/canvas.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/button/label_button_border.h"
 #include "ui/views/painter.h"
 
 namespace {
 
+const int kDisplayFontSize = 13;
+
 scoped_ptr<views::Border> CreateBorder(const int normal_image_set[],
                                        const int hot_image_set[],
                                        const int pushed_image_set[]) {
   scoped_ptr<views::LabelButtonBorder> border(
       new views::LabelButtonBorder(views::Button::STYLE_TEXTBUTTON));
   border->SetPainter(false, views::Button::STATE_NORMAL,
       views::Painter::CreateImageGridPainter(normal_image_set));
   border->SetPainter(false, views::Button::STATE_HOVERED,
       views::Painter::CreateImageGridPainter(hot_image_set));
   border->SetPainter(false, views::Button::STATE_PRESSED,
@@ skipping 21 matching lines @@
                  true),
       browser_(browser) {
   set_animate_on_state_change(false);
   SetTextColor(views::Button::STATE_NORMAL, SK_ColorWHITE);
   SetTextColor(views::Button::STATE_HOVERED, SK_ColorWHITE);
   SetTextColor(views::Button::STATE_PRESSED, SK_ColorWHITE);
   SetTextShadows(gfx::ShadowValues(10,
       gfx::ShadowValue(gfx::Point(), 1.0f, SK_ColorDKGRAY)));
   SetTextSubpixelRenderingEnabled(false);
 
+  // If the default font size is larger than the expected one, derive
+  // the correct font size to match it.
+  int font_size = GetFontList().GetFontSize();
+  if (font_size > kDisplayFontSize) {
+    SetFontList(GetFontList().DeriveWithSizeDelta(

[msw, 2014/08/25 19:17:20]

I think we might need to use the font height instead of the size, similar to my
work in <https://codereview.chromium.org/496873003>, since some fonts of the
same size are taller than others. To do that, we'll need to generalize
PlatformFontWin::DeriveFontWithHeight to other platform fonts and expose that
method to the Font[List] class(es); that's needed here and probably for
RenderTextHarfBuzz if my WIP CL is the right approach.

[noms (inactive), 2014/08/25 19:19:36]

I originally had the font height, but it didn't seem to work as well (in
particular) on Hindi. It worked fin on Linux, but in Hindi (when using english
characters, not hindi ones), the font was way too small :/

I can give it a try again, if you thing that should work. Maybe I was doing
something silly. 

On 2014/08/25 19:17:20, msw wrote:

[msw, 2014/08/25 19:43:48]

Unfortunately, what you had may have been "right". Matching sizes on my Win7
EN-US with "Segoe UI" (size 15, height 20) yields "Mangal" (size 15, height 28)
for Hindi fallback. Any text using those some of those additional 8px of height
could potentially be clipped. You might be able to use the full button height to
bump up the font height a little, like I did for tab titles in r289073.

I'd at least like to see how your height-based fix worked.

[noms (inactive), 2014/08/25 20:06:02]

So what I had was:
  const int kDisplayFontSize = 15; 

  int font_size = GetFontList().GetHeight();
  if (font_size > kDisplayFontSize) {
    SetFontList(GetFontList().DeriveWithSizeDelta(
        kDisplayFontSize - font_size - 1));
  }

With the code above, the button looks like this:
https://drive.google.com/open?id=0B1B1Up4p2NRMdjg0RTg0MnF0aDQ&authuser=1

I've tested my current patch (with size instead of height), and this is what it
looks like when displaying the non-english characters in hindi:
https://drive.google.com/open?id=0B1B1Up4p2NRMdDkxNVBrU3lKM3c&authuser=1, which
is at the right size, i think.

I think either works better than what we have now, so I'll go with whatever you
think is correct.

On 2014/08/25 19:43:48, msw wrote:

[msw, 2014/08/25 20:13:51]

No, you can't interchange font height and size values like that (and that could
cause the text to shrink unreasonably as you noticed). Let's try to fix this as
I described in my original "To do that..." comment, since jshin@'s latest
comments at http://crbug.com/404999 seem to support my height-limiting approach
for height-constrained UI scenarios.

+        kDisplayFontSize - font_size - 1));
+  }
+
   ui::ResourceBundle* rb = &ui::ResourceBundle::GetSharedInstance();
   if (button_style == THEMED_BUTTON) {
     const int kNormalImageSet[] = IMAGE_GRID(IDR_AVATAR_THEMED_BUTTON_NORMAL);
     const int kHotImageSet[] = IMAGE_GRID(IDR_AVATAR_THEMED_BUTTON_HOVER);
     const int kPushedImageSet[] = IMAGE_GRID(IDR_AVATAR_THEMED_BUTTON_PRESSED);
 
     SetBorder(CreateBorder(kNormalImageSet, kHotImageSet, kPushedImageSet));
     set_menu_marker(
         rb->GetImageNamed(IDR_AVATAR_THEMED_BUTTON_DROPARROW).ToImageSkia());
 #if defined(OS_WIN)
@@ skipping 79 matching lines @@
   // We want the button to resize if the new text is shorter.
   SetText(profiles::GetAvatarButtonTextForProfile(browser_->profile()));
   SetMinSize(gfx::Size());
   InvalidateLayout();
 
   // Because the width of the button might have changed, the parent browser
   // frame needs to recalculate the button bounds and redraw it.
   if (parent())
     parent()->Layout();
 }