nsAutoPtr.h has these odd do_QueryObject definitions: http://mxr.mozilla.org/mozilla-central/source/xpcom/base/nsAutoPtr.h#810 They don't really belong in nsAutoPtr.h, and they're the only bits in there that require nsAutoPtr.h to #include nsCOMPtr.h. Let's move them out of there. Steps: 1) Make a new header in xpcom/base/, call it nsQueryObject.h (suggested name). 2) Add nsQueryObject.h to EXPORTS in xpcom/base/moz.build. 3) Move the relevant definitions from nsAutoPtr.h to nsQueryObject.h. 4) Make all the source files that use do_QueryObject #include nsQueryObject.h. We can remove the nsCOMPtr.h include in a different bug, as that's liable to require some fix-and-rebuild cycles.
Seems nice for a first contrib. I'd like to do this.
Great! Let me know if you have questions, either here or on IRC (my IRC nick is froydnj).
Assignee: nobody → denis.volk
Comment on attachment 8525819 [details] [diff] [review] Proposed patch Review of attachment 8525819 [details] [diff] [review]: ----------------------------------------------------------------- This patch looks good, one minor change necessary. ::: xpcom/base/nsQueryObject.h @@ +10,5 @@ > +#include "nsCOMPtr.h" > +#include "nsRefPtr.h" > + > +#include "nsCycleCollectionNoteChild.h" > +#include "mozilla/MemoryReporting.h" These two includes are unnecessary, please remove them.
Attachment #8525819 - Flags: feedback+
Thanks. I removed the unnecessary includes.
Thanks for the quick adjustments! Apologies for the delay in looking at this patch; vacation and US Thanksgiving hit at precisely the wrong times to provide quick feedback for this. In the future, please request review on the patch by following instructions here: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Getting_the_patch_reviewed The patch looks good, so I've pushed it to try to make sure it compiles everywhere: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=eaee2d3b3048
Try's green, marking checkin-needed. Thanks for the patch!
I had to do some minor fixups to get the patch to apply, but apparently I either did it wrong, or something's changed enough since this patch was written so that it's breaking the builds now: https://treeherder.mozilla.org/ui/logviewer.html#?job_id=4442901&repo=mozilla-inbound I had to back it out in https://hg.mozilla.org/integration/mozilla-inbound/rev/da41a9308c2d Can we get an updated patch that applies cleanly to the current tip of the repo?
I spoke to Denis on IRC and I believe he's updating the patch. I'll try to be around next time it lands to fixup any problems that crop up.
You need to log in before you can comment on or make changes to this bug.