Closed
Bug 399289
Opened 17 years ago
Closed 17 years ago
Leak nsBaseURLParser and more with SVG fill attributes
Categories
(Core :: SVG, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jruderman, Assigned: tor)
References
Details
(Keywords: memory-leak, testcase)
Attachments
(3 files, 4 obsolete files)
|
89 bytes,
image/svg+xml
|
Details | |
|
5.51 KB,
patch
|
dbaron
:
review+
dbaron
:
superreview+
dbaron
:
approval1.9+
|
Details | Diff | Splinter Review |
|
5.66 KB,
patch
|
Details | Diff | Splinter Review |
testcase
Steps to reproduce:
1. Run a debug build of Firefox with XPCOM_MEM_LEAK_LOG=2
2. Load the testcase.
3. Quit.
Result: trace-refcnt reports that one of each of the following has leaked:
nsBaseURLParser, nsStandardURL, nsStringBuffer.
Attachment #284319 -
Flags: review?(dbaron)
Comment on attachment 284319 [details] [diff] [review]
maintain refcnt
>Index: layout/style/nsRuleNode.cpp
>+ aResult.~nsStyleSVGPaint();
Add
new (&aResult) nsStyleSVGPaint();
after this. (You may need to #include NEW_H as well.)
>Index: layout/style/nsStyleStruct.cpp
>+ this->~nsStyleSVGPaint();
and
new (this) nsStyleSVGPaint();
here (may need a const_cast?).
With that, r=dbaron.
Attachment #284319 -
Flags: review?(dbaron) → review+
|
||
Updated•17 years ago
|
Assignee: nobody → tor
(In reply to comment #2)
> >Index: layout/style/nsStyleStruct.cpp
> >+ this->~nsStyleSVGPaint();
>
> and
>
> new (this) nsStyleSVGPaint();
Should nsStyleContentData::operator= have a similar new?
Attachment #284319 -
Attachment is obsolete: true
(In reply to comment #3)
> Should nsStyleContentData::operator= have a similar new?
Yes (and the compiler should be able to optimize it away there as well).
Attachment #284541 -
Attachment is obsolete: true
Attachment #284557 -
Flags: superreview?(roc)
Attachment #284557 -
Flags: superreview?(roc) → superreview+
Attachment #284557 -
Attachment description: add nsStyleContenetData new → add nsStyleContentData new
Attachment #284557 -
Flags: approval1.9?
Comment on attachment 284557 [details] [diff] [review]
add nsStyleContentData new
a1.9=dbaron
Attachment #284557 -
Flags: approval1.9? → approval1.9+
Checked in.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Backed out - was causing tinderbox test failures.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
What test? Why did it fail?
|
Assignee | |
Comment 11•17 years ago
|
||
Tp and reftests were crashing. This was due to the lack of a constructor for nsStyleSVGPaint, so we would occasionally get unlucky and have the unitialized mType set to eStyleSVGPaintType_Server so we would try destroying random memory.
Attachment #285129 -
Flags: review?(dbaron)
Attachment #285129 -
Flags: superreview+
Attachment #285129 -
Flags: review?(dbaron)
Attachment #285129 -
Flags: review+
Attachment #285129 -
Flags: approval1.9+
|
|
Assignee | |
Comment 12•17 years ago
|
||
Comment on attachment 285129 [details] [diff] [review]
add nsStyleSVGPaint constructor
Causing wrong paint style problems.
Attachment #285129 -
Attachment is obsolete: true
|
|
Assignee | |
Comment 13•17 years ago
|
||
SetSVGPaint was throwing away any value already set by ComputeSVGData's COMPUTE_START_INHERITED. This version passes the mochitest style tests.
Attachment #284557 -
Attachment is obsolete: true
Attachment #285251 -
Flags: review?(dbaron)
Comment on attachment 285251 [details] [diff] [review]
be more careful about when structure is cleared
>+nsStyleSVGPaint::SetType(nsStyleSVGPaintType aType) {
Brace on its own line.
r+sr=dbaron
Attachment #285251 -
Flags: superreview+
Attachment #285251 -
Flags: review?(dbaron)
Attachment #285251 -
Flags: review+
Attachment #285251 -
Flags: approval1.9+
|
|
Assignee | |
Comment 15•17 years ago
|
||
|
|
Assignee | |
Comment 16•17 years ago
|
||
Checked in.
Status: REOPENED → RESOLVED
Closed: 17 years ago → 17 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•