[webkit-reviews] review denied: [Bug 38758] [gtk] web fonts not loaded properly in scribd html5 reader : [Attachment 56913] second try

[bugzilla-daemon at webkit.org]

Xan Lopez <xan.lopez at gmail.com> has denied Gustavo Noronha (kov)
<gns at gnome.org>'s request for review:
Bug 38758: [gtk] web fonts not loaded properly in scribd html5 reader
https://bugs.webkit.org/show_bug.cgi?id=38758

Attachment 56913: second try
https://bugs.webkit.org/attachment.cgi?id=56913&action=review

------- Additional Comments from Xan Lopez <xan.lopez at gmail.com>
>+    // Handle generic family types specially, because fontconfig does not
know them, but we have
>+    // code to fallback correctly in our platform data implementation. Times
New Roman is treated
>+    // specially, as it is used as last fallback.
>+    if (!family.length() || family.startsWith("-webkit-")
>+	  || (fontDescription.genericFamily() != FontDescription::NoFamily)
>+	  || isWellKnownFontName(family) || (family == "Times New Roman"))
>+	  return new FontPlatformData(fontDescription, family);

Not sure if it makes sense to not have Time News Roman in the well known list
at this point.

>+
>+    // First check the font exists.
>+    CString familyNameString = family.string().utf8();
>+    const char* fcfamily = familyNameString.data();
>+
>+    FcPattern* pattern = FcPatternCreate();
>+    if (!FcPatternAddString(pattern, FC_FAMILY, reinterpret_cast<const
FcChar8*>(fcfamily))) {
>+	  FcPatternDestroy(pattern);
>+	  return 0;
>+    }
>+
>+    FcConfigSubstitute(0, pattern, FcMatchPattern);
>+    FcDefaultSubstitute(pattern);
>+
>+    FcObjectSet* objectSet = FcObjectSetCreate();
>+    if (!FcObjectSetAdd(objectSet, FC_FAMILY)) {
>+	  FcPatternDestroy(pattern);
>+	  FcObjectSetDestroy(objectSet);
>+	  return 0;
>+    }
>+
>+    FcFontSet* fontSet = FcFontList(0, pattern, objectSet);
>+    FcPatternDestroy(pattern);
>+    FcObjectSetDestroy(objectSet);
>+
>+    if (!fontSet)
>+	  return 0;
>+
>+    if (!fontSet->fonts) {
>+	  FcFontSetDestroy(fontSet);
>+	  return 0;
>+    }
>+
>+    FcFontSetDestroy(fontSet);
>+
>     return new FontPlatformData(fontDescription, family);

Most of the code here is just freeing a couple of data structures all the time,
guess we should just use GOwnPtr.

Looks good other than that.