Fix a few misc include/using issues in imagelib

RESOLVED FIXED in Firefox 51

Status

()

Core
ImageLib
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

Trunk
mozilla51
Points:
---

Firefox Tracking Flags

(firefox51 fixed)

Details

Attachments

(4 attachments, 1 obsolete attachment)

Filing this bug to fix some include/using related issues that I found while poking around imagelib as part of some reviews.
Created attachment 8778383 [details] [diff] [review]
(DO NOT LAND) patch to tweak mozbuild to un-unify two .cpp files

Here's the patch that I used for testing this bug.
Created attachment 8778386 [details] [diff] [review]
part 1: Remove a stale/unused #include from SurfaceCache.cpp

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)
Created attachment 8778388 [details] [diff] [review]
part 2: Add missing "using" decl for IntRect in IDecodingTask.cpp

IDecodingTask.cpp uses "IntRect" and won't build on its own without this "using" decl.
Attachment #8778388 - Flags: review?(seth.bugzilla)
Created attachment 8778394 [details] [diff] [review]
part 3: Include <algorithm> in image/SourceBuffer.h, to provide std::min

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).
Attachment #8778394 - Flags: review?(seth.bugzilla)
Attachment #8778388 - Attachment is patch: true
Attachment #8778394 - Attachment is patch: true
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)
Created attachment 8778397 [details] [diff] [review]
part 1: Remove a stale/unused #include from SurfaceCache.cpp
Attachment #8778397 - 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.

Comment 10

a year ago
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

Comment 11

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/27c0dd5afde6
https://hg.mozilla.org/mozilla-central/rev/461d7103db3a
https://hg.mozilla.org/mozilla-central/rev/d289b7a8ad1f
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox51: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.