PlatformFontWin::DeriveFontWithHeight insensitive to base font's height

ui/gfx/platform_font_win.cc

Index: ui/gfx/platform_font_win.cc
diff --git a/ui/gfx/platform_font_win.cc b/ui/gfx/platform_font_win.cc
index 4acaf64b34abdb2ff9abb8b0755e588741f8f1e1..05250a38efd3fc4193a35a70146c77480b079fd6 100644
--- a/ui/gfx/platform_font_win.cc
+++ b/ui/gfx/platform_font_win.cc
@@ -14,6 +14,7 @@
 #include "base/strings/string_util.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/win/scoped_gdi_object.h"
 #include "base/win/scoped_hdc.h"
 #include "base/win/scoped_select_object.h"
 #include "base/win/win_util.h"
@@ -116,7 +117,7 @@ Font PlatformFontWin::DeriveFontWithHeight(int height, int style) {
   SetLogFontStyle(style, &font_info);
 
   HFONT hfont = CreateFontIndirect(&font_info);
-  return Font(new PlatformFontWin(CreateHFontRef(hfont)));
+  return DeriveWithCorrectedSize(hfont);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -235,6 +236,12 @@ PlatformFontWin::HFontRef* PlatformFontWin::CreateHFontRef(HFONT font) {
     GetTextMetrics(screen_dc, &font_metrics);
   }
 
+  return CreateHFontRef(font, font_metrics);
+}
+
+PlatformFontWin::HFontRef* PlatformFontWin::CreateHFontRef(
+    HFONT font,
+    const TEXTMETRIC& font_metrics) {
   const int height = std::max<int>(1, font_metrics.tmHeight);
   const int baseline = std::max<int>(1, font_metrics.tmAscent);
   const int cap_height =
@@ -254,6 +261,40 @@ PlatformFontWin::HFontRef* PlatformFontWin::CreateHFontRef(HFONT font) {
                       ave_char_width, style);
 }
 
+Font PlatformFontWin::DeriveWithCorrectedSize(HFONT base_font) {

[Alexei Svitkine (slow), 2014/06/02 15:42:55]

Make this a free-standing function in the anon namespace at the top of the file
instead.

[Tomasz Moniuszko, 2014/06/11 13:11:56]

This method calls PlatformFontWin::CreateHFontRef which is a private method so
it cannot be called from anonymous namespace.

+  base::win::ScopedGetDC screen_dc(NULL);
+  gfx::ScopedSetMapMode mode(screen_dc, MM_TEXT);
+
+  base::win::ScopedGDIObject<HFONT> best_font(base_font);
+  TEXTMETRIC best_font_metrics;
+  {
+    base::win::ScopedSelectObject scoped_font(screen_dc, best_font);
+    GetTextMetrics(screen_dc, &best_font_metrics);

[Alexei Svitkine (slow), 2014/06/02 15:42:55]

Nit: Make a helper function in the anon namespace for getting the metrics given
a font and a dc, so these blocks can be much shorter, i.e.:

TEXTMETRIC best_font_metrics;
GetTextMetricsForFont(screen_dc, best_font, &best_font_metrics);

You can use it both here and in CreateHFontRef() above.

[Tomasz Moniuszko, 2014/06/11 13:11:55]

Done.

+  }
+
+  LOGFONT font_info;
+  GetObject(base_font, sizeof(LOGFONT), &font_info);
+
+  do {
+    font_info.lfHeight =
+        -(best_font_metrics.tmHeight - best_font_metrics.tmInternalLeading + 1);

[Alexei Svitkine (slow), 2014/06/02 15:42:55]

Nit: Add a comment explaining this logic.

[Tomasz Moniuszko, 2014/06/11 13:11:55]

Done.

+    base::win::ScopedGDIObject<HFONT> font(CreateFontIndirect(&font_info));
+    TEXTMETRIC font_metrics;
+    {
+      base::win::ScopedSelectObject scoped_font(screen_dc, font);
+      GetTextMetrics(screen_dc, &font_metrics);
+    }
+    if (font_metrics.tmHeight > best_font_metrics.tmHeight) {

[Alexei Svitkine (slow), 2014/06/02 15:42:55]

Nit: No {}'s.

[Tomasz Moniuszko, 2014/06/11 13:11:56]

Done.

+      break;
+    }
+    best_font.Set(font.release());
+    best_font_metrics = font_metrics;
+  } while (true);
+
+  return Font(new PlatformFontWin(
+      CreateHFontRef(best_font.release())));

[Alexei Svitkine (slow), 2014/06/02 15:42:55]

Nit: Does this fit on the previous line?

[Tomasz Moniuszko, 2014/06/11 13:11:56]

Done.

+}
+
 PlatformFontWin::PlatformFontWin(HFontRef* hfont_ref) : font_ref_(hfont_ref) {
 }