206615 2020-01-22 15:13:19 -0800 REGRESSION: [ Mac WK2 ] animations/suspend-resume-animation-events.html is a flaky failure 2020-01-25 12:07:25 -0800 1 1 1 Unclassified WebKit Animations WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 jacob_uphoff darin ap darin dino ryanhaddad tsavell webkit-bot-watchers-bugzilla webkit-bug-importer oldest_to_newest 1609769 0 jacob_uphoff 2020-01-22 15:13:19 -0800 This is a flakey failure on Mac wk2 I did not try to reproduce, however failure goes back as far as I can see the history. History: https://results.webkit.org/?suite=layout-tests&test=animations%2Fsuspend-resume-animation-events.html Diff: --- /Volumes/Data/slave/catalina-release-tests-wk2/build/layout-test-results/animations/suspend-resume-animation-events-expected.txt +++ /Volumes/Data/slave/catalina-release-tests-wk2/build/layout-test-results/animations/suspend-resume-animation-events-actual.txt @@ -4,5 +4,4 @@ start animation move iteration animation move -end animation move 1609798 1 webkit-bug-importer 2020-01-22 15:44:27 -0800 <rdar://problem/58814195> 1609799 2 webkit-bug-importer 2020-01-22 15:44:27 -0800 <rdar://problem/58814196> 1610162 3 ryanhaddad 2020-01-23 10:03:49 -0800 This became very frequent starting on 1/7/2020, around seemingly unrelated commit r254181. Maybe this commit adding animations logging could be related? Add some more Animations logging ​https://bugs.webkit.org/show_bug.cgi?id=205890 https://trac.webkit.org/changeset/254177/webkit 1611033 4 darin 2020-01-25 06:55:38 -0800 This test is inherently racy and timing-dependent. If the 350ms timeout fires too soon, we would move on to the next test without seeing the end of the animation. I think we can make it much less racy by having each timer set the subsequent one instead of setting all three timers at once. I’ll try that, but no easy way to test if it worked. 1611034 5 388773 darin 2020-01-25 06:58:07 -0800 Created attachment 388773 Patch 1611088 6 388773 ap 2020-01-25 11:54:59 -0800 Comment on attachment 388773 Patch The change looks harmless with regards to what this tests check for, and likely beneficial for test reliability. However, I do not understand what is wrong with the test as is. The animation starts before any setTimeouts, and is shorter. Wouldn’t it be an actual WebKit bug if there is raciness? 1611089 7 darin 2020-01-25 11:58:41 -0800 (In reply to Alexey Proskuryakov from comment #6) > The animation starts before any setTimeouts, and is shorter. Wouldn’t it be > an actual WebKit bug if there is raciness? As far as I know, WebKit does not attempt to have a single unified timeline between JavaScript timers and webpage animation. Unless there’s something I’m not aware of. Could be a neat feature and might help webpages work more reliably. Not sure I’d call it a "bug" exactly. 1611093 8 darin 2020-01-25 12:05:13 -0800 I’m going to land this change, even though I’m not sure it will eliminate the flakiness. If this attempt fails then we might need to find another way to write the test. And I think it’s exciting to contemplate an architecture change where we unify JavaScript timers with animation timing and possibly even other time bases that affect web pages. 1611094 9 darin 2020-01-25 12:07:25 -0800 Committed r255124: <https://trac.webkit.org/changeset/255124> 388773 2020-01-25 06:58:07 -0800 2020-01-25 11:54:59 -0800 Patch bug-206615-20200125065801.patch text/plain 2258 darin U3VidmVyc2lvbiBSZXZpc2lvbjogMjU1MTE2CmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9DaGFu Z2VMb2cgYi9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKaW5kZXggNzZjNTViNTA4ZmJlMzQ1YzQ3OTMw YmU4Y2Q1OWJhZTkxYTUxNjgzMS4uZjVjZWJkN2U2ZTE3MWE1MjM4ZTljYzcyYmUyZjUyYTNkMmVk MjlmOCAxMDA2NDQKLS0tIGEvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCisrKyBiL0xheW91dFRlc3Rz L0NoYW5nZUxvZwpAQCAtMSwzICsxLDEzIEBACisyMDIwLTAxLTI1ICBEYXJpbiBBZGxlciAgPGRh cmluQGFwcGxlLmNvbT4KKworICAgICAgICBSRUdSRVNTSU9OOiBbIE1hYyBXSzIgXSBhbmltYXRp b25zL3N1c3BlbmQtcmVzdW1lLWFuaW1hdGlvbi1ldmVudHMuaHRtbCBpcyBhIGZsYWt5IGZhaWx1 cmUKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIwNjYx NQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogYW5p bWF0aW9ucy9zdXNwZW5kLXJlc3VtZS1hbmltYXRpb24tZXZlbnRzLmh0bWw6IE1ha2UgdGVzdCBz bGlnaHRseSBsZXNzIHRpbWluZworICAgICAgICBkZXBlbmRlbnQuCisKIDIwMjAtMDEtMjQgIFJ5 b3N1a2UgTml3YSAgPHJuaXdhQHdlYmtpdC5vcmc+CiAKICAgICAgICAgW2lPU10gUkVHUkVTU0lP TihyMjU0Njk5KSA6IGZhc3QvZm9ybXMvaW50ZXJhY3RpdmUtdmFsaWRhdGlvbi1yZW1vdmUtbm9k ZS1pbi1oYW5kbGVyLmh0bWwgaXMgdGltaW5nIG91dApkaWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMv YW5pbWF0aW9ucy9zdXNwZW5kLXJlc3VtZS1hbmltYXRpb24tZXZlbnRzLmh0bWwgYi9MYXlvdXRU ZXN0cy9hbmltYXRpb25zL3N1c3BlbmQtcmVzdW1lLWFuaW1hdGlvbi1ldmVudHMuaHRtbAppbmRl eCA2YTI4YzdlM2M4ZjEyYzlkOTY0YTdjZDhjZTczNjY1NjcwNzMxMDJjLi4zOTljZTFjYmNlYWIw YTcxZDFkYzYyZTBjMGFiMTY5NDNkZTU0MjBmIDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9hbmlt YXRpb25zL3N1c3BlbmQtcmVzdW1lLWFuaW1hdGlvbi1ldmVudHMuaHRtbAorKysgYi9MYXlvdXRU ZXN0cy9hbmltYXRpb25zL3N1c3BlbmQtcmVzdW1lLWFuaW1hdGlvbi1ldmVudHMuaHRtbApAQCAt MjcsNDEgKzI3LDQyIEBACiAgICAgfQogICA8L3N0eWxlPgogICA8c2NyaXB0IHR5cGU9InRleHQv amF2YXNjcmlwdCIgY2hhcnNldD0idXRmLTgiPgotICAgIAorCiAgICAgZnVuY3Rpb24gc3VzcGVu ZCgpCiAgICAgewogICAgICAgICBpZiAod2luZG93LmludGVybmFscykKICAgICAgICAgICAgIGlu dGVybmFscy5zdXNwZW5kQW5pbWF0aW9ucygpOworCisgICAgICAgIHNldFRpbWVvdXQocmVzdW1l LCA2MCk7CiAgICAgfQotICAgIAorCiAgICAgZnVuY3Rpb24gcmVzdW1lKCkKICAgICB7CiAgICAg ICAgIGlmICh3aW5kb3cuaW50ZXJuYWxzKQogICAgICAgICAgICAgaW50ZXJuYWxzLnJlc3VtZUFu aW1hdGlvbnMoKTsKKworICAgICAgICBzZXRUaW1lb3V0KGZ1bmN0aW9uKCkKKyAgICAgICAgewor ICAgICAgICAgICAgaWYgKHdpbmRvdy50ZXN0UnVubmVyKQorICAgICAgICAgICAgICAgIHRlc3RS dW5uZXIubm90aWZ5RG9uZSgpOworICAgICAgICB9LCAyNTApOwogICAgIH0KLSAgICAKKwogICAg IGZ1bmN0aW9uIHN0YXJ0VGVzdCgpCiAgICAgewogICAgICAgICBpZiAod2luZG93LnRlc3RSdW5u ZXIpIHsKICAgICAgICAgICAgIHRlc3RSdW5uZXIuZHVtcEFzVGV4dCgpOwogICAgICAgICAgICAg dGVzdFJ1bm5lci53YWl0VW50aWxEb25lKCk7CiAgICAgICAgIH0KLSAgICAgICAgCisKICAgICAg ICAgc2V0VGltZW91dChzdXNwZW5kLCA0MCk7Ci0gICAgICAgIHNldFRpbWVvdXQocmVzdW1lLCAx MDApOwotICAgICAgICBzZXRUaW1lb3V0KGZ1bmN0aW9uKCkKLSAgICAgICAgewotICAgICAgICAg ICAgaWYgKHdpbmRvdy50ZXN0UnVubmVyKQotICAgICAgICAgICAgICAgIHRlc3RSdW5uZXIubm90 aWZ5RG9uZSgpOwotICAgICAgICB9LCAzNTApOwogICAgIH0KLSAgICAKKwogICAgIGZ1bmN0aW9u IGxvZ0V2ZW50KGUsIHBoYXNlKQogICAgIHsKICAgICAgICAgdmFyIGxvZyA9IGRvY3VtZW50Lmdl dEVsZW1lbnRCeUlkKCdsb2cnKTsKICAgICAgICAgbG9nLmlubmVySFRNTCA9IGxvZy5pbm5lckhU TUwgKyBwaGFzZSArICcgYW5pbWF0aW9uICcgKyBlLmFuaW1hdGlvbk5hbWUgKyAnPGJyPic7CiAg ICAgfQotCiAgICAgCiAgIDwvc2NyaXB0PgogPC9oZWFkPgo=