Closed Bug 1292632 Opened 3 years ago Closed 3 years ago

Fix a few misc include/using issues in imagelib

Categories

(Core :: ImageLib, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: dholbert, Assigned: dholbert)

Details

Attachments

(4 files, 1 obsolete file)

Filing this bug to fix some include/using related issues that I found while poking around imagelib as part of some reviews.
I noticed an include with comment...
>  // Workaround for flaw in bug 921753 part 2.
...which was included in the initial version of SurfaceCache.h:
https://hg.mozilla.org/mozilla-central/diff/59dd9491503a/image/src/SurfaceCache.cpp#l1.23

I think like that's pointing to the flaw that you called out in bug 921753 comment 16, which ehsan promptly fixed.  So, presumably we don't need this "workaround" for that flaw anymore. :)  (At least, I can build without it.)
Attachment #8778386 - Flags: review?(seth.bugzilla)
IDecodingTask.cpp uses "IntRect" and won't build on its own without this "using" decl.
Attachment #8778388 - Flags: review?(seth.bugzilla)
IDecodingTask.cpp still won't quite build on its own until I add this patch, to fix a latent problem in SourceBuffer.h (which calls std::min without including <algorithm> -- until now).
Comment on attachment 8778386 [details] [diff] [review]
part 1: Remove a stale/unused #include from SurfaceCache.cpp

erm, sorry, attached the wrong patch for "part 1"
Attachment #8778386 - Attachment is obsolete: true
Attachment #8778386 - Flags: review?(seth.bugzilla)
Comment on attachment 8778397 [details] [diff] [review]
part 1: Remove a stale/unused #include from SurfaceCache.cpp

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

Thanks Daniel!
Attachment #8778397 - Flags: review?(seth.bugzilla) → review+
Comment on attachment 8778388 [details] [diff] [review]
part 2: Add missing "using" decl for IntRect in IDecodingTask.cpp

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

Looks good. For the particular case of mozilla::gfx in ImageLib I usually just do "using namespace mozilla::gfx", since usages are so pervasive. But this works too.
Attachment #8778388 - Flags: review?(seth.bugzilla) → review+
Attachment #8778394 - Flags: review?(seth.bugzilla) → review+
(In reply to Daniel Holbert [:dholbert] (mostly OOTO until Aug 9th) from comment #2)
> I think like that's pointing to the flaw that you called out in bug 921753
> comment 16, which ehsan promptly fixed.  So, presumably we don't need this
> "workaround" for that flaw anymore. :)  (At least, I can build without it.)

Yep! I should've fixed it right away so it didn't ossify into a mysterious comment to puzzle later maintainers years down the line. Glad we can remove that.
Pushed by dholbert@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/27c0dd5afde6
part 1: Remove a stale/unused #include from SurfaceCache.cpp. r=seth
https://hg.mozilla.org/integration/mozilla-inbound/rev/461d7103db3a
part 2: Add missing "using" decl for IntRect in IDecodingTask.cpp. r=seth
https://hg.mozilla.org/integration/mozilla-inbound/rev/d289b7a8ad1f
part 3: Include <algorithm> in image/SourceBuffer.h, to provide std::min. r=seth
You need to log in before you can comment on or make changes to this bug.