[webkit-reviews] review granted: [Bug 209773] Hit test with clipPath referencing parent element causes infinite recursion : [Attachment 395768] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Doug Kelly <dougk at apple.com>'s
request for review:
Bug 209773: Hit test with clipPath referencing parent element causes infinite
recursion
https://bugs.webkit.org/show_bug.cgi?id=209773

Attachment 395768: Patch

https://bugs.webkit.org/attachment.cgi?id=395768&action=review




--- Comment #10 from Geoffrey Garen <ggaren at apple.com> ---
Comment on attachment 395768
  --> https://bugs.webkit.org/attachment.cgi?id=395768
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=395768&action=review

r=me

> Source/WebCore/rendering/svg/SVGRenderSupport.h:107
> +    static WeakHashSet<RenderElement>& getVisitedElements();

WebKit style is to name a C++ accessor by the accessed value's name. In this
case, "visitedElements()". (We usually reserve the "get" prefix for something
that returns an out parameter.)

> Source/WebCore/rendering/svg/SVGRenderSupport.h:108
> +    static WeakHashSet<RenderElement>& getVisitedElements();
> +    WeakPtr<RenderElement> m_element;

Upon reflection, I think it would be slightly clearer to make these pointers
RefPtr. We don't actually intend for them to become null or disappear -- we
just want a safe pointer idiom.