Closed Bug 1098958 Opened 5 years ago Closed 5 years ago
Images cut off
[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.
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
Here's a try job: https://tbpl.mozilla.org/?tree=Try&rev=bdbe1d8d2310
Try looks good. Pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/a5fcba591258
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+
You need to log in before you can comment on or make changes to this bug.