Get all font unittests running with DirectWrite on Windows 7+

ui/gfx/platform_font_win_unittest.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 "base/logging.h"
 #include "base/memory/ref_counted.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
@@ skipping 25 matching lines @@
     do {
       expected_font = expected_font.Derive(-1, 0);
     } while (expected_font.GetHeight() > target_height);
   }
   return expected_font;
 }
 
 }  // namespace
 
 TEST(PlatformFontWinTest, DeriveFontWithHeight) {
-  // TODO(ananta): Fix this test for DirectWrite. http://crbug.com/442010
-  if (gfx::win::IsDirectWriteEnabled())
-    return;
-
   const Font base_font;
   PlatformFontWin* platform_font =
       static_cast<PlatformFontWin*>(base_font.platform_font());
 
   for (int i = -10; i < 10; i++) {
     const int target_height = base_font.GetHeight() + i;
     Font expected_font = AdjustFontSizeForHeight(base_font, target_height);
     ASSERT_LE(expected_font.GetHeight(), target_height);
-
-    Font derived_font = platform_font->DeriveFontWithHeight(target_height, 0);
+    // With DirectWrite the height of the expected font calculated above may be

[msw, 2015/01/09 23:40:22]

I might just nuke AdjustFontSizeForHeight and expected_font, and just ensure
that DeriveFontWithHeight does what it promises for this range of heights,
something like below. Then we can just remove the weird comparison against the
test function and these comments. Bonus points for moving any tests that work
cross-platform to a new platform_font_unittest.cc file. What do you think?

for (int i = -10; i < 10; i++) {
  const int target_height = base_font.GetHeight() + i;
  Font derived_font = platform_font->DeriveFontWithHeight(target_height, 0);
  EXPECT_LE(derived_font.GetHeight(), target_height);
  // A derived Font one size larger should exceed the target height.
  EXPECT_GT(derived_font.Derive(1, 0).GetHeight(), target_height);
  EXPECT_EQ(platform_font->GetActualFontNameForTesting(),
            derived_font.GetActualFontNameForTesting());
  EXPECT_EQ(0, derived_font.GetStyle());

  ... Ditto for Font::BOLD ...
  ... Delete the bottom with |rederived_font| ...
}

[ananta, 2015/01/10 00:43:15]

DeriveFontWithHeight is windows specific at the moment. I left the
implementation in the platform_font_win_unittest.cc file. Changed the test on
the lines you mentioned. However the checks are a touch different. Comments in
the test should clarify this.

+    // lesser or greater by 1 px from the desired height due to vagaries in the
+    // metrics calculation.
+    // The AdjustFontSizeForHeight function and the DeriveFontWithHeight
+    // function don't work the same way and thus we end up with mismatched
+    // fonts with DirectWrite. To workaround this we use the expected fonts
+    // height for the tests below.
+    Font derived_font = platform_font->DeriveFontWithHeight(
+        expected_font.GetHeight(), 0);
     EXPECT_EQ(expected_font.GetFontName(), derived_font.GetFontName());
-    EXPECT_EQ(expected_font.GetFontSize(), derived_font.GetFontSize());
+    // Wierdly enough for some font heights like 24 and above when the font is
+    // created via CreateFontIndirect and we get the metrics for the same, the
+    // height returned by the GetTextMetric API is 23. This causes the derived
+    // fonts size to be off by 1 px from the expected font size. We only see
+    // this with DirectWrite because with GDI the AdjustFontSizeForHeight
+    // function returns a font with height 23 and everything works well.
+    EXPECT_LE(abs(expected_font.GetFontSize() - derived_font.GetFontSize()),
+              1);
     EXPECT_LE(expected_font.GetHeight(), target_height);
     EXPECT_EQ(0, derived_font.GetStyle());
 
-    derived_font = platform_font->DeriveFontWithHeight(target_height,
-                                                       Font::BOLD);
+    derived_font = platform_font->DeriveFontWithHeight(
+        expected_font.GetHeight(), Font::BOLD);
     EXPECT_EQ(expected_font.GetFontName(), derived_font.GetFontName());
-    EXPECT_EQ(expected_font.GetFontSize(), derived_font.GetFontSize());
+    // Please refer to the comment above as to why this check is needed.
+    EXPECT_LE(abs(expected_font.GetFontSize() - derived_font.GetFontSize()),
+              1);
     EXPECT_LE(expected_font.GetHeight(), target_height);
     EXPECT_EQ(Font::BOLD, derived_font.GetStyle());
 
     // Test that deriving from the new font has the expected result.
     Font rederived_font = derived_font.Derive(1, 0);
     expected_font = Font(derived_font.GetFontName(),
                          derived_font.GetFontSize() + 1);
     EXPECT_EQ(expected_font.GetFontName(), rederived_font.GetFontName());
     EXPECT_EQ(expected_font.GetFontSize(), rederived_font.GetFontSize());
     EXPECT_EQ(expected_font.GetHeight(), rederived_font.GetHeight());
   }
 }
 
 TEST(PlatformFontWinTest, DeriveFontWithHeight_Consistency) {
-  // TODO(ananta): Fix this test for DirectWrite. http://crbug.com/442010
-  if (gfx::win::IsDirectWriteEnabled())
-    return;
   gfx::Font arial_12("Arial", 12);
   ASSERT_GT(16, arial_12.GetHeight());
   gfx::Font derived_1 = static_cast<PlatformFontWin*>(
       arial_12.platform_font())->DeriveFontWithHeight(16, 0);
 
   gfx::Font arial_15("Arial", 15);
   ASSERT_LT(16, arial_15.GetHeight());
   gfx::Font derived_2 = static_cast<PlatformFontWin*>(
       arial_15.platform_font())->DeriveFontWithHeight(16, 0);
 
@@ skipping 96 matching lines @@
     EXPECT_EQ(h_font_gdi->font_name(), h_font_skia->font_name());
 
     EXPECT_LE(abs(h_font_gdi->cap_height() - h_font_skia->cap_height()), 1);
     EXPECT_LE(abs(h_font_gdi->baseline() - h_font_skia->baseline()), 1);
     EXPECT_LE(abs(h_font_gdi->height() - h_font_skia->height()), 1);
   }
 }
 
 
 }  // namespace gfx