Bug 207223 – [WinCairo] Set up fonts for layout tests for test bot image

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

[Stephan Szabo]

Add code to the wincairo port to set up (and remove) the fonts in the appropriate test runner directory before and after test run.

Make this so that it can be turned on for the webkitdev/buildbot images that were having problems due to ahem not being properly registered but not default to keep existing behavior as close as possible.

[Stephan Szabo]

Created attachment 389700 [details]
Patch

Checking to see if the ctypes additions messes up style check or any other ports.

[Stephan Szabo]

Comment on attachment 389700 [details]
Patch

As most have come back okay...

This sets up the fonts in the directory under the current test runner. This reduced on a manual run in the buildbot image the text-only failures from 620 to 310 and image-only failures from 118 to 17.

[Fujii Hironori]

I don't think this is WebKit's right way to do this.
See other ports

Bug 42153 – Activate test fonts for layout tests in WebKitTestRunner (on Mac)
GTK port: r87760
https://trac.webkit.org/browser/webkit/trunk/Tools/DumpRenderTree/win/DumpRenderTree.cpp#L326

[Stephan Szabo]

(In reply to Fujii Hironori from comment #3)
> I don't think this is WebKit's right way to do this.
> See other ports
> 
> Bug 42153 – Activate test fonts for layout tests in WebKitTestRunner (on Mac)
> GTK port: r87760
> https://trac.webkit.org/browser/webkit/trunk/Tools/DumpRenderTree/win/
> DumpRenderTree.cpp#L326

To prevent an extra cycle with a day apart, looking at the GTK one, would you find a version where it got the effective equivalent of the FONTS_CONF_DIR from environment acceptable (with unset being don't do anything)? I'm a little worried about burning that in at compile time.

[Fujii Hironori]

I don't understand. Why does FONTS_CONF_DIR matter? WinCairo doesn't use FontConfig.

[Stephan Szabo]

Ah, sorry to be unclear, it's also used as part of the path which is used to refer to the ttf files.

For example:
https://github.com/WebKit/webkit/blob/master/Tools/WebKitTestRunner/InjectedBundle/gtk/ActivateFontsGtk.cpp#L118

https://github.com/WebKit/webkit/blob/master/Tools/WebKitTestRunner/InjectedBundle/gtk/ActivateFontsGtk.cpp#L135 

---

I was preferring something like the win DRT environment variable WEBKIT_TESTFONTS for finding the ttf files at runtime. 

So, the first part of the question was whether that seemed acceptable to you.

The second part of the question was whether not having the variable set could be an early out and not try to find/set up the fonts. DRT has a fallback to a exePath() + DumpRenderTree.resources if the environment variable is not set.

[Stephan Szabo]

Created attachment 389892 [details]
Patch

[Fujii Hironori]

Comment on attachment 389892 [details]
Patch

LGTM

[WebKit Commit Bot]

Comment on attachment 389892 [details]
Patch

Clearing flags on attachment: 389892

Committed r255900: <https://trac.webkit.org/changeset/255900>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/59213021>