Changeset 180561 in webkit

Timestamp:

Feb 24, 2015, 8:46:18 AM (10 years ago)

Author:

akling@apple.com

Message:

[Cocoa] Break internal reference cycle in WebCore::Font.
<​https://webkit.org/b/141941>
<rdar://problem/19650570>

Reviewed by Antti Koivisto.

The Cocoa implementation of Font::platformCreateScaledFont() tried to be smart and use the FontCache.
This didn't work out well when scaling a 0pt Font, since scaling 0pt by any factor will return 0pt.

We'd have a 0pt font, scale it by 0.7 to get a small-caps variant, and then cache that small-caps
variant (really "this") in Font::m_derivedData->smallCaps.

Fix this by having Cocoa Font scaling do exactly what other platforms do: create a new Font every time.
This stops us from accumulating tons of abandoned Font objects over time.

• platform/graphics/Font.cpp:

(WebCore::Font::verticalRightOrientationFont):
(WebCore::Font::uprightOrientationFont):
(WebCore::Font::smallCapsFont):
(WebCore::Font::emphasisMarkFont):
(WebCore::Font::brokenIdeographFont):
(WebCore::Font::nonSyntheticItalicFont): Add assertions to guard against reference cycles inside a Font.

• platform/graphics/cocoa/FontCocoa.mm:

(WebCore::Font::platformCreateScaledFont): Always create a new Font when scaling an existing Font to a different size.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/Font.cpp (modified) (6 diffs)
• platform/graphics/cocoa/FontCocoa.mm (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2015-02-24  Andreas Kling  <akling@apple.com>
+
+        [Cocoa] Break internal reference cycle in WebCore::Font.
+        <https://webkit.org/b/141941>
+        <rdar://problem/19650570>
+
+        Reviewed by Antti Koivisto.
+
+        The Cocoa implementation of Font::platformCreateScaledFont() tried to be smart and use the FontCache.
+        This didn't work out well when scaling a 0pt Font, since scaling 0pt by any factor will return 0pt.
+
+        We'd have a 0pt font, scale it by 0.7 to get a small-caps variant, and then cache that small-caps
+        variant (really "this") in Font::m_derivedData->smallCaps.
+
+        Fix this by having Cocoa Font scaling do exactly what other platforms do: create a new Font every time.
+        This stops us from accumulating tons of abandoned Font objects over time.
+
+        * platform/graphics/Font.cpp:
+        (WebCore::Font::verticalRightOrientationFont):
+        (WebCore::Font::uprightOrientationFont):
+        (WebCore::Font::smallCapsFont):
+        (WebCore::Font::emphasisMarkFont):
+        (WebCore::Font::brokenIdeographFont):
+        (WebCore::Font::nonSyntheticItalicFont): Add assertions to guard against reference cycles inside a Font.
+
+        * platform/graphics/cocoa/FontCocoa.mm:
+        (WebCore::Font::platformCreateScaledFont): Always create a new Font when scaling an existing Font to a different size.
+
 2015-02-24  Xabier Rodriguez Calvar <calvaris@igalia.com> and Youenn Fablet  <youenn.fablet@crf.canon.fr>
 

trunk/Source/WebCore/platform/graphics/Font.cpp

@@ -288 +288 @@
         m_derivedFontData->verticalRightOrientation = create(verticalRightPlatformData, isCustomFont(), false, true);
     }
+    ASSERT(m_derivedFontData->verticalRightOrientation != this);
     return m_derivedFontData->verticalRightOrientation;
 }
@@ -297 +298 @@
     if (!m_derivedFontData->uprightOrientation)
         m_derivedFontData->uprightOrientation = create(m_platformData, isCustomFont(), false, true);
+    ASSERT(m_derivedFontData->uprightOrientation != this);
     return m_derivedFontData->uprightOrientation;
 }
@@ -306 +308 @@
     if (!m_derivedFontData->smallCaps)
         m_derivedFontData->smallCaps = createScaledFont(fontDescription, smallCapsFontSizeMultiplier);
-
+    ASSERT(m_derivedFontData->smallCaps != this);
     return m_derivedFontData->smallCaps;
 }
@@ -316 +318 @@
     if (!m_derivedFontData->emphasisMark)
         m_derivedFontData->emphasisMark = createScaledFont(fontDescription, emphasisMarkFontSizeMultiplier);
-
+    ASSERT(m_derivedFontData->emphasisMark != this);
     return m_derivedFontData->emphasisMark;
 }
@@ -328 +330 @@
         m_derivedFontData->brokenIdeograph->m_isBrokenIdeographFallback = true;
     }
+    ASSERT(m_derivedFontData->brokenIdeograph != this);
     return m_derivedFontData->brokenIdeograph;
 }
@@ -342 +345 @@
         m_derivedFontData->nonSyntheticItalic = create(nonSyntheticItalicFontPlatformData, isCustomFont());
     }
+    ASSERT(m_derivedFontData->nonSyntheticItalic != this);
     return m_derivedFontData->nonSyntheticItalic;
 }

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

@@ -340 +340 @@
         scaledFontData.m_syntheticOblique = (fontTraits & NSItalicFontMask) && !(scaledFontTraits & NSItalicFontMask);
 
-        return FontCache::singleton().fontForPlatformData(scaledFontData);
+        return Font::create(scaledFontData);
     }
     END_BLOCK_OBJC_EXCEPTIONS;
@@ -361 +361 @@
         scaledFontData.m_syntheticOblique = (fontTraits & kCTFontItalicTrait) && !(scaledFontTraits & kCTFontTraitItalic);
 
-        return FontCache::singleton().fontForPlatformData(scaledFontData);
+        return Font::create(scaledFontData);
     }