Closed Bug 671101 Opened 13 years ago Closed 6 years ago
Shell in framescripts leaks
I still need to figure out how to narrow it down, but I was using const webNavigation = docShell.QueryInterface(Ci.nsIWebNavigation) in a frame script and that causes the current browser-chrome tests to finish with 48 extra non gc'ed docshells (see bug 658738 comment 49).
Found one test that is able to trigger it and leak one extra docshell.. Note that running the full browser-chrome suite adds about 40. To test: apply this patch, recompile browser/base, and run TEST_PATH=browser/base/content/test/inspector/browser_inspector_treePanel_output.js make mochitest-browser-chrome After the test completes, in the output summary, it says: " INFO TEST-START | Shutdown Browser Chrome Test Summary Passed: 12 Failed: 0 Todo: 0 *** End BrowserChrome Test Results *** --DOCSHELL 095B99C8 == 7 --DOCSHELL 095B99C8 == 6 --DOCSHELL 095B99C8 == 5 .... (warnings removed) " 7 docshells with this attached patch, 6 docshells without the patch
This content.js frame script is loaded here: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#1418 and this is all in-process.
Does this mean all frame scripts leak docshells, or only some?
I suspect this is because docshells are not cycle-collected. So for the moment, don't do that, just write an accessor?
reply to Dietrich Ayala (:dietrich) from comment #3) > Does this mean all frame scripts leak docshells, or only some? Only some, when they use a reference to a docshell, and it's also triggered by a number of tests (not all of them) (In reply to Benjamin Smedberg [:bsmedberg] from comment #4) > So for the moment, don't do that, just write an accessor? In the bug where this was first found (bug 668307) we've already done that to land the code: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/content.js#43
(In reply to Felipe Gomes (:felipe) from comment #1) ... > INFO TEST-START | Shutdown > Browser Chrome Test Summary > Passed: 12 > Failed: 0 > Todo: 0 > > *** End BrowserChrome Test Results *** > --DOCSHELL 095B99C8 == 7 > --DOCSHELL 095B99C8 == 6 > --DOCSHELL 095B99C8 == 5 > .... (warnings removed) > " > > 7 docshells with this attached patch, 6 docshells without the patch I can't actually reproduce this. After *** End BrowserChrome Test Results *** I get 5 docshells, with or without the patch.
(In reply to Olli Pettay [:smaug] from comment #6) > I can't actually reproduce this. > > After *** End BrowserChrome Test Results *** I get 5 docshells, with or > without the patch. Did you apply Felipe's patch? Felipe, are you sure your patch triggers this?
(In reply to Dão Gottwald [:dao] from comment #7) > (In reply to Olli Pettay [:smaug] from comment #6) > > I can't actually reproduce this. > > > > After *** End BrowserChrome Test Results *** I get 5 docshells, with or > > without the patch. > > Did you apply Felipe's patch? I did say "with or without the patch."
This still happens, it just doesn't happen on the inspector tests that I pointed out on comment 0. A full mochitest-browser-chrome run adds 50 docshells with this patch. I'm going to try to find another test that causes the problem by itself.
A reminder comment about this bug is being removed in bug 911496, so I'm needinfo'ing myself to remind me to test this again
docShell is used a lot in frame scripts these days, so I imagine it's fair to say it's not leaking anymore, otherwise we'd have seen it.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.