Leak of SVGFontFaceElement when RenderStyle holds onto a FontRances which uses it
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 26 Mar 2019 03:34:43 +0000 (03:34 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 26 Mar 2019 03:34:43 +0000 (03:34 +0000)
https://bugs.webkit.org/show_bug.cgi?id=196059

Reviewed by Zalan Bujtas.

SVGFontFaceElement keeps its RenderStyle alive via ElementRareData but RenderStyle can hold onto FontRanges
and therefore CSSFontSource, which in turn keeps SVGFontFaceElement alive, making a reference cycle.

More precisely, there are two reference cycles:
SVGFontFaceElement (1) -> ElementRareData -> StyleInheritedData -> FontCascade -> FontCascadeFonts (2)
FontCascadeFonts (2) -> FontRanges (3)
FontCascadeFonts (2) -> CSSFontSelector -> CSSFontFaceSet -> CSSSegmentedFontFace -> FontRanges (3)
FontRanges (3) -> CSSFontAccessor > CSSFontFace > CSSFontSource -> SVGFontFaceElement (1)

No new tests. Unfortunately, writing a test proved to be intractable. The leak can be reproduced by running
svg/text/text-text-05-t.svg then svg/zoom/page/zoom-img-preserveAspectRatio-support-1.html consecutively.

* css/CSSFontFaceSource.cpp:
(WebCore::CSSFontFaceSource::CSSFontFaceSource):
(WebCore::CSSFontFaceSource::load):
(WebCore::CSSFontFaceSource::font):
(WebCore::CSSFontFaceSource::isSVGFontFaceSource const):
* css/CSSFontFaceSource.h:

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

Source/WebCore/ChangeLog
Source/WebCore/css/CSSFontFaceSource.cpp
Source/WebCore/css/CSSFontFaceSource.h

index 896d4b9..fad1694 100644 (file)
@@ -1,3 +1,29 @@
+2019-03-20  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Leak of SVGFontFaceElement when RenderStyle holds onto a FontRances which uses it
+        https://bugs.webkit.org/show_bug.cgi?id=196059
+
+        Reviewed by Zalan Bujtas.
+
+        SVGFontFaceElement keeps its RenderStyle alive via ElementRareData but RenderStyle can hold onto FontRanges
+        and therefore CSSFontSource, which in turn keeps SVGFontFaceElement alive, making a reference cycle.
+
+        More precisely, there are two reference cycles:
+        SVGFontFaceElement (1) -> ElementRareData -> StyleInheritedData -> FontCascade -> FontCascadeFonts (2)
+        FontCascadeFonts (2) -> FontRanges (3)
+        FontCascadeFonts (2) -> CSSFontSelector -> CSSFontFaceSet -> CSSSegmentedFontFace -> FontRanges (3)
+        FontRanges (3) -> CSSFontAccessor > CSSFontFace > CSSFontSource -> SVGFontFaceElement (1)
+
+        No new tests. Unfortunately, writing a test proved to be intractable. The leak can be reproduced by running
+        svg/text/text-text-05-t.svg then svg/zoom/page/zoom-img-preserveAspectRatio-support-1.html consecutively.
+
+        * css/CSSFontFaceSource.cpp:
+        (WebCore::CSSFontFaceSource::CSSFontFaceSource):
+        (WebCore::CSSFontFaceSource::load):
+        (WebCore::CSSFontFaceSource::font):
+        (WebCore::CSSFontFaceSource::isSVGFontFaceSource const):
+        * css/CSSFontFaceSource.h:
+
 2019-03-25  Fujii Hironori  <Hironori.Fujii@sony.com>
 
         Unreviewed, rolling out r243450.
index 09527f1..7f94842 100644 (file)
@@ -79,7 +79,8 @@ CSSFontFaceSource::CSSFontFaceSource(CSSFontFace& owner, const String& familyNam
     , m_face(owner)
     , m_immediateSource(WTFMove(arrayBufferView))
 #if ENABLE(SVG_FONTS)
-    , m_svgFontFaceElement(fontFace)
+    , m_svgFontFaceElement(makeWeakPtr(fontFace))
+    , m_hasSVGFontFaceElement(m_svgFontFaceElement)
 #endif
 {
 #if !ENABLE(SVG_FONTS)
@@ -154,8 +155,8 @@ void CSSFontFaceSource::load(CSSFontSelector* fontSelector)
     } else {
         bool success = false;
 #if ENABLE(SVG_FONTS)
-        if (m_svgFontFaceElement) {
-            if (is<SVGFontElement>(m_svgFontFaceElement->parentNode())) {
+        if (m_hasSVGFontFaceElement) {
+            if (m_svgFontFaceElement && is<SVGFontElement>(m_svgFontFaceElement->parentNode())) {
                 ASSERT(!m_inDocumentCustomPlatformData);
                 SVGFontElement& fontElement = downcast<SVGFontElement>(*m_svgFontFaceElement->parentNode());
                 if (auto otfFont = convertSVGToOTFFont(fontElement))
@@ -195,12 +196,11 @@ RefPtr<Font> CSSFontFaceSource::font(const FontDescription& fontDescription, boo
 {
     ASSERT(status() == Status::Success);
 
-    SVGFontFaceElement* fontFaceElement = nullptr;
 #if ENABLE(SVG_FONTS)
-    fontFaceElement = m_svgFontFaceElement.get();
+    bool usesInDocumentSVGFont = m_hasSVGFontFaceElement;
 #endif
 
-    if (!m_font && !fontFaceElement) {
+    if (!m_font && !usesInDocumentSVGFont) {
         if (m_immediateSource) {
             if (!m_immediateFontCustomPlatformData)
                 return nullptr;
@@ -222,12 +222,11 @@ RefPtr<Font> CSSFontFaceSource::font(const FontDescription& fontDescription, boo
         return result;
     }
 
-    // In-Document SVG Fonts
-    if (!fontFaceElement)
+    if (!usesInDocumentSVGFont)
         return nullptr;
 
 #if ENABLE(SVG_FONTS)
-    if (!is<SVGFontElement>(m_svgFontFaceElement->parentNode()))
+    if (!m_svgFontFaceElement || !is<SVGFontElement>(m_svgFontFaceElement->parentNode()))
         return nullptr;
     if (!m_inDocumentCustomPlatformData)
         return nullptr;
@@ -241,7 +240,7 @@ RefPtr<Font> CSSFontFaceSource::font(const FontDescription& fontDescription, boo
 #if ENABLE(SVG_FONTS)
 bool CSSFontFaceSource::isSVGFontFaceSource() const
 {
-    return m_svgFontFaceElement || is<CachedSVGFont>(m_font.get());
+    return m_hasSVGFontFaceElement || is<CachedSVGFont>(m_font.get());
 }
 #endif
 
index f7e0094..2379cac 100644 (file)
@@ -93,11 +93,14 @@ private:
     std::unique_ptr<FontCustomPlatformData> m_immediateFontCustomPlatformData;
 
 #if ENABLE(SVG_FONTS)
-    RefPtr<SVGFontFaceElement> m_svgFontFaceElement;
+    WeakPtr<SVGFontFaceElement> m_svgFontFaceElement;
 #endif
     std::unique_ptr<FontCustomPlatformData> m_inDocumentCustomPlatformData;
 
     Status m_status { Status::Pending };
+#if ENABLE(SVG_FONTS)
+    bool m_hasSVGFontFaceElement;
+#endif
 };
 
 } // namespace WebCore