<!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>[177689] trunk/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/177689">177689</a></dd>
<dt>Author</dt> <dd>mmaxfield@apple.com</dd>
<dt>Date</dt> <dd>2014-12-23 11:23:48 -0800 (Tue, 23 Dec 2014)</dd>
</dl>

<h3>Log Message</h3>
<pre>platform/mac/editing/input/devanagari-ligature.html is flaky on Yosemite, ligature fails to form
https://bugs.webkit.org/show_bug.cgi?id=138683

Reviewed by Darin Adler.

This patch changes how we check fonts for equality. In particular, this patch adds a
objectForEqualityCheck() to Cocoa's FontPlatformData, and callers should pass this object
to CFEqual() to determine if two platform fonts are equal. This patch also migrates all
call sites to using this function.

I don't want to implement operator==() because there are many cases where the same font
is compared against many others, and this solution is cleaner than caching a comparison
object inside the font object itself.

No new tests because this is covered by platform/mac/editing/input/devanagari-ligature.html.

* platform/graphics/FontPlatformData.h:
* platform/graphics/cocoa/FontPlatformDataCocoa.mm:
(WebCore::FontPlatformData::objectForEqualityCheck):
* platform/graphics/mac/ComplexTextControllerCoreText.mm:
(WebCore::ComplexTextController::collectComplexTextRunsForCharacters):
* platform/graphics/mac/GlyphPageTreeNodeMac.cpp:
(WebCore::GlyphPage::fill):
* platform/graphics/mac/SimpleFontDataMac.mm:
(WebCore::SimpleFontData::canRenderCombiningCharacterSequence):
* platform/spi/cocoa/CoreTextSPI.h:</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsFontPlatformDatah">trunk/Source/WebCore/platform/graphics/FontPlatformData.h</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicscocoaFontPlatformDataCocoamm">trunk/Source/WebCore/platform/graphics/cocoa/FontPlatformDataCocoa.mm</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsmacComplexTextControllerCoreTextmm">trunk/Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.mm</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsmacGlyphPageTreeNodeMaccpp">trunk/Source/WebCore/platform/graphics/mac/GlyphPageTreeNodeMac.cpp</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsmacSimpleFontDataMacmm">trunk/Source/WebCore/platform/graphics/mac/SimpleFontDataMac.mm</a></li>
<li><a href="#trunkSourceWebCoreplatformspicocoaCoreTextSPIh">trunk/Source/WebCore/platform/spi/cocoa/CoreTextSPI.h</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (177688 => 177689)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog        2014-12-23 18:45:05 UTC (rev 177688)
+++ trunk/Source/WebCore/ChangeLog        2014-12-23 19:23:48 UTC (rev 177689)
</span><span class="lines">@@ -1,5 +1,34 @@
</span><span class="cx"> 2014-12-23  Myles C. Maxfield  &lt;mmaxfield@apple.com&gt;
</span><span class="cx"> 
</span><ins>+        platform/mac/editing/input/devanagari-ligature.html is flaky on Yosemite, ligature fails to form
+        https://bugs.webkit.org/show_bug.cgi?id=138683
+
+        Reviewed by Darin Adler.
+
+        This patch changes how we check fonts for equality. In particular, this patch adds a
+        objectForEqualityCheck() to Cocoa's FontPlatformData, and callers should pass this object
+        to CFEqual() to determine if two platform fonts are equal. This patch also migrates all
+        call sites to using this function.
+
+        I don't want to implement operator==() because there are many cases where the same font
+        is compared against many others, and this solution is cleaner than caching a comparison
+        object inside the font object itself.
+
+        No new tests because this is covered by platform/mac/editing/input/devanagari-ligature.html.
+
+        * platform/graphics/FontPlatformData.h:
+        * platform/graphics/cocoa/FontPlatformDataCocoa.mm:
+        (WebCore::FontPlatformData::objectForEqualityCheck):
+        * platform/graphics/mac/ComplexTextControllerCoreText.mm:
+        (WebCore::ComplexTextController::collectComplexTextRunsForCharacters):
+        * platform/graphics/mac/GlyphPageTreeNodeMac.cpp:
+        (WebCore::GlyphPage::fill):
+        * platform/graphics/mac/SimpleFontDataMac.mm:
+        (WebCore::SimpleFontData::canRenderCombiningCharacterSequence):
+        * platform/spi/cocoa/CoreTextSPI.h:
+
+2014-12-23  Myles C. Maxfield  &lt;mmaxfield@apple.com&gt;
+
</ins><span class="cx">         [SVG -&gt; OTF Converter] Make Placeholder a move-only type
</span><span class="cx">         https://bugs.webkit.org/show_bug.cgi?id=139870
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsFontPlatformDatah"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/FontPlatformData.h (177688 => 177689)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/FontPlatformData.h        2014-12-23 18:45:05 UTC (rev 177688)
+++ trunk/Source/WebCore/platform/graphics/FontPlatformData.h        2014-12-23 19:23:48 UTC (rev 177689)
</span><span class="lines">@@ -120,6 +120,8 @@
</span><span class="cx">     void setFont(CTFontRef);
</span><span class="cx"> 
</span><span class="cx">     CTFontRef ctFont() const;
</span><ins>+    static RetainPtr&lt;CFTypeRef&gt; objectForEqualityCheck(CTFontRef);
+    RetainPtr&lt;CFTypeRef&gt; objectForEqualityCheck() const;
</ins><span class="cx"> 
</span><span class="cx">     bool allowsLigatures() const;
</span><span class="cx">     bool roundsGlyphAdvances() const;
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicscocoaFontPlatformDataCocoamm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/cocoa/FontPlatformDataCocoa.mm (177688 => 177689)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/cocoa/FontPlatformDataCocoa.mm        2014-12-23 18:45:05 UTC (rev 177688)
+++ trunk/Source/WebCore/platform/graphics/cocoa/FontPlatformDataCocoa.mm        2014-12-23 19:23:48 UTC (rev 177689)
</span><span class="lines">@@ -24,6 +24,7 @@
</span><span class="cx"> #import &quot;config.h&quot;
</span><span class="cx"> #import &quot;FontPlatformData.h&quot;
</span><span class="cx"> 
</span><ins>+#import &quot;CoreTextSPI.h&quot;
</ins><span class="cx"> #import &quot;SharedBuffer.h&quot;
</span><span class="cx"> #import &quot;WebCoreSystemInterface.h&quot;
</span><span class="cx"> #import &lt;wtf/text/WTFString.h&gt;
</span><span class="lines">@@ -289,6 +290,25 @@
</span><span class="cx">     return m_ctFont.get();
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+RetainPtr&lt;CFTypeRef&gt; FontPlatformData::objectForEqualityCheck(CTFontRef ctFont)
+{
+#if (PLATFORM(IOS) &amp;&amp; __IPHONE_OS_VERSION_MIN_REQUIRED &lt;= 80000) || (PLATFORM(MAC) &amp;&amp; __MAC_OS_X_VERSION_MIN_REQUIRED &lt;= 101000)
+    auto fontDescriptor = adoptCF(CTFontCopyFontDescriptor(ctFont));
+    // FIXME: https://bugs.webkit.org/show_bug.cgi?id=138683 This is a shallow pointer compare for web fonts
+    // because the URL contains the address of the font. This means we might erroneously get false negatives.
+    RetainPtr&lt;CFURLRef&gt; url = adoptCF(static_cast&lt;CFURLRef&gt;(CTFontDescriptorCopyAttribute(fontDescriptor.get(), kCTFontReferenceURLAttribute)));
+    ASSERT(CFGetTypeID(url.get()) == CFURLGetTypeID());
+    return url;
+#else
+    return adoptCF(CTFontCopyGraphicsFont(ctFont, 0));
+#endif
+}
+
+RetainPtr&lt;CFTypeRef&gt; FontPlatformData::objectForEqualityCheck() const
+{
+    return objectForEqualityCheck(ctFont());
+}
+
</ins><span class="cx"> PassRefPtr&lt;SharedBuffer&gt; FontPlatformData::openTypeTable(uint32_t table) const
</span><span class="cx"> {
</span><span class="cx">     if (RetainPtr&lt;CFDataRef&gt; data = adoptCF(CGFontCopyTableForTag(cgFont(), table)))
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsmacComplexTextControllerCoreTextmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.mm (177688 => 177689)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.mm        2014-12-23 18:45:05 UTC (rev 177688)
+++ trunk/Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.mm        2014-12-23 19:23:48 UTC (rev 177689)
</span><span class="lines">@@ -249,14 +249,14 @@
</span><span class="cx">             CFDictionaryRef runAttributes = CTRunGetAttributes(ctRun);
</span><span class="cx">             CTFontRef runFont = static_cast&lt;CTFontRef&gt;(CFDictionaryGetValue(runAttributes, kCTFontAttributeName));
</span><span class="cx">             ASSERT(CFGetTypeID(runFont) == CTFontGetTypeID());
</span><del>-            if (!CFEqual(runFont, fontData-&gt;platformData().ctFont())) {
</del><ins>+            RetainPtr&lt;CFTypeRef&gt; runFontEqualityObject = FontPlatformData::objectForEqualityCheck(runFont);
+            if (!CFEqual(runFontEqualityObject.get(), fontData-&gt;platformData().objectForEqualityCheck().get())) {
</ins><span class="cx">                 // Begin trying to see if runFont matches any of the fonts in the fallback list.
</span><del>-                RetainPtr&lt;CGFontRef&gt; runCGFont = adoptCF(CTFontCopyGraphicsFont(runFont, 0));
</del><span class="cx">                 unsigned i = 0;
</span><span class="cx">                 for (const FontData* candidateFontData = m_font.fontDataAt(i); candidateFontData; candidateFontData = m_font.fontDataAt(++i)) {
</span><span class="cx">                     runFontData = candidateFontData-&gt;fontDataForCharacter(baseCharacter);
</span><del>-                    RetainPtr&lt;CGFontRef&gt; cgFont = adoptCF(CTFontCopyGraphicsFont(runFontData-&gt;platformData().ctFont(), 0));
-                    if (CFEqual(cgFont.get(), runCGFont.get()))
</del><ins>+                    RetainPtr&lt;CFTypeRef&gt; runFontEqualityObject = runFontData-&gt;platformData().objectForEqualityCheck();
+                    if (CFEqual(runFontEqualityObject.get(), runFontEqualityObject.get()))
</ins><span class="cx">                         break;
</span><span class="cx">                     runFontData = 0;
</span><span class="cx">                 }
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsmacGlyphPageTreeNodeMaccpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/mac/GlyphPageTreeNodeMac.cpp (177688 => 177689)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/mac/GlyphPageTreeNodeMac.cpp        2014-12-23 18:45:05 UTC (rev 177688)
+++ trunk/Source/WebCore/platform/graphics/mac/GlyphPageTreeNodeMac.cpp        2014-12-23 19:23:48 UTC (rev 177689)
</span><span class="lines">@@ -111,9 +111,7 @@
</span><span class="cx">         Vector&lt;CFIndex, 512&gt; indexVector;
</span><span class="cx">         bool done = false;
</span><span class="cx"> 
</span><del>-        // For the CGFont comparison in the loop, use the CGFont that Core Text assigns to the CTFont. This may
-        // be non-CFEqual to fontData-&gt;platformData().cgFont().
-        RetainPtr&lt;CGFontRef&gt; cgFont = adoptCF(CTFontCopyGraphicsFont(fontData-&gt;platformData().ctFont(), 0));
</del><ins>+        RetainPtr&lt;CFTypeRef&gt; fontEqualityObject = fontData-&gt;platformData().objectForEqualityCheck();
</ins><span class="cx"> 
</span><span class="cx">         for (CFIndex r = 0; r &lt; runCount &amp;&amp; !done ; ++r) {
</span><span class="cx">             // CTLine could map characters over multiple fonts using its own font fallback list.
</span><span class="lines">@@ -123,9 +121,7 @@
</span><span class="cx"> 
</span><span class="cx">             CFDictionaryRef attributes = CTRunGetAttributes(ctRun);
</span><span class="cx">             CTFontRef runFont = static_cast&lt;CTFontRef&gt;(CFDictionaryGetValue(attributes, kCTFontAttributeName));
</span><del>-            RetainPtr&lt;CGFontRef&gt; runCGFont = adoptCF(CTFontCopyGraphicsFont(runFont, 0));
-            // Use CGFont here as CFEqual for CTFont counts all attributes for font.
-            bool gotBaseFont = CFEqual(cgFont.get(), runCGFont.get());
</del><ins>+            bool gotBaseFont = CFEqual(fontEqualityObject.get(), FontPlatformData::objectForEqualityCheck(runFont).get());
</ins><span class="cx">             if (gotBaseFont || fontData-&gt;platformData().isCompositeFontReference()) {
</span><span class="cx">                 // This run uses the font we want. Extract glyphs.
</span><span class="cx">                 CFIndex glyphCount = CTRunGetGlyphCount(ctRun);
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsmacSimpleFontDataMacmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/mac/SimpleFontDataMac.mm (177688 => 177689)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/mac/SimpleFontDataMac.mm        2014-12-23 18:45:05 UTC (rev 177688)
+++ trunk/Source/WebCore/platform/graphics/mac/SimpleFontDataMac.mm        2014-12-23 19:23:48 UTC (rev 177689)
</span><span class="lines">@@ -480,7 +480,7 @@
</span><span class="cx">     if (!addResult.isNewEntry)
</span><span class="cx">         return addResult.iterator-&gt;value;
</span><span class="cx"> 
</span><del>-    RetainPtr&lt;CGFontRef&gt; cgFont = adoptCF(CTFontCopyGraphicsFont(platformData().ctFont(), 0));
</del><ins>+    RetainPtr&lt;CFTypeRef&gt; fontEqualityObject = platformData().objectForEqualityCheck();
</ins><span class="cx"> 
</span><span class="cx">     ProviderInfo info = { characters, length, getCFStringAttributes(0, platformData().orientation()) };
</span><span class="cx">     RetainPtr&lt;CTLineRef&gt; line = adoptCF(CTLineCreateWithUniCharProvider(&amp;provideStringAndAttributes, 0, &amp;info));
</span><span class="lines">@@ -493,8 +493,7 @@
</span><span class="cx">         ASSERT(CFGetTypeID(ctRun) == CTRunGetTypeID());
</span><span class="cx">         CFDictionaryRef runAttributes = CTRunGetAttributes(ctRun);
</span><span class="cx">         CTFontRef runFont = static_cast&lt;CTFontRef&gt;(CFDictionaryGetValue(runAttributes, kCTFontAttributeName));
</span><del>-        RetainPtr&lt;CGFontRef&gt; runCGFont = adoptCF(CTFontCopyGraphicsFont(runFont, 0));
-        if (!CFEqual(runCGFont.get(), cgFont.get()))
</del><ins>+        if (!CFEqual(fontEqualityObject.get(), FontPlatformData::objectForEqualityCheck(runFont).get()))
</ins><span class="cx">             return false;
</span><span class="cx">     }
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformspicocoaCoreTextSPIh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/spi/cocoa/CoreTextSPI.h (177688 => 177689)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/spi/cocoa/CoreTextSPI.h        2014-12-23 18:45:05 UTC (rev 177688)
+++ trunk/Source/WebCore/platform/spi/cocoa/CoreTextSPI.h        2014-12-23 19:23:48 UTC (rev 177689)
</span><span class="lines">@@ -48,6 +48,8 @@
</span><span class="cx"> typedef const UniChar* (*CTUniCharProviderCallback)(CFIndex stringIndex, CFIndex* charCount, CFDictionaryRef* attributes, void* refCon);
</span><span class="cx"> typedef void (*CTUniCharDisposeCallback)(const UniChar* chars, void* refCon);
</span><span class="cx"> 
</span><ins>+extern const CFStringRef kCTFontReferenceURLAttribute;
+
</ins><span class="cx"> #if PLATFORM(IOS) || (PLATFORM(MAC) &amp;&amp; __MAC_OS_X_VERSION_MIN_REQUIRED &gt; 1080)
</span><span class="cx"> #if !USE(APPLE_INTERNAL_SDK)
</span><span class="cx"> typedef CF_OPTIONS(uint32_t, CTFontTransformOptions)
</span></span></pre>
</div>
</div>

</body>
</html>