MLK: nsBorderEdge out of nsBorderEdges out of nsTableCellFrame::InitCellFrame()

VERIFIED WORKSFORME

Status

()

P3
critical
VERIFIED WORKSFORME
19 years ago
17 years ago

People

(Reporter: bruce, Assigned: karnaze)

Tracking

({memory-leak})

Trunk
Future
memory-leak
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nsbeta3-][rtm-], URL)

Attachments

(1 attachment)

(Reporter)

Description

19 years ago
This leak is present when loading sample #4 in viewer.  To fix it, nsBorderEdges
(as defined at the URL above), needs to have a destructor to release the
nsBorderEdge stuff stuffed into the mBorderEdges->mEdges nsVoidArray.

         MLK: 288 bytes leaked in 12 blocks
         This memory was allocated from:
               malloc         [rtlib.o]
               __bUiLtIn_nEw  [libxpcom.so]
               __builtin_new  [rtlib.o]
               nsTableCellFrame::InitCellFrame(int) [nsTableCellFrame.cpp:176]
                        PRInt32 rowspan =
tableFrame->GetEffectiveRowSpan(*this);
                        PRInt32 i;
                        for (i=0; i<rowspan; i++) {
               =>         nsBorderEdge *borderToAdd = new nsBorderEdge();
                         
mBorderEdges->mEdges[NS_SIDE_LEFT].AppendElement(borderToAdd);
                          borderToAdd = new nsBorderEdge();
                         
mBorderEdges->mEdges[NS_SIDE_RIGHT].AppendElement(borderToAdd);
               nsCellMap::AppendCell(nsTableCellMap&,nsTableCellFrame&,int,int)
[nsCellMap.cpp:716]
              
nsCellMap::ExpandWithRows(nsIPresContext*,nsTableCellMap&,nsVoidArray&,int)
[nsCellMap.cpp:928]
              
nsCellMap::InsertRows(nsIPresContext*,nsTableCellMap&,nsVoidArray&,int,int)
[nsCellMap.cpp:629]
              
nsTableCellMap::InsertRows(nsIPresContext*,nsTableRowGroupFrame&,nsVoidArray&,int,int)
[nsCellMap.cpp:277]
              
nsTableFrame::InsertRows(nsIPresContext&,nsTableRowGroupFrame&,nsVoidArray&,int,int)
[nsTableFrame.cpp:992]
               nsTableFrame::InsertRowGroups(nsIPresContext&,nsIFrame*,int)
[nsTableFrame.cpp:1134]
               nsTableFrame::AppendRowGroups(nsIPresContext&,nsIFrame*)
[nsTableFrame.cpp:1050]
              
nsTableFrame::SetInitialChildList(nsIPresContext*,nsIAtom*,nsIFrame*)
[nsTableFrame.cpp:343]
              
nsCSSFrameConstructor::ConstructTableFrame(nsIPresShell*,nsIPresContext*,nsFrameConstructorState&,nsIContent*,nsIFrame*,nsIStyleContext*,nsIFrame*&,nsTableCreator&)
[nsCSSFrameConstructor.cpp:1506]
              
nsCSSFrameConstructor::ConstructFrameByDisplayType(nsIPresShell*,nsIPresContext*,nsFrameConstructorState&,const
nsStyleDisplay*,nsIContent*,nsIFrame*,nsIStyleContext*,nsFrameItems&)
[nsCSSFrameConstructor.cpp:5621]
              
nsCSSFrameConstructor::ConstructFrame(nsIPresShell*,nsIPresContext*,nsFrameConstructorState&,nsIContent*,nsIFrame*,nsFrameItems&)
[nsCSSFrameConstructor.cpp:6139]
              
nsCSSFrameConstructor::ContentAppended(nsIPresContext*,nsIContent*,int)
[nsCSSFrameConstructor.cpp:6745]
               StyleSetImpl::ContentAppended(nsIPresContext*,nsIContent*,int)
[nsStyleSet.cpp:956]
               PresShell::ContentAppended(nsIDocument*,nsIContent*,int)
[nsPresShell.cpp:2745]
               nsDocument::ContentAppended(nsIContent*,int)
[nsDocument.cpp:1654]
               nsHTMLDocument::ContentAppended(nsIContent*,int)
[nsHTMLDocument.cpp:1204]
               HTMLContentSink::NotifyAppend(nsIContent*,int)
[nsHTMLContentSink.cpp:3980]
               SinkContext::FlushTags(int) [nsHTMLContentSink.cpp:1919]
               SinkContext::DidAddContent(nsIContent*,int)
[nsHTMLContentSink.cpp:1237]
               SinkContext::FlushText(int*,int) [nsHTMLContentSink.cpp:2021]
               SinkContext::FlushTextAndRelease(int*)
[nsHTMLContentSink.cpp:419]
               SinkContext::CloseContainer(const nsIParserNode&)
[nsHTMLContentSink.cpp:1353]
               HTMLContentSink::CloseContainer(const nsIParserNode&)
[nsHTMLContentSink.cpp:2917]
               CNavDTD::CloseContainer(const nsIParserNode*,nsHTMLTag,int)
[CNavDTD.cpp:3010]
               CNavDTD::CloseContainersTo(int,nsHTMLTag,int) [CNavDTD.cpp:3046]
               CNavDTD::CloseContainersTo(nsHTMLTag,int) [CNavDTD.cpp:3216]
         Block of 24 bytes (12 times); last block at 0xc4e308
I don't think we want to add a destructor.  The comment above the struct
definition shows:

  * owner of this struct is responsible for freeing any data stored in mEdges

I'll attach a patch to delete each of the elements in the mBorderEdges->mEdges
array.
Created attachment 6890 [details] [diff] [review]
Add DeleteBorderEdges to properly free nsBorderEdge structures.
(Reporter)

Comment 3

19 years ago
This fixed the leak for me.  I didn't see other places in the tree that needed 
to free this data as well.
karnaze, can you review and/or apply?  Thanks!
(Assignee)

Comment 5

19 years ago
I'm reluctant to spend any time on this because I am planning to rewrite the 
border collapse code. If someone wants to check it in, please do so.
Status: NEW → ASSIGNED
Target Milestone: --- → M16
(Assignee)

Comment 6

19 years ago
Moving to M17.
Target Milestone: M16 → M17
Is that ``M17 for the rewrite'' or ``M17 for checking in the patch here that
fixed the memory leak''?
(Assignee)

Comment 8

19 years ago
For the rewrite. 

Comment 9

19 years ago
Adding donttest keyword so this doesn't show up on the bugathon search page.
Keywords: donttest
(Assignee)

Comment 10

19 years ago
Adding dependency on 41262.
Depends on: 41262

Updated

19 years ago
Keywords: nsbeta3

Updated

19 years ago
Keywords: patch

Comment 11

19 years ago
Tentatively marking beta3- because the fix for 41262 is claimed to make this a 
moot bug. Failure to fix 41262 should cause this bug to be reconsidered.
Whiteboard: [nsbeta3-]

Updated

19 years ago
Target Milestone: M17 → Future

Comment 12

18 years ago
41262 is minused. opening up for re-consideration as we need to nail down leaks.
Whiteboard: [nsbeta3-]
(Reporter)

Comment 13

18 years ago
*** Bug 51595 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 14

18 years ago
Months ago, this patch was created and tested.  A few months later, yet still 
months ago, that it was asked that karnaze be given time to do his thing.

requesting a= to check this patch in.
a=brendan@mozilla.org

Time is money, eh?

/be
(Assignee)

Comment 16

18 years ago
Marking nsbeta3+. Any takers on checking this in?
Whiteboard: [nsbeta3+]

Comment 17

18 years ago
r=buster.  commercial tree closed, shaver, you wanna check this in?  already 
a=brendan, and a=karnaze (module owner)

Comment 18

18 years ago
This has all the requirements for checkin, it should be grandfathered in.
Getting people's attention...
Keywords: approval
(Assignee)

Comment 19

18 years ago
Adding rtm+.
Keywords: rtm
Whiteboard: [nsbeta3+] → [nsbeta3+][rtm+]

Comment 20

18 years ago
Not holding PR3 for this. Marking nsbeta3-
Whiteboard: [nsbeta3+][rtm+] → [nsbeta3-][rtm+]
(Assignee)

Comment 21

18 years ago
Marking rtm-, removing dependency on 41462, and adding dependency on 49490. When 
49490 is fixed this will be fixed.
Depends on: 49490
No longer depends on: 41262
Whiteboard: [nsbeta3-][rtm+] → [nsbeta3-][rtm-]
"When 49490 is fixed this will be fixed."

karnaze: you fixed bug 49490 quite a while ago. There's a patch sitting here - 
is it ready for checkin?

Gerv 
Keywords: mozilla0.9

Comment 23

18 years ago
This patch has all it needs to get checked in (a=, r=), shaver - Just Do It! :)

Comment 24

18 years ago
QA contact update
QA Contact: chrisd → amar
(Assignee)

Comment 25

18 years ago
I'm marking this worksforme, because the code in question has been removed and 
will be replaced at some point.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → WORKSFORME

Comment 26

17 years ago
 Since the code has been removed I am marking it verified.... If you think that 
if this bug has to be reopened please reopen this bug
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.