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

Status: REOPENED

(Core :: Layout, defect)

Description

[Seth Fowler [:seth] [:s2h]]

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.

Comment 1

[Seth Fowler [:seth] [:s2h]]

Attachment: Make test-animated-image-layers-*.html use a GIF with a finite loop count

Here's the patch.

Comment 2

[Seth Fowler [:seth] [:s2h]]

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.

Comment 3

[Seth Fowler [:seth] [:s2h]]

Attachment: Make test-animated-image-layers-*.html use a GIF with a finite loop count

Here's the correct patch.

Comment 4

[Seth Fowler [:seth] [:s2h]]

Try job:

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

Comment 5

[Seth Fowler [:seth] [:s2h]]

Updating the summary to reflect the fact that these tests are getting disabled until this can get resolved.

Comment 6

[Timothy Nikkel (:tnikkel)]

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?

Comment 7

[Seth Fowler [:seth] [:s2h]]

(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...

Comment 8

[Seth Fowler [:seth] [:s2h]]

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.

Comment 9

[Seth Fowler [:seth] [:s2h]]

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.

Comment 10

[Timothy Nikkel (:tnikkel)]

(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?

Comment 11

[Seth Fowler [:seth] [:s2h]]

(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 12

[Timothy Nikkel (:tnikkel)]

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.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

FWIW, these tests are completely disabled on Android & Linux now.

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.