Bug 141900 – [iOS] Implement WTR::activateFonts() to fix missing fonts in WebKit2 test runs

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[David Kilzer (:ddkilzer)]

Implement WTR::activateFonts() for iOS.

The current ActivateFontsIOS.mm is just a placeholder, so fonts like Ahem are not being rendered properly when running tests.

This fixes the fast/text/ruby-justification-flush.html reference test for iOS WebKit2 runs, and possibly others.  The same test already passes with WebKit1 because the font activation code in DumpRenderTree already exists for iOS.

[David Kilzer (:ddkilzer)]

Created attachment 247109 [details]
Patch v1

[Myles C. Maxfield]

Comment on attachment 247109 [details]
Patch v1

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

> Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mmTools/WebKitTestRunner/InjectedBundle/mac/ActivateFonts.mm:275
>          exit(1);
>      }

I thought we actually linked the contents of these font files into a rodata section in the actual executable. Do we not do this any more? Does this work on iOS?

> Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mmTools/WebKitTestRunner/InjectedBundle/mac/ActivateFonts.mm:278
>      swizzleNSFontManagerMethods();

See https://bugs.webkit.org/show_bug.cgi?id=132159 for implementing the whitelist on iOS

[David Kilzer (:ddkilzer)]

(In reply to comment #2)
> Comment on attachment 247109 [details]
> Patch v1
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=247109&action=review
> 
> > Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mmTools/WebKitTestRunner/InjectedBundle/mac/ActivateFonts.mm:275
> >          exit(1);
> >      }
> 
> I thought we actually linked the contents of these font files into a rodata
> section in the actual executable. Do we not do this any more? Does this work
> on iOS?

This does work on iOS.  See related code in DumpRenderTree.  Dan Bernstein fixed this a while ago.

> > Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mmTools/WebKitTestRunner/InjectedBundle/mac/ActivateFonts.mm:278
> >      swizzleNSFontManagerMethods();
> 
> See https://bugs.webkit.org/show_bug.cgi?id=132159 for implementing the
> whitelist on iOS

Why is Bug 132159 OS X only?  Why aren't we fixing both platforms at the same time?

[Myles C. Maxfield]

(In reply to comment #3)
> (In reply to comment #2)
> > Comment on attachment 247109 [details]
> > Patch v1
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=247109&action=review
> > 
> > > Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mmTools/WebKitTestRunner/InjectedBundle/mac/ActivateFonts.mm:275
> > >          exit(1);
> > >      }
> > 
> > I thought we actually linked the contents of these font files into a rodata
> > section in the actual executable. Do we not do this any more? Does this work
> > on iOS?
> 
> This does work on iOS.  See related code in DumpRenderTree.  Dan Bernstein
> fixed this a while ago.
> 
> > > Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mmTools/WebKitTestRunner/InjectedBundle/mac/ActivateFonts.mm:278
> > >      swizzleNSFontManagerMethods();
> > 
> > See https://bugs.webkit.org/show_bug.cgi?id=132159 for implementing the
> > whitelist on iOS
> 
> Why is Bug 132159 OS X only?  Why aren't we fixing both platforms at the
> same time?

Ultimately it's for both, but the initial goal of the patch is to do something which requires a CoreText whitelist. Baby steps.

[David Kilzer (:ddkilzer)]

Committed r180557: <http://trac.webkit.org/changeset/180557>