Bug 141900 - [iOS] Implement WTR::activateFonts() to fix missing fonts in WebKit2 test runs
Summary: [iOS] Implement WTR::activateFonts() to fix missing fonts in WebKit2 test runs
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-02-23 02:00 PST by David Kilzer (:ddkilzer)
Modified: 2015-02-24 05:03 PST (History)
6 users (show)

See Also:

Attachments
Patch v1 (15.70 KB, patch)
2015-02-23 02:16 PST, David Kilzer (:ddkilzer) 		mmaxfield: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2015-02-23 02:00:31 PST 
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.
Comment 1 David Kilzer (:ddkilzer) 2015-02-23 02:16:23 PST 
Created attachment 247109 [details]
Patch v1
Comment 2 Myles C. Maxfield 2015-02-23 12:37:40 PST 
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
Comment 3 David Kilzer (:ddkilzer) 2015-02-23 14:53:28 PST 
(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?
Comment 4 Myles C. Maxfield 2015-02-23 16:00:07 PST 
(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.
Comment 5 David Kilzer (:ddkilzer) 2015-02-24 05:03:49 PST 
Committed r180557: <http://trac.webkit.org/changeset/180557>