Inspired by njn's war on unnecessary #includes, I decided to look at nsCycleCollector.h, which has a bunch of random things mashed together. Here's a complete list of things that use stuff in this file, and what they use: - xpcprivate.h: nsCycleCollectionJSRuntime - nsJSEnvironment.cpp: _suspectedCount, _collect, _forgetSkippable, nsCycleCollectorResults - nsXPCOMInit.cpp: CCThreadingModel, _startup, nsCycleCollectorInit, _shutdownThreads, _shutdown - FragmentOrElement.cpp: _setForgetSkippableCallback, _setBeforeUnlinkCallback nsXPConnect.cpp: _registerJSRuntime, _forgetJSRuntime - only in nsCycleCollector.cpp: nsCycleCollectorLoggerConstructor - nowhere (?!?!): NS_CYCLE_COLLECTOR_LOGGER_CID Files that include nsCC.h but don't need to: /xpcom/glue/nsISupportsImpl.h <-- !!!! /content/base/src/nsDocument.cpp /content/base/src/nsINode.cpp /content/base/src/Element.cpp /dom/base/nsGlobalWindow.cpp /layout/build/nsLayoutStatics.cpp Files that include nsCC.h and should: /xpcom/base/nsCycleCollector.cpp /dom/base/nsJSEnvironment.cpp /content/base/src/FragmentOrElement.cpp /js/xpconnect/src/xpcprivate.h Files that must be picking it up indirectly: nsXPCOMInit.cpp nsXPConnect.cpp nsISupportsImpl.h includes this but never uses it! That's terrible, as that file is included basically everywhere. I propose the following changes: - remove nsCycleCollector.h from the files that don't use it - add it to the files that do use it - split out nsCycleCollectionJSRuntime into its own header, include that in xpcprivate.h - delete the unused NS_CYCLE_COLLECTOR_LOGGER_CID and nsCycleCollectorLoggerConstructor from the header, unless there's some XPCOM magic that might be able to use this? Do you know, Olli, why this is there? Obviously, finer grained splitting could be used, but xpcprivate.h is the only header file that uses anything, so it isn't worth worrying about otherwise.

A similar analysis for nsCycleCollectorUtils.h which only defines NS_IsCycleCollectorThread. Files that include it: nsISupportsImpl.h nsCycleCollector.cpp nsThreadManager.cpp Files that use it: nsISupportsImpl.h (debug+!XPCOM_GLUE_AVOID_NSPR) XPCJSRuntime.cpp (debug) nsXPConnect.cpp nsThreadManager.cpp I'll change it to be included in all the CPP files that use it, and hide it behind an #ifdef in nsISupportsImpl.h.

(nsCycleCollector.cpp also uses nsCycleCollectorUtils.cpp, of course...)

NS_CYCLE_COLLECTOR_LOGGER_CID and nsCycleCollectorLoggerConstructor are used in some kind of XPCOM build thing via a macro, so I will leave them alone.

I cleaned up the mode line and indentation for nsCycleCollector.h and nsCycleCollectorJSRuntime.h while I was here. Note that the splinter review for those is bogus due to bug 829708.

Suspect and Forget were initially defined in nsCycleCollector.h, which is why I think they were included in nsISupportsImpl.h, but bug 386025 moved them to nsXPCOM.h, so this file has only been uselessly included for 6 years. ;) Here is the changeset for that commit: http://hg.mozilla.org/mozilla-central/rev/9119e2dc0aea

Builds locally. T-push to try: https://tbpl.mozilla.org/?tree=Try&rev=fbb215174e22

Comment on attachment 742451 [details] [diff] [review] include more carefully, plus a few misc. fixups Clean Linux64-based T-try run. Pretend the commit message is more like "Clean up usage of nsCycleCollector.h and nsCycleCollectorUtils.h". bsmedberg, if you could review the changes to nsXPComInit.cpp, nsISupportsImpl.h and the Makefile I'd appreciate it. Changing these core XPCOM things always makes me nervous. Comments welcome on anything else, of course.

Comment on attachment 742451 [details] [diff] [review] include more carefully, plus a few misc. fixups nsISupportsImpl.h is a bit odd, but ok because there is #if defined(DEBUG) && !defined(XPCOM_GLUE_AVOID_NSPR)

Thanks for the reviews. https://hg.mozilla.org/integration/mozilla-inbound/rev/b8e61a083340