Closed Bug 901146 Opened 11 years ago Closed 11 years ago

Fix misleading comment in nsImageFrame.cpp

Categories

(Core :: Layout: Images, Video, and HTML Frames, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: seth, Assigned: seth)

Details

Attachments

(1 file, 1 obsolete file)

(From bug 825771 comment 36:) The current behavior doesn't make sense because BuildDisplayList will display the image if the first frame is partially decoded, but if it wasn't available when BuildDisplayList was called so that we end up calling DisplayAltFeedback when painting instead, then DisplayAltFeedback discovers that the first frame is now partially decoded, it won't actually paint it unless the first frame is completely decoded. Whether the image gets displayed before the first frame is fully decoded is therefore determined by a race on the timing of the call to BuildDisplayList.
Proposed patch. This is a one-liner that makes us display the image in DisplayAltFeedback if we have STATUS_SIZE_AVAILABLE, rather than waiting for STATUS_FRAME_COMPLETE. This matches the BuildDisplayList code. Try job here: https://tbpl.mozilla.org/?tree=Try&rev=b3f5beb781a7
Attachment #785276 - Flags: review?(dholbert)
The only caller of DisplayAltFeedback is here http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsImageFrame.cpp#1064 which shows that aRequest here is either gIconLoad->mLoadingImage or gIconLoad->mBrokenImage, not the request of the image we are asked to display for this image frame. So it's a different request from the request in BuildDisplayList.
Wow, good catch. Alright, let's update the comment at the top of that block to clarify what the situation is. This also makes it clear that I don't need to touch that code at all in bug 825771, so I'll remove that portion of the patch on that bug. (And request review from you, just to make sure I don't let any other mistakes slip through!)
Attachment #785319 - Flags: review?(tnikkel)
Attachment #785276 - Flags: review?(dholbert)
Attachment #785276 - Attachment is obsolete: true
Summary: nsImageFrame should display the image in the same cases in DisplayAltFeedback and BuildDisplayList → Fix misleading comment in nsImageFrame.cpp
No longer blocks: 825771
Attachment #785319 - Flags: review?(tnikkel) → review+
Wow, this somehow got removed from my patch queue without getting pushed. Nine months ago. Pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/d71e43379733
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Product: Core → Core Graveyard
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: