409674 - No leak logging for DOM windows or documents

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Bill Gianopoulos [:WG9s]]

The checkin for bug 403830 seems to have resulted in bypassing the leak logging for DOM windows and documents.  This means that dbaron's leak gauge now only works for docshell leaks.

Comment 1

[Matt]

Can confirm this bug. Don't know if the bug mentioned by Bill caused it or not. Seems serious as I'd like to contribute to leak testing for v3 using leak-gauge.pl

Comment 2

[Matt]

Also, I actually do believe this could be considered a blocker as it halts a major way of testing development of the browser. For instance to see if any new changes to code cause memory leaks.

Comment 3

[Jesse Ruderman]

You can still use trace-refcnt logging, but that takes significantly more time to get working ;)

Comment 4

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

What's the regression window that led you to bug 403830?  I don't see anything in the patch there that would have done this.

Comment 5

[Bill Gianopoulos [:WG9s]]

(In reply to comment #4)
> What's the regression window that led you to bug 403830?  I don't see anything
> in the patch there that would have done this.
> 
I did a pull by time from the tree specifying 2007-12-15 01:40 -0800.  The resultant build worked fine.  I then updated the tree to 2007-12-15 02:00 -0800 and that build shows the issue.  This is the only check-in between those 2 times.

Comment 6

[Bill Gianopoulos [:WG9s]]

Just to be absolutely sure, I repeated the above process and the regression window is definitely:

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-12-15+01%3A40&maxdate=2007-12-15+02%3A00&cvsroot=%2Fcvsroot

The only 2 checkins are bug 403830 and its subsequent regression fix.

Comment 7

[Bill Gianopoulos [:WG9s]]

(In reply to comment #6)
> The only 2 checkins are bug 403830 and its subsequent regression fix.
                                                        ^^^^^^^^^^
                                                        bustage 

Comment 8

[Steve England [:stevee]]

FWIW as well
Works: 20071214_2343_firefox-3.0b3pre.en-US.win32
Broken: 20071215_0201_firefox-3.0b3pre.en-US.win32
Checkins to module PhoenixTinderbox between 2007-12-14 23:43 and 2007-12-15 02:00 :
http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1197704580&maxdate=1197712859

Comment 9

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Oh, the problem looks like that prlog.h is included in nsNodeInfoManager.h.  It really shouldn't be included in header files since some users need to define FORCE_PR_LOGGING before including it for the first time.  Either that or we need to move the FORCE_PR_LOGGING up to before all the includes -- but that might force on logging done by the nsNodeInfoManager.h header file that we don't want elsewhere.

Comment 10

[Bill Gianopoulos [:WG9s]]

(In reply to comment #9)
> Oh, the problem looks like that prlog.h is included in nsNodeInfoManager.h.  It
That seems to be exactly the issue.

Oddly, just removing the include, seems to resolve the issue, result in no compiler or linker issues and passing all unit tests.

Comment 11

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Want to post the patch, or should I (probably later)?

Comment 12

[Bill Gianopoulos [:WG9s]]

Attachment: patch v1

Comment 13

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 294493 [details] [diff] [review]
patch v1

r+sr=dbaron

Comment 14

[Reed Loden [:reed]]

Comment on attachment 294493 [details] [diff] [review]
patch v1

dbaron, can you grant approval, too?

Comment 15

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 294493 [details] [diff] [review]
patch v1

a=beltzner for 1.9

Comment 16

[Reed Loden [:reed]]

Checking in content/base/src/nsNodeInfoManager.h;
/cvsroot/mozilla/content/base/src/nsNodeInfoManager.h,v  <--  nsNodeInfoManager.h
new revision: 1.32; previous revision: 1.31
done