Last Comment Bug 673806 - test_SVGLengthList-2.xhtml leaks when parentnode is strong (bug 335998)
: test_SVGLengthList-2.xhtml leaks when parentnode is strong (bug 335998)
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: SVG (show other bugs)
: unspecified
: x86_64 Linux
: -- normal (vote)
: ---
Assigned To: Olli Pettay [:smaug]
:
: Jet Villegas (:jet)
Mentors:
Depends on:
Blocks: strongparent
  Show dependency treegraph
 
Reported: 2011-07-24 13:15 PDT by Olli Pettay [:smaug]
Modified: 2011-07-25 11:15 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (273 bytes, application/xhtml+xml)
2011-07-24 13:45 PDT, Olli Pettay [:smaug]
no flags Details
testcase (118 bytes, patch)
2011-07-24 13:50 PDT, Olli Pettay [:smaug]
no flags Details | Diff | Splinter Review
patch (713 bytes, patch)
2011-07-25 03:00 PDT, Olli Pettay [:smaug]
dholbert: review+
Details | Diff | Splinter Review

Description Olli Pettay [:smaug] 2011-07-24 13:15:38 PDT
This is a regression, but I don't know whether the problem is in
SVG code or elsewhere.
Comment 1 Olli Pettay [:smaug] 2011-07-24 13:45:55 PDT
Created attachment 548039 [details]
testcase
Comment 2 Olli Pettay [:smaug] 2011-07-24 13:50:26 PDT
Created attachment 548041 [details] [diff] [review]
testcase
Comment 3 Thomas Ahlblom 2011-07-24 14:30:20 PDT
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] 2011-07-24 14:36:02 PDT
Apply the patch for bug bug 335998 and open the testcase.
Comment 5 Daniel Holbert [:dholbert] 2011-07-24 15:19:20 PDT
(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] 2011-07-24 15:37:13 PDT
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] 2011-07-25 03:00:45 PDT
Created attachment 548132 [details] [diff] [review]
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] 2011-07-25 10:34:05 PDT
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] 2011-07-25 11:15:14 PDT
http://hg.mozilla.org/mozilla-central/rev/504a1a927d39

Note You need to log in before you can comment on or make changes to this bug.