Changeset 243483 in webkit

Timestamp:

Mar 25, 2019, 8:34:43 PM (7 years ago)

Author:

rniwa@webkit.org

Message:

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:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• css/CSSFontFaceSource.cpp (modified) (5 diffs)
• css/CSSFontFaceSource.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

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

trunk/Source/WebCore/css/CSSFontFaceSource.cpp

@@ -80 +80 @@
     , m_immediateSource(WTFMove(arrayBufferView))
 #if ENABLE(SVG_FONTS)
-    , m_svgFontFaceElement(fontFace)
+    , m_svgFontFaceElement(makeWeakPtr(fontFace))
+    , m_hasSVGFontFaceElement(m_svgFontFaceElement)
 #endif
 {
@@ -155 +156 @@
         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());
@@ -196 +197 @@
     ASSERT(status() == Status::Success);
 
-    SVGFontFaceElement* fontFaceElement = nullptr;
-#if ENABLE(SVG_FONTS)
-    fontFaceElement = m_svgFontFaceElement.get();
-#endif
-
-    if (!m_font && !fontFaceElement) {
+#if ENABLE(SVG_FONTS)
+    bool usesInDocumentSVGFont = m_hasSVGFontFaceElement;
+#endif
+
+    if (!m_font && !usesInDocumentSVGFont) {
         if (m_immediateSource) {
             if (!m_immediateFontCustomPlatformData)
@@ -223 +223 @@
     }
 
-    // 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)
@@ -242 +241 @@
 bool CSSFontFaceSource::isSVGFontFaceSource() const
 {
-    return m_svgFontFaceElement || is<CachedSVGFont>(m_font.get());
-}
-#endif
-
-}
+    return m_hasSVGFontFaceElement || is<CachedSVGFont>(m_font.get());
+}
+#endif
+
+}

trunk/Source/WebCore/css/CSSFontFaceSource.h

@@ -94 +94 @@
 
 #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
 };