<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[37222] trunk</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/37222">37222</a></dd>
<dt>Author</dt> <dd>mitz@apple.com</dd>
<dt>Date</dt> <dd>2008-10-02 23:20:41 -0700 (Thu, 02 Oct 2008)</dd>
</dl>

<h3>Log Message</h3>
<pre>WebCore:

        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.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkWebCoreChangeLog">trunk/WebCore/ChangeLog</a></li>
<li><a href="#trunkWebCoresvgSVGFontDatah">trunk/WebCore/svg/SVGFontData.h</a></li>
<li><a href="#trunkWebCoresvgSVGFontFaceElementcpp">trunk/WebCore/svg/SVGFontFaceElement.cpp</a></li>
<li><a href="#trunkWebCoresvgSVGFontFaceElementh">trunk/WebCore/svg/SVGFontFaceElement.h</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestssvgcustomfontfacenotindocumentexpectedtxt">trunk/LayoutTests/svg/custom/font-face-not-in-document-expected.txt</a></li>
<li><a href="#trunkLayoutTestssvgcustomfontfacenotindocumentsvg">trunk/LayoutTests/svg/custom/font-face-not-in-document.svg</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (37221 => 37222)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog        2008-10-03 01:59:41 UTC (rev 37221)
+++ trunk/LayoutTests/ChangeLog        2008-10-03 06:20:41 UTC (rev 37222)
</span><span class="lines">@@ -1,3 +1,10 @@
</span><ins>+2008-10-02  Dan Bernstein  &lt;mitz@apple.com&gt;
+
+        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.
+
</ins><span class="cx"> 2008-10-01  Simon Fraser  &lt;simon.fraser@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Reviewed by Dave Hyatt
</span></span></pre></div>
<a id="trunkLayoutTestssvgcustomfontfacenotindocumentexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/svg/custom/font-face-not-in-document-expected.txt (0 => 37222)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/svg/custom/font-face-not-in-document-expected.txt                                (rev 0)
+++ trunk/LayoutTests/svg/custom/font-face-not-in-document-expected.txt        2008-10-03 06:20:41 UTC (rev 37222)
</span><span class="lines">@@ -0,0 +1,3 @@
</span><ins>+PASS
+This tests that &lt;font-face&gt; elements that are not in the document have no effect.
+Test result: PASS
</ins></span></pre></div>
<a id="trunkLayoutTestssvgcustomfontfacenotindocumentsvg"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/svg/custom/font-face-not-in-document.svg (0 => 37222)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/svg/custom/font-face-not-in-document.svg                                (rev 0)
+++ trunk/LayoutTests/svg/custom/font-face-not-in-document.svg        2008-10-03 06:20:41 UTC (rev 37222)
</span><span class="lines">@@ -0,0 +1,20 @@
</span><ins>+&lt;svg xmlns=&quot;http://www.w3.org/2000/svg&quot; xmlns:xlink=&quot;http://www.w3.org/1999/xlink&quot;&gt;
+&lt;font id=&quot;f&quot;&gt;
+    &lt;font-face font-family=&quot;epic&quot; units-per-em=&quot;1000&quot; /&gt;
+    &lt;glyph unicode=&quot;PASS&quot; horiz-adv-x=&quot;1000&quot; /&gt;
+&lt;/font&gt;
+
+&lt;text id=&quot;t&quot; y=&quot;50&quot; font-size=&quot;50&quot; font-family=&quot;epic&quot;&gt;PASS&lt;/text&gt;
+&lt;text y=&quot;72&quot;&gt;This tests that &amp;lt;font-face&amp;gt; elements that are not in the document have no effect.&lt;/text&gt;
+&lt;text y=&quot;96&quot;&gt;Test result: &lt;tspan id=&quot;result&quot;&gt;&lt;/tspan&gt;&lt;/text&gt;
+&lt;script&gt;
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+
+    var f = document.getElementById(&quot;f&quot;);
+    f.parentNode.removeChild(f);
+
+    var pass = document.getElementById(&quot;t&quot;).getEndPositionOfChar(0).x != 50;
+    document.getElementById(&quot;result&quot;).appendChild(document.createTextNode(pass ? &quot;PASS&quot; : &quot;FAIL&quot;));
+&lt;/script&gt;
+&lt;/svg&gt;
</ins></span></pre></div>
<a id="trunkWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/ChangeLog (37221 => 37222)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/ChangeLog        2008-10-03 01:59:41 UTC (rev 37221)
+++ trunk/WebCore/ChangeLog        2008-10-03 06:20:41 UTC (rev 37222)
</span><span class="lines">@@ -1,3 +1,29 @@
</span><ins>+2008-10-02  Dan Bernstein  &lt;mitz@apple.com&gt;
+
+        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:
+
</ins><span class="cx"> 2008-10-01  Simon Fraser  &lt;simon.fraser@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Reviewed by Dave Hyatt
</span></span></pre></div>
<a id="trunkWebCoresvgSVGFontDatah"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/svg/SVGFontData.h (37221 => 37222)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/svg/SVGFontData.h        2008-10-03 01:59:41 UTC (rev 37221)
+++ trunk/WebCore/svg/SVGFontData.h        2008-10-03 06:20:41 UTC (rev 37222)
</span><span class="lines">@@ -31,7 +31,7 @@
</span><span class="cx">     SVGFontData(SVGFontFaceElement*);
</span><span class="cx">     virtual ~SVGFontData();
</span><span class="cx"> 
</span><del>-    SVGFontFaceElement* svgFontFaceElement() const { return m_svgFontFaceElement.get(); }
</del><ins>+    SVGFontFaceElement* svgFontFaceElement() const { return m_svgFontFaceElement; }
</ins><span class="cx"> 
</span><span class="cx">     float horizontalOriginX() const { return m_horizontalOriginX; }
</span><span class="cx">     float horizontalOriginY() const { return m_horizontalOriginY; }
</span><span class="lines">@@ -42,7 +42,13 @@
</span><span class="cx">     float verticalAdvanceY() const { return m_verticalAdvanceY; }
</span><span class="cx"> 
</span><span class="cx"> private:
</span><del>-    RefPtr&lt;SVGFontFaceElement&gt; m_svgFontFaceElement;
</del><ins>+    // 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;
</ins><span class="cx"> 
</span><span class="cx">     float m_horizontalOriginX;
</span><span class="cx">     float m_horizontalOriginY;
</span></span></pre></div>
<a id="trunkWebCoresvgSVGFontFaceElementcpp"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/svg/SVGFontFaceElement.cpp (37221 => 37222)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/svg/SVGFontFaceElement.cpp        2008-10-03 01:59:41 UTC (rev 37221)
+++ trunk/WebCore/svg/SVGFontFaceElement.cpp        2008-10-03 06:20:41 UTC (rev 37222)
</span><span class="lines">@@ -1,6 +1,7 @@
</span><span class="cx"> /*
</span><span class="cx">    Copyright (C) 2007 Eric Seidel &lt;eric@webkit.org&gt;
</span><span class="cx">    Copyright (C) 2007, 2008 Nikolas Zimmermann &lt;zimmermann@kde.org&gt;
</span><ins>+   Copyright (C) 2008 Apple Inc. All rights reserved.
</ins><span class="cx">     
</span><span class="cx">    This library is free software; you can redistribute it and/or
</span><span class="cx">    modify it under the terms of the GNU Library General Public
</span><span class="lines">@@ -53,7 +54,6 @@
</span><span class="cx">     m_styleDeclaration-&gt;setParent(document()-&gt;mappedElementSheet());
</span><span class="cx">     m_styleDeclaration-&gt;setStrictParsing(true);
</span><span class="cx">     m_fontFaceRule-&gt;setDeclaration(m_styleDeclaration.get());
</span><del>-    document()-&gt;mappedElementSheet()-&gt;append(m_fontFaceRule);
</del><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> SVGFontFaceElement::~SVGFontFaceElement()
</span><span class="lines">@@ -121,7 +121,8 @@
</span><span class="cx">     int propId = cssPropertyIdForSVGAttributeName(attr-&gt;name());
</span><span class="cx">     if (propId &gt; 0) {
</span><span class="cx">         m_styleDeclaration-&gt;setProperty(propId, attr-&gt;value(), false);
</span><del>-        rebuildFontFace();
</del><ins>+        if (inDocument())
+            rebuildFontFace();
</ins><span class="cx">         return;
</span><span class="cx">     }
</span><span class="cx">     
</span><span class="lines">@@ -286,11 +287,7 @@
</span><span class="cx"> 
</span><span class="cx"> void SVGFontFaceElement::rebuildFontFace()
</span><span class="cx"> {
</span><del>-    // Ignore changes until we live in the tree
-    if (!parentNode()) {
-        m_fontElement = 0;
-        return;
-    }
</del><ins>+    ASSERT(inDocument());
</ins><span class="cx"> 
</span><span class="cx">     // we currently ignore all but the first src element, alternatively we could concat them
</span><span class="cx">     SVGFontFaceSrcElement* srcElement = 0;
</span><span class="lines">@@ -317,8 +314,11 @@
</span><span class="cx"> 
</span><span class="cx">         list = CSSValueList::createCommaSeparated();
</span><span class="cx">         list-&gt;append(CSSFontFaceSrcValue::createLocal(fontFamily()));
</span><del>-    } else if (srcElement)
-        list = srcElement-&gt;srcValue();
</del><ins>+    } else {
+        m_fontElement = 0;
+        if (srcElement)
+            list = srcElement-&gt;srcValue();
+    }
</ins><span class="cx"> 
</span><span class="cx">     if (!list)
</span><span class="cx">         return;
</span><span class="lines">@@ -345,24 +345,22 @@
</span><span class="cx"> 
</span><span class="cx"> void SVGFontFaceElement::insertedIntoDocument()
</span><span class="cx"> {
</span><del>-    rebuildFontFace();
</del><span class="cx">     SVGElement::insertedIntoDocument();
</span><del>-}
-
-void SVGFontFaceElement::childrenChanged(bool changedByParser, Node* beforeChange, Node* afterChange, int childCountDelta)
-{
-    SVGElement::childrenChanged(changedByParser, beforeChange, afterChange, childCountDelta);
</del><ins>+    document()-&gt;mappedElementSheet()-&gt;append(m_fontFaceRule);
</ins><span class="cx">     rebuildFontFace();
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-void SVGFontFaceElement::willMoveToNewOwnerDocument()
</del><ins>+void SVGFontFaceElement::removedFromDocument()
</ins><span class="cx"> {
</span><span class="cx">     removeFromMappedElementSheet();
</span><ins>+    SVGElement::removedFromDocument();
</ins><span class="cx"> }
</span><span class="cx"> 
</span><del>-void SVGFontFaceElement::didMoveToNewOwnerDocument()
</del><ins>+void SVGFontFaceElement::childrenChanged(bool changedByParser, Node* beforeChange, Node* afterChange, int childCountDelta)
</ins><span class="cx"> {
</span><del>-    document()-&gt;mappedElementSheet()-&gt;append(m_fontFaceRule);
</del><ins>+    SVGElement::childrenChanged(changedByParser, beforeChange, afterChange, childCountDelta);
+    if (inDocument())
+        rebuildFontFace();
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void SVGFontFaceElement::removeFromMappedElementSheet()
</span><span class="lines">@@ -377,6 +375,7 @@
</span><span class="cx">             break;
</span><span class="cx">         }
</span><span class="cx">     }
</span><ins>+    document()-&gt;updateStyleSelector();
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> } // namespace WebCore
</span></span></pre></div>
<a id="trunkWebCoresvgSVGFontFaceElementh"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/svg/SVGFontFaceElement.h (37221 => 37222)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/svg/SVGFontFaceElement.h        2008-10-03 01:59:41 UTC (rev 37221)
+++ trunk/WebCore/svg/SVGFontFaceElement.h        2008-10-03 06:20:41 UTC (rev 37222)
</span><span class="lines">@@ -1,6 +1,7 @@
</span><span class="cx"> /*
</span><span class="cx">    Copyright (C) 2007 Eric Seidel &lt;eric@webkit.org&gt;
</span><span class="cx">    Copyright (C) 2007 Nikolas Zimmermann &lt;zimmermann@kde.org&gt;
</span><ins>+   Copyright (C) 2008 Apple Inc. All rights reserved.
</ins><span class="cx"> 
</span><span class="cx">    This library is free software; you can redistribute it and/or
</span><span class="cx">    modify it under the terms of the GNU Library General Public
</span><span class="lines">@@ -39,8 +40,7 @@
</span><span class="cx"> 
</span><span class="cx">         virtual void childrenChanged(bool changedByParser = false, Node* beforeChange = 0, Node* afterChange = 0, int childCountDelta = 0);
</span><span class="cx">         virtual void insertedIntoDocument();
</span><del>-        virtual void willMoveToNewOwnerDocument();
-        virtual void didMoveToNewOwnerDocument();
</del><ins>+        virtual void removedFromDocument();
</ins><span class="cx"> 
</span><span class="cx">         unsigned unitsPerEm() const;
</span><span class="cx">         int xHeight() const;
</span></span></pre>
</div>
</div>

</body>
</html>