Bug 186997

Summary: [Web Animations] Ensure animations are updated prior to requestAnimationFrame callbacks
Product: WebKit Reporter: Antoine Quint <graouts>
Component: AnimationsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ddkilzer, dino, realdawei, ross.kirsling, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
See Also: https://bugs.webkit.org/show_bug.cgi?id=183826

Description Antoine Quint 2018-06-25 02:51:07 PDT
The HTML5 event loop mandates that animations are updated prior to requestAnimationFrame callbacks being fired. We need to implement this if only to reduce test flakiness since some tests use a rAF callback to check that animations have been updated. This is needed to fix https://bugs.webkit.org/show_bug.cgi?id=183826.
Comment 1 Radar WebKit Bug Importer 2018-06-25 02:51:35 PDT
<rdar://problem/41419414>
Comment 2 Radar WebKit Bug Importer 2018-06-25 02:51:42 PDT
<rdar://problem/41419416>
Comment 3 Antoine Quint 2018-06-25 02:54:42 PDT
Committed r233140: <https://trac.webkit.org/changeset/233140>
Comment 4 David Kilzer (:ddkilzer) 2018-06-25 05:43:43 PDT
(In reply to Antoine Quint from comment #3)
> Committed r233140: <https://trac.webkit.org/changeset/233140>

Attempt to fix Windows build failure:

Committed r233144: <https://trac.webkit.org/changeset/233144>
Comment 5 Dawei Fenton (:realdawei) 2018-06-25 09:25:32 PDT
(In reply to David Kilzer (:ddkilzer) from comment #4)
> (In reply to Antoine Quint from comment #3)
> > Committed r233140: <https://trac.webkit.org/changeset/233140>
> 
> Attempt to fix Windows build failure:
> 
> Committed r233144: <https://trac.webkit.org/changeset/233144>

unfortunately Windows is still failing to build:

Sample error:
https://build.webkit.org/builders/Apple%20High%20Sierra%20Release%20WK2%20(Tests)/builds/5228/steps/layout-test/logs/stdio

C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\dom/Document.cpp(2448): error C2027: use of undefined type 'WebCore::DocumentAnimationScheduler' (compiling source file C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\DerivedSources\WebCore\unified-sources\UnifiedSource206.cpp) [C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\Source\WebCore\WebCore.vcxproj]
  c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\dom\Document.h(83): note: see declaration of 'WebCore::DocumentAnimationScheduler' (compiling source file C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\DerivedSources\WebCore\unified-sources\UnifiedSource206.cpp)
Comment 6 Ross Kirsling 2018-06-25 10:15:09 PDT
The first error before r233144 was
> WebCore\dom/Document.cpp(2448): error C2027: use of undefined type 'WebCore::DocumentAnimationScheduler'

The first error after r233144 is now
> WebCore\animation/KeyframeEffectReadOnly.cpp(707): error C2027: use of undefined type 'WebCore::FrameView'
Comment 7 Ross Kirsling 2018-06-25 10:32:45 PDT
Looks like DocumentAnimationScheduler really is an undefined type on Windows:
- https://github.com/WebKit/webkit/blob/master/Source/WTF/wtf/Platform.h#L1138-L1140
- https://github.com/WebKit/webkit/blob/master/Source/WebCore/animation/DocumentAnimationScheduler.h#L28

So I think r233144 could be reversed in favor of three extra #ifs in Document.cpp (unless we wanted to make a stub implementation).
Comment 8 David Kilzer (:ddkilzer) 2018-06-25 12:24:29 PDT
(In reply to Ross Kirsling from comment #7)
> Looks like DocumentAnimationScheduler really is an undefined type on Windows:
> -
> https://github.com/WebKit/webkit/blob/master/Source/WTF/wtf/Platform.h#L1138-
> L1140
> -
> https://github.com/WebKit/webkit/blob/master/Source/WebCore/animation/
> DocumentAnimationScheduler.h#L28
> 
> So I think r233144 could be reversed in favor of three extra #ifs in
> Document.cpp (unless we wanted to make a stub implementation).

Thanks!  Just landed:

Committed r233163: <https://trac.webkit.org/changeset/233163>
Comment 9 David Kilzer (:ddkilzer) 2018-06-25 13:34:28 PDT
(In reply to David Kilzer (:ddkilzer) from comment #8)
> (In reply to Ross Kirsling from comment #7)
> > Looks like DocumentAnimationScheduler really is an undefined type on Windows:
> > -
> > https://github.com/WebKit/webkit/blob/master/Source/WTF/wtf/Platform.h#L1138-
> > L1140
> > -
> > https://github.com/WebKit/webkit/blob/master/Source/WebCore/animation/
> > DocumentAnimationScheduler.h#L28
> > 
> > So I think r233144 could be reversed in favor of three extra #ifs in
> > Document.cpp (unless we wanted to make a stub implementation).
> 
> Thanks!  Just landed:
> 
> Committed r233163: <https://trac.webkit.org/changeset/233163>

And:

Committed r233170: < https://trac.webkit.org/changeset/233170>
Comment 10 Ross Kirsling 2018-06-25 13:35:47 PDT
(In reply to David Kilzer (:ddkilzer) from comment #9)
> (In reply to David Kilzer (:ddkilzer) from comment #8)
> > (In reply to Ross Kirsling from comment #7)
> > > Looks like DocumentAnimationScheduler really is an undefined type on Windows:
> > > -
> > > https://github.com/WebKit/webkit/blob/master/Source/WTF/wtf/Platform.h#L1138-
> > > L1140
> > > -
> > > https://github.com/WebKit/webkit/blob/master/Source/WebCore/animation/
> > > DocumentAnimationScheduler.h#L28
> > > 
> > > So I think r233144 could be reversed in favor of three extra #ifs in
> > > Document.cpp (unless we wanted to make a stub implementation).
> > 
> > Thanks!  Just landed:
> > 
> > Committed r233163: <https://trac.webkit.org/changeset/233163>
> 
> And:
> 
> Committed r233170: < https://trac.webkit.org/changeset/233170>

You beat me to the punch. :)
Apparently KeyframeEffectReadOnly.cpp had a legitimate separate issue, but builds should go green after this one.