Bug 15141 – GIFImageReader can decode incorrectly at different packet boundaries

• Status: RESOLVED
• Resolution: FIXED
• Component: Images

[Peter Kasting]

WebCore/platform/image-decoders/gif/GIFImageReader.cpp (not used by Safari, but used by Cairo/QT) has bugs relating to how it was borrowed from the Mozilla codebase, which provides data to the decoder differently.

In the Mozilla world, the decoder is called with only the new data seen since the last call, so a hold buffer is used to store not-yet-decoded data that should be prepended to the next incoming chunk.  In the WebKit world, the decoder is provided the entire data stream up to the current point every time.  To fit these two together, GIFImageDecoder.cpp contains some wrapper code that lets the reader call back to say "I've decoded up to this point", and then the wrapper will call the reader with only the data from there the next time.

The problem is that this callback isn't actually used in a couple places where it should be, meaning that depending on where your packet boundaries are, GIFs fail to decode or decode incorrectly.

The true fix to this would be to rewrite the reader and its wrapper to eliminate the hold buffer entirely and be fully aware of the way data is going to be provided.  However, that's a slightly trickier task than what I propose to do in this bug, which is to at least insert the necessary callback calls so that the decoder always gets data from the right point.  Patch coming shortly.

[Peter Kasting]

Created attachment 16198 [details]
patch v1

Contains the aforementioned fix, plus one other tiny change: when the GIF reader fails, return failure rather than success, so we stop decoding (as Mozilla does) instead of trying again and again.

[Maciej Stachowiak]

Comment on attachment 16198 [details]
patch v1

r=me

[Mark Rowe (bdash)]

Landed in r26580.