[Cocoa] Break internal reference cycle in WebCore::Font.
authorakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 24 Feb 2015 16:46:18 +0000 (16:46 +0000)
committerakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 24 Feb 2015 16:46:18 +0000 (16:46 +0000)
<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.

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

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/Font.cpp
Source/WebCore/platform/graphics/cocoa/FontCocoa.mm

index a8cb9ccbca39a165e3ae6ded24650f3a3dda751e..95d42c1c032ad8ac87f503e84031d4d5a75e03c4 100644 (file)
@@ -1,3 +1,31 @@
+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>
 
         [Streams API] Reading ReadableStream ready and closed attributes should not always create a new promise
index 8e2138a243999278ef044150e5d9a5c134cc9b3e..549da8757dffaf8c197cd59da48611ad59bc9589 100644 (file)
@@ -287,6 +287,7 @@ PassRefPtr<Font> Font::verticalRightOrientationFont() const
         verticalRightPlatformData.setOrientation(Horizontal);
         m_derivedFontData->verticalRightOrientation = create(verticalRightPlatformData, isCustomFont(), false, true);
     }
+    ASSERT(m_derivedFontData->verticalRightOrientation != this);
     return m_derivedFontData->verticalRightOrientation;
 }
 
@@ -296,6 +297,7 @@ PassRefPtr<Font> Font::uprightOrientationFont() const
         m_derivedFontData = std::make_unique<DerivedFontData>(isCustomFont());
     if (!m_derivedFontData->uprightOrientation)
         m_derivedFontData->uprightOrientation = create(m_platformData, isCustomFont(), false, true);
+    ASSERT(m_derivedFontData->uprightOrientation != this);
     return m_derivedFontData->uprightOrientation;
 }
 
@@ -305,7 +307,7 @@ PassRefPtr<Font> Font::smallCapsFont(const FontDescription& fontDescription) con
         m_derivedFontData = std::make_unique<DerivedFontData>(isCustomFont());
     if (!m_derivedFontData->smallCaps)
         m_derivedFontData->smallCaps = createScaledFont(fontDescription, smallCapsFontSizeMultiplier);
-
+    ASSERT(m_derivedFontData->smallCaps != this);
     return m_derivedFontData->smallCaps;
 }
 
@@ -315,7 +317,7 @@ PassRefPtr<Font> Font::emphasisMarkFont(const FontDescription& fontDescription)
         m_derivedFontData = std::make_unique<DerivedFontData>(isCustomFont());
     if (!m_derivedFontData->emphasisMark)
         m_derivedFontData->emphasisMark = createScaledFont(fontDescription, emphasisMarkFontSizeMultiplier);
-
+    ASSERT(m_derivedFontData->emphasisMark != this);
     return m_derivedFontData->emphasisMark;
 }
 
@@ -327,6 +329,7 @@ PassRefPtr<Font> Font::brokenIdeographFont() const
         m_derivedFontData->brokenIdeograph = create(m_platformData, isCustomFont(), false);
         m_derivedFontData->brokenIdeograph->m_isBrokenIdeographFallback = true;
     }
+    ASSERT(m_derivedFontData->brokenIdeograph != this);
     return m_derivedFontData->brokenIdeograph;
 }
 
@@ -341,6 +344,7 @@ PassRefPtr<Font> Font::nonSyntheticItalicFont() const
 #endif
         m_derivedFontData->nonSyntheticItalic = create(nonSyntheticItalicFontPlatformData, isCustomFont());
     }
+    ASSERT(m_derivedFontData->nonSyntheticItalic != this);
     return m_derivedFontData->nonSyntheticItalic;
 }
 
index b22735ccbb1016b5410f6ebc7cb488cdbf7b5999..507a92b4cff8a3bf2a0b6b4966a361a6c374eb26 100644 (file)
@@ -339,7 +339,7 @@ PassRefPtr<Font> Font::platformCreateScaledFont(const FontDescription&, float sc
         scaledFontData.m_syntheticBold = (fontTraits & NSBoldFontMask) && !(scaledFontTraits & NSBoldFontMask);
         scaledFontData.m_syntheticOblique = (fontTraits & NSItalicFontMask) && !(scaledFontTraits & NSItalicFontMask);
 
-        return FontCache::singleton().fontForPlatformData(scaledFontData);
+        return Font::create(scaledFontData);
     }
     END_BLOCK_OBJC_EXCEPTIONS;
 
@@ -360,7 +360,7 @@ PassRefPtr<Font> Font::platformCreateScaledFont(const FontDescription&, float sc
         scaledFontData.m_syntheticBold = (fontTraits & kCTFontBoldTrait) && !(scaledFontTraits & kCTFontTraitBold);
         scaledFontData.m_syntheticOblique = (fontTraits & kCTFontItalicTrait) && !(scaledFontTraits & kCTFontTraitItalic);
 
-        return FontCache::singleton().fontForPlatformData(scaledFontData);
+        return Font::create(scaledFontData);
     }
 
     return nullptr;