<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[180825] releases/WebKitGTK/webkit-2.8/Source/WebCore</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/180825">180825</a></dd>
<dt>Author</dt> <dd>carlosgc@webkit.org</dd>
<dt>Date</dt> <dd>2015-02-28 03:00:46 -0800 (Sat, 28 Feb 2015)</dd>
</dl>

<h3>Log Message</h3>
<pre>Merge <a href="http://trac.webkit.org/projects/webkit/changeset/180563">r180563</a> - [GTK] Fonts loaded via @font-face look bad
https://bugs.webkit.org/show_bug.cgi?id=140994

Patch by Michael Catanzaro &lt;mcatanzaro@igalia.com&gt; on 2015-02-24
Reviewed by Martin Robinson.

We've had several complaints that woff fonts look bad on some websites. This seems to be a
combination of multiple issues. For one, we don't look at Fontconfig settings at all when
creating a web font. This commit changes FontPlatformData::initializeWithFontFace to instead
use sane default settings from Fontconfig when loading a web font, rather than accepting the
default settings from GTK+, which are normally overridden by Fontconfig when loading system
fonts. However, we will hardcode the hinting setting for web fonts to always force use of
the light autohinter, so that we do not use a font's native hints. This avoids compatibility
issues when fonts with poor native hinting are only tested in browsers that do not use the
native hints. It also allows us to sidestep future security vulnerabilities in FreeType's
bytecode interpreter.

The net result of this is that there should be little noticable difference if you have GTK+
set to use slight hinting (which forces use of the autohinter) unless you have customized
Fontconfig configuration, but a dramatic improvement with particular fonts if you use medium
or full hinting. This should reduce complaints about our font rendering on &quot;fancy sites.&quot;

No new tests, since the affected fonts we've found are not freely redistributable.

* platform/graphics/freetype/FontCustomPlatformDataFreeType.cpp:
(WebCore::FontCustomPlatformData::FontCustomPlatformData): Force web fonts to be autohinted.
* platform/graphics/freetype/FontPlatformDataFreeType.cpp:
(WebCore::getDefaultCairoFontOptions): Renamed to disambiguate.
(WebCore::getDefaultFontconfigOptions): Added.
(WebCore::FontPlatformData::initializeWithFontFace): Always call
FontPlatformData::setCairoOptionsFromFontConfigPattern. If the FontPlatformData was not
created with an FcPattern (e.g. because this is a web font), call
getDefaultFontconfigOptions to get a sane default FcPattern.
(WebCore::FontPlatformData::setOrientation): Renamed call to getDefaultCairoFontOptions.
(WebCore::getDefaultFontOptions): Deleted.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#releasesWebKitGTKwebkit28SourceWebCoreChangeLog">releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog</a></li>
<li><a href="#releasesWebKitGTKwebkit28SourceWebCoreplatformgraphicsfreetypeFontCustomPlatformDataFreeTypecpp">releases/WebKitGTK/webkit-2.8/Source/WebCore/platform/graphics/freetype/FontCustomPlatformDataFreeType.cpp</a></li>
<li><a href="#releasesWebKitGTKwebkit28SourceWebCoreplatformgraphicsfreetypeFontPlatformDataFreeTypecpp">releases/WebKitGTK/webkit-2.8/Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="releasesWebKitGTKwebkit28SourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog (180824 => 180825)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog        2015-02-28 10:56:57 UTC (rev 180824)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog        2015-02-28 11:00:46 UTC (rev 180825)
</span><span class="lines">@@ -1,3 +1,54 @@
</span><ins>+2015-02-26  Michael Catanzaro  &lt;mcatanzaro@igalia.com&gt;
+
+        [FreeType] REGRESSION(r180563): Introduced crashes
+        https://bugs.webkit.org/show_bug.cgi?id=142044
+
+        Reviewed by Martin Robinson.
+
+        No new tests, should be caught by any woff font test.
+
+        Use optionsPattern, not m_pattern, when m_pattern may be null.
+
+        * platform/graphics/freetype/FontPlatformDataFreeType.cpp:
+        (WebCore::FontPlatformData::initializeWithFontFace):
+
+2015-02-24  Michael Catanzaro  &lt;mcatanzaro@igalia.com&gt;
+
+        [GTK] Fonts loaded via @font-face look bad
+        https://bugs.webkit.org/show_bug.cgi?id=140994
+
+        Reviewed by Martin Robinson.
+
+        We've had several complaints that woff fonts look bad on some websites. This seems to be a
+        combination of multiple issues. For one, we don't look at Fontconfig settings at all when
+        creating a web font. This commit changes FontPlatformData::initializeWithFontFace to instead
+        use sane default settings from Fontconfig when loading a web font, rather than accepting the
+        default settings from GTK+, which are normally overridden by Fontconfig when loading system
+        fonts. However, we will hardcode the hinting setting for web fonts to always force use of
+        the light autohinter, so that we do not use a font's native hints. This avoids compatibility
+        issues when fonts with poor native hinting are only tested in browsers that do not use the
+        native hints. It also allows us to sidestep future security vulnerabilities in FreeType's
+        bytecode interpreter.
+
+        The net result of this is that there should be little noticable difference if you have GTK+
+        set to use slight hinting (which forces use of the autohinter) unless you have customized
+        Fontconfig configuration, but a dramatic improvement with particular fonts if you use medium
+        or full hinting. This should reduce complaints about our font rendering on &quot;fancy sites.&quot;
+
+        No new tests, since the affected fonts we've found are not freely redistributable.
+
+        * platform/graphics/freetype/FontCustomPlatformDataFreeType.cpp:
+        (WebCore::FontCustomPlatformData::FontCustomPlatformData): Force web fonts to be autohinted.
+        * platform/graphics/freetype/FontPlatformDataFreeType.cpp:
+        (WebCore::getDefaultCairoFontOptions): Renamed to disambiguate.
+        (WebCore::getDefaultFontconfigOptions): Added.
+        (WebCore::FontPlatformData::initializeWithFontFace): Always call
+        FontPlatformData::setCairoOptionsFromFontConfigPattern. If the FontPlatformData was not
+        created with an FcPattern (e.g. because this is a web font), call
+        getDefaultFontconfigOptions to get a sane default FcPattern.
+        (WebCore::FontPlatformData::setOrientation): Renamed call to getDefaultCairoFontOptions.
+        (WebCore::getDefaultFontOptions): Deleted.
+
</ins><span class="cx"> 2015-02-24  Dhi Aurrahman  &lt;diorahman@rockybars.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Always serialize :lang()'s arguments to strings
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit28SourceWebCoreplatformgraphicsfreetypeFontCustomPlatformDataFreeTypecpp"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/platform/graphics/freetype/FontCustomPlatformDataFreeType.cpp (180824 => 180825)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.8/Source/WebCore/platform/graphics/freetype/FontCustomPlatformDataFreeType.cpp        2015-02-28 10:56:57 UTC (rev 180824)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/platform/graphics/freetype/FontCustomPlatformDataFreeType.cpp        2015-02-28 11:00:46 UTC (rev 180825)
</span><span class="lines">@@ -35,9 +35,21 @@
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> FontCustomPlatformData::FontCustomPlatformData(FT_Face freeTypeFace, SharedBuffer&amp; buffer)
</span><del>-    : m_fontFace(cairo_ft_font_face_create_for_ft_face(freeTypeFace, 0))
</del><ins>+    : m_fontFace(cairo_ft_font_face_create_for_ft_face(freeTypeFace, FT_LOAD_FORCE_AUTOHINT))
</ins><span class="cx"> {
</span><del>-    // FIXME Should we be setting some hinting options here?
</del><ins>+    // FT_LOAD_FORCE_AUTOHINT prohibits use of the font's native hinting. This
+    // is a safe option for custom fonts because (a) some such fonts may have
+    // broken hinting, which site admins may not notice if other browsers do not
+    // use the native hints, and (b) allowing native hints exposes the FreeType
+    // bytecode interpreter to potentially-malicious input. Treating web fonts
+    // differently than system fonts is non-ideal, but the result of autohinting
+    // is always decent, whereas native hints sometimes look terrible, and
+    // unlike system fonts where Fontconfig may change the hinting settings on a
+    // per-font basis, the same settings are used for all web fonts. Note that
+    // Chrome is considering switching from autohinting to native hinting in
+    // https://code.google.com/p/chromium/issues/detail?id=173207 but this is
+    // more risk than we want to assume for now. See
+    // https://bugs.webkit.org/show_bug.cgi?id=140994 before changing this.
</ins><span class="cx"> 
</span><span class="cx">     buffer.ref(); // This is balanced by the buffer-&gt;deref() in releaseCustomFontData.
</span><span class="cx">     static cairo_user_data_key_t bufferKey;
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit28SourceWebCoreplatformgraphicsfreetypeFontPlatformDataFreeTypecpp"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp (180824 => 180825)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.8/Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp        2015-02-28 10:56:57 UTC (rev 180824)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp        2015-02-28 11:00:46 UTC (rev 180825)
</span><span class="lines">@@ -26,6 +26,7 @@
</span><span class="cx"> #include &quot;FontPlatformData.h&quot;
</span><span class="cx"> 
</span><span class="cx"> #include &quot;FontDescription.h&quot;
</span><ins>+#include &quot;RefPtrCairo.h&quot;
</ins><span class="cx"> #include &lt;cairo-ft.h&gt;
</span><span class="cx"> #include &lt;cairo.h&gt;
</span><span class="cx"> #include &lt;fontconfig/fcfreetype.h&gt;
</span><span class="lines">@@ -103,7 +104,7 @@
</span><span class="cx">         cairo_font_options_set_hint_style(options, CAIRO_HINT_STYLE_NONE);
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-static cairo_font_options_t* getDefaultFontOptions()
</del><ins>+static cairo_font_options_t* getDefaultCairoFontOptions()
</ins><span class="cx"> {
</span><span class="cx"> #if PLATFORM(GTK)
</span><span class="cx">     if (GdkScreen* screen = gdk_screen_get_default()) {
</span><span class="lines">@@ -115,6 +116,24 @@
</span><span class="cx">     return cairo_font_options_create();
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+static FcPattern* getDefaultFontconfigOptions()
+{
+    // Get some generic default settings from fontconfig for web fonts. Strategy
+    // from Behdad Esfahbod in https://code.google.com/p/chromium/issues/detail?id=173207#c35
+    // For web fonts, the hint style is overridden in FontCustomPlatformData::FontCustomPlatformData
+    // so Fontconfig will not affect the hint style, but it may disable hinting completely.
+    static FcPattern* pattern = nullptr;
+    static std::once_flag flag;
+    std::call_once(flag, [](FcPattern*) {
+        pattern = FcPatternCreate();
+        FcConfigSubstitute(nullptr, pattern, FcMatchPattern);
+        FcDefaultSubstitute(pattern);
+        FcPatternDel(pattern, FC_FAMILY);
+        FcConfigSubstitute(nullptr, pattern, FcMatchFont);
+    }, pattern);
+    return pattern;
+}
+
</ins><span class="cx"> static void rotateCairoMatrixForVerticalOrientation(cairo_matrix_t* matrix)
</span><span class="cx"> {
</span><span class="cx">     // The resulting transformation matrix for vertical glyphs (V) is a
</span><span class="lines">@@ -283,7 +302,9 @@
</span><span class="cx"> 
</span><span class="cx"> void FontPlatformData::initializeWithFontFace(cairo_font_face_t* fontFace, const FontDescription&amp; fontDescription)
</span><span class="cx"> {
</span><del>-    cairo_font_options_t* options = getDefaultFontOptions();
</del><ins>+    cairo_font_options_t* options = getDefaultCairoFontOptions();
+    FcPattern* optionsPattern = m_pattern ? m_pattern.get() : getDefaultFontconfigOptions();
+    setCairoFontOptionsFromFontConfigPattern(options, optionsPattern);
</ins><span class="cx"> 
</span><span class="cx">     cairo_matrix_t ctm;
</span><span class="cx">     cairo_matrix_init_identity(&amp;ctm);
</span><span class="lines">@@ -292,32 +313,26 @@
</span><span class="cx">     // Instead we scale we scale the font to a very tiny size and just abort rendering later on.
</span><span class="cx">     float realSize = m_size ? m_size : 1;
</span><span class="cx"> 
</span><ins>+    // FontConfig may return a list of transformation matrices with the pattern, for instance,
+    // for fonts that are oblique. We use that to initialize the cairo font matrix.
</ins><span class="cx">     cairo_matrix_t fontMatrix;
</span><del>-    if (!m_pattern)
-        cairo_matrix_init_scale(&amp;fontMatrix, realSize, realSize);
-    else {
-        setCairoFontOptionsFromFontConfigPattern(options, m_pattern.get());
</del><ins>+    FcMatrix fontConfigMatrix, *tempFontConfigMatrix;
+    FcMatrixInit(&amp;fontConfigMatrix);
</ins><span class="cx"> 
</span><del>-        // FontConfig may return a list of transformation matrices with the pattern, for instance,
-        // for fonts that are oblique. We use that to initialize the cairo font matrix.
-        FcMatrix fontConfigMatrix, *tempFontConfigMatrix;
-        FcMatrixInit(&amp;fontConfigMatrix);
</del><ins>+    // These matrices may be stacked in the pattern, so it's our job to get them all and multiply them.
+    for (int i = 0; FcPatternGetMatrix(optionsPattern, FC_MATRIX, i, &amp;tempFontConfigMatrix) == FcResultMatch; i++)
+        FcMatrixMultiply(&amp;fontConfigMatrix, &amp;fontConfigMatrix, tempFontConfigMatrix);
+    cairo_matrix_init(&amp;fontMatrix, fontConfigMatrix.xx, -fontConfigMatrix.yx,
+        -fontConfigMatrix.xy, fontConfigMatrix.yy, 0, 0);
</ins><span class="cx"> 
</span><del>-        // These matrices may be stacked in the pattern, so it's our job to get them all and multiply them.
-        for (int i = 0; FcPatternGetMatrix(m_pattern.get(), FC_MATRIX, i, &amp;tempFontConfigMatrix) == FcResultMatch; i++)
-            FcMatrixMultiply(&amp;fontConfigMatrix, &amp;fontConfigMatrix, tempFontConfigMatrix);
-        cairo_matrix_init(&amp;fontMatrix, fontConfigMatrix.xx, -fontConfigMatrix.yx,
-                          -fontConfigMatrix.xy, fontConfigMatrix.yy, 0, 0);
</del><ins>+    // We requested an italic font, but Fontconfig gave us one that was neither oblique nor italic.
+    int actualFontSlant;
+    if (fontDescription.italic() &amp;&amp; FcPatternGetInteger(optionsPattern, FC_SLANT, 0, &amp;actualFontSlant) == FcResultMatch)
+        m_syntheticOblique = actualFontSlant == FC_SLANT_ROMAN;
</ins><span class="cx"> 
</span><del>-        // We requested an italic font, but Fontconfig gave us one that was neither oblique nor italic.
-        int actualFontSlant;
-        if (fontDescription.italic() &amp;&amp; FcPatternGetInteger(m_pattern.get(), FC_SLANT, 0, &amp;actualFontSlant) == FcResultMatch)
-            m_syntheticOblique = actualFontSlant == FC_SLANT_ROMAN;
</del><ins>+    // The matrix from FontConfig does not include the scale. 
+    cairo_matrix_scale(&amp;fontMatrix, realSize, realSize);
</ins><span class="cx"> 
</span><del>-        // The matrix from FontConfig does not include the scale. 
-        cairo_matrix_scale(&amp;fontMatrix, realSize, realSize);
-    }
-
</del><span class="cx">     if (syntheticOblique()) {
</span><span class="cx">         static const float syntheticObliqueSkew = -tanf(14 * acosf(0) / 90);
</span><span class="cx">         cairo_matrix_t skew = {1, 0, syntheticObliqueSkew, 1, 0, 0};
</span><span class="lines">@@ -388,7 +403,7 @@
</span><span class="cx">     cairo_matrix_t fontMatrix;
</span><span class="cx">     cairo_scaled_font_get_font_matrix(m_scaledFont, &amp;fontMatrix);
</span><span class="cx"> 
</span><del>-    cairo_font_options_t* options = getDefaultFontOptions();
</del><ins>+    cairo_font_options_t* options = getDefaultCairoFontOptions();
</ins><span class="cx"> 
</span><span class="cx">     // In case of vertical orientation, rotate the transformation matrix.
</span><span class="cx">     // Otherwise restore the horizontal orientation matrix.
</span></span></pre>
</div>
</div>

</body>
</html>