In FontPlatformDataHarfBuzz.cpp, FontPlatformData::emSizeInFontUnits(), the Android-specific code was added when skia's Android port didn't support Typeface::getAdvancedTypefaceMetrics(). Skia added Android support of the method in http://codereview.appspot.com/5396045, so the Android-specific code can be removed.
Since getAdvancedTypefaceMetrics() is only called to get the em size in font units (the caller disregards all other returned information), I don't think this would be the right approach. See the following bugs for more context (also referred to by a TODO in the code): Bug 75702 ([Chromium] Use SkFontHost::GetUnitsPerEm instead of advanced type metrics for Android in FontPlatformData) Bug 75961 ([Chromium] Switch to using the new SkTypeface::GetUnitsPerEm API) http://code.google.com/p/skia/issues/detail?id=441 (Add SkTypeface::GetUnitsPerEm()) +reed
(In reply to comment #1) > Since getAdvancedTypefaceMetrics() is only called to get the em size in font units (the caller disregards all other returned information), I don't think this would be the right approach. See the following bugs for more context (also referred to by a TODO in the code): > > Bug 75702 ([Chromium] Use SkFontHost::GetUnitsPerEm instead of advanced type metrics for Android in FontPlatformData) > Bug 75961 ([Chromium] Switch to using the new SkTypeface::GetUnitsPerEm API) > > http://code.google.com/p/skia/issues/detail?id=441 (Add SkTypeface::GetUnitsPerEm()) > > +reed Yes, I saw those bugs, but this one just removes the Android-specific code with the bugs (to switch to SkTypeface::GetUnitsPerEm()) still open. That is, it combines two different 'incorrect' ways into one :) Found this by the way when cleaning up Android-specific code in the file in downstream. I'm OK to delay this until we fix 75961.
Are you proposing to switch Android back to getAdvancedTypefaceMetrics(), and thus removing the compile-time branch, until emSizeInFontUnits() is available for all platforms?
(In reply to comment #3) > Are you proposing to switch Android back to getAdvancedTypefaceMetrics(), and thus removing the compile-time branch, until emSizeInFontUnits() is available for all platforms? Yes. But now I think this is unnecessary as there's no diff about this in downstream/upstream. Closing this bug.