Bug 186507 – [Web Animations] Make WPT test at timing-model/timelines/document-timelines.html pass reliably

• Status: RESOLVED
• Resolution: FIXED
• Component: Animations

[Antoine Quint]

The WPT test at imported/w3c/web-platform-tests/web-animations/timing-model/timelines/document-timelines.html is failing reliably, flaky or timing out.

[Radar WebKit Bug Importer]

<rdar://problem/41000257>

[Antoine Quint]

Created attachment 343908 [details]
Patch

[EWS Watchlist]

Comment on attachment 343908 [details]
Patch

Attachment 343908 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8383887

New failing tests:
http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-audio.html

[EWS Watchlist]

Created attachment 343919 [details]
Archive of layout-test-results from ews204 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews204  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Dean Jackson]

Comment on attachment 343908 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=343908&action=review

> Source/WebCore/animation/DocumentTimeline.cpp:159
> +        // animations, so we schedule the invalidation task and registere a whenIdle callback on the VM, which will

typo register

> Source/WebCore/animation/DocumentTimeline.cpp:164
> +        RefPtr<DocumentTimeline> self;
> +        m_waitingOnVMIdle = true;
> +        m_document->vm().whenIdle([self, this]() {

You never set or use self.

> Source/WebCore/animation/DocumentTimeline.cpp:215
> +    // We want to make sure we only clear the cached currenr time if we're not currently running

typo current

> Source/WebCore/animation/DocumentTimeline.cpp:219
> +    if (!m_waitingOnVMIdle && !m_invalidationTaskQueue.hasPendingTasks())

Are you sure you'll always have an empty invalidationTaskQueue when the whenIdle callback happens? Or I guess it could fire, but there still be a task to run, which will itself call maybeClearCachedCurrentTime.

[Antoine Quint]

Committed r233394: <https://trac.webkit.org/changeset/233394>

[Antoine Quint]

(In reply to Dean Jackson from comment #5)
> Comment on attachment 343908 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=343908&action=review
> 
> > Source/WebCore/animation/DocumentTimeline.cpp:219
> > +    if (!m_waitingOnVMIdle && !m_invalidationTaskQueue.hasPendingTasks())
> 
> Are you sure you'll always have an empty invalidationTaskQueue when the
> whenIdle callback happens? Or I guess it could fire, but there still be a
> task to run, which will itself call maybeClearCachedCurrentTime.

That's right, both conditions need to be true, so we call maybeClearCachedCurrentTime() in both the whenIdle callback and performInvalidationTask().