This morning, releng landed split mochitests, so that mochitests are separated into 5 parts. The remaining regression from this is that there's a substantial leak on Windows only, on part 2/5 of the mochitests. It shows up on the old-style mochitests and the debug mochitests. I can reproduce the leak locally by running all of part 2/5 (--total-chunks=5 --this-chunk=2 --chunk-by-dir=4) or just by running with --test-path=dom/tests/mochitest/bugs/
Note that we tried increasing the cycle collector shutdown collections limit from 5 to 10, and it didn't help: http://hg.mozilla.org/mozilla-central/rev/d6add78eff55 http://hg.mozilla.org/mozilla-central/rev/11a42c6d5aa8 http://hg.mozilla.org/mozilla-central/rev/69a4a21d9b84
It would not surprise me if this is related to bug 523885.
I think my first guess at what to take a refcnt log of might have been right: I logged reference counts on the one nsHTMLHtmlElement leaked. There was one outstanding reference, and it looks like it was the one taken in nsIPresShell::SetCapturingContent (code added last month in bug 503943, http://hg.mozilla.org/mozilla-central/rev/eda2433181c9 ). But I'm going to sleep now.
(And the leak as a result of splitting sort of makes sense for this leak, since this would leak until somebody called SetCapturingContent on some other piece of content; in parts 3, 4, or 5, there's probably a test that also does capturing and therefore lets the memory for this test get freed.)
The capturing content is a global and should be cleared when either the mouse is released, the view containing it is destroyed or hidden, or when any window is deactivated. Is nsIPresShell::gCaptureInfo::mContent still set at the end of the test run? There's only one test that calls setCapture directly: toolkit/content/tests/widgets/test_mousecapture.xul But setCapture can be called directly by other elements though, for example: <select>, <splitter>, <frameset>. If it's an nsHTMLHtmlElement element that's leaking though it seems like it might be a selection drag capture.
The capturing content is set to the root element of the *harness* page while running dom/tests/mochitest/bugs/test_bug397571.html and stays that way until the end of the mochitest run (the point of the last call to ~PresShell).
I just landed a *workaround* for this leak: http://hg.mozilla.org/mozilla-central/rev/3e855f1a3f75 though we should still find a real fix for it.
(I don't know whether it's possible to trigger it without nsIDOMWindowUtils.sendMouseEvent, though.)
OK, test_bug397671 runs near the end and no mouse related tests run after it, which is why the capture is still active, as no mouseup ever occurred. The patch here is correct as the test should be firing a mouseup as well. The test_bug397671 runs during inline script which may be occurring before the widget hierarchy is set properly perhaps, thus attempting to get the widget to fire the mousedown event at during nsIDOMWindowUtils.sendMouseEvent retrieves the parent instead.
Is it possible to hit this leak as a result of user action, or can it only result from nsIDOMWindowUtils event synthesis? (What about standard DOM event synthesis? Hopefully we're checking that the events are trusted...)
(In reply to comment #10) > Is it possible to hit this leak as a result of user action, or can it only > result from nsIDOMWindowUtils event synthesis? (What about standard DOM event > synthesis? Hopefully we're checking that the events are trusted...) Would be worth testing but I don't think there's a problem. Real user events should always have the right widget set from the native widget code. Also, the user can release the capture by just letting go of the mouse button ;) Synthetic DOM events shouldn't be a problem either. Mouse capture is only allowed to be enabled during a real mousedown event (enabled in PresShell::HandleEventInternal)
Then I think we should at least add assertions to DOMWindowUtils to make sure that mousedowns are paired with mouseups.
Created attachment 408008 [details] testcase? This shows a problem with our mousedown/up handling. And it also shows that mousedown without mouseup is possible, and that is why nsIDOMWindowUtils shouldn't assert about it.
Note, I'm using linux with the testcase. Not sure what other OSes do.
The capture is also released when a window is deactivated so the testcase shouldn't cause a problem there. It may be that a mouseup should be fired at the document anyway though.
I don't know that I can say that this is really a release blocker, but fixing the leaks in our tests would be a really good thing to do. Marking a blocker for now, to make sure this doesn't get lost... Olli, can you investigate a bit more here?
(In reply to comment #15) > The capture is also released when a window is deactivated Actually this doesn't seem to work. Investigating.
Ok, so seems like the problem with attachment 408008 [details] is that in that case we set the capturing content right *after* dismissing the alert. So deactivating the window and activating alert happens before setting capturing content. And the mouseup is processed before we set the capturing content. A bit tricky case. Need to play a bit with nsEventStateManager::mNormalLMouseEventInProcess.
Created attachment 472471 [details] [diff] [review] patch So before setting capturing content, nsEventStateManager::EventStatusOK is called. If we make sNormalLMouseEventInProcess global, this should fix the problem, at least for this case. I posted the patch to tryserver. And note, if a testcase sends a mousedown, without mouseup, it is "ok" to keep the capturing content, IMHO.
The mochitest leak part of this patch should have been fixed in Bug 528829.
(In reply to comment #20) > The mochitest leak part of this patch should have been fixed in Bug 528829. ...if this bug ...
And still wrong. ...of this bug ...