DOMWindow leak with <col onload>

VERIFIED FIXED

Status

()

defect
VERIFIED FIXED
13 years ago
12 years ago

People

(Reporter: jruderman, Assigned: bernd_mozilla)

Tracking

(Blocks 1 bug, 4 keywords)

Trunk
PowerPC
macOS
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8.1 +
blocking1.8.0.5 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: regression from 329900)

Attachments

(4 attachments)

(Reporter)

Description

13 years ago
The following causes leak-gauge to report a DOMWindow leak:

<html xmlns="http://www.w3.org/1999/xhtml">
<body><table><col onload="3"/>foo</table></body>
</html>

bclary found this bug; I made a reduced testcase.
(Reporter)

Comment 1

13 years ago
Posted file simple testcase
(Assignee)

Comment 2

13 years ago
this causes the following assert on a slighlty dated trunk:

###!!! ASSERTION: nsGenericElement's event listener manager hash not empty at sh
utdown!: 'sEventListenerManagersHash.entryCount == 0', file d:/moz_src/mozilla/c
ontent/base/src/nsGenericElement.cpp, line 835
(Assignee)

Comment 3

13 years ago
we create two col frames for it but destroy only one

col create 03700DF8
frame: TableCol(col)(0) (03700DF8) style: 03700E5C {}
Wrong parent style context:  style: 03700D4C :-moz-table-column-group {}
should be using:  style: 036FF870 {}

col create 037015F4
VP 036DEFF0 r=0 a=14235,15480 c=14235,15480 cnt=151
 scroll 036DF1D0 r=0 a=14235,15480 c=14235,15480 cnt=152
  canvas 036DF084 r=0 a=14235,UC c=14235,UC cnt=153
   area 036FF7B0 r=0 a=14235,UC c=14235,UC cnt=154
    text 036FF804 r=0 a=14235,UC c=UC,UC cnt=155
    text 036FF804 d=0,0
    block 036FF910 r=0 a=14235,UC c=13995,UC cnt=156
     tblO 036FFA24 r=0 a=13995,UC c=0,UC cnt=157
      tbl 036FFB4C r=0 a=13995,UC c=UC,UC cnt=158
       rowG 03700FB0 r=0 a=UC,UC c=UC,UC cnt=159
        row 037010F8 r=0 a=UC,UC c=UC,UC cnt=160
         cell 03701258 r=0 a=UC,UC c=UC,UC cnt=161
          block 037012B8 r=0 a=UC,UC c=UC,UC cnt=162
           text 03701460 r=0 a=UC,UC c=UC,UC cnt=163
           text 03701460 d=300,285 me=300
          block 037012B8 d=300,300 me=300
         cell 03701258 d=300,300 me=300
        row 037010F8 d=UC,300
       rowG 03700FB0 d=UC,300
       colG 037014D0 r=0 a=UC,UC c=UC,UC cnt=164
        col 037015F4 r=0 a=0,0 c=0,UC cnt=165
        col 037015F4 d=0,0
       colG 037014D0 d=0,0
       rowG 03700FB0 r=2 a=300,UC c=300,UC cnt=166
        row 037010F8 r=2 a=300,UC c=300,UC cnt=167
         cell 03701258 r=2 a=300,UC c=300,UC cnt=168
          block 037012B8 r=2 a=300,UC c=300,UC cnt=169
          block 037012B8 d=300,300 me=300
         cell 03701258 d=300,300 me=300
        row 037010F8 d=300,300
       rowG 03700FB0 d=300,300
       colG 037014D0 r=2 a=300,UC c=300,UC cnt=170
        col 037015F4 r=0 a=0,0 c=0,UC cnt=171
        col 037015F4 d=0,0
       colG 037014D0 d=0,0
      tbl 036FFB4C d=360,360
     tblO 036FFA24 d=360,360
    block 036FF910 d=13995,360
    text 03701688 r=0 a=14235,UC c=UC,UC cnt=172
    text 03701688 d=0,0
   area 036FF7B0 d=14235,600
  canvas 036DF084 d=14235,600
 scroll 036DF1D0 d=14235,15480
VP 036DEFF0 d=14235,15480

col frame destroy 037015F4

###!!! ASSERTION: nsGenericElement's event listener manager hash not empty at sh
utdown!: 'sEventListenerManagersHash.entryCount == 0', file d:/moz_src/mozilla/c
ontent/base/src/nsGenericElement.cpp, line 835

I placed a printf inside nsTableColFrame::nsTableColFrame() and nsTableColFrame::~nsTableColFrame()
(Assignee)

Comment 4

13 years ago
We loose the content based col frame and create a col frame for the cell content later at nsTableFrame::InsertRows
(Assignee)

Comment 5

13 years ago
we loose anonymous colgroups if other anonymous frames exists.
even with this patch and correct removal the assert persists
(Assignee)

Comment 6

13 years ago
Posted file testcase
(Reporter)

Updated

13 years ago
Assignee: events → nobody
Component: DOM: Events → Layout: Tables
QA Contact: ian → layout.tables
(Assignee)

Updated

13 years ago
Blocks: 329900
(Assignee)

Comment 7

13 years ago
Posted patch patchSplinter Review
Assignee: nobody → bernd_mozilla
Status: NEW → ASSIGNED
(Assignee)

Updated

13 years ago
Attachment #216917 - Flags: superreview?(bzbarsky)
Attachment #216917 - Flags: review?(bzbarsky)
(Assignee)

Updated

13 years ago
Blocks: 325434
Comment on attachment 216917 [details] [diff] [review]
patch

r+sr=bzbarsky, I guess.

We really need a better system for this stuff.  :(
Attachment #216917 - Flags: superreview?(bzbarsky)
Attachment #216917 - Flags: superreview+
Attachment #216917 - Flags: review?(bzbarsky)
Attachment #216917 - Flags: review+
(Assignee)

Comment 9

13 years ago
fix checked in into trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Depends on: 333493
Flags: blocking1.8.1?
Flags: blocking1.8.0.5?
Whiteboard: required by 329900
If we take bug 329900 on old branches we definitely don't want to be leaking DOMWindows so we'd need this fix also.
Whiteboard: required by 329900 → regression from 329900
(Assignee)

Comment 11

13 years ago
Boris I would like to get with this together with 329900 on 1.8.1 what do you think?
Comment on attachment 216917 [details] [diff] [review]
patch

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #216917 - Flags: approval1.8.0.5+
Attachment #216917 - Flags: approval-branch-1.8.1+
Flags: blocking1.8.1?
Flags: blocking1.8.1+
Flags: blocking1.8.0.5?
Flags: blocking1.8.0.5+
If we take this on the branches, shouldn't we take bug 333493 too?
Comment on attachment 216917 [details] [diff] [review]
patch

removing branch approvals from this patch in favor of the regression-fix cummulative patch in bug 333493
Attachment #216917 - Flags: approval1.8.0.5-
Attachment #216917 - Flags: approval1.8.0.5+
Attachment #216917 - Flags: approval-branch-1.8.1-
Attachment #216917 - Flags: approval-branch-1.8.1+
(Assignee)

Comment 15

13 years ago
fixed on the 1.8 branch by the cumulative patch in bug 333493
Keywords: fixed1.8.1
(Assignee)

Comment 16

13 years ago
fixed on branch 1.8.0.5
Keywords: fixed1.8.0.5

Comment 17

13 years ago
Verified FIXED using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.5) Gecko/20060622 Firefox/1.5.0.5.

Leak gauge shows no leaked DOM windows (whereas it did in 1.5.0.4).
Status: RESOLVED → VERIFIED
(Reporter)

Comment 18

12 years ago
I checked in both testcases as crashtests.

Bernd, was the testcase in comment 6 intended to be a reftest instead?
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.