Closed Bug 1282275 Opened 3 years ago Closed 3 years ago

Return IDecodingTask objects instead of Decoder objects from DecoderFactory functions

Categories

(Core :: ImageLib, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: seth, Assigned: seth)

References

Details

Attachments

(1 file, 1 obsolete file)

Since most callers are going to really need IDecodingTasks and not Decoders, and also because IDecodingTasks are going to require some configuration in the future, it's better to have DecoderFactory return IDecodingTasks directly.

Because callers still need access to the underlying Decoder for some purposes, we have to add a GetDecoder() method to IDecodingTask to do this. It's worth noting that upcoming patches will make GetDecoder() unnecessary for all callsites in the browser itself, because of other architectural changes. We'll still need it for GTests, though, so I anticipate it sticking around.
OK, here's the patch.

When I actually implemented this I noticed that if I change the
CreateAnonymousDecoder() / CreateAnonymousMetadataDecoder() functions, I end up
having to change a *lot* of GTests. This is because there are a huge number of
SurfacePipe tests and SurfacePipes currently need Decoder objects - not because
they do any decoding, but because currently it is the Decoder object which owns
the surfaces that the SurfacePipe writes to. Within the next couple of days I'm
going to eliminate this dependency, and I'll have to touch all of those tests
*again* if I make this change to those two DecoderFactory functions now. So I
think the best bet is to hold off for those two cases, in the interest of
reducing code churn and the burden on reviewers. (I'm only thinking of you! =)
Attachment #8765215 - Flags: review?(dholbert)
I've rebased this against the updated version of IDecodingTask; all that has
changed is that raw pointers have been replaced with NotNull.
Attachment #8765301 - Flags: review?(dholbert)
Attachment #8765215 - Attachment is obsolete: true
Attachment #8765215 - Flags: review?(dholbert)
Comment on attachment 8765301 [details] [diff] [review]
Return IDecodingTask objects instead of Decoder objects from most DecoderFactory functions.

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

r=me
Attachment #8765301 - Flags: review?(dholbert) → review+
Pushed by mfowler@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/626bc70dd8f9
Return IDecodingTask objects instead of Decoder objects from most DecoderFactory functions. r=dholbert
https://hg.mozilla.org/mozilla-central/rev/626bc70dd8f9
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.