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

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

BUG=none
TEST=trybots

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=30891

[akalin, 2009-11-03 20:23:59]

+dmac for code review.

[akalin (wrong akalin), 2009-11-03 20:53:35]

On 2009/11/03 20:23:59, akalin wrote:
> +dmac for code review.

Also worth pointing out, nothing actually uses font.height() on OS X yet (which
is why this didn't break anything).

[dmac, 2009-11-03 21:07:51]

http://codereview.chromium.org/351024/diff/2001/3001
File app/gfx/font_mac.mm (right):

http://codereview.chromium.org/351024/diff/2001/3001#newcode36
Line 36: NSLayoutManager* layout_manager =
use a scoped_nsobject<NSLayouManager> here, and get rid of the autorelease.

In the font.h header, can we clearly mark that height_ is lineHeight and NOT
ascender - descender?

[akalin (wrong akalin), 2009-11-03 21:26:46]

On 2009/11/03 21:07:51, dmac wrote:
> http://codereview.chromium.org/351024/diff/2001/3001
> File app/gfx/font_mac.mm (right):
> 
> http://codereview.chromium.org/351024/diff/2001/3001#newcode36
> Line 36: NSLayoutManager* layout_manager =
> use a scoped_nsobject<NSLayouManager> here, and get rid of the autorelease.

Done.

> 
> In the font.h header, can we clearly mark that height_ is lineHeight and NOT
> ascender - descender?

Actually on Linux it appears that it's just ascender + descender.  Added
explanatory comment, though.

Please take another look.

[dmac, 2009-11-03 23:09:22]

LGTM

http://codereview.chromium.org/351024/diff/4001/4002
File app/gfx/font.h (right):

http://codereview.chromium.org/351024/diff/4001/4002#newcode77
Line 77: // greater than just ascent + descent.
I would expand on this comment and say that on the mac and windows it includes
leading and on linux it does not. If you can explain why that would be even
better.

[akalin (wrong akalin), 2009-11-03 23:45:01]

On Tue, Nov 3, 2009 at 4:09 PM,  <dmaclach@chromium.org> wrote:
> LGTM
>
>
> http://codereview.chromium.org/351024/diff/4001/4002
> File app/gfx/font.h (right):
>
> http://codereview.chromium.org/351024/diff/4001/4002#newcode77
> Line 77: // greater than just ascent + descent.
> I would expand on this comment and say that on the mac and windows it
> includes leading and on linux it does not. If you can explain why that
> would be even better.

Done.  I actually don't know why, but it's probably something we'll
have to revisit in the future, so I mentioned that.

Submitting.

-- 
Frederick Akalin
Software Engineer