Re: [PATCH] gdi32: Don't enumerate fonts when OpenFontFace fails (try 2).

[Alexandre Julliard]

Qian Hong  writes:
> dlls/gdi32/freetype.c |   17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)

It's causing a test failure:

../../../tools/runtest -q -P wine -M gdiplus.dll -T ../../.. -p gdiplus_test.exe.so font.c && touch font.ok
font.c:400: Test failed: Expected 0, got 3
font.c:401: Test failed: got NULL font collection
make[1]: *** [font.ok] Error 2

[PATCH] gdi32: Don't enumerate fonts when OpenFontFace fails (try 2).

[Qian Hong]

Hello,

On Fri, Dec 7, 2012 at 6:28 AM, Alexandre Julliard  wrote:
> ../../../tools/runtest -q -P wine -M gdiplus.dll -T ../../.. -p gdiplus_test.exe.so font.c && touch font.ok
> font.c:400: Test failed: Expected 0, got 3
> font.c:401: Test failed: got NULL font collection
> make[1]: *** [font.ok] Error 2

I can reproduce the failure with run gdiplus font test right after
gdi32 font test, however, if a `wineserver -k` is executed  between
the two test, then the gdiplus failure will disappear.

The attach patch should fix this problem.

This time I grepped
EnumFontFamilies/CreateScalableFontResource/AddFontResource in all
dlls and manually run similar tests with related dlls
(usp10/dwrite/oleaut32/comdlg32/gdiplus...), each test is executed
either right after gdi32/font test or independently, hopefully this
time such failure will not happen again.

This patch is here for fixing the side effect of
gdi32/tests/font.c:test_CreateScalableFontResource().
usp10/tests/usp10.c:test_ScriptGetFontProperties() fails due to this
side effect because the font file is removed but the wine_test font is
not removed from the font list since RemoveFontResource() is a stub in
Wine.

However, the attached solution is not exactly match what Windows does:
On Windows, if the font file is deleted after
CreateScalableFontResource() and AddFontResource(), EnumFontFamilies
still enumerate the missing font until next reboot. Do we need to
follow what Windows does in this case?
I don't know if there is any real world app rely on this behavior, if
no I don't have a strong motivation for implementing it.

IMO the best solution is to correctly implement RemoveFontResource()
so Bug 8292 will be fixed, then we don't need to worry about the
broken test in usp10, which is the original reason for this patch.

Would you like me send this patch which is still an improvement, or
just leave the broken test there until RemoveFontResource is
implement?

Thanks for any suggestions.

--
Regards,
Qian Hong

-
Sent from Ubuntu
http://www.ubuntu.com/

[PATCH] gdi32: Don't enumerate fonts when OpenFontFace fails (try 2).

[Alexandre Julliard]

Qian Hong  writes:
> IMO the best solution is to correctly implement RemoveFontResource()
> so Bug 8292 will be fixed, then we don't need to worry about the
> broken test in usp10, which is the original reason for this patch.
>
> Would you like me send this patch which is still an improvement, or
> just leave the broken test there until RemoveFontResource is
> implement?

Either way we probably need to improve handling of font files being
deleted unexpectedly. But it's certainly reasonable to implement
RemoveFontResource first and then see what remains to be fixed.