Closed Bug 1098958 Opened 5 years ago Closed 5 years ago

Images cut off

Categories

(Core :: ImageLib, defect)

35 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla37
Tracking Status
firefox34 --- unaffected
firefox35 + wontfix
firefox36 + fixed
firefox37 + fixed

People

(Reporter: alice0775, Assigned: seth)

References

()

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

[Tracking Requested - why for this release]: regressed in Aurora35.0a2 and Nightly36.0a1

Steps To reproduce:
1. Clear cache
2. Put mouse pointer to the bottom of the browser screen
3. Open http://ie.microsoft.com/testdrive/Performance/FlyingImages/Default.html
   --- observe images
4. Change Image size (Default IE Logo, Large IE Logo,Mix Photo, My Browser, All Browsers)
   --- observe images

Actual Results:
Images cut off at bottom

Expected Results:
Image should draw all its height

Regression window(m-c)
Good:
https://hg.mozilla.org/mozilla-central/rev/43ab820c7b68
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:35.0) Gecko/20100101 Firefox/35.0 ID:20140926183438
Bad:
https://hg.mozilla.org/mozilla-central/rev/818f353b7aa6
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:35.0) Gecko/20100101 Firefox/35.0 ID:20140926185339
Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=43ab820c7b68&tochange=818f353b7aa6

Regressed by:
818f353b7aa6	Seth Fowler — Bug 1069369 - Remove all manual discarding during frame lookup. r=tn a=kwierso


Note:
there is other Bug 1098895 in Aurora35.0a2.
See Also: → 1098895
Flags: needinfo?(seth)
I can confirm the intermittent issue on 36 (GNU/Linux).
[Tracking Requested - why for this release]:
Since debugging showed that the images involved here are getting decoded, notified about, etc. successfully, my next theory was that the layers::ImageContainer was falling out of sync. This proved to be correct, and the fix is extremely straightforward.

I do feel like we're starting to get too much logic about invalidations in FinishedSomeDecoding; a future refactoring should try to clean this stuff up a little. At least it's the *only* place in RasterImage we have logic about invalidations due to decoding now.
Attachment #8534554 - Flags: review?(tnikkel)
Assignee: nobody → seth
Status: NEW → ASSIGNED
Before that patch, I could reproduce the problem 75% of the time on a shift-reload. After the patch, I cannot reproduce it at all. That said, this bug is timing dependent, so I think we should ask QA for a verification of the fix.

This also needs an uplift to Aurora. (And a try job, but try is closed right now.)
Attachment #8534554 - Flags: review?(tnikkel) → review+
Thanks for the review! I made a minor tweak; now we only call UpdateImageContainer if the default decode flags are used. Doing it otherwise is pointless, since we use the default decode flags version of the image in UpdateImageContainer.
Attachment #8534554 - Attachment is obsolete: true
Try looks good. Pushed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/a5fcba591258
Flags: needinfo?(seth)
https://hg.mozilla.org/mozilla-central/rev/a5fcba591258
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Comment on attachment 8534720 [details] [diff] [review]
Call UpdateImageContainer whenever we send invalidations in RasterImage

This needs an uplift to Aurora. I am very sorry to say that because the dependencies here are *massive*, we cannot uplift to 35, even though it's affected. =( I'll look into the possibility of making a different version of the patch that could land on 35.

Approval Request Comment
[Feature/regressing bug #]: 1069369
[User impact if declined]: Sometimes part of an image will be cut off.
[Describe test coverage new/current, TBPL]: On m-c. Current tests should cover this; it's not clear to me right now why we're not getting intermittent oranges from this issue. (OTOH, there are several unexplained intermittent orange bugs in ImageLib right now, so maybe we are...)
[Risks and why]: Low risk. We just update the version of the image displayed onscreen more often.
[String/UUID change made/needed]: None.
Attachment #8534720 - Flags: approval-mozilla-aurora?
Attachment #8534720 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Depends on: 1124051
No longer depends on: 1124051
Duplicate of this bug: 1125210
Duplicate of this bug: 1126991
You need to log in before you can comment on or make changes to this bug.