Bug 160898 - [Win] Add tests for linked fonts.
Summary: [Win] Add tests for linked fonts.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Per Arne Vollan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-08-16 07:00 PDT by Per Arne Vollan
Modified: 2016-08-17 07:41 PDT (History)
4 users (show)

See Also:


Attachments
Patch (7.33 KB, patch)
2016-08-16 07:24 PDT, Per Arne Vollan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Per Arne Vollan 2016-08-16 07:00:36 PDT
Add tests for https://trac.webkit.org/changeset/204502.
Comment 1 Per Arne Vollan 2016-08-16 07:24:52 PDT
Created attachment 286174 [details]
Patch
Comment 2 Brent Fulgham 2016-08-16 10:13:02 PDT
Comment on attachment 286174 [details]
Patch

Looks good! r=me.
Comment 3 Myles C. Maxfield 2016-08-16 11:10:25 PDT
Comment on attachment 286174 [details]
Patch

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

> Source/WebCore/platform/graphics/win/FontCacheWin.cpp:99
> +WEBCORE_EXPORT void appendLinkedFonts(WCHAR* linkedFonts, unsigned length, Vector<String>* result)

There is no way to test this without exporting the function? This seems suspicious.
Comment 4 Per Arne Vollan 2016-08-16 11:45:48 PDT
(In reply to comment #3)
> Comment on attachment 286174 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=286174&action=review
> 
> > Source/WebCore/platform/graphics/win/FontCacheWin.cpp:99
> > +WEBCORE_EXPORT void appendLinkedFonts(WCHAR* linkedFonts, unsigned length, Vector<String>* result)
> 
> There is no way to test this without exporting the function? This seems
> suspicious.

Since we have a separate test executable for WebCore, I thought this was the way to do it. I might have missed something, though.

Thanks for looking into this :-)
Comment 5 Per Arne Vollan 2016-08-16 11:46:29 PDT
(In reply to comment #2)
> Comment on attachment 286174 [details]
> Patch
> 
> Looks good! r=me.

Thanks for reviewing!
Comment 6 Myles C. Maxfield 2016-08-16 12:52:25 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > Comment on attachment 286174 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=286174&action=review
> > 
> > > Source/WebCore/platform/graphics/win/FontCacheWin.cpp:99
> > > +WEBCORE_EXPORT void appendLinkedFonts(WCHAR* linkedFonts, unsigned length, Vector<String>* result)
> > 
> > There is no way to test this without exporting the function? This seems
> > suspicious.
> 
> Since we have a separate test executable for WebCore, I thought this was the
> way to do it. I might have missed something, though.
> 
> Thanks for looking into this :-)

I mean, isn't there a way a regular layout test can trigger this code path?
Comment 7 Per Arne Vollan 2016-08-17 07:19:49 PDT
(In reply to comment #6)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > Comment on attachment 286174 [details]
> > > Patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=286174&action=review
> > > 
> > > > Source/WebCore/platform/graphics/win/FontCacheWin.cpp:99
> > > > +WEBCORE_EXPORT void appendLinkedFonts(WCHAR* linkedFonts, unsigned length, Vector<String>* result)
> > > 
> > > There is no way to test this without exporting the function? This seems
> > > suspicious.
> > 
> > Since we have a separate test executable for WebCore, I thought this was the
> > way to do it. I might have missed something, though.
> > 
> > Thanks for looking into this :-)
> 
> I mean, isn't there a way a regular layout test can trigger this code path?

Normally, there will be a comma in the linked font list, since this is read from the Windows registry. I think it would be possible to write a layout test for this, but it will require a bit more work.
Comment 8 WebKit Commit Bot 2016-08-17 07:41:00 PDT
Comment on attachment 286174 [details]
Patch

Clearing flags on attachment: 286174

Committed r204558: <http://trac.webkit.org/changeset/204558>
Comment 9 WebKit Commit Bot 2016-08-17 07:41:05 PDT
All reviewed patches have been landed.  Closing bug.