Changeset 116498 in webkit
- Timestamp:
- May 9, 2012 12:17:09 AM (12 years ago)
- Location:
- trunk
- Files:
-
- 4 added
- 8 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/LayoutTests/ChangeLog
r116491 r116498 1 2012-05-09 Nikolas Zimmermann <nzimmermann@rim.com> 2 3 REGRESSION(r105057): Infinite loop inside SVGTextLayoutEngine::currentLogicalCharacterMetrics 4 https://bugs.webkit.org/show_bug.cgi?id=83405 5 6 Reviewed by Darin Adler. 7 8 Add two new testcases covering the problem. 9 10 * svg/text/add-tspan-position-bug-expected.html: Added. 11 * svg/text/add-tspan-position-bug.html: Added. 12 * svg/text/modify-tspan-position-bug-expected.html: Added. 13 * svg/text/modify-tspan-position-bug.html: Added. 14 1 15 2012-05-08 Kent Tamura <tkent@chromium.org> 2 16 -
trunk/Source/WebCore/ChangeLog
r116497 r116498 1 2012-05-09 Nikolas Zimmermann <nzimmermann@rim.com> 2 3 REGRESSION(r105057): Infinite loop inside SVGTextLayoutEngine::currentLogicalCharacterMetrics 4 https://bugs.webkit.org/show_bug.cgi?id=83405 5 6 Reviewed by Darin Adler. 7 8 Dynamically adding tspans carrying position information in the x/y/dx/dy/rotate lists is broken. 9 To avoid mistakes like this in future, simplify the calling code in RenderSVGInlineText and centralize 10 the managment of all caches (text positioning element cache / metrics map / layout attributes) in 11 RenderSVGText. This avoids the hack in SVGRootInlineBox::computePerCharacterLayoutInformation() which 12 called textRoot->rebuildLayoutAttributes(), which was used to fix previous security issues with this code. 13 Instead correctly handle destruction of RenderSVGInlineText in RenderSVGText, keeping the m_layoutAttributes 14 synchronized with the current state of the render tree. Fixes highcharts problems. 15 16 Tests: svg/text/add-tspan-position-bug.html 17 svg/text/modify-tspan-position-bug.html 18 19 * rendering/svg/RenderSVGInline.cpp: 20 (WebCore::RenderSVGInline::addChild): 21 * rendering/svg/RenderSVGInlineText.cpp: 22 (WebCore::RenderSVGInlineText::willBeDestroyed): 23 (WebCore::RenderSVGInlineText::setTextInternal): 24 (WebCore::RenderSVGInlineText::styleDidChange): 25 * rendering/svg/RenderSVGText.cpp: 26 (WebCore::recursiveUpdateMetrics): 27 (WebCore::RenderSVGText::subtreeChildAdded): 28 (WebCore::RenderSVGText::subtreeChildWillBeDestroyed): 29 (WebCore::recursiveCollectLayoutAttributes): 30 (WebCore::checkLayoutAttributesConsistency): 31 (WebCore::RenderSVGText::subtreeChildWasDestroyed): 32 (WebCore::RenderSVGText::subtreeStyleChanged): 33 (WebCore::RenderSVGText::subtreeTextChanged): 34 (WebCore::RenderSVGText::layout): 35 (WebCore::RenderSVGText::addChild): 36 (WebCore::RenderSVGText::rebuildAllLayoutAttributes): 37 (WebCore::RenderSVGText::rebuildLayoutAttributes): 38 * rendering/svg/RenderSVGText.h: 39 (WebCore::RenderSVGText::layoutAttributes): 40 * rendering/svg/SVGRootInlineBox.cpp: 41 (WebCore::SVGRootInlineBox::computePerCharacterLayoutInformation): 42 * rendering/svg/SVGTextLayoutAttributesBuilder.cpp: 43 (WebCore::SVGTextLayoutAttributesBuilder::buildLayoutAttributes): 44 1 45 2012-05-08 Dongwoo Im <dw.im@samsung.com> 2 46 -
trunk/Source/WebCore/rendering/svg/RenderSVGInline.cpp
r115981 r116498 125 125 RenderInline::addChild(child, beforeChild); 126 126 if (RenderSVGText* textRenderer = RenderSVGText::locateRenderSVGTextAncestor(this)) 127 textRenderer-> layoutAttributesChanged(child);127 textRenderer->subtreeChildAdded(child); 128 128 } 129 129 -
trunk/Source/WebCore/rendering/svg/RenderSVGInlineText.cpp
r115215 r116498 82 82 83 83 Vector<SVGTextLayoutAttributes*> affectedAttributes; 84 textRenderer->layoutAttributesWillBeDestroyed(this, affectedAttributes); 85 84 textRenderer->subtreeChildWillBeDestroyed(this, affectedAttributes); 86 85 RenderText::willBeDestroyed(); 87 if (affectedAttributes.isEmpty()) 88 return; 89 90 if (!documentBeingDestroyed()) 91 textRenderer->rebuildLayoutAttributes(affectedAttributes); 86 textRenderer->subtreeChildWasDestroyed(this, affectedAttributes); 92 87 } 93 88 … … 95 90 { 96 91 RenderText::setTextInternal(text); 97 98 // When the underlying text content changes, call both textDOMChanged() & layoutAttributesChanged() 99 // The former will clear the SVGTextPositioningElement cache, which depends on the textLength() of 100 // the RenderSVGInlineText objects, and thus needs to be rebuild. The latter will assure that the 101 // SVGTextLayoutAttributes associated with the RenderSVGInlineText will be updated. 102 if (RenderSVGText* textRenderer = RenderSVGText::locateRenderSVGTextAncestor(this)) { 103 textRenderer->invalidateTextPositioningElements(); 104 textRenderer->layoutAttributesChanged(this); 105 } 92 if (RenderSVGText* textRenderer = RenderSVGText::locateRenderSVGTextAncestor(this)) 93 textRenderer->subtreeTextChanged(this); 106 94 } 107 95 … … 128 116 // The text metrics may be influenced by style changes. 129 117 if (RenderSVGText* textRenderer = RenderSVGText::locateRenderSVGTextAncestor(this)) 130 textRenderer-> layoutAttributesChanged(this);118 textRenderer->subtreeStyleChanged(this); 131 119 } 132 120 -
trunk/Source/WebCore/rendering/svg/RenderSVGText.cpp
r115981 r116498 111 111 } 112 112 113 static inline void recursiveUpdateLayoutAttributes(RenderObject* start, SVGTextLayoutAttributesBuilder& builder) 114 { 115 if (start->isSVGInlineText()) { 116 builder.buildLayoutAttributesForTextRenderer(toRenderSVGInlineText(start)); 117 return; 118 } 119 120 for (RenderObject* child = start->firstChild(); child; child = child->nextSibling()) 121 recursiveUpdateLayoutAttributes(child, builder); 122 } 123 124 void RenderSVGText::layoutAttributesChanged(RenderObject* child) 113 void RenderSVGText::subtreeChildAdded(RenderObject* child) 125 114 { 126 115 ASSERT(child); 127 116 if (m_needsPositioningValuesUpdate) 128 117 return; 118 119 // The positioning elements cache doesn't include the new 'child' yet. Clear the 120 // cache, as the next buildLayoutAttributesForTextRenderer() call rebuilds it. 121 invalidateTextPositioningElements(); 122 129 123 FontCachePurgePreventer fontCachePurgePreventer; 130 recursiveUpdateLayoutAttributes(child, m_layoutAttributesBuilder); 124 for (RenderObject* descendant = child; descendant; descendant = descendant->nextInPreOrder(child)) { 125 if (descendant->isSVGInlineText()) 126 m_layoutAttributesBuilder.buildLayoutAttributesForTextRenderer(toRenderSVGInlineText(descendant)); 127 } 128 131 129 rebuildLayoutAttributes(); 132 130 } … … 163 161 } 164 162 165 void RenderSVGText:: layoutAttributesWillBeDestroyed(RenderSVGInlineText* text, Vector<SVGTextLayoutAttributes*>& affectedAttributes)163 void RenderSVGText::subtreeChildWillBeDestroyed(RenderSVGInlineText* text, Vector<SVGTextLayoutAttributes*>& affectedAttributes) 166 164 { 167 165 ASSERT(text); 166 167 // The positioning elements cache depends on the size of each text renderer in the 168 // subtree. If this changes, clear the cache. It's going to be rebuilt below. 169 invalidateTextPositioningElements(); 170 168 171 if (m_needsPositioningValuesUpdate) 169 172 return; 170 173 174 // This logic requires that the 'text' child is still inserted in the tree. 171 175 bool stopAfterNext = false; 172 176 SVGTextLayoutAttributes* previous = 0; … … 177 181 if (next) 178 182 affectedAttributes.append(next); 183 184 SVGTextLayoutAttributes* currentLayoutAttributes = text->layoutAttributes(); 185 186 size_t position = m_layoutAttributes.find(currentLayoutAttributes); 187 ASSERT(position != notFound); 188 m_layoutAttributes.remove(position); 189 190 ASSERT(!m_layoutAttributes.contains(currentLayoutAttributes)); 191 } 192 193 static inline void recursiveCollectLayoutAttributes(RenderObject* start, Vector<SVGTextLayoutAttributes*>& attributes) 194 { 195 for (RenderObject* child = start->firstChild(); child; child = child->nextSibling()) { 196 if (child->isSVGInlineText()) { 197 attributes.append(toRenderSVGInlineText(child)->layoutAttributes()); 198 continue; 199 } 200 201 recursiveCollectLayoutAttributes(child, attributes); 202 } 203 } 204 205 static inline void checkLayoutAttributesConsistency(RenderSVGText* text, Vector<SVGTextLayoutAttributes*>& expectedLayoutAttributes) 206 { 207 #ifndef NDEBUG 208 Vector<SVGTextLayoutAttributes*> newLayoutAttributes; 209 recursiveCollectLayoutAttributes(text, newLayoutAttributes); 210 ASSERT(newLayoutAttributes == expectedLayoutAttributes); 211 #else 212 UNUSED_PARAM(text); 213 UNUSED_PARAM(expectedLayoutAttributes); 214 #endif 215 } 216 217 void RenderSVGText::subtreeChildWasDestroyed(RenderSVGInlineText*, Vector<SVGTextLayoutAttributes*>& affectedAttributes) 218 { 219 if (documentBeingDestroyed() || affectedAttributes.isEmpty()) 220 return; 221 222 checkLayoutAttributesConsistency(this, m_layoutAttributes); 223 224 size_t size = affectedAttributes.size(); 225 for (size_t i = 0; i < size; ++i) 226 m_layoutAttributesBuilder.rebuildMetricsForTextRenderer(affectedAttributes[i]->context()); 227 } 228 229 void RenderSVGText::subtreeStyleChanged(RenderSVGInlineText* text) 230 { 231 ASSERT(text); 232 if (m_needsPositioningValuesUpdate) 233 return; 234 235 // Only update the metrics cache, but not the text positioning element cache 236 // nor the layout attributes cached in the leaf #text renderers. 237 FontCachePurgePreventer fontCachePurgePreventer; 238 for (RenderObject* descendant = text; descendant; descendant = descendant->nextInPreOrder(text)) { 239 if (descendant->isSVGInlineText()) 240 m_layoutAttributesBuilder.rebuildMetricsForTextRenderer(toRenderSVGInlineText(descendant)); 241 } 242 } 243 244 void RenderSVGText::subtreeTextChanged(RenderSVGInlineText* text) 245 { 246 ASSERT(text); 247 248 // The positioning elements cache depends on the size of each text renderer in the 249 // subtree. If this changes, clear the cache. It's going to be rebuilt below. 250 invalidateTextPositioningElements(); 251 252 if (m_needsPositioningValuesUpdate) 253 return; 254 255 FontCachePurgePreventer fontCachePurgePreventer; 256 for (RenderObject* descendant = text; descendant; descendant = descendant->nextInPreOrder(text)) { 257 if (descendant->isSVGInlineText()) 258 m_layoutAttributesBuilder.buildLayoutAttributesForTextRenderer(toRenderSVGInlineText(descendant)); 259 } 179 260 } 180 261 … … 186 267 // do not always cause the position elements to be marked invalid before use. 187 268 m_layoutAttributesBuilder.clearTextPositioningElements(); 188 }189 190 static inline void recursiveUpdateScaledFont(RenderObject* start)191 {192 for (RenderObject* child = start->firstChild(); child; child = child->nextSibling()) {193 if (child->isSVGInlineText()) {194 toRenderSVGInlineText(child)->updateScaledFont();195 continue;196 }197 198 recursiveUpdateScaledFont(child);199 }200 269 } 201 270 … … 216 285 // or the transform to the root context has changed then recompute the on-screen font size. 217 286 if (m_needsTextMetricsUpdate || SVGRenderSupport::findTreeRootObject(this)->isLayoutSizeChanged()) { 218 recursiveUpdateScaledFont(this); 219 rebuildLayoutAttributes(true); 287 for (RenderObject* descendant = this; descendant; descendant = descendant->nextInPreOrder(this)) { 288 if (descendant->isSVGInlineText()) 289 toRenderSVGInlineText(descendant)->updateScaledFont(); 290 } 291 292 rebuildAllLayoutAttributes(); 220 293 updateCachedBoundariesInParents = true; 221 294 m_needsTextMetricsUpdate = false; … … 365 438 { 366 439 RenderSVGBlock::addChild(child, beforeChild); 367 layoutAttributesChanged(child);440 subtreeChildAdded(child); 368 441 } 369 442 … … 381 454 } 382 455 383 static inline void recursiveCollectLayoutAttributes(RenderObject* start, Vector<SVGTextLayoutAttributes*>& attributes) 384 { 385 for (RenderObject* child = start->firstChild(); child; child = child->nextSibling()) { 386 if (child->isSVGInlineText()) { 387 attributes.append(toRenderSVGInlineText(child)->layoutAttributes()); 388 continue; 389 } 390 391 recursiveCollectLayoutAttributes(child, attributes); 392 } 393 } 394 395 void RenderSVGText::rebuildLayoutAttributes(bool performFullRebuild) 396 { 397 if (performFullRebuild) 398 m_layoutAttributes.clear(); 399 456 void RenderSVGText::rebuildAllLayoutAttributes() 457 { 458 m_layoutAttributes.clear(); 459 recursiveCollectLayoutAttributes(this, m_layoutAttributes); 460 if (m_layoutAttributes.isEmpty()) 461 return; 462 463 m_layoutAttributesBuilder.rebuildMetricsForWholeTree(this); 464 } 465 466 void RenderSVGText::rebuildLayoutAttributes() 467 { 400 468 if (m_layoutAttributes.isEmpty()) { 401 recursiveCollectLayoutAttributes(this, m_layoutAttributes); 402 if (m_layoutAttributes.isEmpty() || !performFullRebuild) 403 return; 404 405 m_layoutAttributesBuilder.rebuildMetricsForWholeTree(this); 406 return; 407 } 408 409 Vector<SVGTextLayoutAttributes*> affectedAttributes; 410 rebuildLayoutAttributes(affectedAttributes); 411 } 412 413 void RenderSVGText::rebuildLayoutAttributes(Vector<SVGTextLayoutAttributes*>& affectedAttributes) 414 { 469 rebuildAllLayoutAttributes(); 470 return; 471 } 472 415 473 // Detect changes in layout attributes and only measure those text parts that have changed! 416 474 Vector<SVGTextLayoutAttributes*> newLayoutAttributes; … … 421 479 } 422 480 423 // Compare m_layoutAttributes with newLayoutAttributes to figure out which attributes got added /removed.481 // Compare m_layoutAttributes with newLayoutAttributes to figure out which attributes got added. 424 482 size_t size = newLayoutAttributes.size(); 425 483 for (size_t i = 0; i < size; ++i) { … … 429 487 } 430 488 431 size = affectedAttributes.size();432 for (size_t i = 0; i < size; ++i)433 m_layoutAttributesBuilder.rebuildMetricsForTextRenderer(affectedAttributes[i]->context());434 435 489 m_layoutAttributes = newLayoutAttributes; 436 490 } -
trunk/Source/WebCore/rendering/svg/RenderSVGText.h
r115981 r116498 49 49 50 50 bool needsReordering() const { return m_needsReordering; } 51 Vector<SVGTextLayoutAttributes*>& layoutAttributes() { return m_layoutAttributes; } 52 53 void subtreeChildAdded(RenderObject*); 54 void subtreeChildWillBeDestroyed(RenderSVGInlineText*, Vector<SVGTextLayoutAttributes*>& affectedAttributes); 55 void subtreeChildWasDestroyed(RenderSVGInlineText*, Vector<SVGTextLayoutAttributes*>& affectedAttributes); 56 void subtreeStyleChanged(RenderSVGInlineText*); 57 void subtreeTextChanged(RenderSVGInlineText*); 51 58 52 59 // Call this method when either the children of a DOM text element have changed, or the length of 53 60 // the text in any child element has changed. 54 61 void invalidateTextPositioningElements(); 55 56 void layoutAttributesChanged(RenderObject*);57 void layoutAttributesWillBeDestroyed(RenderSVGInlineText*, Vector<SVGTextLayoutAttributes*>& affectedAttributes);58 void rebuildLayoutAttributes(bool performFullRebuild = false);59 void rebuildLayoutAttributes(Vector<SVGTextLayoutAttributes*>& affectedAttributes);60 61 Vector<SVGTextLayoutAttributes*>& layoutAttributes() { return m_layoutAttributes; }62 62 63 63 private: … … 92 92 virtual void updateFirstLetter(); 93 93 94 void rebuildAllLayoutAttributes(); 95 void rebuildLayoutAttributes(); 96 94 97 bool m_needsReordering : 1; 95 98 bool m_needsPositioningValuesUpdate : 1; -
trunk/Source/WebCore/rendering/svg/SVGRootInlineBox.cpp
r110285 r116498 74 74 ASSERT(textRoot); 75 75 76 textRoot->rebuildLayoutAttributes();77 76 Vector<SVGTextLayoutAttributes*>& layoutAttributes = textRoot->layoutAttributes(); 78 77 if (layoutAttributes.isEmpty()) -
trunk/Source/WebCore/rendering/svg/SVGTextLayoutAttributesBuilder.cpp
r107862 r116498 27 27 #include "SVGTextPositioningElement.h" 28 28 29 // Set to a value > 0 to dump the text layout attributes30 #define DUMP_TEXT_LAYOUT_ATTRIBUTES 031 32 29 namespace WebCore { 33 30 … … 174 171 for (unsigned i = 0; i < size; ++i) 175 172 fillCharacterDataMap(m_textPositions[i]); 176 177 #if DUMP_TEXT_LAYOUT_ATTRIBUTES > 0178 fprintf(stderr, "\nDumping ALL layout attributes for RenderSVGText, renderer=%p, node=%p (m_textLength: %i)\n", textRoot, textRoot->node(), m_textLength);179 m_characterDataMap.dump();180 #endif181 173 } 182 174
Note: See TracChangeset
for help on using the changeset viewer.