Bug 16941

Summary: [GTK] FontCustomPlatformData.cpp leaks FT_Faces
Product: WebKit Reporter: Alp Toker <alp>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: gns, mrobinson, webkit.review.bot, xan.lopez
Priority: P2 Keywords: Cairo, Gtk
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch for this issue none

Description Alp Toker 2008-01-19 09:54:02 PST
The newly created memory face needs to be destroyed manually according to the documentation.

Since we also need to free the SharedBuffer, we can't just add another user key I think.

It might be better to add the newly created FontCustomPlatformData as the user data and do the freeing from there. Not sure yet what the best fix is.

    FT_Face face;
    error = FT_New_Memory_Face(library, reinterpret_cast<const FT_Byte*>(buffer->data()), buffer->size(), 0, &face);
    if (error)
        return 0;

    buffer->ref();
    cairo_font_face_t* fontFace = cairo_ft_font_face_create_for_ft_face(face, 0);

    static cairo_user_data_key_t bufferKey;
    cairo_font_face_set_user_data(fontFace, &bufferKey, buffer, releaseData);

    return new FontCustomPlatformData(fontFace);
Comment 1 Alp Toker 2008-01-23 18:00:11 PST
From SimpleFontDataGtk.cpp:

void SimpleFontData::platformDestroy()
{
    if (!isCustomFont()) {
        if (m_font.m_pattern && ((FcPattern*)-1 != m_font.m_pattern)) {
            FcPatternDestroy(m_font.m_pattern);
            m_font.m_pattern = 0;
        }

        if (m_font.m_scaledFont) {
            cairo_scaled_font_destroy(m_font.m_scaledFont);
            m_font.m_scaledFont = 0;
        }
    }

    delete m_smallCapsFontData;
}


This isCustomFont() check looks overzealous as it won't free m_font.m_scaledFont which exists even in custom WebFonts. So, another potential leak here.
Comment 2 Alp Toker 2008-08-27 17:55:24 PDT
This can probably be fixed by carrying the resources in the FontCustomPlatformData structure and freeing them in ~FontCustomPlatformData.
Comment 3 Martin Robinson 2010-09-21 16:59:38 PDT
Created attachment 68308 [details]
Patch for this issue
Comment 4 WebKit Review Bot 2010-09-21 17:00:55 PDT
Attachment 68308 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/platform/graphics/cairo/FontCustomPlatformData.h:29:  FT_Face is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Martin Robinson 2010-09-21 17:02:25 PDT
This is a false positive. We should update check-webkit-style to ignore typedefs like these.
Comment 6 Xan Lopez 2010-09-21 22:43:23 PDT
Comment on attachment 68308 [details]
Patch for this issue

r=me
Comment 7 Martin Robinson 2010-09-22 08:01:38 PDT
Committed r68041: <http://trac.webkit.org/changeset/68041>