[webkit-reviews] review denied: [Bug 83405] REGRESSION(r105057): Infinite loop inside SVGTextLayoutEngine::currentLogicalCharacterMetrics : [Attachment 141980] Patch v4

[bugzilla-daemon at webkit.org]

Rob Buis <rwlbuis at gmail.com> has denied Nikolas Zimmermann
<zimmermann at kde.org>'s request for review:
Bug 83405: REGRESSION(r105057): Infinite loop inside
SVGTextLayoutEngine::currentLogicalCharacterMetrics
https://bugs.webkit.org/show_bug.cgi?id=83405

Attachment 141980: Patch v4
https://bugs.webkit.org/attachment.cgi?id=141980&action=review

------- Additional Comments from Rob Buis <rwlbuis at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=141980&action=review


Tests look good. Some minor points to fix.

> Source/WebCore/ChangeLog:10
> +	   the managment of all caches (text positioning element cache /
metrics map / layout attributes) in

Typo: management.

> Source/WebCore/rendering/svg/RenderSVGBlock.h:38
> +    virtual void willBeDestroyed();

Better use OVERRIDE.

> Source/WebCore/rendering/svg/RenderSVGText.cpp:203
> +#ifndef  NDEBUG

Two spaces.

> Source/WebCore/rendering/svg/RenderSVGText.cpp:204
> +    // Verify that m_layoutAttributes only differs by maxium one entry.

Typo: maximum

> Source/WebCore/rendering/svg/RenderSVGText.cpp:214
> +#ifndef NDEBUG

Could the method be debug-only?

> Source/WebCore/rendering/svg/RenderSVGText.cpp:293
> +	  
ASSERT(!m_layoutAttributesBuilder.numberOfTextPositioningElements());

We are doing this a lot, a helper method possible?

> Source/WebCore/rendering/svg/RenderSVGText.cpp:376
> +	   // If the root layout size changed (eg. window size changes) or or
the transform to the root

or or? :)