Last Comment Bug 376509 - Leak of nested <svg:svg>
: Leak of nested <svg:svg>
Status: RESOLVED FIXED
: fixed1.8.0.12, fixed1.8.1.4, mlk, testcase
Product: Core
Classification: Components
Component: SVG (show other bugs)
: 1.8 Branch
: All All
: -- normal (vote)
: ---
Assigned To: General SVG Bugs
: Hixie (not reading bugmail)
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-04-04 09:50 PDT by tor
Modified: 2007-04-09 17:17 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
remove extra addref (1.03 KB, patch)
2007-04-04 09:50 PDT, tor
jwatt: review+
roc: superreview+
dveditz: approval1.8.1.4+
dveditz: approval1.8.0.12+
Details | Diff | Splinter Review

Description tor 2007-04-04 09:50:08 PDT
Created attachment 260601 [details] [diff] [review]
remove extra addref

The branch has a bug where once you nest <svg:svg> to three levels, we will leak everything past the second level.  Example:

<svg xmlns="http://www.w3.org/2000/svg">
  <svg id="leaking">
     <svg>
     </svg>
  </svg>
</svg>

Everything at the id="leaking" level and below is leaked.

This is happening because the nsSVGInnerSVGFrame::GetCoordContextProvider is QIing its content to a type and raising the refcount before returning it as an already_AddRefed<>.  The manual refcount is not needed because the QI already does that.
Comment 1 tor 2007-04-04 13:37:13 PDT
Comment on attachment 260601 [details] [diff] [review]
remove extra addref

This simple change fixes a large memory leak in SVG files with a deep (>2) nested structure, which is pretty easy to do with machine generated content.

Only affects SVG documents, so low risk from the point of view of what most content the browser deals with.  Change itself is also simple, just removing an extraneous addref.
Comment 2 Daniel Veditz [:dveditz] 2007-04-06 12:12:19 PDT
Comment on attachment 260601 [details] [diff] [review]
remove extra addref

approved for 1.8.0.12 and 1.8.1.4, a=dveditz for release-drivers
Comment 3 tor 2007-04-06 12:50:51 PDT
Checked in on MOZILLA_1_8_BRANCH and MOZILLA_1_8_0_BRANCH.

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