467008 - [FIX]Pointing to a non-existing binding locally seems to leak documents

Status: VERIFIED FIXED

(Core :: XBL, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, open it locally, I see these leaks with the nspr log tool and leak gauge:
Results of processing log leak1.log :
Leaked document at address 30ea400.
 ... with URI "jar:file:///C:/Documents%20and%20Settings/mw/Bureaublad/moz/firefox20081127/chrome/toolkit.jar!/content/global/bindings/scrollbar.xml".
 ... with URI "chrome://global/content/bindings/scrollbar.xml".
Leaked document at address 30ea800.
 ... with URI "file:///C:/Documents%20and%20Settings/mw/Bureaublad/notexisting".
Leaked content nodes associated with node info manager at address 312a490.
 ... with document URI "file:///C:/Documents%20and%20Settings/mw/Bureaublad/notexistingbindingleak.htm".
Leaked content nodes associated with node info manager at address 3141160.
 ... with document URI "jar:file:///C:/Documents%20and%20Settings/mw/Bureaublad/moz/firefox20081127/chrome/toolkit.jar!/content/global/bindings/scrollbar.xml".
 ... with document URI "chrome://global/content/bindings/scrollbar.xml".
Leaked content nodes associated with node info manager at address 3141a90.
 ... with document URI "file:///C:/Documents%20and%20Settings/mw/Bureaublad/notexisting".

Summary:
Leaked 0 out of 0 DOM Windows
Leaked 2 out of 36 documents
Leaked 0 out of 5 docshells
Leaked content nodes in 3 out of 46 documents

It doesn't seem to leak when opened from a network.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Do I need to enable some pref? 
I get Security Error: Content at file:///home/smaug/mozilla/tests/ may not load data from file:///home/smaug/mozilla/tests/notexisting#x.

Comment 2

[Martijn Wargers (dead)]

Ah, apparently you need to have the security.fileuri.strict_origin_policy pref set to false.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

I hate our XBL code.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 351317 [details] [diff] [review]
Fix

Should we add these things to the cycle collector as well? If so, would be great with a new bug on that.

Comment 5

[Boris Zbarsky [:bzbarsky]]

I think we have an existing bug on that, actually.  The issue here was that we'd stick it in the hashtable too, so we want this patch anyway.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/89dbc542270b.

Do we need a mochitest here?  In that case bug 424484 blocks the test.  Or can leaks be detected in reftest?

Comment 7

[Boris Zbarsky [:bzbarsky]]

Er, I guess for file:// in our default config we can't use reftest here, so we do need mochitest.

Comment 8

[Martijn Wargers (dead)]

Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20081214 Minefield/3.2a1pre

Thanks for fixing this, Boris, very useful to me.