The native font handle created from webfont data is invalid. In much the same way it's done in WinApple, we first need to install the webfont, create a native font handle from the new font name, and pass the font handle to the FontPlatformData object, where it's needed to get the glyphs.
Created attachment 204821 [details] Patch
Comment on attachment 204821 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=204821&action=review I think this looks good, but I'd like Martin Robinson to double-check that we aren't doing something bad to the Gtk+ port. I also had a few questions I'd like you to double-check for me before I r+ the patch. Thanks! > Source/WebCore/platform/graphics/FontPlatformData.h:109 > + FontPlatformData(HFONT, cairo_font_face_t*, float size, bool bold, bool italic); It seems like this should break Gtk+, but the EWS says its okay... It seems like this should maybe be protected by USE(WINCAIRO). > Source/WebCore/platform/graphics/win/FontCustomPlatformDataCairo.cpp:82 > FontCustomPlatformData* createFontCustomPlatformData(SharedBuffer* buffer) Should this be declared static now? It's not used outside this module, is it? > Source/WebCore/platform/graphics/win/FontCustomPlatformDataCairo.h:-49 > - cairo_font_face_t* m_fontFace; Is there no benefit to holding the cairo_font_fact_t*? Is it needed anywhere else? > Source/WebCore/platform/graphics/win/FontPlatformDataCairoWin.cpp:60 > + : m_font(RefCountedGDIHandle<HFONT>::create(font)) Good!
CC'ing mrobinson to double-check that there are no Gtk+ problems with this patch.
Comment on attachment 204821 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=204821&action=review >> Source/WebCore/platform/graphics/FontPlatformData.h:109 >> + FontPlatformData(HFONT, cairo_font_face_t*, float size, bool bold, bool italic); > > It seems like this should break Gtk+, but the EWS says its okay... It seems like this should maybe be protected by USE(WINCAIRO). I think that perhaps GTK+ and EFL are not including this file, but are instead including WebKit/Source/WebCore/platform/graphics/freetype/FontPlatformData.h. Looks good to me. :)
(In reply to comment #2) Thanks for reviewing, Brent! > > Source/WebCore/platform/graphics/win/FontCustomPlatformDataCairo.cpp:82 > > FontCustomPlatformData* createFontCustomPlatformData(SharedBuffer* buffer) > > Should this be declared static now? It's not used outside this module, is it? Yes, it's used by WebCore\loader\cache\CachedFont.cpp. > > Source/WebCore/platform/graphics/win/FontCustomPlatformDataCairo.h:-49 > > - cairo_font_face_t* m_fontFace; > > Is there no benefit to holding the cairo_font_fact_t*? Is it needed anywhere else? I don't think it's currently needed elsewhere (no compile errors). The FontCustomPlatformData object is only used to create a FontPlatformData object via the fontPlatformData() method, and the cairo_font_face_t pointer is only used to create a cairo_scaled_font_t pointer in the FontPlatformData constructor, so I believe it can be a temporary variable.
Created attachment 204915 [details] Patch
(In reply to comment #6) > Created an attachment (id=204915) [details] > Patch Modified patch according to review, moved windows specific constructor into PLATFORM(WIN) section.
Comment on attachment 204915 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=204915&action=review This patch looks great. Could you just make the "#elif" change I asked for when you land the change? > Source/WebCore/platform/graphics/FontPlatformData.h:107 > #if USE(CAIRO) nit: This should probably have been an "#elif USE(CAIRO)", since we never use both graphics packages together (they are exclusive).
Created attachment 204973 [details] Patch
(In reply to comment #8) > nit: This should probably have been an "#elif USE(CAIRO)", since we never use both graphics packages together (they are exclusive). Changed the patch to use "#elif USE(CAIRO)". If the patch is ok, could you also commit+ it, I don't have committer rights :)
Comment on attachment 204973 [details] Patch Thank you for making those changes! r=me.
Comment on attachment 204973 [details] Patch Clearing flags on attachment: 204973 Committed r151743: <http://trac.webkit.org/changeset/151743>
All reviewed patches have been landed. Closing bug.