Fixed font height on OS X. Also added some unittests.

BUG=none
TEST=trybots

Review URL: http://codereview.chromium.org/351024

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@30891 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 12 insertions, 6 deletions

diff --git a/app/gfx/font.h b/app/gfx/font.h
index ccc1a64..80a85ec 100644
--- a/app/gfx/font.h
+++ b/app/gfx/font.h
@@ -73,7 +73,10 @@ class Font {
   Font DeriveFont(int size_delta, int style) const;
 
   // Returns the number of vertical pixels needed to display characters from
-  // the specified font.
+  // the specified font.  This may include some leading, i.e. height may be
+  // greater than just ascent + descent.  Specifically, the Windows and Mac
+  // implementations include leading and the Linux one does not.  This may
+  // need to be revisited in the future.
   int height() const;
 
   // Returns the baseline, or ascent, of the font.
diff --git a/app/gfx/font_mac.mm b/app/gfx/font_mac.mm
index 56cc6bc..abbf6f4 100644
--- a/app/gfx/font_mac.mm
+++ b/app/gfx/font_mac.mm
@@ -7,6 +7,7 @@
 #include <Cocoa/Cocoa.h>
 
 #include "base/logging.h"
+#include "base/scoped_nsobject.h"
 #include "base/sys_string_conversions.h"
 
 namespace gfx {
@@ -33,10 +34,9 @@ Font::Font()
 
 void Font::calculateMetrics() {
   NSFont* font = nativeFont();
-  // TODO(akalin): This is the wrong height to use!  Use either the height
-  // of the bounding rect for the font or ascender - descender; this needs
-  // further investigation.  Width may be wrong, too.
-  height_ = [font xHeight];
+  scoped_nsobject<NSLayoutManager> layout_manager(
+      [[NSLayoutManager alloc] init]);
+  height_ = [layout_manager defaultLineHeightForFont:font];
   ascent_ = [font ascender];
   avg_width_ = [font boundingRectForGlyph:[font glyphWithName:@"x"]].size.width;
 }
diff --git a/app/gfx/font_unittest.cc b/app/gfx/font_unittest.cc
index 405c130..4e866e7 100644
--- a/app/gfx/font_unittest.cc
+++ b/app/gfx/font_unittest.cc
@@ -31,11 +31,14 @@ TEST_F(FontTest, LoadArialBold) {
 TEST_F(FontTest, Ascent) {
   Font cf(Font::CreateFont(L"Arial", 16));
   ASSERT_GT(cf.baseline(), 2);
+  ASSERT_LE(cf.baseline(), 22);
 }
 
 TEST_F(FontTest, Height) {
   Font cf(Font::CreateFont(L"Arial", 16));
-  ASSERT_LT(cf.baseline(), 22);
+  ASSERT_GE(cf.height(), 16);
+  // TODO(akalin): Figure out why height is so large on Linux.
+  ASSERT_LE(cf.height(), 26);
 }
 
 TEST_F(FontTest, AvgWidths) {