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

RESOLVED FIXED

Status

()

Core
SVG
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: smaug, Assigned: smaug)

Tracking

({regression})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

118 bytes, patch
Details | Diff | Splinter Review
713 bytes, patch
dholbert
: review+
Details | Diff | Splinter Review
(Assignee)

Description

6 years ago
This is a regression, but I don't know whether the problem is in
SVG code or elsewhere.
(Assignee)

Comment 1

6 years ago
Created attachment 548039 [details]
testcase
(Assignee)

Comment 2

6 years ago
Created attachment 548041 [details] [diff] [review]
testcase
Attachment #548039 - Attachment is obsolete: true

Comment 3

6 years ago
What are the steps to reproduce this leak? Should one use any specific debug tool or extension to detect it?
(Assignee)

Comment 4

6 years ago
Apply the patch for bug bug 335998 and open the testcase.
(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.
(Assignee)

Comment 6

6 years ago
bug 669584 didn't affect to this.

I don't know yet the regression range.

Whatever attributeName does, it somehow causes the leak.
(Assignee)

Comment 7

6 years ago
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.
Assignee: nobody → Olli.Pettay
Attachment #548132 - Flags: review?(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!
Attachment #548132 - Flags: review?(dholbert) → review+
(Assignee)

Updated

6 years ago
Keywords: regressionwindow-wanted
(Assignee)

Comment 9

6 years ago
http://hg.mozilla.org/mozilla-central/rev/504a1a927d39
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
Blocks: 335998
You need to log in before you can comment on or make changes to this bug.