Created attachment 643037 [details] [diff] [review] Patch I think this should be ok even when MOZILLA_INTERNAL_API is undefined because an embedders need to include Loader.h already.
Attachment #643037 - Flags: review?(bzbarsky)
Why does an embedder need to include Loader.h already?
Err they shouldn't, but it actually doesn't matter, because they won't instantiate the template.
Hmm, ok. Then why not do the same thing with nsNodeInfoManager?
I can, but nsIDocument doesn't actually addref nsNodeInfoManager, instead it holds onto nsNodeInfo which holds the nsNodeInfoManager. I wasn't sure what to do about this, but we can definitely have nsNodeInfoManager be a nsRefPtr.
Comment on attachment 643037 [details] [diff] [review] Patch I see. Could you please fix the totally misleading comment on mNodeInfoManager to say that we hold a ref to it via mNodeInfo but not directly? Also, no need for the #ifdef in nsDocument.cpp: that macro is so totally defined there, it's not even funny. ;) And finally, the NS_RELEASE(mCSSLoader) in ~nsDocument needs to go away, right? r=me with that. Especially that last part.
Attachment #643037 - Flags: review?(bzbarsky) → review+
Target Milestone: --- → mozilla17
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.