Changeset 177689 in webkit

Timestamp:

Dec 23, 2014 11:23:48 AM (9 years ago)

Author:

mmaxfield@apple.com

Message:

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:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/FontPlatformData.h (modified) (1 diff)
• platform/graphics/cocoa/FontPlatformDataCocoa.mm (modified) (2 diffs)
• platform/graphics/mac/ComplexTextControllerCoreText.mm (modified) (1 diff)
• platform/graphics/mac/GlyphPageTreeNodeMac.cpp (modified) (2 diffs)
• platform/graphics/mac/SimpleFontDataMac.mm (modified) (2 diffs)
• platform/spi/cocoa/CoreTextSPI.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/platform/graphics/FontPlatformData.h

@@ -121 +121 @@
 
     CTFontRef ctFont() const;
+    static RetainPtr<CFTypeRef> objectForEqualityCheck(CTFontRef);
+    RetainPtr<CFTypeRef> objectForEqualityCheck() const;
 
     bool allowsLigatures() const;

trunk/Source/WebCore/platform/graphics/cocoa/FontPlatformDataCocoa.mm

@@ -25 +25 @@
 #import "FontPlatformData.h"
 
+#import "CoreTextSPI.h"
 #import "SharedBuffer.h"
 #import "WebCoreSystemInterface.h"
@@ -290 +291 @@
 }
 
+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
 {

trunk/Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.mm

@@ -250 +250 @@
             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;

trunk/Source/WebCore/platform/graphics/mac/GlyphPageTreeNodeMac.cpp

@@ -112 +112 @@
         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) {
@@ -124 +122 @@
             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.

trunk/Source/WebCore/platform/graphics/mac/SimpleFontDataMac.mm

@@ -481 +481 @@
         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()) };
@@ -494 +494 @@
         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;
     }

trunk/Source/WebCore/platform/spi/cocoa/CoreTextSPI.h

@@ -49 +49 @@
 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)