WebCore:
authormitz@apple.com <mitz@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 3 Oct 2008 06:20:41 +0000 (06:20 +0000)
committermitz@apple.com <mitz@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 3 Oct 2008 06:20:41 +0000 (06:20 +0000)
        Reviewed by Geoffrey Garen and Sam Weinig.

        - fix SVGFontFaceElement leaks seen in Acid3
        - make font-face elements take effect only when they are in the document tree

        Test: svg/custom/font-face-not-in-document.svg

        * svg/SVGFontData.h: Changed the m_svgFontFaceElement member from a
        RefPtr to a plain pointer to break a ref cycle.
        (WebCore::SVGFontData::svgFontFaceElement):

        * svg/SVGFontFaceElement.cpp: Changed to insert and remove the
        @font-face rule from the document's mapped element sheet when the
        element is inserted and removed from the document, and to update it
        only when the element is in the document.
        (WebCore::SVGFontFaceElement::SVGFontFaceElement):
        (WebCore::SVGFontFaceElement::parseMappedAttribute):
        (WebCore::SVGFontFaceElement::rebuildFontFace):
        (WebCore::SVGFontFaceElement::insertedIntoDocument):
        (WebCore::SVGFontFaceElement::removedFromDocument):
        (WebCore::SVGFontFaceElement::childrenChanged):
        (WebCore::SVGFontFaceElement::removeFromMappedElementSheet):
        * svg/SVGFontFaceElement.h:

LayoutTests:

        Reviewed by Geoffrey Garen and Sam Weinig.

        * svg/custom/font-face-not-in-document-expected.txt: Added.
        * svg/custom/font-face-not-in-document.svg: Added.

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

LayoutTests/ChangeLog
LayoutTests/svg/custom/font-face-not-in-document-expected.txt [new file with mode: 0644]
LayoutTests/svg/custom/font-face-not-in-document.svg [new file with mode: 0644]
WebCore/ChangeLog
WebCore/svg/SVGFontData.h
WebCore/svg/SVGFontFaceElement.cpp
WebCore/svg/SVGFontFaceElement.h

index c4c3e18..fc1c4c5 100644 (file)
@@ -1,3 +1,10 @@
+2008-10-02  Dan Bernstein  <mitz@apple.com>
+
+        Reviewed by Geoffrey Garen and Sam Weinig.
+
+        * svg/custom/font-face-not-in-document-expected.txt: Added.
+        * svg/custom/font-face-not-in-document.svg: Added.
+
 2008-10-01  Simon Fraser  <simon.fraser@apple.com>
 
         Reviewed by Dave Hyatt
diff --git a/LayoutTests/svg/custom/font-face-not-in-document-expected.txt b/LayoutTests/svg/custom/font-face-not-in-document-expected.txt
new file mode 100644 (file)
index 0000000..2844c0c
--- /dev/null
@@ -0,0 +1,3 @@
+PASS
+This tests that <font-face> elements that are not in the document have no effect.
+Test result: PASS
diff --git a/LayoutTests/svg/custom/font-face-not-in-document.svg b/LayoutTests/svg/custom/font-face-not-in-document.svg
new file mode 100644 (file)
index 0000000..1f1aef7
--- /dev/null
@@ -0,0 +1,20 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+<font id="f">
+    <font-face font-family="epic" units-per-em="1000" />
+    <glyph unicode="PASS" horiz-adv-x="1000" />
+</font>
+
+<text id="t" y="50" font-size="50" font-family="epic">PASS</text>
+<text y="72">This tests that &lt;font-face&gt; elements that are not in the document have no effect.</text>
+<text y="96">Test result: <tspan id="result"></tspan></text>
+<script>
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+
+    var f = document.getElementById("f");
+    f.parentNode.removeChild(f);
+
+    var pass = document.getElementById("t").getEndPositionOfChar(0).x != 50;
+    document.getElementById("result").appendChild(document.createTextNode(pass ? "PASS" : "FAIL"));
+</script>
+</svg>
index 5744e4e..f89679e 100644 (file)
@@ -1,3 +1,29 @@
+2008-10-02  Dan Bernstein  <mitz@apple.com>
+
+        Reviewed by Geoffrey Garen and Sam Weinig.
+
+        - fix SVGFontFaceElement leaks seen in Acid3
+        - make font-face elements take effect only when they are in the document tree
+
+        Test: svg/custom/font-face-not-in-document.svg
+
+        * svg/SVGFontData.h: Changed the m_svgFontFaceElement member from a
+        RefPtr to a plain pointer to break a ref cycle.
+        (WebCore::SVGFontData::svgFontFaceElement):
+
+        * svg/SVGFontFaceElement.cpp: Changed to insert and remove the
+        @font-face rule from the document's mapped element sheet when the
+        element is inserted and removed from the document, and to update it
+        only when the element is in the document.
+        (WebCore::SVGFontFaceElement::SVGFontFaceElement):
+        (WebCore::SVGFontFaceElement::parseMappedAttribute):
+        (WebCore::SVGFontFaceElement::rebuildFontFace):
+        (WebCore::SVGFontFaceElement::insertedIntoDocument):
+        (WebCore::SVGFontFaceElement::removedFromDocument):
+        (WebCore::SVGFontFaceElement::childrenChanged):
+        (WebCore::SVGFontFaceElement::removeFromMappedElementSheet):
+        * svg/SVGFontFaceElement.h:
+
 2008-10-01  Simon Fraser  <simon.fraser@apple.com>
 
         Reviewed by Dave Hyatt
index cb2192c..4df3db2 100644 (file)
@@ -31,7 +31,7 @@ public:
     SVGFontData(SVGFontFaceElement*);
     virtual ~SVGFontData();
 
-    SVGFontFaceElement* svgFontFaceElement() const { return m_svgFontFaceElement.get(); }
+    SVGFontFaceElement* svgFontFaceElement() const { return m_svgFontFaceElement; }
 
     float horizontalOriginX() const { return m_horizontalOriginX; }
     float horizontalOriginY() const { return m_horizontalOriginY; }
@@ -42,7 +42,13 @@ public:
     float verticalAdvanceY() const { return m_verticalAdvanceY; }
 
 private:
-    RefPtr<SVGFontFaceElement> m_svgFontFaceElement;
+    // Ths SVGFontFaceElement is kept alive --
+    // 1) in the external font case: by the CSSFontFaceSource, which holds a reference to the external SVG document
+    //    containing the element;
+    // 2) in the in-document font case: by virtue of being in the document tree and making sure that when it is removed
+    //    from the document, it removes the @font-face rule it owns from the document's mapped element sheet and forces
+    //    a style update.
+    SVGFontFaceElement* m_svgFontFaceElement;
 
     float m_horizontalOriginX;
     float m_horizontalOriginY;
index 3de32dd..4bc0580 100644 (file)
@@ -1,6 +1,7 @@
 /*
    Copyright (C) 2007 Eric Seidel <eric@webkit.org>
    Copyright (C) 2007, 2008 Nikolas Zimmermann <zimmermann@kde.org>
+   Copyright (C) 2008 Apple Inc. All rights reserved.
     
    This library is free software; you can redistribute it and/or
    modify it under the terms of the GNU Library General Public
@@ -53,7 +54,6 @@ SVGFontFaceElement::SVGFontFaceElement(const QualifiedName& tagName, Document* d
     m_styleDeclaration->setParent(document()->mappedElementSheet());
     m_styleDeclaration->setStrictParsing(true);
     m_fontFaceRule->setDeclaration(m_styleDeclaration.get());
-    document()->mappedElementSheet()->append(m_fontFaceRule);
 }
 
 SVGFontFaceElement::~SVGFontFaceElement()
@@ -121,7 +121,8 @@ void SVGFontFaceElement::parseMappedAttribute(MappedAttribute* attr)
     int propId = cssPropertyIdForSVGAttributeName(attr->name());
     if (propId > 0) {
         m_styleDeclaration->setProperty(propId, attr->value(), false);
-        rebuildFontFace();
+        if (inDocument())
+            rebuildFontFace();
         return;
     }
     
@@ -286,11 +287,7 @@ String SVGFontFaceElement::fontFamily() const
 
 void SVGFontFaceElement::rebuildFontFace()
 {
-    // Ignore changes until we live in the tree
-    if (!parentNode()) {
-        m_fontElement = 0;
-        return;
-    }
+    ASSERT(inDocument());
 
     // we currently ignore all but the first src element, alternatively we could concat them
     SVGFontFaceSrcElement* srcElement = 0;
@@ -317,8 +314,11 @@ void SVGFontFaceElement::rebuildFontFace()
 
         list = CSSValueList::createCommaSeparated();
         list->append(CSSFontFaceSrcValue::createLocal(fontFamily()));
-    } else if (srcElement)
-        list = srcElement->srcValue();
+    } else {
+        m_fontElement = 0;
+        if (srcElement)
+            list = srcElement->srcValue();
+    }
 
     if (!list)
         return;
@@ -345,24 +345,22 @@ void SVGFontFaceElement::rebuildFontFace()
 
 void SVGFontFaceElement::insertedIntoDocument()
 {
-    rebuildFontFace();
     SVGElement::insertedIntoDocument();
-}
-
-void SVGFontFaceElement::childrenChanged(bool changedByParser, Node* beforeChange, Node* afterChange, int childCountDelta)
-{
-    SVGElement::childrenChanged(changedByParser, beforeChange, afterChange, childCountDelta);
+    document()->mappedElementSheet()->append(m_fontFaceRule);
     rebuildFontFace();
 }
 
-void SVGFontFaceElement::willMoveToNewOwnerDocument()
+void SVGFontFaceElement::removedFromDocument()
 {
     removeFromMappedElementSheet();
+    SVGElement::removedFromDocument();
 }
 
-void SVGFontFaceElement::didMoveToNewOwnerDocument()
+void SVGFontFaceElement::childrenChanged(bool changedByParser, Node* beforeChange, Node* afterChange, int childCountDelta)
 {
-    document()->mappedElementSheet()->append(m_fontFaceRule);
+    SVGElement::childrenChanged(changedByParser, beforeChange, afterChange, childCountDelta);
+    if (inDocument())
+        rebuildFontFace();
 }
 
 void SVGFontFaceElement::removeFromMappedElementSheet()
@@ -377,6 +375,7 @@ void SVGFontFaceElement::removeFromMappedElementSheet()
             break;
         }
     }
+    document()->updateStyleSelector();
 }
 
 } // namespace WebCore
index 9240767..3c28a89 100644 (file)
@@ -1,6 +1,7 @@
 /*
    Copyright (C) 2007 Eric Seidel <eric@webkit.org>
    Copyright (C) 2007 Nikolas Zimmermann <zimmermann@kde.org>
+   Copyright (C) 2008 Apple Inc. All rights reserved.
 
    This library is free software; you can redistribute it and/or
    modify it under the terms of the GNU Library General Public
@@ -39,8 +40,7 @@ namespace WebCore {
 
         virtual void childrenChanged(bool changedByParser = false, Node* beforeChange = 0, Node* afterChange = 0, int childCountDelta = 0);
         virtual void insertedIntoDocument();
-        virtual void willMoveToNewOwnerDocument();
-        virtual void didMoveToNewOwnerDocument();
+        virtual void removedFromDocument();
 
         unsigned unitsPerEm() const;
         int xHeight() const;