This is a regression from bug 435296. I'm looking at this callstack: #0 imgContainer::GetWidth (this=0x1679ffb0, aWidth=0xbfffa664) at /Users/bzbarsky/mozilla/css-frameconst/mozilla/modules/libpr0n/src/imgContainer.cpp:347 #1 0x11caa8e3 in nsImageFrame::UpdateIntrinsicSize (this=0x170c0f0, aImage=0x1679ffb0) at /Users/bzbarsky/mozilla/css-frameconst/mozilla/layout/generic/nsImageFrame.cpp:282 #2 0x11cab7bd in nsImageFrame::EnsureIntrinsicSize (this=0x170c0f0, aPresContext=0x12ab600) at /Users/bzbarsky/mozilla/css-frameconst/mozilla/layout/generic/nsImageFrame.cpp:649 #3 0x11caf907 in nsImageFrame::ComputeSize (this=0x170c0f0, aRenderingContext=0x167a0c30, aCBSize=@0xbfffa7c8, aAvailableWidth=66300, aMargin=@0xbfffa7c0, aBorder=@0xbfffa7b8, aPadding=@0xbfffa7b0, aShrinkWrap=1) at /Users/bzbarsky/mozilla/css-frameconst/mozilla/layout/generic/nsImageFrame.cpp:672 Now in frame 2, STATUS_SIZE_AVAILABLE is set, so we assume we can get the size from the imgContainer. but GetWidth/GetHeight actually return error and don't set the out param, ever since bug 435296 landed. That means we end up using random values (whatever was on the stack) for the image width/height. I'm not sure whether the right fix is to change the nsImageFrame code or to change imgContainer to return the size if it has one, or to change imgRequest to not claim STATUS_SIZE_AVAILABLE in this situation, but _something_ needs to get changed.

My preference would be to make the imgIRequest status match whatever is going on (whichever way that's done). All sorts of things rely on that status being reliable.

Picking a random owner, but if Bobby is the right owner, please ressign. This is causing random reftest and page bustage over here...

Boris, I'm hoping to look at decode-on-draw regressions RSN. Do you have a particular URL/reftest where this happens?

I'll comment next time I'm reftesting in a debug build and hit this... I think all it really takes to reproduce, though, is an image with a correct size header and then bogus image data somewhere down the line. Plus luck on the timing.

This should make it so that, if we find an error, we'll clear our status bits. I'm pretty sure this is 100% safe, but there is a somewhat-worrying comment in OnStopDecode about being about the _load_ and not the _decode_, so I want bholley to take a long, hard look at this.

Comment on attachment 408095 [details] [diff] [review] clear success bits on failure in OnStopDecode The comment isn't relevant here. That's saying that above imgRequest.cpp, OnStopDecode is about loads, but it's still what it should be in imgRequest.cpp and below. r=bholley Flagging bz for review to make sure he's ok with potentially setting STATUS_ERROR in more cases than we used to. In particular, the old system would just squelch all image decoding errors, and return the cached size even if the rest of decoding failed. This gets into the bigger issue of bug 514033. But I'm still chugging along at a snail's pace through other bugs. so...

Comment on attachment 408095 [details] [diff] [review] clear success bits on failure in OnStopDecode r=bzbarsky. Sorry for the terrible lag here. :(