It'd be much better if imgTools::DecodeImage worked off-main-thread, especially now that we have DOM interfaces like ImageBitmap that can trigger decoding in workers. I think the best way to do this is to eliminate DecodeImage's dependency on RasterImage and just implement it directly against Decoder/DecodePool. That will require reducing the coupling between Decoder/DecodePool and RasterImage, but I don't think that will actually be a huge amount of work.

Finally, all the prerequisites are in place to implement this. Here's the patch. Mostly pretty straightforward, although the code in imgTools::DecodeSurface() is unfortunately a bit verbose because of the interaction with nsIInputStream. Part 2 will follow with tests.

In the interests of cleaner code, I've added a new part 1 which moves the code that reads from an nsIInputStream into a SourceBuffer directly into the SourceBuffer class. This allows RasterImage and the new method we're adding in part 2 to share the same code.

This version of part 2 is the same except that: - It uses the new code added in part 1 to append from an nsIInputStream to a SourceBuffer. - I moved the new method from imgITools to ImageOps. Placing the new method on imgITools seemed pointless because it's a C++-only method, and putting it there requires an ugly boilerplate do_CreateInstance() call at every callsite.

This part adds reasonably comprehensive tests for DecodeToSurface(). The patch is a bit bigger than it otherwise might be because these are the first gtests in ImageLib. I'd like to add many more in the long term, though, so I've tried to keep things as reusable as possible. Everything should be pretty straightforward - there are shared helper functions and testcases in Common.h, with the testcases being implemented as data URIs. Each of the tests loads one of these data URIs, packages up the resulting nsIInputStream in a runnable, and runs DecodeToSurface() off-main-thread. We then verify some basic properties of the surface, and walk over its data using the IsSolidColor() function to verify that DecodeToSurface() produced the results we expect. In this case, every test but the corrupt one is intended to produce an all-green surface. There's a slightly ugly thing happening here - I needed to force XPCOM to initialize the image module at the beginning to ensure the right initialization order - that's done in the ImageModuleAvailable test, as documented in the comment there. Otherwise I got failures in the Necko code (!!). I don't like this, but it doesn't seem worth resolving as part of this bug.

Minor tweak here to make sure we use the right calling convention for the callback.

I reworked the tests to load the testcase images from a file instead of using data URIs, because MSVC has a 16k limit on string literals. =\ Really, this may be for the best, though. Having the testcase images in separate files is definitely convenient.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3e8dd82cbf50 https://hg.mozilla.org/integration/mozilla-inbound/rev/44dc3228c00b https://hg.mozilla.org/integration/mozilla-inbound/rev/4d7cd400d799

https://hg.mozilla.org/mozilla-central/rev/3e8dd82cbf50 https://hg.mozilla.org/mozilla-central/rev/44dc3228c00b https://hg.mozilla.org/mozilla-central/rev/4d7cd400d799 https://hg.mozilla.org/mozilla-central/rev/ca56c20306df