Bug 15971 - Public GIF image decoder erroneously fails if no new data has come in
Summary: Public GIF image decoder erroneously fails if no new data has come in
Alias: None
Product: WebKit
Classification: Unclassified
Component: Images (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
Depends on:
Reported: 2007-11-13 11:57 PST by Peter Kasting
Modified: 2007-11-19 04:23 PST (History)
0 users

See Also:

patch v1 (1.29 KB, patch)
2007-11-13 14:10 PST, Peter Kasting
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Kasting 2007-11-13 11:57:37 PST
GIFImageReader.cpp (not used by Safari, but used by the Cairo/Qt ports) erroneously marks GIF decoding as having failed if we ask it to decode when no new data has come in since the last request to decode.  Instead, this should just result in doing nothing.

This looks like an artifact of the process of porting from the Mozilla codebase.  I think in Mozilla's codebase, this function is never called unless new data has come in, whereas in WebKit, the wrapper code does not guarantee that.  Therefore, I could fix this by putting a check in the wrapper that doesn't try to decode when there isn't new data.  But I think making the underlying decoder fail in this case is wrong, since there's nothing dangerous about such a call (in fact, if the check in the decoder is removed entirely, we'd run all the way through the function without doing anything, and return successfully).  So instead I'm planning to fix this by just changing the "return false" to "return true" in the beginning of GIFImageReader.cpp:read().

Patch coming in a bit.
Comment 1 Peter Kasting 2007-11-13 14:10:08 PST
Created attachment 17240 [details]
patch v1

Simple fix.
Comment 2 Alp Toker 2007-11-13 22:17:28 PST
I'm curious, how did you notice this? Is there a test case for this issue? A link to some online content will do..

We are getting crashes and corruption with the GIF image loader in the GTK+ port, particularly with animated GIFs, but I don't notice this patch fixing that problem.
Comment 3 Peter Kasting 2007-11-14 23:05:43 PST
I was able to repro this pretty consistently with http://transit.511.org/providers/maps/SF_7122007102947.gif (a 3201x2840 GIF).
Comment 4 Darin Adler 2007-11-16 20:17:27 PST
Comment on attachment 17240 [details]
patch v1

Looks fine, r=me.
Comment 5 Mark Rowe (bdash) 2007-11-19 04:23:04 PST
Landed in r27897.