Summary: | [Web Animations] Make imported/mozilla/css-animations/test_animation-pausing.html pass reliably | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Antoine Quint <graouts> | ||||
Component: | Animations | Assignee: | Antoine Quint <graouts> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | cdumez, dbates, dino, esprehn+autocc, ews-watchlist, kangil.han, realdawei, ryanhaddad, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | Safari Technology Preview | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
See Also: |
https://bugs.webkit.org/show_bug.cgi?id=186484 https://bugs.webkit.org/show_bug.cgi?id=183821 https://bugs.webkit.org/show_bug.cgi?id=186997 |
||||||
Attachments: |
|
Description
Antoine Quint
2018-03-21 08:37:23 PDT
Created attachment 343269 [details]
Patch
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? (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. First and main part of the reviewed patch landed as part of https://bugs.webkit.org/show_bug.cgi?id=186997. Committed r233141: <https://trac.webkit.org/changeset/233141> |