In the debugger mochitests that exercise the debugger UI we are a bit sloppy in the tear down path: some times the tab is removed before the debugging protocol has a chance to wind down properly. This is evident by protocol output from one test that is interspersed with the next: INFO TEST-END | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_pause-resume.js | finished in 992ms TEST-INFO | checking window state DBG-SERVER: Got: { "from": "conn8.tab3", "type": "tabDetached" } DBG-SERVER: Got: { "to": "conn8.context4", "type": "detach" } TEST-START | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_propertyview-01.js DBG-SERVER: Got: { "from": "conn8.context4", "error": "noSuchActor" } DBG-SERVER: Got: { "to": "conn8.tab3", "type": "detach" }

This patch works, but it increases the number of leaking windows for some reason. These are not test-breaking leaks, they can only be observed by test result inspection, like bug 728606.

Fixed the leak in this patch and fixed a few more leaked DOMWindows.

This patch does the job and in addition to that plugs 20-30% of the newly-reported DOMWindow leaks on average, in my local testing. Do you feel like this is enough for this patch or should I keep iterating on it until I have them all fixed?

This version fixes all leaks, including the one in bug 728606. I've changed slightly the semantics of the browser_dbg_clean-exit.js test: instead of abruptly killing the debugger in the middle of a pause by removing the tab, I am now calling the handler of the tabClose event. I verified that this modified test still breaks without the fix in bug 725360.

Comment on attachment 602872 [details] [diff] [review] [in-fx-team] Working patch v2 here we go. This looks better. I was struggling a bit with the aOnDebugging() function passing in the debuggee object. This seems to handle the exit better with the cleanup function. Wondering if it's worth adding a default registerCleanupFunction in head.js to avoid copying that everywhere, but i leave that up to you.

(In reply to Rob Campbell [:rc] (robcee) from comment #5) > Comment on attachment 602872 [details] [diff] [review] > Working patch v2 > > here we go. > > This looks better. I was struggling a bit with the aOnDebugging() function > passing in the debuggee object. This seems to handle the exit better with > the cleanup function. > > Wondering if it's worth adding a default registerCleanupFunction in head.js > to avoid copying that everywhere, but i leave that up to you. The problem is that the cleanup function needs references to a bunch of local variables in order to nullify them, and I thought the call with all those arguments would look silly. I want to revisit this at some point, but I didn't have any better ideas right now.