imported/w3c/web-platform-tests/web-animations/timing-model/animations/updating-the-finished-state.html Description:This Mac wk2 test is flaky failing. History: https://results.webkit.org/?suite=layout-tests&test=imported%2Fw3c%2Fweb-platform-tests%2Fweb-animations%2Ftiming-model%2Fanimations%2Fupdating-the-finished-state.html Diff: --- /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/web-animations/timing-model/animations/updating-the-finished-state-expected.txt +++ /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/web-animations/timing-model/animations/updating-the-finished-state-actual.txt @@ -1,3 +1,5 @@ + +Harness Error (TIMEOUT), message = null PASS Updating the finished state when playing past end PASS Updating the finished state when seeking past end @@ -23,7 +25,7 @@ PASS Animation finished promise is replaced after seeking back to start PASS Animation finished promise is replaced after replaying from start PASS Animation finish event is fired again after seeking back to start -PASS Animation finish event is fired again after replaying from start +TIMEOUT Animation finish event is fired again after replaying from start Test timed out PASS finish event is not fired at the end of the active interval when the endDelay has not expired PASS finish event is fired after the endDelay has expired
<rdar://problem/58867580>
Only timing out in release mode is a weird symptom! Why is Debug fine?
Created attachment 388891 [details] Patch
Created attachment 388896 [details] Update Test Expectations
Comment on attachment 388896 [details] Update Test Expectations Clearing flags on attachment: 388896 Committed r255168: <https://trac.webkit.org/changeset/255168>
Still happens on bots, not reproducing locally.
When this test times out, the failure is that the second .onfinish function is not called.
Increasing the test duration does not help.
We get in a situation where the hold time is still resolved within the play() call and we return early, not scheduling any further update, and thus failing to ever advance that animation's time and finish it.
This is the branch that we should get into but aren't inside WebAnimation::play(): `if (effectivePlaybackRate() > 0 && autoRewind == AutoRewind::Yes && (!localTime || localTime.value() < 0_s || localTime.value() >= endTime))` I expect it'll be a rounding issue when comparing times, we should probably use `timeEpsilon`.
That's it! In the bad case, these are the values we have: localTime = 0.000999999999999994s, endTime = 0.001000s In the good case: localTime = 0.001s, endTime = 0.001000s We just need to use timeEpsilon when comparing localTime and endTime.
Created attachment 393268 [details] Patch
Comment on attachment 393268 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393268&action=review > Source/WebCore/ChangeLog:12 > + Because we could end up in situation where localTime was very marginally smaller than endTime inside of WebAnimation::play(), we would end up > + with an unresolved hold time and we would return before calling WebAnimation::timingDidChange() and thus scheduling an animation update from > + the timeline because we'd assume it was paused. As a result, the animation would never end and the test would wait for a "finish" event which > + would never come. It's kind of weird to pick a line wrap value that is so large. Maybe you should rewrap to 80-100 chars? > Source/WebCore/animation/WebAnimation.cpp:931 > + if (effectivePlaybackRate() > 0 && autoRewind == AutoRewind::Yes && (!localTime || localTime.value() < 0_s || (localTime.value() + timeEpsilon) >= endTime)) { Not now, but would it make sense to have helpers for nearly_equals or nearly_greater than? I think we have some helpers for that already (maybe with a better name).
Committed r258274: <https://trac.webkit.org/changeset/258274>