Closed
Bug 671101
Opened 13 years ago
Closed 6 years ago
docShell in framescripts leaks
Categories
(Core :: DOM: Navigation, defect)
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: Felipe, Unassigned)
References
(Blocks 1 open bug)
Details
(Whiteboard: [e10s])
Attachments
(1 file)
675 bytes,
patch
|
Details | Diff | Splinter Review |
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).
Reporter | ||
Comment 1•13 years ago
|
||
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
Reporter | ||
Comment 2•13 years ago
|
||
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.
Comment 3•12 years ago
|
||
Does this mean all frame scripts leak docshells, or only some?
Comment 4•12 years ago
|
||
I suspect this is because docshells are not cycle-collected. So for the moment, don't do that, just write an accessor?
Reporter | ||
Comment 5•12 years ago
|
||
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
Updated•12 years ago
|
Assignee: nobody → Olli.Pettay
Comment 6•12 years ago
|
||
(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.
Updated•12 years ago
|
Assignee: Olli.Pettay → nobody
Comment 7•12 years ago
|
||
(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?
Comment 8•12 years ago
|
||
(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."
Reporter | ||
Comment 9•12 years ago
|
||
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.
Reporter | ||
Comment 10•10 years ago
|
||
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
Flags: needinfo?(felipc)
Reporter | ||
Comment 11•6 years ago
|
||
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
Flags: needinfo?(felipc)
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•