1282670 - Misc #include / forward-declaration cleanup in imagelib

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Daniel Holbert [:dholbert]]

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).

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: part 0 (NOT FOR LANDING): Move imagelib c++ files to SOURCES instead of UNIFIED_SOURCES

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).

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: part 1: add missing #includes and "using" declarations

This patch fixes the build bustage that's caused (revealed) by my previous patch, by adding #includes and "using" declarations as-needed.

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: part 2: Remove unnecessary/redundant string-related #include & "using" declarations in imagelib

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.)

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: part 3: Make several imagelib .cpp files #inclue their corresponding .h file as their very first #include, per Gecko convention. r?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).

Comment 5

[Seth Fowler [:seth] [:s2h]]

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.

Comment 6

[Seth Fowler [:seth] [:s2h]]

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!

Comment 7

[Seth Fowler [:seth] [:s2h]]

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)

Comment 8

[Daniel Holbert [:dholbert]]

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).

Comment 9

[Pulsebot]

Pushed by [e-mail]:
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

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a7d76de3b61b
https://hg.mozilla.org/mozilla-central/rev/02fe9f94bde1
https://hg.mozilla.org/mozilla-central/rev/7339ed5e0379