Relanding this with font test fixes for gdi.

ui/gfx/platform_font_win.cc

 // Copyright (c) 2012 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 "ui/gfx/platform_font_win.h"
 
 #include <algorithm>
 #include <dwrite.h>
 #include <math.h>
 #include <windows.h>
@@ skipping 193 matching lines @@
   InitWithCopyOfHFONT(native_font);
 }
 
 PlatformFontWin::PlatformFontWin(const std::string& font_name,
                                  int font_size) {
   InitWithFontNameAndSize(font_name, font_size);
 }
 
 Font PlatformFontWin::DeriveFontWithHeight(int height, int style) {
   DCHECK_GE(height, 0);
-  if (GetHeight() == height && GetStyle() == style)
-    return Font(this);
+
+  // Create a font with a height near that of the target height.
+  LOGFONT font_info;
+  GetObject(GetNativeFont(), sizeof(LOGFONT), &font_info);
+  font_info.lfHeight = height;
+  SetLogFontStyle(style, &font_info);
+
+  HFONT hfont = CreateFontIndirect(&font_info);
+  Font font(new PlatformFontWin(CreateHFontRef(hfont)));
+
+  const int min_font_size = GetMinimumFontSize();
 
   // CreateFontIndirect() doesn't return the largest size for the given height
   // when decreasing the height. Iterate to find it.
-  if (GetHeight() > height) {
-    const int min_font_size = GetMinimumFontSize();
-    Font font = DeriveFont(-1, style);
+  if (font.GetHeight() > height) {

[msw, 2015/01/13 21:59:57]

This check could potentially return a font smaller than the min size. If
|height|=10, |min_font_size|==10, |font.GetHeight()|==11,
|font.GetFontSize()|==9, this if statement would pass; then the first Derive
statement could make: |font.GetHeight()|==10, |font.GetFontSize()|==8, which
would skip the while loop and return the |font| at size 8, smaller than the min
size of 10. Let's rewrite this loop to handle that case and match the structure
of the new loop below. Does this look right?

while ((font.GetHeight() > height) &&
       (font.GetFontSize() >= min_font_size)) {
  Font derived_font = font.Derive(-1, style);
  if ((derived_font.GetFontSize() < min_font_size) ||
      (derived_font.GetFontSize() == font.GetFontSize()))
    break;
  DCHECK_LT(derived_font.GetFontSize(), font.GetFontSize());
  font = derived_font;
}

[ananta, 2015/01/13 22:21:46]

Yes. I put this block inside an if (font.GetHeight() > height).

+    font = font.Derive(-1, style);
     int font_height = font.GetHeight();
     int font_size = font.GetFontSize();
     while (font_height > height && font_size != min_font_size) {
       font = font.Derive(-1, style);
       if (font_height == font.GetHeight() && font_size == font.GetFontSize())
         break;
       font_height = font.GetHeight();
       font_size = font.GetFontSize();
     }
     return font;
   }
 
-  LOGFONT font_info;
-  GetObject(GetNativeFont(), sizeof(LOGFONT), &font_info);
-  font_info.lfHeight = height;
-  SetLogFontStyle(style, &font_info);
-
-  HFONT hfont = CreateFontIndirect(&font_info);
-  return DeriveWithCorrectedSize(hfont);
+  // Respect the minimum font size constraint.

[msw, 2015/01/13 21:59:57]

nit: move this comment just above the |min_font_size| decl.

[ananta, 2015/01/13 22:21:46]

Done.

+  while ((font.GetHeight() <= height) ||
+         (font.GetFontSize() < min_font_size)) {
+    Font derived_font = font.Derive(1, style);
+    if (derived_font.GetHeight() > height && !min_font_size)

[msw, 2015/01/13 21:59:57]

I think this should be:
  if ((derived_font.GetHeight() > height) &&
      (font.GetFontSize() >= min_font_size))
Otherwise, given |height|==10, |min_font_size|==5, |font.GetHeight()|=10,
|font.GetFontSize()|=7, |derived_font.GetHeight()|==11, this check as-is would
fail and we'd wind up returning the |derived_font| with a height larger than
requested, even though the |font| already passes the minimum size requirement.

[ananta, 2015/01/13 22:21:46]

Done.

[ananta, 2015/01/13 22:21:46]

Done.

+      break;
+    DCHECK_GT(derived_font.GetFontSize(), font.GetFontSize());
+    font = derived_font;
+  }
+  return font;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // PlatformFontWin, PlatformFont implementation:
 
 Font PlatformFontWin::DeriveFont(int size_delta, int style) const {
   LOGFONT font_info;
   GetObject(GetNativeFont(), sizeof(LOGFONT), &font_info);
   const int requested_font_size = font_ref_->requested_font_size();
   font_info.lfHeight = AdjustFontSize(-requested_font_size, size_delta);
@@ skipping 165 matching lines @@
     style |= Font::ITALIC;
   if (font_metrics.tmUnderlined)
     style |= Font::UNDERLINE;
   if (font_metrics.tmWeight >= kTextMetricWeightBold)
     style |= Font::BOLD;
 
   return new HFontRef(font, font_size, height, baseline, cap_height,
                       ave_char_width, style);
 }
 
-Font PlatformFontWin::DeriveWithCorrectedSize(HFONT base_font) {
-  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;
-  GetTextMetricsForFont(screen_dc, best_font, &best_font_metrics);
-
-  LOGFONT font_info;
-  GetObject(base_font, sizeof(LOGFONT), &font_info);
-
-  // Set |lfHeight| to negative value to indicate it's the size, not the height.
-  font_info.lfHeight =
-      -(best_font_metrics.tmHeight - best_font_metrics.tmInternalLeading);
-
-  do {
-    // Increment font size. Prefer font with greater size if its height isn't
-    // greater than height of base font.
-    font_info.lfHeight = AdjustFontSize(font_info.lfHeight, 1);
-    base::win::ScopedGDIObject<HFONT> font(CreateFontIndirect(&font_info));
-    TEXTMETRIC font_metrics;
-    GetTextMetricsForFont(screen_dc, font, &font_metrics);
-    if (font_metrics.tmHeight > best_font_metrics.tmHeight)
-      break;
-    best_font.Set(font.release());
-    best_font_metrics = font_metrics;
-  } while (true);
-
-  return Font(new PlatformFontWin(CreateHFontRef(best_font.release())));
-}
-
 // static
 PlatformFontWin::HFontRef* PlatformFontWin::CreateHFontRefFromSkia(
     HFONT gdi_font,
     const TEXTMETRIC& font_metrics) {
   LOGFONT font_info = {0};
   GetObject(gdi_font, sizeof(LOGFONT), &font_info);
 
   // If the font height is passed in as 0, assume the height to be -1 to ensure
   // that we return the metrics for a 1 point font.
   // If the font height is positive it represents the rasterized font's cell
@@ skipping 44 matching lines @@
   SkPaint paint;
   paint.setAntiAlias(!!antialiasing);
   paint.setTypeface(skia_face.get());
   paint.setTextSize(-font_info.lfHeight);
   SkPaint::FontMetrics skia_metrics;
   paint.getFontMetrics(&skia_metrics);
 
   // The calculations below are similar to those in the CreateHFontRef
   // function. The height, baseline and cap height are rounded up to ensure
   // that they match up closely with GDI.
-  const int height = std::ceil(
-      skia_metrics.fDescent - skia_metrics.fAscent + skia_metrics.fLeading);
+  const int height = std::ceil(skia_metrics.fDescent - skia_metrics.fAscent);
   const int baseline = std::max<int>(1, std::ceil(-skia_metrics.fAscent));
   const int cap_height = std::ceil(paint.getTextSize() *
       static_cast<double>(dwrite_font_metrics.capHeight) /
           dwrite_font_metrics.designUnitsPerEm);
 
   // The metrics retrieved from skia don't have the average character width. In
   // any case if we get the average character width from skia then use that or
   // use the text extent technique as documented by microsoft. See
   // GetAverageCharWidthInDialogUnits for details.
   const int ave_char_width =
@@ skipping 87 matching lines @@
   return new PlatformFontWin(native_font);
 }
 
 // static
 PlatformFont* PlatformFont::CreateFromNameAndSize(const std::string& font_name,
                                                   int font_size) {
   return new PlatformFontWin(font_name, font_size);
 }
 
 }  // namespace gfx