Bug 207223 - [WinCairo] Set up fonts for layout tests for test bot image
Summary: [WinCairo] Set up fonts for layout tests for test bot image
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-02-04 13:45 PST by Stephan Szabo
Modified: 2020-02-05 20:46 PST (History)
8 users (show)

See Also:


Attachments
Patch (2.66 KB, patch)
2020-02-04 13:50 PST, Stephan Szabo
no flags Details | Formatted Diff | Diff
Patch (2.44 KB, patch)
2020-02-05 15:51 PST, Stephan Szabo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stephan Szabo 2020-02-04 13:45:39 PST
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.
Comment 1 Stephan Szabo 2020-02-04 13:50:39 PST
Created attachment 389700 [details]
Patch

Checking to see if the ctypes additions messes up style check or any other ports.
Comment 2 Stephan Szabo 2020-02-04 14:34:50 PST
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.
Comment 3 Fujii Hironori 2020-02-04 18:34:43 PST
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
Comment 4 Stephan Szabo 2020-02-04 19:25:16 PST
(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.
Comment 5 Fujii Hironori 2020-02-05 03:16:54 PST
I don't understand. Why does FONTS_CONF_DIR matter? WinCairo doesn't use FontConfig.
Comment 6 Stephan Szabo 2020-02-05 10:32:54 PST
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.
Comment 7 Stephan Szabo 2020-02-05 15:51:35 PST
Created attachment 389892 [details]
Patch
Comment 8 Fujii Hironori 2020-02-05 19:26:43 PST
Comment on attachment 389892 [details]
Patch

LGTM
Comment 9 WebKit Commit Bot 2020-02-05 20:45:04 PST
Comment on attachment 389892 [details]
Patch

Clearing flags on attachment: 389892

Committed r255900: <https://trac.webkit.org/changeset/255900>
Comment 10 WebKit Commit Bot 2020-02-05 20:45:06 PST
All reviewed patches have been landed.  Closing bug.
Comment 11 Radar WebKit Bug Importer 2020-02-05 20:46:13 PST
<rdar://problem/59213021>