[webkit-reviews] review granted: [Bug 63290] Stack overflow with enormous SVG filter : [Attachment 238892] Patch

[bugzilla-daemon at webkit.org]

Dean Jackson <dino at apple.com> has granted Said Abou-Hallawa
<sabouhallawa at apple.com>'s request for review:
Bug 63290: Stack overflow with enormous SVG filter
https://bugs.webkit.org/show_bug.cgi?id=63290

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

------- Additional Comments from Dean Jackson <dino at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=238892&action=review


Looks great. Minor comments. Upload a new patch and we'll land this!!

> Source/WebCore/platform/graphics/filters/FilterEffect.cpp:107
> +unsigned FilterEffect::collectEffects(const FilterEffect*effect,
HashSet<const FilterEffect*>& allEffects)

This could be a static function just in this file, not in the class.

> Source/WebCore/platform/graphics/filters/FilterEffect.h:82
> +    static unsigned collectEffects(const FilterEffect*, HashSet<const
FilterEffect*>&);

No need for this if we just declare it in the .cpp.

> LayoutTests/ChangeLog:13
> +	   Test if an SVG filter with deeply nested tree of FilterEffects can
be loaded
> +	   with no crash.  Make sure other valid filters can still be
referenced by SVG 
> +	   drawing elements.
> +
> +	   * svg/filters/svg-deeply-nested-crash-expected.txt: Added.
> +	   * svg/filters/svg-deeply-nested-crash.html: Added.

Now we should have one more test that is the <200 but >100 case.