REGRESSION(r105057): Infinite loop inside SVGTextLayoutEngine::currentLogicalCharacte...
authorzimmermann@webkit.org <zimmermann@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 May 2012 07:17:09 +0000 (07:17 +0000)
committerzimmermann@webkit.org <zimmermann@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 May 2012 07:17:09 +0000 (07:17 +0000)
https://bugs.webkit.org/show_bug.cgi?id=83405

Reviewed by Darin Adler.

Source/WebCore:

Dynamically adding tspans carrying position information in the x/y/dx/dy/rotate lists is broken.
To avoid mistakes like this in future, simplify the calling code in RenderSVGInlineText and centralize
the managment of all caches (text positioning element cache / metrics map / layout attributes) in
RenderSVGText. This avoids the hack in SVGRootInlineBox::computePerCharacterLayoutInformation() which
called textRoot->rebuildLayoutAttributes(), which was used to fix previous security issues with this code.
Instead correctly handle destruction of RenderSVGInlineText in RenderSVGText, keeping the m_layoutAttributes
synchronized with the current state of the render tree. Fixes highcharts problems.

Tests: svg/text/add-tspan-position-bug.html
       svg/text/modify-tspan-position-bug.html

* rendering/svg/RenderSVGInline.cpp:
(WebCore::RenderSVGInline::addChild):
* rendering/svg/RenderSVGInlineText.cpp:
(WebCore::RenderSVGInlineText::willBeDestroyed):
(WebCore::RenderSVGInlineText::setTextInternal):
(WebCore::RenderSVGInlineText::styleDidChange):
* rendering/svg/RenderSVGText.cpp:
(WebCore::recursiveUpdateMetrics):
(WebCore::RenderSVGText::subtreeChildAdded):
(WebCore::RenderSVGText::subtreeChildWillBeDestroyed):
(WebCore::recursiveCollectLayoutAttributes):
(WebCore::checkLayoutAttributesConsistency):
(WebCore::RenderSVGText::subtreeChildWasDestroyed):
(WebCore::RenderSVGText::subtreeStyleChanged):
(WebCore::RenderSVGText::subtreeTextChanged):
(WebCore::RenderSVGText::layout):
(WebCore::RenderSVGText::addChild):
(WebCore::RenderSVGText::rebuildAllLayoutAttributes):
(WebCore::RenderSVGText::rebuildLayoutAttributes):
* rendering/svg/RenderSVGText.h:
(WebCore::RenderSVGText::layoutAttributes):
* rendering/svg/SVGRootInlineBox.cpp:
(WebCore::SVGRootInlineBox::computePerCharacterLayoutInformation):
* rendering/svg/SVGTextLayoutAttributesBuilder.cpp:
(WebCore::SVGTextLayoutAttributesBuilder::buildLayoutAttributes):

LayoutTests:

Add two new testcases covering the problem.

* svg/text/add-tspan-position-bug-expected.html: Added.
* svg/text/add-tspan-position-bug.html: Added.
* svg/text/modify-tspan-position-bug-expected.html: Added.
* svg/text/modify-tspan-position-bug.html: Added.

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

12 files changed:
LayoutTests/ChangeLog
LayoutTests/svg/text/add-tspan-position-bug-expected.html [new file with mode: 0644]
LayoutTests/svg/text/add-tspan-position-bug.html [new file with mode: 0644]
LayoutTests/svg/text/modify-tspan-position-bug-expected.html [new file with mode: 0644]
LayoutTests/svg/text/modify-tspan-position-bug.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/svg/RenderSVGInline.cpp
Source/WebCore/rendering/svg/RenderSVGInlineText.cpp
Source/WebCore/rendering/svg/RenderSVGText.cpp
Source/WebCore/rendering/svg/RenderSVGText.h
Source/WebCore/rendering/svg/SVGRootInlineBox.cpp
Source/WebCore/rendering/svg/SVGTextLayoutAttributesBuilder.cpp

index b25b73d..0e980fa 100644 (file)
@@ -1,3 +1,17 @@
+2012-05-09  Nikolas Zimmermann  <nzimmermann@rim.com>
+
+        REGRESSION(r105057): Infinite loop inside SVGTextLayoutEngine::currentLogicalCharacterMetrics
+        https://bugs.webkit.org/show_bug.cgi?id=83405
+
+        Reviewed by Darin Adler.
+
+        Add two new testcases covering the problem.
+
+        * svg/text/add-tspan-position-bug-expected.html: Added.
+        * svg/text/add-tspan-position-bug.html: Added.
+        * svg/text/modify-tspan-position-bug-expected.html: Added.
+        * svg/text/modify-tspan-position-bug.html: Added.
+
 2012-05-08  Kent Tamura  <tkent@chromium.org>
 
         Unreviewed, a trivial test fix to remove a platform-dependency.
diff --git a/LayoutTests/svg/text/add-tspan-position-bug-expected.html b/LayoutTests/svg/text/add-tspan-position-bug-expected.html
new file mode 100644 (file)
index 0000000..d1f889b
--- /dev/null
@@ -0,0 +1,8 @@
+<!DOCTYPE html>
+<html>
+<body>
+<svg>
+<text id="text" y="50"><tspan>Should be on</tspan><tspan dy="30">different lines</tspan></text>
+</svg>
+</body>
+</html>
diff --git a/LayoutTests/svg/text/add-tspan-position-bug.html b/LayoutTests/svg/text/add-tspan-position-bug.html
new file mode 100644 (file)
index 0000000..d33c57f
--- /dev/null
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<html>
+<body onload="loaded()">
+<svg>
+<title>This test used to be laid out on a single line, instead of multiple ones</title>
+<text id="text" y="50"></text>
+
+<script>
+var text = document.getElementsByTagName("text")[0];
+
+function addSpans() {
+    var tspan1 = document.createElementNS("http://www.w3.org/2000/svg", "tspan");
+    tspan1.appendChild(document.createTextNode("Should be on"));
+
+    var tspan2 = document.createElementNS("http://www.w3.org/2000/svg", "tspan");
+    tspan2.setAttribute("dy", "30");
+    tspan2.appendChild(document.createTextNode("different lines"));
+
+    text.appendChild(tspan1);
+    text.appendChild(tspan2);
+
+    if (window.layoutTestController)
+        layoutTestController.notifyDone();
+}
+
+if (window.layoutTestController)
+    layoutTestController.waitUntilDone();
+
+function loaded() {
+    // Bug is only trigger from another loop.
+    setTimeout(addSpans, 0);
+}
+</script>
+</svg>
+</body>
+</html>
diff --git a/LayoutTests/svg/text/modify-tspan-position-bug-expected.html b/LayoutTests/svg/text/modify-tspan-position-bug-expected.html
new file mode 100644 (file)
index 0000000..d1f889b
--- /dev/null
@@ -0,0 +1,8 @@
+<!DOCTYPE html>
+<html>
+<body>
+<svg>
+<text id="text" y="50"><tspan>Should be on</tspan><tspan dy="30">different lines</tspan></text>
+</svg>
+</body>
+</html>
diff --git a/LayoutTests/svg/text/modify-tspan-position-bug.html b/LayoutTests/svg/text/modify-tspan-position-bug.html
new file mode 100644 (file)
index 0000000..4ea9d07
--- /dev/null
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<html>
+<body onload="loaded()">
+<svg>
+<title>This test used to be laid out on a single line, instead of multiple ones</title>
+<text id="text" y="50"></text>
+
+<script>
+var text = document.getElementsByTagName("text")[0];
+
+function addSpans() {
+    var tspan1 = document.createElementNS("http://www.w3.org/2000/svg", "tspan");
+    tspan1.appendChild(document.createTextNode("Should be on"));
+
+    tspan2 = document.createElementNS("http://www.w3.org/2000/svg", "tspan");
+    tspan2.appendChild(document.createTextNode("different lines"));
+
+    text.appendChild(tspan1);
+    text.appendChild(tspan2);
+
+    // Bug is only trigger from another loop.
+    setTimeout(moveSpan, 0);
+}
+
+function moveSpan() {
+    tspan2.setAttribute("dy", "30");
+
+    if (window.layoutTestController)
+        layoutTestController.notifyDone();
+}
+
+if (window.layoutTestController)
+    layoutTestController.waitUntilDone();
+
+function loaded() {
+    // Bug is only trigger from another loop.
+    setTimeout(addSpans, 0);
+}
+</script>
+</svg>
+</body>
+</html>
index cf20506..996fe28 100644 (file)
@@ -1,3 +1,47 @@
+2012-05-09  Nikolas Zimmermann  <nzimmermann@rim.com>
+
+        REGRESSION(r105057): Infinite loop inside SVGTextLayoutEngine::currentLogicalCharacterMetrics
+        https://bugs.webkit.org/show_bug.cgi?id=83405
+
+        Reviewed by Darin Adler.
+
+        Dynamically adding tspans carrying position information in the x/y/dx/dy/rotate lists is broken.
+        To avoid mistakes like this in future, simplify the calling code in RenderSVGInlineText and centralize
+        the managment of all caches (text positioning element cache / metrics map / layout attributes) in
+        RenderSVGText. This avoids the hack in SVGRootInlineBox::computePerCharacterLayoutInformation() which
+        called textRoot->rebuildLayoutAttributes(), which was used to fix previous security issues with this code.
+        Instead correctly handle destruction of RenderSVGInlineText in RenderSVGText, keeping the m_layoutAttributes
+        synchronized with the current state of the render tree. Fixes highcharts problems.
+
+        Tests: svg/text/add-tspan-position-bug.html
+               svg/text/modify-tspan-position-bug.html
+
+        * rendering/svg/RenderSVGInline.cpp:
+        (WebCore::RenderSVGInline::addChild):
+        * rendering/svg/RenderSVGInlineText.cpp:
+        (WebCore::RenderSVGInlineText::willBeDestroyed):
+        (WebCore::RenderSVGInlineText::setTextInternal):
+        (WebCore::RenderSVGInlineText::styleDidChange):
+        * rendering/svg/RenderSVGText.cpp:
+        (WebCore::recursiveUpdateMetrics):
+        (WebCore::RenderSVGText::subtreeChildAdded):
+        (WebCore::RenderSVGText::subtreeChildWillBeDestroyed):
+        (WebCore::recursiveCollectLayoutAttributes):
+        (WebCore::checkLayoutAttributesConsistency):
+        (WebCore::RenderSVGText::subtreeChildWasDestroyed):
+        (WebCore::RenderSVGText::subtreeStyleChanged):
+        (WebCore::RenderSVGText::subtreeTextChanged):
+        (WebCore::RenderSVGText::layout):
+        (WebCore::RenderSVGText::addChild):
+        (WebCore::RenderSVGText::rebuildAllLayoutAttributes):
+        (WebCore::RenderSVGText::rebuildLayoutAttributes):
+        * rendering/svg/RenderSVGText.h:
+        (WebCore::RenderSVGText::layoutAttributes):
+        * rendering/svg/SVGRootInlineBox.cpp:
+        (WebCore::SVGRootInlineBox::computePerCharacterLayoutInformation):
+        * rendering/svg/SVGTextLayoutAttributesBuilder.cpp:
+        (WebCore::SVGTextLayoutAttributesBuilder::buildLayoutAttributes):
+
 2012-05-08  Dongwoo Im  <dw.im@samsung.com>
 
         NavigatorRegisterProtocolHandler can call ChromeClient directly.
index ab8cc1a..98af8e9 100644 (file)
@@ -124,7 +124,7 @@ void RenderSVGInline::addChild(RenderObject* child, RenderObject* beforeChild)
 {
     RenderInline::addChild(child, beforeChild);
     if (RenderSVGText* textRenderer = RenderSVGText::locateRenderSVGTextAncestor(this))
-        textRenderer->layoutAttributesChanged(child);
+        textRenderer->subtreeChildAdded(child);
 }
 
 }
index 09d446b..a0df411 100644 (file)
@@ -81,28 +81,16 @@ void RenderSVGInlineText::willBeDestroyed()
     }
 
     Vector<SVGTextLayoutAttributes*> affectedAttributes;
-    textRenderer->layoutAttributesWillBeDestroyed(this, affectedAttributes);
-
+    textRenderer->subtreeChildWillBeDestroyed(this, affectedAttributes);
     RenderText::willBeDestroyed();
-    if (affectedAttributes.isEmpty())
-        return;
-
-    if (!documentBeingDestroyed())
-        textRenderer->rebuildLayoutAttributes(affectedAttributes);
+    textRenderer->subtreeChildWasDestroyed(this, affectedAttributes);
 }
 
 void RenderSVGInlineText::setTextInternal(PassRefPtr<StringImpl> text)
 {
     RenderText::setTextInternal(text);
-
-    // When the underlying text content changes, call both textDOMChanged() & layoutAttributesChanged()
-    // The former will clear the SVGTextPositioningElement cache, which depends on the textLength() of
-    // the RenderSVGInlineText objects, and thus needs to be rebuild. The latter will assure that the
-    // SVGTextLayoutAttributes associated with the RenderSVGInlineText will be updated.
-    if (RenderSVGText* textRenderer = RenderSVGText::locateRenderSVGTextAncestor(this)) {
-        textRenderer->invalidateTextPositioningElements();
-        textRenderer->layoutAttributesChanged(this);
-    }
+    if (RenderSVGText* textRenderer = RenderSVGText::locateRenderSVGTextAncestor(this))
+        textRenderer->subtreeTextChanged(this);
 }
 
 void RenderSVGInlineText::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
@@ -127,7 +115,7 @@ void RenderSVGInlineText::styleDidChange(StyleDifference diff, const RenderStyle
 
     // The text metrics may be influenced by style changes.
     if (RenderSVGText* textRenderer = RenderSVGText::locateRenderSVGTextAncestor(this))
-        textRenderer->layoutAttributesChanged(this);
+        textRenderer->subtreeStyleChanged(this);
 }
 
 InlineTextBox* RenderSVGInlineText::createTextBox()
index f512ab8..f0f0b9c 100644 (file)
@@ -110,24 +110,22 @@ void RenderSVGText::mapLocalToContainer(RenderBoxModelObject* repaintContainer,
     SVGRenderSupport::mapLocalToContainer(this, repaintContainer, transformState, wasFixed);
 }
 
-static inline void recursiveUpdateLayoutAttributes(RenderObject* start, SVGTextLayoutAttributesBuilder& builder)
-{
-    if (start->isSVGInlineText()) {
-        builder.buildLayoutAttributesForTextRenderer(toRenderSVGInlineText(start));
-        return;
-    }
-
-    for (RenderObject* child = start->firstChild(); child; child = child->nextSibling())
-        recursiveUpdateLayoutAttributes(child, builder);
-}
-
-void RenderSVGText::layoutAttributesChanged(RenderObject* child)
+void RenderSVGText::subtreeChildAdded(RenderObject* child)
 {
     ASSERT(child);
     if (m_needsPositioningValuesUpdate)
         return;
+
+    // The positioning elements cache doesn't include the new 'child' yet. Clear the
+    // cache, as the next buildLayoutAttributesForTextRenderer() call rebuilds it.
+    invalidateTextPositioningElements();
+
     FontCachePurgePreventer fontCachePurgePreventer;
-    recursiveUpdateLayoutAttributes(child, m_layoutAttributesBuilder);
+    for (RenderObject* descendant = child; descendant; descendant = descendant->nextInPreOrder(child)) {
+        if (descendant->isSVGInlineText())
+            m_layoutAttributesBuilder.buildLayoutAttributesForTextRenderer(toRenderSVGInlineText(descendant));
+    }
+
     rebuildLayoutAttributes();
 }
 
@@ -162,12 +160,18 @@ static inline bool findPreviousAndNextAttributes(RenderObject* start, RenderSVGI
     return false;
 }
 
-void RenderSVGText::layoutAttributesWillBeDestroyed(RenderSVGInlineText* text, Vector<SVGTextLayoutAttributes*>& affectedAttributes)
+void RenderSVGText::subtreeChildWillBeDestroyed(RenderSVGInlineText* text, Vector<SVGTextLayoutAttributes*>& affectedAttributes)
 {
     ASSERT(text);
+
+    // The positioning elements cache depends on the size of each text renderer in the
+    // subtree. If this changes, clear the cache. It's going to be rebuilt below.
+    invalidateTextPositioningElements();
+
     if (m_needsPositioningValuesUpdate)
         return;
 
+    // This logic requires that the 'text' child is still inserted in the tree.
     bool stopAfterNext = false;
     SVGTextLayoutAttributes* previous = 0;
     SVGTextLayoutAttributes* next = 0;
@@ -176,29 +180,94 @@ void RenderSVGText::layoutAttributesWillBeDestroyed(RenderSVGInlineText* text, V
         affectedAttributes.append(previous);
     if (next)
         affectedAttributes.append(next);
-}
 
-void RenderSVGText::invalidateTextPositioningElements()
-{
-    // Clear the text positioning elements. This should be called when either the children
-    // of a DOM text element have changed, or the length of the text in any child element
-    // has changed. Failure to clear may leave us with invalid elements, as other code paths
-    // do not always cause the position elements to be marked invalid before use.
-    m_layoutAttributesBuilder.clearTextPositioningElements();
+    SVGTextLayoutAttributes* currentLayoutAttributes = text->layoutAttributes();
+
+    size_t position = m_layoutAttributes.find(currentLayoutAttributes);
+    ASSERT(position != notFound);
+    m_layoutAttributes.remove(position);
+
+    ASSERT(!m_layoutAttributes.contains(currentLayoutAttributes));
 }
 
-static inline void recursiveUpdateScaledFont(RenderObject* start)
+static inline void recursiveCollectLayoutAttributes(RenderObject* start, Vector<SVGTextLayoutAttributes*>& attributes)
 {
     for (RenderObject* child = start->firstChild(); child; child = child->nextSibling()) {
         if (child->isSVGInlineText()) {
-            toRenderSVGInlineText(child)->updateScaledFont();
+            attributes.append(toRenderSVGInlineText(child)->layoutAttributes());
             continue;
         }
 
-        recursiveUpdateScaledFont(child);
+        recursiveCollectLayoutAttributes(child, attributes);
+    }
+}
+
+static inline void checkLayoutAttributesConsistency(RenderSVGText* text, Vector<SVGTextLayoutAttributes*>& expectedLayoutAttributes)
+{
+#ifndef NDEBUG
+    Vector<SVGTextLayoutAttributes*> newLayoutAttributes;
+    recursiveCollectLayoutAttributes(text, newLayoutAttributes);
+    ASSERT(newLayoutAttributes == expectedLayoutAttributes);
+#else
+    UNUSED_PARAM(text);
+    UNUSED_PARAM(expectedLayoutAttributes);
+#endif
+}
+
+void RenderSVGText::subtreeChildWasDestroyed(RenderSVGInlineText*, Vector<SVGTextLayoutAttributes*>& affectedAttributes)
+{
+    if (documentBeingDestroyed() || affectedAttributes.isEmpty())
+        return;
+
+    checkLayoutAttributesConsistency(this, m_layoutAttributes);
+
+    size_t size = affectedAttributes.size();
+    for (size_t i = 0; i < size; ++i)
+        m_layoutAttributesBuilder.rebuildMetricsForTextRenderer(affectedAttributes[i]->context());
+}
+
+void RenderSVGText::subtreeStyleChanged(RenderSVGInlineText* text)
+{
+    ASSERT(text);
+    if (m_needsPositioningValuesUpdate)
+        return;
+
+    // Only update the metrics cache, but not the text positioning element cache
+    // nor the layout attributes cached in the leaf #text renderers.
+    FontCachePurgePreventer fontCachePurgePreventer;
+    for (RenderObject* descendant = text; descendant; descendant = descendant->nextInPreOrder(text)) {
+        if (descendant->isSVGInlineText())
+            m_layoutAttributesBuilder.rebuildMetricsForTextRenderer(toRenderSVGInlineText(descendant));
+    }
+}
+
+void RenderSVGText::subtreeTextChanged(RenderSVGInlineText* text)
+{
+    ASSERT(text);
+
+    // The positioning elements cache depends on the size of each text renderer in the
+    // subtree. If this changes, clear the cache. It's going to be rebuilt below.
+    invalidateTextPositioningElements();
+
+    if (m_needsPositioningValuesUpdate)
+        return;
+
+    FontCachePurgePreventer fontCachePurgePreventer;
+    for (RenderObject* descendant = text; descendant; descendant = descendant->nextInPreOrder(text)) {
+        if (descendant->isSVGInlineText())
+            m_layoutAttributesBuilder.buildLayoutAttributesForTextRenderer(toRenderSVGInlineText(descendant));
     }
 }
 
+void RenderSVGText::invalidateTextPositioningElements()
+{
+    // Clear the text positioning elements. This should be called when either the children
+    // of a DOM text element have changed, or the length of the text in any child element
+    // has changed. Failure to clear may leave us with invalid elements, as other code paths
+    // do not always cause the position elements to be marked invalid before use.
+    m_layoutAttributesBuilder.clearTextPositioningElements();
+}
+
 void RenderSVGText::layout()
 {
     ASSERT(needsLayout());
@@ -215,8 +284,12 @@ void RenderSVGText::layout()
     // If the root layout size changed (eg. window size changes) or the positioning values change
     // or the transform to the root context has changed then recompute the on-screen font size.
     if (m_needsTextMetricsUpdate || SVGRenderSupport::findTreeRootObject(this)->isLayoutSizeChanged()) {
-        recursiveUpdateScaledFont(this);
-        rebuildLayoutAttributes(true);
+        for (RenderObject* descendant = this; descendant; descendant = descendant->nextInPreOrder(this)) {
+            if (descendant->isSVGInlineText())
+                toRenderSVGInlineText(descendant)->updateScaledFont();
+        }
+
+        rebuildAllLayoutAttributes();
         updateCachedBoundariesInParents = true;
         m_needsTextMetricsUpdate = false;
     }
@@ -364,7 +437,7 @@ FloatRect RenderSVGText::repaintRectInLocalCoordinates() const
 void RenderSVGText::addChild(RenderObject* child, RenderObject* beforeChild)
 {
     RenderSVGBlock::addChild(child, beforeChild);
-    layoutAttributesChanged(child);
+    subtreeChildAdded(child);
 }
 
 // Fix for <rdar://problem/8048875>. We should not render :first-line CSS Style
@@ -380,38 +453,23 @@ void RenderSVGText::updateFirstLetter()
 {
 }
 
-static inline void recursiveCollectLayoutAttributes(RenderObject* start, Vector<SVGTextLayoutAttributes*>& attributes)
+void RenderSVGText::rebuildAllLayoutAttributes()
 {
-    for (RenderObject* child = start->firstChild(); child; child = child->nextSibling()) {
-        if (child->isSVGInlineText()) {
-            attributes.append(toRenderSVGInlineText(child)->layoutAttributes());
-            continue;
-        }
+    m_layoutAttributes.clear();
+    recursiveCollectLayoutAttributes(this, m_layoutAttributes);
+    if (m_layoutAttributes.isEmpty())
+        return;
 
-        recursiveCollectLayoutAttributes(child, attributes);
-    }
+    m_layoutAttributesBuilder.rebuildMetricsForWholeTree(this);
 }
 
-void RenderSVGText::rebuildLayoutAttributes(bool performFullRebuild)
+void RenderSVGText::rebuildLayoutAttributes()
 {
-    if (performFullRebuild)
-        m_layoutAttributes.clear();
-
     if (m_layoutAttributes.isEmpty()) {
-        recursiveCollectLayoutAttributes(this, m_layoutAttributes);
-        if (m_layoutAttributes.isEmpty() || !performFullRebuild)
-            return;
-
-        m_layoutAttributesBuilder.rebuildMetricsForWholeTree(this);
+        rebuildAllLayoutAttributes();
         return;
     }
 
-    Vector<SVGTextLayoutAttributes*> affectedAttributes;
-    rebuildLayoutAttributes(affectedAttributes);
-}
-
-void RenderSVGText::rebuildLayoutAttributes(Vector<SVGTextLayoutAttributes*>& affectedAttributes)
-{
     // Detect changes in layout attributes and only measure those text parts that have changed!
     Vector<SVGTextLayoutAttributes*> newLayoutAttributes;
     recursiveCollectLayoutAttributes(this, newLayoutAttributes);
@@ -420,7 +478,7 @@ void RenderSVGText::rebuildLayoutAttributes(Vector<SVGTextLayoutAttributes*>& af
         return;
     }
 
-    // Compare m_layoutAttributes with newLayoutAttributes to figure out which attributes got added/removed.
+    // Compare m_layoutAttributes with newLayoutAttributes to figure out which attributes got added.
     size_t size = newLayoutAttributes.size();
     for (size_t i = 0; i < size; ++i) {
         SVGTextLayoutAttributes* attributes = newLayoutAttributes[i];
@@ -428,10 +486,6 @@ void RenderSVGText::rebuildLayoutAttributes(Vector<SVGTextLayoutAttributes*>& af
             m_layoutAttributesBuilder.rebuildMetricsForTextRenderer(attributes->context());
     }
 
-    size = affectedAttributes.size();
-    for (size_t i = 0; i < size; ++i)
-        m_layoutAttributesBuilder.rebuildMetricsForTextRenderer(affectedAttributes[i]->context());
-
     m_layoutAttributes = newLayoutAttributes;
 }
 
index 5665b44..23a0b6a 100644 (file)
@@ -48,18 +48,18 @@ public:
     static const RenderSVGText* locateRenderSVGTextAncestor(const RenderObject*);
 
     bool needsReordering() const { return m_needsReordering; }
+    Vector<SVGTextLayoutAttributes*>& layoutAttributes() { return m_layoutAttributes; }
+
+    void subtreeChildAdded(RenderObject*);
+    void subtreeChildWillBeDestroyed(RenderSVGInlineText*, Vector<SVGTextLayoutAttributes*>& affectedAttributes);
+    void subtreeChildWasDestroyed(RenderSVGInlineText*, Vector<SVGTextLayoutAttributes*>& affectedAttributes);
+    void subtreeStyleChanged(RenderSVGInlineText*);
+    void subtreeTextChanged(RenderSVGInlineText*);
 
     // Call this method when either the children of a DOM text element have changed, or the length of
     // the text in any child element has changed.
     void invalidateTextPositioningElements();
 
-    void layoutAttributesChanged(RenderObject*);
-    void layoutAttributesWillBeDestroyed(RenderSVGInlineText*, Vector<SVGTextLayoutAttributes*>& affectedAttributes);
-    void rebuildLayoutAttributes(bool performFullRebuild = false);
-    void rebuildLayoutAttributes(Vector<SVGTextLayoutAttributes*>& affectedAttributes);
-
-    Vector<SVGTextLayoutAttributes*>& layoutAttributes() { return m_layoutAttributes; }
-
 private:
     virtual const char* renderName() const { return "RenderSVGText"; }
     virtual bool isSVGText() const { return true; }
@@ -91,6 +91,9 @@ private:
     virtual RenderBlock* firstLineBlock() const;
     virtual void updateFirstLetter();
 
+    void rebuildAllLayoutAttributes();
+    void rebuildLayoutAttributes();
+
     bool m_needsReordering : 1;
     bool m_needsPositioningValuesUpdate : 1;
     bool m_needsTransformUpdate : 1;
index 57146c9..34e93a0 100644 (file)
@@ -73,7 +73,6 @@ void SVGRootInlineBox::computePerCharacterLayoutInformation()
     RenderSVGText* textRoot = toRenderSVGText(block());
     ASSERT(textRoot);
 
-    textRoot->rebuildLayoutAttributes();
     Vector<SVGTextLayoutAttributes*>& layoutAttributes = textRoot->layoutAttributes();
     if (layoutAttributes.isEmpty())
         return;
index 02a2123..067bf6b 100644 (file)
@@ -26,9 +26,6 @@
 #include "RenderSVGText.h"
 #include "SVGTextPositioningElement.h"
 
-// Set to a value > 0 to dump the text layout attributes
-#define DUMP_TEXT_LAYOUT_ATTRIBUTES 0
-
 namespace WebCore {
 
 SVGTextLayoutAttributesBuilder::SVGTextLayoutAttributesBuilder()
@@ -173,11 +170,6 @@ void SVGTextLayoutAttributesBuilder::buildLayoutAttributes(RenderSVGText* textRo
     unsigned size = m_textPositions.size();
     for (unsigned i = 0; i < size; ++i)
         fillCharacterDataMap(m_textPositions[i]);
-
-#if DUMP_TEXT_LAYOUT_ATTRIBUTES > 0
-    fprintf(stderr, "\nDumping ALL layout attributes for RenderSVGText, renderer=%p, node=%p (m_textLength: %i)\n", textRoot, textRoot->node(), m_textLength);
-    m_characterDataMap.dump();
-#endif
 }
 
 static inline void updateCharacterData(unsigned i, float& lastRotation, SVGCharacterData& data, const SVGLengthContext& lengthContext, const SVGLengthList* xList, const SVGLengthList* yList, const SVGLengthList* dxList, const SVGLengthList* dyList, const SVGNumberList* rotateList)