Misc #include / forward-declaration cleanup in imagelib

RESOLVED FIXED in Firefox 50

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

Trunk
mozilla50
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(firefox50 fixed)

Details

Attachments

(4 attachments)

After I reviewed bug 1282275 on an airplane today, I wrote a stack of patches to do some miscellaneous cleanup of #includes & forward declarations in imagelib. My patches layer on top of the patches on bug 1282275 (and bug 1282259).
I'm attaching this "part 0" patch just for reference -- it's what I used to shake out the issues that I address in the next patch (and to validate that my cleanup in subsequent patches isn't overzealous).
Attachment #8765734 - Attachment description: part 0 (NOT FOR LANDING): Move imagelib c++ files to SOURCES → part 0 (NOT FOR LANDING): Move imagelib c++ files to SOURCES instead of UNIFIED_SOURCES
This patch fixes the build bustage that's caused (revealed) by my previous patch, by adding #includes and "using" declarations as-needed.
Attachment #8765736 - Flags: review?(seth)
This part just removes some string-related includes/forward-declares that aren't in fact needed/used in the files where they live.

(I did a quick spot check for each of these, to be sure the types in question aren't used, and I verified that this builds with "part 0", to be sure I'm not accidentally replacing a legitimate #include with a unified-build dependency.)

(My rough approach to find code to fix here was to grep for "string", and look for files that only/mostly had includes & forward-declarations, and did not have any grep matches for the corresponding types.)
Attachment #8765737 - Attachment is patch: true
Attachment #8765737 - Flags: review?(seth)
This reorders the #include ordering of several .cpp files, so that they include their own .h file first.

(or second in some cases, just after a logging header that apparently needs to be included first, to avoid the possibility of some conflict, as hinted at in some nearby code-comments).
Attachment #8765738 - Flags: review?(seth)
Comment on attachment 8765736 [details] [diff] [review]
part 1: add missing #includes and "using" declarations

Review of attachment 8765736 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good! Thanks for fixing this stuff.
Attachment #8765736 - Flags: review?(seth) → review+
Comment on attachment 8765737 [details] [diff] [review]
part 2: Remove unnecessary/redundant string-related #include & "using" declarations in imagelib

Review of attachment 8765737 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!
Attachment #8765737 - Flags: review?(seth) → review+
Comment on attachment 8765738 [details] [diff] [review]
part 3: Make several imagelib .cpp files #inclue their corresponding .h file as their very first #include, per Gecko convention. r?seth

Review of attachment 8765738 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for doing this! It'd be nice to fix the ImageLogging.h thing at some point. We should really just move to the more modern Gecko logging system. (or the gfx-specific one, perhaps even better)
Attachment #8765738 - Flags: review?(seth) → review+
Thanks for the reviews!

I'll land these patches after bug 1282275 has made it into the tree (since "part 2" here would cause some minor bitrot for your DecoderFactory changes in that bug).
Flags: needinfo?(dholbert)
Pushed by dholbert@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a7d76de3b61b
part 1: Add missing #include & "using" declarations in imagelib. r=seth
https://hg.mozilla.org/integration/mozilla-inbound/rev/02fe9f94bde1
part 2: Remove unnecessary/redundant string-related #include & "using" declarations in imagelib. r=seth
https://hg.mozilla.org/integration/mozilla-inbound/rev/7339ed5e0379
part 3: Make several imagelib .cpp files #inclue their corresponding .h file as their very first #include, per Gecko convention. r=seth
Flags: needinfo?(dholbert)
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.