Created attachment 618852 [details] [diff] [review] patch As far as I know this is unused. The xpcshell test for it has memory leaks.
This was never used by Gecko core, just extensions. Does xforms still use it?
This blog post suggests not: http://philipp.wagner.name/blog/2011/07/the-future-of-mozilla-xforms/
Also, I should note that the alternative to removing XTF is to patch the cycle collector code that traces through JS objects to handle a case that only XTF uses. This is a fairly hot path, so we'd be slowing down the common case to make XTF not leak.
Why does it matter if XTF leaks?
I used to use it in JS land, but it was never really stable enough and didn't quite meet my needs. +1 for killing this off. I did have someone ask for a copy of my XTF JS-based library a couple months ago...
Rafael and I spent about two days tracking down this leak in the hope that it was related to the incremental GC leak. If we leave this in, someone else is liable to waste more time on it.
Also, it would be nice if we check for leaks in xpcshell tests. Right now they're just ignored. That seems bad to me.
Comment on attachment 618852 [details] [diff] [review] patch XTF is still used by extensions. We could just fix XTF's classinfo. I can do that
Or, at least this should be reviewed by aaronr and surkov and philipp
But still, I think fixing the leak should be trivial. Better to do that first, and remove XTF after telling everyone that it is being removed. XForms isn't the only addon which has used XTF.
OK. Are you going to somehow make it so we don't use a JS object for classinfo, or have the cycle collector traverse through the classinfo?
I was thinking to create a new class for the classinfo and have a weak pointer from classinfo to the element class and vise versa and manually disconnect them from each other when needed.
(In reply to Bill McCloskey (:billm) from comment #6) > Rafael and I spent about two days tracking down this leak in the hope that > it was related to the incremental GC leak. If we leave this in, someone else > is liable to waste more time on it. This also blocks enabling useful assertions that some global objects are not leaked.
smaug, what extensions other than XForms use this? It's being an annoyance as we're changing DOM code... and forcing some things to be virtual that can be non-virtual otherwise.
It was used also by X+V, but that is long gone. And I think now XForms has finally died.
I'm also in favor of removing this.
Yeah, I think we can remove XTF (and XMLEvents). ESR17 will still have XTF for some time.
(In reply to Olli Pettay [:smaug] from comment #15) > And I think now XForms has finally died. Yes, I would agree with you on that. Let's remove XTF if it helps.
Comment on attachment 681485 [details] [diff] [review] WIP Review of attachment 681485 [details] [diff] [review]: ----------------------------------------------------------------- Do we need to add the xpt file to some removed-files file?
ted> no, that doesn't matter ted> we only ship one combined .xpt file
Comment on attachment 681485 [details] [diff] [review] WIP Compiles and runs locally. Will push to try with the XML events removal patch.
Comment on attachment 681485 [details] [diff] [review] WIP r=me, but I believe you can now make nsGenericElement::HasAttribute/GetAttribute/RemoveAttribute non-virtual. Followup patch for that is fine.
FWIW, I pulled add-on ping data for XForms from the past 7 days. Here is the XForms install base for active Firefox instances that were able to submit a ping over that period (note that this data says nothing about whether the add-on is enabled): Nov 14: 2278 Nov 13: 2302 Nov 12: 2224 Nov 11: 1758 Nov 10: 1300 Nov 9 : 1980 Nov 8 : 2338 Nov 7 : 2413
Any information about FF version? 3.6 perhaps?
Here's the breakdown of install by FF version for 20121114: 16.0.2 1656 12.0 118 15.0.1 103 16.0.1 75 14.0.1 56 10.0.10 29 13.0.1 29 11.0 22 15.0 19 10.0.2 16 17.0 16 4.0.1 14 9.0.1 12 10.0 11 7.0.1 11 8.0.1 10 10.0.6 8 16.0 7 10.0.7 6 8.0 5 10.0.1 5 10.0.4 5 13.0 5 6.0.2 4 5.0 4 6.0 4 10.0.5 3 10.0.9 3 10.0.8 3 18.0a2 3 4.0 3 10.0.3 2 5.0.1 2 17.0a2 2 18.0a1 2 19.0a1 1 15.2.1-x64 1 4.0b12 1 6.0.1 1 17.0a1 1
The XForms extension only ever worked up to Firefox 7 IIRC, and already the 4 version was quite crashy. But those latest code changes were never released. The last version published on AMO was supporting up to Firefox 3.6, the last version I released as preview on my homepage was for 4.0. So we can be pretty sure that that install data is meaningless, as the extension will almost certainly be disabled for everything above Firefox 4.0.