Abandoned Memory: SVGFontElement and Corresponding SVGDocument Never Deconstructed
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 24 Jan 2013 19:18:22 +0000 (19:18 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 24 Jan 2013 19:18:22 +0000 (19:18 +0000)
https://bugs.webkit.org/show_bug.cgi?id=66438

Reviewed by Dirk Schulze.

The memory leak was caused by SVGFontFaceElement storing its own parent in a RefPtr.

Fixed the bug by storing a raw pointer instead, and clearing the pointer in removedFrom
when the node detached from the document. Also added several sanity check assertions.

* svg/SVGFontFaceElement.cpp:
(WebCore::SVGFontFaceElement::SVGFontFaceElement):
(WebCore::SVGFontFaceElement::associatedFontElement):
(WebCore::SVGFontFaceElement::rebuildFontFace):
(WebCore::SVGFontFaceElement::insertedInto):
(WebCore::SVGFontFaceElement::removedFrom):
* svg/SVGFontFaceElement.h:
(SVGFontFaceElement):

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

Source/WebCore/ChangeLog
Source/WebCore/svg/SVGFontFaceElement.cpp
Source/WebCore/svg/SVGFontFaceElement.h

index 7a37f47..0557dad 100644 (file)
@@ -1,3 +1,24 @@
+2013-01-24  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Abandoned Memory: SVGFontElement and Corresponding SVGDocument Never Deconstructed
+        https://bugs.webkit.org/show_bug.cgi?id=66438
+
+        Reviewed by Dirk Schulze.
+
+        The memory leak was caused by SVGFontFaceElement storing its own parent in a RefPtr.
+
+        Fixed the bug by storing a raw pointer instead, and clearing the pointer in removedFrom
+        when the node detached from the document. Also added several sanity check assertions.
+
+        * svg/SVGFontFaceElement.cpp:
+        (WebCore::SVGFontFaceElement::SVGFontFaceElement):
+        (WebCore::SVGFontFaceElement::associatedFontElement):
+        (WebCore::SVGFontFaceElement::rebuildFontFace):
+        (WebCore::SVGFontFaceElement::insertedInto):
+        (WebCore::SVGFontFaceElement::removedFrom):
+        * svg/SVGFontFaceElement.h:
+        (SVGFontFaceElement):
+
 2013-01-22  Robert Hogan  <robert@webkit.org>
 
         Inline Containing Only Collapsed Whitespace Not Getting a Linebox
index 73a08f9..366a968 100644 (file)
@@ -49,6 +49,7 @@ using namespace SVGNames;
 inline SVGFontFaceElement::SVGFontFaceElement(const QualifiedName& tagName, Document* document)
     : SVGElement(tagName, document)
     , m_fontFaceRule(StyleRuleFontFace::create())
+    , m_fontElement(0)
 {
     ASSERT(hasTagName(font_faceTag));
     RefPtr<StylePropertySet> styleDeclaration = StylePropertySet::create(CSSStrictMode);
@@ -262,13 +263,17 @@ String SVGFontFaceElement::fontFamily() const
 
 SVGFontElement* SVGFontFaceElement::associatedFontElement() const
 {
-    return m_fontElement.get();
+    ASSERT(parentNode() == m_fontElement);
+    ASSERT(!parentNode() || parentNode()->hasTagName(SVGNames::fontTag));
+    return m_fontElement;
 }
 
 void SVGFontFaceElement::rebuildFontFace()
 {
-    if (!inDocument())
+    if (!inDocument()) {
+        ASSERT(!m_fontElement);
         return;
+    }
 
     // we currently ignore all but the first src element, alternatively we could concat them
     SVGFontFaceSrcElement* srcElement = 0;
@@ -316,8 +321,10 @@ void SVGFontFaceElement::rebuildFontFace()
 Node::InsertionNotificationRequest SVGFontFaceElement::insertedInto(ContainerNode* rootParent)
 {
     SVGElement::insertedInto(rootParent);
-    if (!rootParent->inDocument())
+    if (!rootParent->inDocument()) {
+        ASSERT(!m_fontElement);
         return InsertionDone;
+    }
     document()->accessSVGExtensions()->registerSVGFontFaceElement(this);
 
     rebuildFontFace();
@@ -329,11 +336,13 @@ void SVGFontFaceElement::removedFrom(ContainerNode* rootParent)
     SVGElement::removedFrom(rootParent);
 
     if (rootParent->inDocument()) {
+        m_fontElement = 0;
         document()->accessSVGExtensions()->unregisterSVGFontFaceElement(this);
         m_fontFaceRule->mutableProperties()->parseDeclaration(emptyString(), 0);
 
         document()->styleResolverChanged(DeferRecalcStyle);
-    }
+    } else
+        ASSERT(!m_fontElement);
 }
 
 void SVGFontFaceElement::childrenChanged(bool changedByParser, Node* beforeChange, Node* afterChange, int childCountDelta)
index fd93dad..543f6d0 100644 (file)
@@ -61,7 +61,7 @@ private:
     virtual void removedFrom(ContainerNode*) OVERRIDE;
 
     RefPtr<StyleRuleFontFace> m_fontFaceRule;
-    RefPtr<SVGFontElement> m_fontElement;
+    SVGFontElement* m_fontElement;
 };
 
 } // namespace WebCore