Created attachment 312643[details]
Archive of layout-test-results from ews100 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 312644[details]
Archive of layout-test-results from ews114 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 312650[details]
Archive of layout-test-results from ews101 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 312652[details]
Archive of layout-test-results from ews114 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114 Port: mac-elcapitan Platform: Mac OS X 10.11.6
What's happening in mac and mac-debug EWS?
There is a comment in TestExpectations of mac-wk1:
https://trac.webkit.org/browser/webkit/trunk/LayoutTests/platform/mac-wk1/TestExpectations?rev=217880#L123> # Animated image throttling behaves differently on WK1.
> fast/images/animated-gif-body-outside-viewport.html [ Skip ]
> fast/images/animated-gif-window-resizing.html [ Skip ]
> fast/images/animated-gif-zooming.html [ Skip ]
I need to do the following trick as other test cases do.
> // Force layout and display so the image frame starts decoding.
> document.body.offsetHeight;
> testRunner.display();
Comment on attachment 312847[details]
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=312847&action=review> Source/WebCore/ChangeLog:8
> + Test: fast/images/animated-gif-paint-after-animation.html
Please move this line after the comment below.
> Source/WebCore/platform/graphics/BitmapImage.cpp:-191
> - ASSERT(!canAnimate() && !m_currentFrame);
You can use:
ASSERT(!canAnimate() && (!m_currentFrame || m_animationFinished));
> LayoutTests/fast/images/animated-gif-paint-after-animation.html:12
> + window.onload = function() {
Instead of listening to the onload event you can move all the script at the end of <body> tag.
> LayoutTests/fast/images/animated-gif-paint-after-animation.html:15
> + img.src = 'resources/animated-red-green-blue-1000x1000-repeat-1.gif';
Usually setting the image source comes after defining the onload function:
img.onload = function() {
....
}
img.src = "...".
> LayoutTests/fast/images/animated-gif-paint-after-animation.html:25
> + window.setTimeout(function() {
> + testRunner.notifyDone();
> + }, 500);
Instead of using the css animation and the setTimeout(), I think this can be replaced by the following:
// Force the page to be displayed. The image element is already invalidated when the frame finished decoding.
testRunner.display();
// Change the width of the img element
img.width = "100"
// Force a final layout and display so the image last frame is drawn.
document.body.offsetHeight;
testRunner.display();
testRunner.notifyDone();
> LayoutTests/fast/images/animated-gif-paint-after-animation.html:34
> + @keyframes n {
> + from { width : 10px; }
> + to { width: 20px; }
> + }
Why do you need to add this animation? I think you want to force a last redraw after the last frame is drawn. You can change the width of the element manually after receiving the webkitImageFrameReady event; see above.
> LayoutTests/fast/images/animated-gif-paint-after-animation.html:36
> + display: block;
Is setting the display to "block" really needed for the test?
Comment on attachment 313992[details]
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=313992&action=review> LayoutTests/fast/images/animated-gif-paint-after-animation.html:31
> + window.setTimeout(function() {
> + // Force a final layout and display so the image last frame is drawn.
> + img.width = "100";
> + if (window.testRunner)
> + testRunner.notifyDone();
> + }, 500);
Using the setTimeout() makes the test non deterministic. There is no guarantee here that the image will draw its last frame twice in 500ms. Also the 500ms is way too long period to wait for. As I said earlier, you can control the layout and display yourself so you can know exactly when the test should end:
// Force the page to be displayed. The image element is already invalidated when the frame finished decoding.
testRunner.display();
// Change the width of the img element
img.width = "100";
// notifyDone() will force the last layout and display.
testRunner.notifyDone();
Thank you for the review.
It needs to wait at least 30ms for finishing the animation.
I've found the way without setTimeout by counting decoded frames.
Created attachment 314119[details]
Archive of layout-test-results from ews117 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 314150[details]
Archive of layout-test-results from ews103 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-elcapitan Platform: Mac OS X 10.11.6
(In reply to Fujii Hironori from comment #24)
> Thank you for the review.
> It needs to wait at least 30ms for finishing the animation.
> I've found the way without setTimeout by counting decoded frames.
I was assuming your animated image has only two frames. Just delete the last frame of the image and have it only contain two frames: red and green. Then your code should be simpler
img.addEventListener("webkitImageFrameReady", function() {
// Force the page to be displayed. The image element is already invalidated when the frame finished decoding.
if (window.testRunner)
testRunner.display();
// Change the width of the img element
img.width = "100";
// notifyDone() will force the last layout and display.
if (window.testRunner)
testRunner.notifyDone();
});
img.src = 'resources/animated-red-green-1000x1000-repeat-1.gif';
Also make sure the test asserts without your patch.
(In reply to Build Bot from comment #29)
> Created attachment 314150[details]
> Archive of layout-test-results from ews103 for mac-elcapitan
>
> The attached test failures were seen while running run-webkit-tests on the
> mac-ews.
> Bot: ews103 Port: mac-elcapitan Platform: Mac OS X 10.11.6
The test is timing out because DRT does not force drawing when a repaint happens. Instead DRT waits for notifyDone() to do the last draw. Your code relies on having the event webkitImageFrameReady is called two times and the image is drawn four times:
1st time: draws frame 0 and request frame 1 for decoding.
2nd time: draws frame 1 and request frame 2 for decoding.
3nd time: draws frame 2.
4th time: draws frame 2.
Only the first draw happens because onload calls testRunner.display(). The second and the third draws do not happen because webkitImageFrameReady callback returns when count != 2. And webkitImageFrameReady can't fire unless a draw happens. The fourth time does not happen because testRunner.notifyDone() is not called.
Changing the image to have two frames only and changing the webkitImageFrameReady callback as I mentioned above should draw the image three times and testRunner.notifyDone() is guaranteed to be called.
Created attachment 314239[details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
2017-06-08 01:13 PDT, Fujii Hironori
2017-06-08 01:26 PDT, Fujii Hironori
2017-06-12 01:34 PDT, Fujii Hironori
2017-06-12 02:34 PDT, Build Bot
2017-06-12 02:53 PDT, Build Bot
2017-06-12 03:22 PDT, Fujii Hironori
2017-06-12 04:05 PDT, Build Bot
2017-06-12 04:45 PDT, Build Bot
2017-06-12 23:21 PDT, Fujii Hironori
2017-06-13 19:30 PDT, Fujii Hironori
2017-06-27 22:37 PDT, Fujii Hironori
2017-06-28 22:26 PDT, Fujii Hironori
2017-06-28 23:54 PDT, Build Bot
2017-06-29 12:18 PDT, Build Bot
2017-06-29 22:28 PDT, Fujii Hironori
2017-06-29 23:32 PDT, Build Bot