Bug 16941 – [GTK] FontCustomPlatformData.cpp leaks FT_Faces

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Alp Toker]

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);

[Alp Toker]

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.

[Alp Toker]

This can probably be fixed by carrying the resources in the FontCustomPlatformData structure and freeing them in ~FontCustomPlatformData.

[Martin Robinson]

Created attachment 68308 [details]
Patch for this issue

[WebKit Review Bot]

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.

[Martin Robinson]

This is a false positive. We should update check-webkit-style to ignore typedefs like these.

[Xan Lopez]

Comment on attachment 68308 [details]
Patch for this issue

r=me

[Martin Robinson]

Committed r68041: <http://trac.webkit.org/changeset/68041>