RESOLVED FIXED 183826
[Web Animations] Make imported/mozilla/css-animations/test_animation-pausing.html pass reliably
https://bugs.webkit.org/show_bug.cgi?id=183826
Summary [Web Animations] Make imported/mozilla/css-animations/test_animation-pausing....
Antoine Quint
Reported 2018-03-21 08:37:23 PDT
The new tests added under imported/mozilla/css-animations and imported/mozilla/css-transitions are mostly failing reliably, flaky or timing out.
Attachments
Patch (38.60 KB, patch)
2018-06-21 13:49 PDT, Antoine Quint
dino: review+
Radar WebKit Bug Importer
Comment 1 2018-06-11 02:33:21 PDT
Antoine Quint
Comment 2 2018-06-21 13:49:00 PDT
Dean Jackson
Comment 3 2018-06-25 02:11:02 PDT
Comment on attachment 343269 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343269&action=review > Source/WebCore/ChangeLog:21 > + The second issue is that some of the sub-tests would be flaky failure because the expected clearly that animations > + would be resolved prior to firing a requestAnimationFrame() callback, as the HTML5 event loop mandates. But until > + now, both DocumentTimeline and ScriptedAnimationController would make calls to DisplayRefreshMonitorManager::scheduleAnimation() > + that were not coordinated and so the order in which the DocumentTimeline and ScriptedAnimationController callbacks > + were performed was not guaranteed. Why isn't this a separate patch? > Source/WebCore/animation/DocumentAnimationScheduler.h:41 > +class DocumentAnimationScheduler : public RefCounted<DocumentAnimationScheduler> > + , public DisplayRefreshMonitorClient { i don't think this should be split on two lines. > Source/WebCore/animation/DocumentTimeline.cpp:-221 > - DisplayRefreshMonitorManager::sharedManager().scheduleAnimation(*this); So is DisplayRefreshMonitorManager::sharedManager() used anywhere now?
Antoine Quint
Comment 4 2018-06-25 02:27:07 PDT
(In reply to Dean Jackson from comment #3) > Comment on attachment 343269 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=343269&action=review > > > Source/WebCore/ChangeLog:21 > > + The second issue is that some of the sub-tests would be flaky failure because the expected clearly that animations > > + would be resolved prior to firing a requestAnimationFrame() callback, as the HTML5 event loop mandates. But until > > + now, both DocumentTimeline and ScriptedAnimationController would make calls to DisplayRefreshMonitorManager::scheduleAnimation() > > + that were not coordinated and so the order in which the DocumentTimeline and ScriptedAnimationController callbacks > > + were performed was not guaranteed. > > Why isn't this a separate patch? This patch is addressing all remaining issues that makes this test fail. I'll land in two separate commits. > > Source/WebCore/animation/DocumentAnimationScheduler.h:41 > > +class DocumentAnimationScheduler : public RefCounted<DocumentAnimationScheduler> > > + , public DisplayRefreshMonitorClient { > > i don't think this should be split on two lines. Will fix. > > Source/WebCore/animation/DocumentTimeline.cpp:-221 > > - DisplayRefreshMonitorManager::sharedManager().scheduleAnimation(*this); > > So is DisplayRefreshMonitorManager::sharedManager() used anywhere now? Sure, DocumentAnimationScheduler uses it.
Antoine Quint
Comment 5 2018-06-25 02:58:21 PDT
First and main part of the reviewed patch landed as part of https://bugs.webkit.org/show_bug.cgi?id=186997.
Antoine Quint
Comment 6 2018-06-25 03:00:49 PDT
Note You need to log in before you can comment on or make changes to this bug.