1298142 - Make all image decoders anonymous

Status: NEW

(Core :: Graphics: ImageLib, defect)

Description

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

In ImageLib, the status quo has been that decoders were associated with a RasterImage object. Each decoder gets a RasterImage passed to its constructor, which was used to send notifications back to the RasterImage and to insert things into the surface cache with the appropriate ImageKey. Recently, though, IDecodingTasks have been added as a kind of "glue layer" between RasterImage objects and decoders, and they're responsible for handling notifications. Similarly, ISurfaceProviders are a "glue layer" between decoders and the surface cache, and decoders no longer interact with the surface cache at all. Because of these changes, there's no reason for decoders to continue to hold a reference to their associated RasterImage.

There's precedent for this; we already have the notion of "anonymous decoders", which aren't associated with a RasterImage. Originally they were primarily used for utility functions and for unit testing, but as the IDecodingTask / ISurfaceProvider conversion has progressed, we've gotten to the point where almost all decoders are anonymous decoders.

Let's complete the job - only metadata decoders remain non-anonymous. Once we make that change, we can remove and simplify some code, and importantly we've further reduced the coupling between different subsystems in ImageLib.

Comment 1

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

Attachment: (Part 1) - Make MetadataDecodingTask store a reference to its associated image.

Pretty much what it says on the tin.

Comment 2

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

Attachment: (Part 2) - Make all decoders anonymous decoders.

OK, this is the big change. We can delete all the code related to
Decoder::mImage, since nothing interacts with it anymore. All of the decoder
subclasses can be updated to stop taking a RasterImage argument to their
constructors. That enables some simplification in DecoderFactory, too.

Overall, 74 fewer lines of code. Nice.

Comment 3

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

Try job here:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d248c53d14bd

Comment 4

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Comment on attachment 8784962 [details] [diff] [review]
(Part 2) - Make all decoders anonymous decoders.

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

Nice indeed.

Sorry for the late reviews; just back from PTO.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.