Fix and reenable test-animated-image-layers-*.html

REOPENED
Assigned to

Status

()

Core
Layout
REOPENED
3 years ago
3 years ago

People

(Reporter: seth, Assigned: seth)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 years ago
test-animated-image-layers.html and test-animated-image-layers-background.html use a GIF that loops infinitely in the test. This can cause the reftest harness to loop infinitely when trying to take a snapshot, which results in a test failure due to timeout. (See bug 1123563.)

We should modify the GIF to loop a finite number of times. This means that even if we break these tests, it could pass sometimes, because in wonky timing environments like the B2G emulator we might run through every iteration of the animation before send MozReftestInvalidate. However, I'd expect it to at least fail intermittently, in the worst case.
(Assignee)

Comment 1

3 years ago
Created attachment 8580972 [details] [diff] [review]
Make test-animated-image-layers-*.html use a GIF with a finite loop count

Here's the patch.
Attachment #8580972 - Flags: review?(tnikkel)
(Assignee)

Updated

3 years ago
Assignee: nobody → seth
Status: NEW → ASSIGNED
(Assignee)

Comment 2

3 years ago
Comment on attachment 8580972 [details] [diff] [review]
Make test-animated-image-layers-*.html use a GIF with a finite loop count

Sorry, uploaded the wrong patch.
Attachment #8580972 - Attachment is obsolete: true
Attachment #8580972 - Flags: review?(tnikkel)
(Assignee)

Comment 3

3 years ago
Created attachment 8580974 [details] [diff] [review]
Make test-animated-image-layers-*.html use a GIF with a finite loop count

Here's the correct patch.
Attachment #8580974 - Flags: review?(tnikkel)
(Assignee)

Comment 4

3 years ago
Try job:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=72239db31759
(Assignee)

Comment 5

3 years ago
Updating the summary to reflect the fact that these tests are getting disabled until this can get resolved.
Summary: Make test-animated-image-layers-*.html use a GIF with a finite loop count → Fix and reenable test-animated-image-layers-*.html
Hmm, most of the failures are on android, and opt linux x64 builds. Not being able to animate a single gif at 10fps on a regular desktop linux opt build seems a little fishy, given that up until january this test was seemingly working fine?
(Assignee)

Comment 7

3 years ago
(In reply to Timothy Nikkel (:tn) from comment #6)
> Hmm, most of the failures are on android, and opt linux x64 builds. Not
> being able to animate a single gif at 10fps on a regular desktop linux opt
> build seems a little fishy, given that up until january this test was
> seemingly working fine?

We're animating it just fine; the problem is that the reftest snapshot process gets into an infinite loop because the animation loops. We ordinarily do not test with looping animated GIFs, but in this case it's tricky to avoid because we want to test whether the background under the image needs to be repainted when it animates.

I have no idea why this worked before. It makes perfect sense that this could cause problems...
(Assignee)

Comment 8

3 years ago
FWIW, I'm going to resolve this as WORKSFORME for now because I ended up being able to land the other bugs that were blocked on this. I'm still not comfortable with the sensitivity to timing that these tests have, but I think it's better not to devote effort to fixing it if it's not actively causing a problem, and right now it seems like it isn't. We can reopen later if needed.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → WORKSFORME
(Assignee)

Comment 9

3 years ago
Actually, I changed my mind on this. This issue isn't blocking my ability to land anything right now, but we do still want to fix bug 1123563.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
(In reply to Seth Fowler [:seth] from comment #7)
> We're animating it just fine; the problem is that the reftest snapshot
> process gets into an infinite loop because the animation loops.

A desktop optimized build should certainly be able to display one frame of a gif and then find enough idle time after that to determine that there is nothing to paint and fire the mozreftestinvalidate event in 100ms. Wasn't this test working for years before this?
(Assignee)

Comment 11

3 years ago
(In reply to Timothy Nikkel (:tn) from comment #10)
> A desktop optimized build should certainly be able to display one frame of a
> gif and then find enough idle time after that to determine that there is
> nothing to paint and fire the mozreftestinvalidate event in 100ms. Wasn't
> this test working for years before this?

I agree that it's odd that there are desktop failures. Not sure what's up with that.
Comment on attachment 8580974 [details] [diff] [review]
Make test-animated-image-layers-*.html use a GIF with a finite loop count

Well if the test should work, and it was working for a long time I'd really like to know why it doesn't work.
Attachment #8580974 - Flags: review?(tnikkel)
FWIW, these tests are completely disabled on Android & Linux now.
You need to log in before you can comment on or make changes to this bug.