Bug 139599 – fast/images/animated-gif-body-outside-viewport.html is flaky

• Component: Tools / Tests
• Status: RESOLVED WONTFIX

[Alexey Proskuryakov]

This test randomly fails on several Mac WK1 bots:

http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fimages%2Fanimated-gif-body-outside-viewport.html

-PASS isBackgroundAnimated is false
+FAIL isBackgroundAnimated should be false. Was true.

[Alexey Proskuryakov]

Marked as flaky in r177239.

[Chris Dumez]

Created attachment 243241 [details]
WIP Patch

[Chris Dumez]

Created attachment 243247 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 243247 [details]
Patch

Clearing flags on attachment: 243247

Committed r177265: <http://trac.webkit.org/changeset/177265>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

Sadly, the test is still flaky:

https://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=fast%2Fimages%2Fanimated-gif-body-outside-viewport.html

[Chris Dumez]

Hmm, it seems that this is an actual failure on WK1. On WebKit1, it seems FrameView::paintsEntireContents() return true. As a result, FrameView::windowClipRect() returns [0, 0, 1600, 1200] (this is the size of the content, *including* what is not visible) instead of the expected [0, 0, 800, 600]. This means we don't currently don't always properly detect if animated gifs are outside the viewport on WK1.

[Chris Dumez]

(In reply to comment #7)
> Hmm, it seems that this is an actual failure on WK1. On WebKit1, it seems
> FrameView::paintsEntireContents() return true. As a result,
> FrameView::windowClipRect() returns [0, 0, 1600, 1200] (this is the size of
> the content, *including* what is not visible) instead of the expected [0, 0,
> 800, 600]. This means we don't currently don't always properly detect if
> animated gifs are outside the viewport on WK1.

Looks like this was done by Andreas a long time ago:
https://bugs.webkit.org/show_bug.cgi?id=49375

[Simon Fraser (smfr)]

(In reply to comment #7)
> Hmm, it seems that this is an actual failure on WK1. On WebKit1, it seems
> FrameView::paintsEntireContents() return true. As a result,

I think paintsEntireContents() should only be true in a layer-backed WK1 view (if the WebHTMLView has its own layer).

[Chris Dumez]

(In reply to comment #9)
> (In reply to comment #7)
> > Hmm, it seems that this is an actual failure on WK1. On WebKit1, it seems
> > FrameView::paintsEntireContents() return true. As a result,
> 
> I think paintsEntireContents() should only be true in a layer-backed WK1
> view (if the WebHTMLView has its own layer).

Hmm, it seems to be true when I run MiniBrowser:
Tools/Scripts/run-minibrowser --debug

However, it seems to be false when running the layout tests so this is not the source of the failure.

[Chris Dumez]

Hmm, what seems to happen locally (with WK1 DumpRenderTree) is that RenderElement::newImageAnimationFrameAvailable() never gets called. As a result, RenderView::addRendererWithPausedImageAnimations() is never called and we never set the 'hasPausedImageAnimations' flag on the renderer.

Does anyone have an idea why newImageAnimationFrameAvailable() wouldn't get called with WK1/DRT? The function gets called on WK2/WKTR and the test passes just fine there.

[Chris Dumez]

I found an interesting comment in RenderLayerBacking::updateImageContents():
// Image animation is "lazy", in that it automatically stops unless someone is drawing
// the image. So we have to kick the animation each time; this has the downside that the
// image will keep animating, even if its layer is not visible.
image->startAnimation();


Is WK1 using RenderLayerBacking? If not, then it is likely the image is not being animated outside the viewport simply because it is not being drawn.

In this case, this would mean that Antti's optimization only makes sense on WK2. That would make my animated GIF layout tests wk2-specific.

[Simon Fraser (smfr)]

MiniBrowser does layer-backing on Yosemite, because it has a blurred toolbar.

[Chris Dumez]

(In reply to comment #12)
> I found an interesting comment in RenderLayerBacking::updateImageContents():
> // Image animation is "lazy", in that it automatically stops unless someone
> is drawing
> // the image. So we have to kick the animation each time; this has the
> downside that the
> // image will keep animating, even if its layer is not visible.
> image->startAnimation();
> 
> 
> Is WK1 using RenderLayerBacking? If not, then it is likely the image is not
> being animated outside the viewport simply because it is not being drawn.
> 
> In this case, this would mean that Antti's optimization only makes sense on
> WK2. That would make my animated GIF layout tests wk2-specific.

+dino in cc as it seems he added this comment.

[Chris Dumez]

Animated image throttling behaves differently on WebKit1 than on WebKit2 and our layout test infrastructure does not work for WebKit1. This test is WK2 specific. I recently skipped it on WK1.