1. Set nglayout.debug.disable_xul_cache to true. 2. Install 'DOM Fuzz Lite' from https://www.squarefree.com/extensions/domFuzzLite.xpi 3. Run a debug build of Firefox like this: XPCOM_MEM_LEAK_LOG=2 firefox u.html 4. Click the "Quit with leak check" button. Result: during shutdown, it says "Documents: 5" Expected: during shutdown, it should say "Documents: 4"

Something similar happens with <link rel="author"> and <link rel="help">.

Does it just happen with any display:none element?

And I assume the point is that the document doesn't leak _through_ shutdown, so normal leak logging doesn't see it?

> Does it just happen with any display:none element? No. I tried with a display:none div. > And I assume the point is that the document doesn't leak _through_ shutdown, so > normal leak logging doesn't see it? Exactly.

Why does the xul-cache matter here? Also, if this is an easy leak to trigger, should we block on it? How long is the document leaked? Does it leak until shutdown or until you've visited a couple of other pages?

(In reply to comment #6) > Why does the xul-cache matter here? The XUL cache entrains nsDocuments and doesn't listen for memory-pressure events, so it's a distraction when counting nsDocuments. > Also, if this is an easy leak to trigger, should we block on it? > > How long is the document leaked? Does it leak until shutdown or until you've > visited a couple of other pages? I haven't figured this out. If I reload the testcase a few times, I leak more nsDocuments, but there seems to be a limit or something. I could go for blocking. If it really is a leak, it's easy to trigger and therefore bad. If it isn't, we should still fix whatever is breaking my strategy for detecting leak-until-shutdown bugs.

OK, two notes (Jesse, please confirm if you can): 1) The style set is not relevant. It's the _get_ of .style that matters. So just having: meta.style; as the third line of the function still shows the problem. 2) If I comment out the CreateAndDispatchEvent call in nsHTMLMetaElement::UnbindFromTree, the problem also goes away. This would be consistent with <link showing similar issues, since <link> fires such events. Now the thing is... we have no DOMMetaRemoved listeners in our tree.

I added a printf to ~nsDocument to see the URIs of the documents remaining. The difference between the 8 and 9 case is that in the 9 case during shutdown we have two copis of chrome://global/content/bindings/scrollbar.xml around; in the 8 case there is only one. And if I then breakpoint in ~nsDocument and condition it on having that URI, I see them both being destroyed off cycle collection... If I comment out the style get, then I see the nsDocument get destroyed when the async DOM event (which holds a strong ref to the meta element) goes away.

Blocking for now since it's unclear how wide spread this is. Boris, can you dig a bit deeper here? If we learn this is unlikely to cause big problems for normal users I'm not convinced it should block, but until then...

So first note: the only reason we have more than one scrollbar.xml document is that the xul proto cache is disabled..... so each scrollbar gets its own copy.

OK, I think this is invalid. What the extension does is this: function quitWithLeakCheck() { runSoon(a); function a() { dumpln("QA"); closeAllWindows(); runOnTimer(b); dumpln("QAA"); } function b() { dumpln("QB"); mpUntilDone(); runSoon(c); } function c() { dumpln("QC"); bloatStats(d); } function d(objectCounts) { dumpln("QD"); dumpln("Windows: " + objectCounts["nsGlobalWindow"]); dumpln("Documents: " + objectCounts["nsDocument"]); //if (objectCounts["nsGlobalWindow"] > 4) { dumpln("OMGLEAK"); } //if (objectCounts["nsDocument"] > 4) { dumpln("OMGLEAK"); } runSoon(e); } function e() { dumpln("QE"); goQuitApplication(); } } where bloatStats() does an async read of about:bloat, runSoon() just posts a runnable to the main thread, and runOnTimer runs its argument after a time delay. Now the key ingredients of the testcase are: 1) A <meta> element, so that removing it from the DOM posts an nsPLDOMEvent to the event loop which holds a strong ref to the <meta>. 2) A .style get on the <meta>, which ensures that the <meta> is in a cycle with the nsDOMCSSAttributeDeclaration. Now we call that function above. It closes all windows, then sets a timer to run b(). b() does a bunch of cycle collection; as a result the document containing the <meta> is destroyed, which triggers UnbindFromTree on the <meta>, which posts that nsPLDOMEvent. After this, there's a strong ref to the <meta> held by the event loop for the rest of the cycle collections, so the meta, plus anything it references, can't be collected. Then post an event to run c() and go out to the event loop, process the nsPLDOMEvent, but the <meta> is still in a cycle with its style decl, so it's not destroyed yet (in the testcase without .style the <meta> and things it entrains are destroyed here). Now c() is called, which calls bloatStats(), which synchronously (in nsAboutBloat::NewChannel) grabs the stats. There have been no cycle collections since the nsPLDOMEvent ran, so the the <meta> and things it entrains are still alive. So we see an extra xbl binding document as alive; it's destroyed the very next time we cycle collect. Jesse, it sounds like you want to make mpUntilDone take a callback function and go back out to the event loop after each cc it does... That should make this artifact go away. As far as causing problems for users, the only issue here is that we might need an event loop trip between two CCs to really collect everything in a document. Perhaps we should somehow flag things so the <meta> (and <link>) doesn't post the unbind event at all if the document is going away?

Thank you Boris! Given that explanation I don't think this is a blocker as it's really not a true leak, it only causes certain elements in certain cases to take more than one cc to get collected.

(In reply to comment #12) > As far as causing problems for users, the only issue here is that we might need > an event loop trip between two CCs to really collect everything in a document. > Perhaps we should somehow flag things so the <meta> (and <link>) doesn't post > the unbind event at all if the document is going away? I think we should do this.

Is this still an issue?

The issue quoted in comment 14 is, I would think, yes.

Bulk priority change, per :mdaly