platform/mac/editing/input/devanagari-ligature.html is flaky on Yosemite, ligature...

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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@177689 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 9afb0250bda8845f08be931636b716c15f947228..b4d34fd90d1e4be083ade8dffa4ec35f6f5efdfc 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,32 @@
+2014-12-23  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        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  <mmaxfield@apple.com>
 
         [SVG -> OTF Converter] Make Placeholder a move-only type

diff --git a/Source/WebCore/platform/graphics/FontPlatformData.h b/Source/WebCore/platform/graphics/FontPlatformData.h
index 316acd8fbfacf4a602fdf2215a2ce11945e6b78e..d4cd5729695d6fdde71f67f1b4ea101000d47743 100644 (file)
--- a/Source/WebCore/platform/graphics/FontPlatformData.h
+++ b/Source/WebCore/platform/graphics/FontPlatformData.h
@@ -120,6 +120,8 @@ public:
     void setFont(CTFontRef);
 
     CTFontRef ctFont() const;
+    static RetainPtr<CFTypeRef> objectForEqualityCheck(CTFontRef);
+    RetainPtr<CFTypeRef> objectForEqualityCheck() const;
 
     bool allowsLigatures() const;
     bool roundsGlyphAdvances() const;

diff --git a/Source/WebCore/platform/graphics/cocoa/FontPlatformDataCocoa.mm b/Source/WebCore/platform/graphics/cocoa/FontPlatformDataCocoa.mm
index a0e0c2c3f325cda6587005435bb41b7f41a3e2be..1004a9cbf71f6c5f960f66842b7adc345c86fe37 100644 (file)
--- a/Source/WebCore/platform/graphics/cocoa/FontPlatformDataCocoa.mm
+++ b/Source/WebCore/platform/graphics/cocoa/FontPlatformDataCocoa.mm
@@ -24,6 +24,7 @@
 #import "config.h"
 #import "FontPlatformData.h"
 
+#import "CoreTextSPI.h"
 #import "SharedBuffer.h"
 #import "WebCoreSystemInterface.h"
 #import <wtf/text/WTFString.h>
@@ -289,6 +290,25 @@ CTFontRef FontPlatformData::ctFont() const
     return m_ctFont.get();
 }
 
+RetainPtr<CFTypeRef> FontPlatformData::objectForEqualityCheck(CTFontRef ctFont)
+{
+#if (PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED <= 80000) || (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED <= 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<CFURLRef> url = adoptCF(static_cast<CFURLRef>(CTFontDescriptorCopyAttribute(fontDescriptor.get(), kCTFontReferenceURLAttribute)));
+    ASSERT(CFGetTypeID(url.get()) == CFURLGetTypeID());
+    return url;
+#else
+    return adoptCF(CTFontCopyGraphicsFont(ctFont, 0));
+#endif
+}
+
+RetainPtr<CFTypeRef> FontPlatformData::objectForEqualityCheck() const
+{
+    return objectForEqualityCheck(ctFont());
+}
+
 PassRefPtr<SharedBuffer> FontPlatformData::openTypeTable(uint32_t table) const
 {
     if (RetainPtr<CFDataRef> data = adoptCF(CGFontCopyTableForTag(cgFont(), table)))

diff --git a/Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.mm b/Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.mm
index 622866509463d6a6d1d7e439b991114ede123bf8..6c957603f985ca7b88c9fa092564dbff823e7442 100644 (file)
--- a/Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.mm
+++ b/Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.mm
@@ -249,14 +249,14 @@ void ComplexTextController::collectComplexTextRunsForCharacters(const UChar* cp,
             CFDictionaryRef runAttributes = CTRunGetAttributes(ctRun);
             CTFontRef runFont = static_cast<CTFontRef>(CFDictionaryGetValue(runAttributes, kCTFontAttributeName));
             ASSERT(CFGetTypeID(runFont) == CTFontGetTypeID());
-            if (!CFEqual(runFont, fontData->platformData().ctFont())) {
+            RetainPtr<CFTypeRef> runFontEqualityObject = FontPlatformData::objectForEqualityCheck(runFont);
+            if (!CFEqual(runFontEqualityObject.get(), fontData->platformData().objectForEqualityCheck().get())) {
                 // Begin trying to see if runFont matches any of the fonts in the fallback list.
-                RetainPtr<CGFontRef> runCGFont = adoptCF(CTFontCopyGraphicsFont(runFont, 0));
                 unsigned i = 0;
                 for (const FontData* candidateFontData = m_font.fontDataAt(i); candidateFontData; candidateFontData = m_font.fontDataAt(++i)) {
                     runFontData = candidateFontData->fontDataForCharacter(baseCharacter);
-                    RetainPtr<CGFontRef> cgFont = adoptCF(CTFontCopyGraphicsFont(runFontData->platformData().ctFont(), 0));
-                    if (CFEqual(cgFont.get(), runCGFont.get()))
+                    RetainPtr<CFTypeRef> runFontEqualityObject = runFontData->platformData().objectForEqualityCheck();
+                    if (CFEqual(runFontEqualityObject.get(), runFontEqualityObject.get()))
                         break;
                     runFontData = 0;
                 }

diff --git a/Source/WebCore/platform/graphics/mac/GlyphPageTreeNodeMac.cpp b/Source/WebCore/platform/graphics/mac/GlyphPageTreeNodeMac.cpp
index 3e9962b7e30ba8dd9e0dfcf9b016a4e7204bb9bd..6fd5c29e6e209af7dd52604e3f324a1139666d01 100644 (file)
--- a/Source/WebCore/platform/graphics/mac/GlyphPageTreeNodeMac.cpp
+++ b/Source/WebCore/platform/graphics/mac/GlyphPageTreeNodeMac.cpp
@@ -111,9 +111,7 @@ bool GlyphPage::fill(unsigned offset, unsigned length, UChar* buffer, unsigned b
         Vector<CFIndex, 512> indexVector;
         bool done = false;
 
-        // For the CGFont comparison in the loop, use the CGFont that Core Text assigns to the CTFont. This may
-        // be non-CFEqual to fontData->platformData().cgFont().
-        RetainPtr<CGFontRef> cgFont = adoptCF(CTFontCopyGraphicsFont(fontData->platformData().ctFont(), 0));
+        RetainPtr<CFTypeRef> fontEqualityObject = fontData->platformData().objectForEqualityCheck();
 
         for (CFIndex r = 0; r < runCount && !done ; ++r) {
             // CTLine could map characters over multiple fonts using its own font fallback list.
@@ -123,9 +121,7 @@ bool GlyphPage::fill(unsigned offset, unsigned length, UChar* buffer, unsigned b
 
             CFDictionaryRef attributes = CTRunGetAttributes(ctRun);
             CTFontRef runFont = static_cast<CTFontRef>(CFDictionaryGetValue(attributes, kCTFontAttributeName));
-            RetainPtr<CGFontRef> runCGFont = adoptCF(CTFontCopyGraphicsFont(runFont, 0));
-            // Use CGFont here as CFEqual for CTFont counts all attributes for font.
-            bool gotBaseFont = CFEqual(cgFont.get(), runCGFont.get());
+            bool gotBaseFont = CFEqual(fontEqualityObject.get(), FontPlatformData::objectForEqualityCheck(runFont).get());
             if (gotBaseFont || fontData->platformData().isCompositeFontReference()) {
                 // This run uses the font we want. Extract glyphs.
                 CFIndex glyphCount = CTRunGetGlyphCount(ctRun);

diff --git a/Source/WebCore/platform/graphics/mac/SimpleFontDataMac.mm b/Source/WebCore/platform/graphics/mac/SimpleFontDataMac.mm
index 0a6857c1c50e8d5e8740616b861029d97db72f88..f9aa8a8ff4e238aca477bca89280c26a1ed076db 100644 (file)
--- a/Source/WebCore/platform/graphics/mac/SimpleFontDataMac.mm
+++ b/Source/WebCore/platform/graphics/mac/SimpleFontDataMac.mm
@@ -480,7 +480,7 @@ bool SimpleFontData::canRenderCombiningCharacterSequence(const UChar* characters
     if (!addResult.isNewEntry)
         return addResult.iterator->value;
 
-    RetainPtr<CGFontRef> cgFont = adoptCF(CTFontCopyGraphicsFont(platformData().ctFont(), 0));
+    RetainPtr<CFTypeRef> fontEqualityObject = platformData().objectForEqualityCheck();
 
     ProviderInfo info = { characters, length, getCFStringAttributes(0, platformData().orientation()) };
     RetainPtr<CTLineRef> line = adoptCF(CTLineCreateWithUniCharProvider(&provideStringAndAttributes, 0, &info));
@@ -493,8 +493,7 @@ bool SimpleFontData::canRenderCombiningCharacterSequence(const UChar* characters
         ASSERT(CFGetTypeID(ctRun) == CTRunGetTypeID());
         CFDictionaryRef runAttributes = CTRunGetAttributes(ctRun);
         CTFontRef runFont = static_cast<CTFontRef>(CFDictionaryGetValue(runAttributes, kCTFontAttributeName));
-        RetainPtr<CGFontRef> runCGFont = adoptCF(CTFontCopyGraphicsFont(runFont, 0));
-        if (!CFEqual(runCGFont.get(), cgFont.get()))
+        if (!CFEqual(fontEqualityObject.get(), FontPlatformData::objectForEqualityCheck(runFont).get()))
             return false;
     }
 

diff --git a/Source/WebCore/platform/spi/cocoa/CoreTextSPI.h b/Source/WebCore/platform/spi/cocoa/CoreTextSPI.h
index 23362b7718fe6ff79eac3c6d209abd517dd52941..d39b8677767736b3e989035df1fff7c32da81f05 100644 (file)
--- a/Source/WebCore/platform/spi/cocoa/CoreTextSPI.h
+++ b/Source/WebCore/platform/spi/cocoa/CoreTextSPI.h
@@ -48,6 +48,8 @@ extern "C" {
 typedef const UniChar* (*CTUniCharProviderCallback)(CFIndex stringIndex, CFIndex* charCount, CFDictionaryRef* attributes, void* refCon);
 typedef void (*CTUniCharDisposeCallback)(const UniChar* chars, void* refCon);
 
+extern const CFStringRef kCTFontReferenceURLAttribute;
+
 #if PLATFORM(IOS) || (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED > 1080)
 #if !USE(APPLE_INTERNAL_SDK)
 typedef CF_OPTIONS(uint32_t, CTFontTransformOptions)