Closed Bug 1187401 Opened 4 years ago Closed 4 years ago

Remove Decoder::GetDecodeTotallyDone()

Categories

(Core :: ImageLib, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: seth, Assigned: seth)

References

Details

Attachments

(3 files)

We should rework the code in RasterImage::FinalizeDecoder so that Decoder::GetDecodeTotallyDone() is unnecessary, and then remove it. I would've made this change in bug 1187386, but since that's a refactoring bug I wanted to minimize the risk of changing behavior in that bug.
I think we can, and should, use a simpler condition here. Basically, if the
decoder was a full decoder, and it finished and wasn't aborted early, then we
can set mHasBeenDecoded to true. It doesn't matter whether we encountered an
error or not. I don't see any good reason to have different behavior if an error
was encountered, so that issue is ignored in the new condition.

This does mean that we're changing our behavior a little bit here, because
previously if a full decoder finished but did not set Decoder::mDecodeDone, we
wouldn't set RasterImage::mHasBeenDecoded. This seems pointless, though, because
this would always result from a deterministic error condition, so we effectively
*have* decoded the image to the extent that we can. And doing it the way we do
now hurts us, because it means that seriously corrupt, but small, images could
potentially get decoded on the main thread repeatedly when SYNC_DECODE_IF_FAST
is passed, since mHasBeenDecoded will never get set.
Attachment #8648317 - Flags: review?(tnikkel)
This is a pure refactoring patch: since we're not using the return value from
SetMetadata() anymore, let's get rid of it.
Attachment #8648318 - Flags: review?(tnikkel)
I noticed that we don't call DoError() if SetMetadata() sees a negative size.
This could trigger the assertion in FinalizeDecoder() that we always either
encounter an error or get the size in SetMetadata(). It's also inconsistent
behavior. So let's fix that.
Attachment #8648320 - Flags: review?(tnikkel)
Attachment #8648317 - Flags: review?(tnikkel) → review+
Attachment #8648318 - Flags: review?(tnikkel) → review+
Attachment #8648320 - Flags: review?(tnikkel) → review+
You need to log in before you can comment on or make changes to this bug.