[webkit-reviews] review granted: [Bug 183826] [Web Animations] Make imported/mozilla/css-animations/test_animation-pausing.html pass reliably : [Attachment 343269] Patch

[bugzilla-daemon at webkit.org]

Dean Jackson <dino at apple.com> has granted Antoine Quint <graouts at apple.com>'s
request for review:
Bug 183826: [Web Animations] Make
imported/mozilla/css-animations/test_animation-pausing.html pass reliably
https://bugs.webkit.org/show_bug.cgi?id=183826

Attachment 343269: Patch

https://bugs.webkit.org/attachment.cgi?id=343269&action=review




--- Comment #3 from Dean Jackson <dino at apple.com> ---
Comment on attachment 343269
  --> https://bugs.webkit.org/attachment.cgi?id=343269
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?