I believe that this is the stumbleupon bug mentioned in bug 516334. Basically, when an image becomes animated, we set mDiscardable = PR_FASE, which, when mDecodeOnDraw is also false, alters the value of StoringSourceData(). This means that, despite having some unprocessed data in the source data buffer, we start writing incoming data (from AddSourceData()) directly to the decoder (since we start believing that we don't have a source data buffer at all), which overall means that a corrupt data stream goes into the decoder. I discovered this just by running a debug build on stumbleupon, because it triggered an assertion. mDiscardable should really be immutable. We should use LockImage() for cases like this. Nonetheless, this does mean we probably still have a bug handling corrupt gifs. I'm going to see if I can get the offsets of the missing chunk of data and reproduce it.

added a patch. flagging joe for review. Also removed a method prototype for EnableDiscarding(), which didn't have any callers or implementations. Must have snuck in somehow.

Ok, it looks like the Tp4 issue was a crash, not a freeze, which makes sense - all sorts of internal state gets mucked up, and we probably end up closing the decoder twice or something. So this is hopefully the only issue at play here.

Comment on attachment 400563 [details] [diff] [review] patch v1 >+ // the source buffer. Since we're going to fix this anyway later, and >+ // since we didn't kill the source data in the old world either, locking Reference a bug# here? Also, are there any assertions about images being locked when they are destroyed? Are we totally safe in just leaving it locked?

(In reply to comment #3) > (From update of attachment 400563 [details] [diff] [review]) > > >+ // the source buffer. Since we're going to fix this anyway later, and > >+ // since we didn't kill the source data in the old world either, locking > > Reference a bug# here? The bug about making animated images discardable is in the paragraph above. Good enough, no? > > Also, are there any assertions about images being locked when they are > destroyed? Are we totally safe in just leaving it locked? No assertions - I believe we're safe. Someday in the future we might want to assert that, but not now. ;-)

pushed this to try, along with the false/true patch, in rev 856728ef02d0.

pushed to mc as 3f9909b818bd. resolving fixed.

At least mention bug 500402 here. That one is titled: "Make animated images properly discardable". But as long as 500402 is not fixed, which may be hard, it would be good to discard the source data for locked images anyway as soon as decoding finishes. (Even if animated images are discardable, there may be other reasons to lock the image).

bug 500402 depends on a couple of things that need doing (like making animation stopping/starting sane), but after that it shouldn't be too hard. I'd rather spend cycles on that than on this (unless this turns out to be the source of bug 516331, which I kind of doubt).