Bug 143583 – [iOS] Delete hardcoded font fallback tables

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Myles C. Maxfield]

[iOS] Delete hardcoded font fallback tables

[Myles C. Maxfield]

Created attachment 250473 [details]
Patch

[Jon Lee]

rdar://problem/20050226

[Myles C. Maxfield]

Perf says we're good to go with this.

[Myles C. Maxfield]

Created attachment 250770 [details]
Patch

[Darin Adler]

Comment on attachment 250770 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=250770&action=review

What kind of testing did you do?

> Source/WebCore/platform/graphics/ios/FontCacheIOS.mm:450
> +    RetainPtr<CTFontDescriptorRef> fallbackFontDescriptor = adoptCF(CTFontCreatePhysicalFontDescriptorForCharactersWithLanguage(originalFontData->getCTFont(), characters, length, nullptr, nullptr));
> +    if (RetainPtr<CFStringRef> foundFontName = adoptCF(static_cast<CFStringRef>(CTFontDescriptorCopyAttribute(fallbackFontDescriptor.get(), kCTFontNameAttribute))))
> +        font = fontForFamily(description, foundFontName.get(), false);

I think these would read better with auto instead of writing out the types. Seems a little strange that we don’t pass a language in, since webpages do, in fact, sometimes have language information, but I guess this is very low-level code.

[Myles C. Maxfield]

Comment on attachment 250770 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=250770&action=review

>> Source/WebCore/platform/graphics/ios/FontCacheIOS.mm:450
>> +        font = fontForFamily(description, foundFontName.get(), false);
> 
> I think these would read better with auto instead of writing out the types. Seems a little strange that we don’t pass a language in, since webpages do, in fact, sometimes have language information, but I guess this is very low-level code.

I agree - it seems like something we should do. https://bugs.webkit.org/show_bug.cgi?id=143788

[Myles C. Maxfield]

(In reply to comment #5)
> Comment on attachment 250770 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=250770&action=review
> 
> What kind of testing did you do?
> 
> > Source/WebCore/platform/graphics/ios/FontCacheIOS.mm:450
> > +    RetainPtr<CTFontDescriptorRef> fallbackFontDescriptor = adoptCF(CTFontCreatePhysicalFontDescriptorForCharactersWithLanguage(originalFontData->getCTFont(), characters, length, nullptr, nullptr));
> > +    if (RetainPtr<CFStringRef> foundFontName = adoptCF(static_cast<CFStringRef>(CTFontDescriptorCopyAttribute(fallbackFontDescriptor.get(), kCTFontNameAttribute))))
> > +        font = fontForFamily(description, foundFontName.get(), false);
> 
> I think these would read better with auto instead of writing out the types.
> Seems a little strange that we don’t pass a language in, since webpages do,
> in fact, sometimes have language information, but I guess this is very
> low-level code.

I ran PLT with and without this patch, and it didn't report any difference.

As for correctness, I have yet to run layout tests. Note that chosen fallback fonts will change with this patch, which is both expected and desirable. I am going to look at all the test output to make sure nothing is going awry, and if not, update expected results.

[Myles C. Maxfield]

These are the tests which need to be rebaselined. I will do so when I commit this patch today.

editing/selection/vertical-rl-rtl-extend-line-backward-br.html
editing/selection/vertical-rl-rtl-extend-line-backward-p.html
editing/selection/vertical-rl-rtl-extend-line-forward-br.html
editing/selection/vertical-rl-rtl-extend-line-forward-p.html
fast/text/international/danda-space.html
fast/text/international/thai-baht-space.html

[Myles C. Maxfield]

Committed r182894: <http://trac.webkit.org/changeset/182894>