Bug 55362 – Clarify comment about potential memory leak in SVGImage

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Cosmin Truta]

There is the following comment at

// FIXME: If this SVG ends up loading itself, we might leak the world.
// The comment said that the Cache code does not know about CachedImages
// holding Frames and won't know to break the cycle. But 
[code follows]

I tested SVG images with various kinds of content, running testshell under valgrind, but I could not see any leak.
I believe that comment is no longer applicable. (It was written in 2009.)

[Cosmin Truta]

Created attachment 84040 [details]
Patch

[Robert Longson]

bug 15443 suggests that images loading images has not been implemented yet. If so the comment could be true when it is implemented couldn't it?

[Cosmin Truta]

(In reply to comment #2)
> bug 15443 suggests that images loading images has not been implemented yet. If so the comment could be true when it is implemented couldn't it?

SVG images loading other SVG images, via the <image> tag, is supported, albeit in a limited manner.
You can see in the layout test that I submitted that it works. The layout test is, in fact, meant to be run under valgrind, to show that it does not leak memory.

[Cosmin Truta]

I wonder what happened, the bubbles have been purple for the whole day.

[Cosmin Truta]

Created attachment 84626 [details]
Patch

Forgot to use option --binary in git-diff last time. That explains why the previous patch failed to apply.

[Cosmin Truta]

Instead of removing that comment altogher, I will submit a patch explaining why it cannot currently happen, and when (i.e. at what event in the future) it will happen.

[Cosmin Truta]

Created attachment 84636 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 84636 [details]
Patch

Clearing flags on attachment: 84636

Committed r80371: <http://trac.webkit.org/changeset/80371>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.