583825 - Progressive decoding/display of images no longer works

Status: VERIFIED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Joe Drew (not getting mail)]

Visit the URL mentioned. Firefox loads the image properly, but doesn't progressively decode it - instead, you get the alt text while the image downloads, and then once it's done downloading/decoding, you get the full image displayed.

Comment 1

[Bobby Holley (:bholley)]

this is bad.

Comment 2

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Setting image.mem.decodeondraw to true seems to fix this (or at least get around it) for me.

Comment 3

[Alice0775 White]

Regression window:

Works:
http://hg.mozilla.org/mozilla-central/rev/5f857be14db9
Mozilla/5.0 (Windows; Windows NT 6.1; WOW64; rv:2.0b3pre) Gecko/20100727
Minefield/4.0b3pre ID:20100728145944

Fails:
http://hg.mozilla.org/mozilla-central/rev/b8b62b351c09
Mozilla/5.0 (Windows; Windows NT 6.1; WOW64; rv:2.0b3pre) Gecko/20100728
Minefield/4.0b3pre ID:20100728152620

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5f857be14db9&tochange=b8b62b351c09

Comment 5

[Joe Drew (not getting mail)]

Oh hooray, a regression from my bug!

Comment 6

[IU]

This is not an x86_64-only bug.  I see this also on XP 32-bit.

Comment 7

[Bobby Holley (:bholley)]

Figured this bug out.

The issue has to do with the tricky stuff we do with OnStartContainer, as discussed in bug 572520 comment 31. Before that patch, we were setting stateHasSize after sending OnStartContainer, but we'd send imgIRequest::STATUS_SIZE_AVAILABLE _before_. In bug 572520, this was all rolled together into RecordStartContainer(), so they both got set afterwards.

This is a problem, because consumers of OnStartContainer expect STATUS_SIZE_AVAILABLE to be set. In particular, nsImageLoadingContent calls UpdateImageState(), which doesn't set mLoading=false because it doesn't find STATUS_SIZE_AVAILABLE (note the abuse of the concept of "loading" here - in nsImageLoadingContent, "loading" means "size not yet available"). This means that nsImageFrame::ShouldCreateImageFrameFor() returns falls during reflow, which means that the frame doesn't get created until we do another reflow when we get the whole image.

Patch coming right up.

Comment 8

[Bobby Holley (:bholley)]

Attachment: patch v1

Since really we want to send OnStartContainer once per-proxy, it doesn't make the most sense to have such handling be done in imgRequest and imgStatusTracker. This patch moves all that into imgRequestProxy, and fixes the bug.

Comment 9

[Joe Drew (not getting mail)]

Comment on attachment 464503 [details] [diff] [review]
patch v1

r+, looks great. File a followup for the very-likely-to-be-intermittently-faily test for this functionality.

Comment 10

[Bobby Holley (:bholley)]

(In reply to comment #9)
> Comment on attachment 464503 [details] [diff] [review]
> patch v1
> 
> r+, looks great. File a followup for the very-likely-to-be-intermittently-faily
> test for this functionality.

Filed bug 586078.

Comment 11

[Paul-Kenji Cahier]

Is there any chances of seeing this merged to trunk any time soon, or will it have to wait(forever) for that test?

Comment 12

[Bobby Holley (:bholley)]

(In reply to comment #11)
> Is there any chances of seeing this merged to trunk any time soon, or will it
> have to wait(forever) for that test?

No, this is ready to land. It's just difficult to land patches right now because the tree is so busy.

Comment 13

[Shawn Wilsher :sdwilsh]

This is marked as a beta4 blocker, so it needs to land by Friday.  Please ensure that it does so (by landing it yourself, or getting checkin-needed on the bug while keeping in mind the points raised in http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed)

Comment 14

[Bobby Holley (:bholley)]

This just landed:
http://hg.mozilla.org/mozilla-central/rev/706f72b0c151

Resolving fixed.

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b4pre) Gecko/20100813 Minefield/4.0b4pre