673806 - test_SVGLengthList-2.xhtml leaks when parentnode is strong (bug 335998)

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

This is a regression, but I don't know whether the problem is in
SVG code or elsewhere.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Attachment: testcase

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Attachment: testcase

Comment 3

[Thomas Ahlblom]

What are the steps to reproduce this leak? Should one use any specific debug tool or extension to detect it?

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Apply the patch for bug bug 335998 and open the testcase.

Comment 5

[Daniel Holbert [:dholbert]]

(In reply to comment #0)
> This is a regression, but I don't know whether the problem is in
> SVG code or elsewhere.

Any idea how recently this regressed, smaug?

If it was recent-ish, I wonder if it may be related to the checkin from bug 669584 - that's the only SVG-length-list change that I remember going by in very-recent history.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

bug 669584 didn't affect to this.

I don't know yet the regression range.

Whatever attributeName does, it somehow causes the leak.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

Hmm, why wasn't this leaking before :/
Anyway, animation controller isn't cycle collectable object, so its
unlink must be called explicitly.

http://hg.mozilla.org/mozilla-central/rev/ed15cc897a16 doesn't seem to call unlink ever, so I guess it has never been called.

Comment 8

[Daniel Holbert [:dholbert]]

Comment on attachment 548132 [details] [diff] [review]
patch

(In reply to comment #7)
> http://hg.mozilla.org/mozilla-central/rev/ed15cc897a16 doesn't seem to call
> unlink ever, so I guess it has never been called.

You're right, though that's not the right cset to be looking at. :)  nsSMILAnimationController wasn't refcounted at that point -- it only became refcounted when we started using the refresh driver, in http://hg.mozilla.org/mozilla-central/rev/c6e4f04278cd (and *that* cset has no unlink calls, as you'd suspected).

Thanks for catching this!

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

http://hg.mozilla.org/mozilla-central/rev/504a1a927d39