See <http://mxr.mozilla.org/mozilla-central/source/content/xul/content/src/nsXULElement.h#343>. I'm not sure if it's GC-safe to remove the ScriptObjectHolder struct entirely. Mark, feel free to take if you feel like it, else I can have a look myself.
Can't hurt to try ....
Created attachment 614678 [details] [diff] [review] Patch (v1) very rough Ok,, I took a great big ax and came up with this ... I haven't killed the whole ScriptObjectHolder structure ... just lost the mLangID var ... Also, I didn't do a lot of analysis, just made sure it built and mochitest-plain ran it locally. I haven't TRY tested it yet, as I wanted a pair of eyes to reality-review the while thing before I went off too far ... there may yet be room for improvements / enhancements. --mark
Ms2ger, jst, how much should we be concerned about breaking PyXPCOM with these kinds of changes? I'm not opposed to doing it if that's necessary to really improve the code, but I don't know if accidentally breaking it just to clean out some weirdness is necessarily good. I looked over the PyXPCOM code before and I think it was using at least nsScriptObjectHolder. Of course, it seems to only be updated to 10, and isn't using the new templated stuff anyways...
At this point, not at all. Ant using PyXPCOM to access XPCOM objects in Mozilla is very different from executing python code in a script *element* in Mozilla. IOW, AFAIK the script type stuff doesn't matter much, if at all, to PyXPCOM.
Comment on attachment 614678 [details] [diff] [review] Patch (v1) very rough Removing feedback for now ... Bug 742837 - Remove dead code and cleanup around nsDOMScriptObjectFactory changes have to be absorbed before I can repost
Created attachment 615174 [details] [diff] [review] Rebased patch Rebased over my changes
This looks good. Will you run with it from here? Make sure it tests ... get it reviewed and pushed, etc?
Either way works for me; let me know?
Re-built and tested locally ... going for TRY run...
Autoland Patchset: Patches: 615174 Branch: mozilla-central => try Destination: http://hg.mozilla.org/try/pushloghtml?changeset=f23ca2aed382 Try run started, revision f23ca2aed382. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=f23ca2aed382
Try run for f23ca2aed382 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=f23ca2aed382 Results (out of 15 total builds): success: 15 Builds (or logs if builds failed) available at: http://email@example.com
Comment on attachment 615174 [details] [diff] [review] Rebased patch TRY Builds all good ... requesting review from JST...
Created attachment 615625 [details] [diff] [review] Rebased Patch + JS MACRO Nit Last patch plus requested JS MACRO change, Rebased against nightly, rebuilt locally, all mochitest-plain run.
Comment on attachment 615625 [details] [diff] [review] Rebased Patch + JS MACRO Nit Looks good!
This is just a refactoring that is not fixing a regression or whatnot. I think it can just land on birch.
mccr8: well, a similar previous patch caused a perf regression so I don't know how low risk it is Good enough reason to wait IMO.
Landed by itself in case of any perf regressions. https://hg.mozilla.org/integration/mozilla-inbound/rev/08008f60f4e5
5 years ago