[Webkit-unassigned] [Bug 59136] Null deref when no use element exists for SVG element instance

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=59136


Justin Schuh <jschuh at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jschuh at chromium.org




--- Comment #7 from Justin Schuh <jschuh at chromium.org>  2011-04-22 12:10:02 PST ---
(In reply to comment #6)
> (From update of attachment 90628 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=90628&action=review
> 
> Sorry Cris, I fear that involves a bit more work:
> 
> > Source/WebCore/svg/SVGAnimationElement.cpp:347
> > +            useElement->setNeedsStyleRecalc();
> 
> I'm confused a shadowTreeElement w/o correspondingUseElement, what _exactly_ forces us into such a state?
> What happens if you completely disable the shadowTreeElement modifications, if correspondingUseElement is 0, does the animation still work?

The null correspondingUseElement is the same partially detached state that we check for explicitly in SVGElementInstance::invalidateAllInstancesOfElement. However, animation bypasses that code path and pokes properties directly (I assume to reduce the overhead of constant shadow tree destruction and rebuilding). So, it seems reasonable that animation should have the same null check.


> > LayoutTests/svg/custom/use-null-instanceroot-crash.svg:5
> > +        document.getElementById("use_elem").instanceRoot.correspondingElement = 0;
> 
> Hm, that is needed? Or does it even trigger the bug?? In the code changes, you're checking whether correspondingUseElement is zero, not correspondingElement?
> SVGElementInstance.idl defines: readonly attribute SVGElement correspondingElement;
> so this shouldn't take any affect at all, does it?

Referencing the instanceRoot just gets it created earlier than it would be by startAnimation. That's how we end up with a partially detached instanceRoot. You're correct that dereferencing correspondingElement and making an assignment are unnecessary. Sorry, I noticed that and meant to point it out in my earlier informal review.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.