whether a decode error put our image state into error depends on when the decode starts in relation to the network load event for the image

RESOLVED FIXED in mozilla34

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: tnikkel, Assigned: tnikkel)

Tracking

unspecified
mozilla34
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

In nsImageLoadingContent whether a decode error put our image state into error depends on when the decode starts in relation to the network load event for the image. In nsImageLoadingContent::OnStopRequest if a decode has been started then we wait until the decode is complete to fire the load/error event. When we fire this event we also update the image state. However if a decode has not started by the time OnStopRequest runs, but a decode does happen later then when it completes nsImageLoadingContent::Notify does nothing, and our image state stays as non-error.

This is the cause of bug 930894, bug 920493, bug 921207, bug 918419, bug 918712, and is the reason the patch in bug 1008942 causes test failures.
Posted patch patchSplinter Review
Not really sure who would be a good reviewer for this. Kyle, let me know if you have a better idea.
Attachment #8470473 - Flags: review?(khuey)
Given that bz is on vacation I am probably the best candidate.
Comment on attachment 8470473 [details] [diff] [review]
patch

Maybe bz can get to this.
Attachment #8470473 - Flags: review?(bzbarsky)
Comment on attachment 8470473 [details] [diff] [review]
patch

Yeah, this seems pretty reasonable.
Attachment #8470473 - Flags: review?(bzbarsky) → review+
Comment on attachment 8470473 [details] [diff] [review]
patch

Thanks!
Attachment #8470473 - Flags: review?(khuey)
https://hg.mozilla.org/mozilla-central/rev/0e0d6b70a43d
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Can we land this on Aurora33 as well?
Are the intermittent's causing that much trouble for the sheriffs? They seem to be pretty rare right now, and since aurora becomes beta in a few days I wouldn't mind the extra bake time.
If you're worried about baking, we can live without it.
You need to log in before you can comment on or make changes to this bug.