Bug 191542 - [Web Animations] Don't schedule animation frames or update style while an accelerated animation is running
Summary: [Web Animations] Don't schedule animation frames or update style while an acc...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Animations (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-11-12 04:13 PST by Antoine Quint
Modified: 2018-11-16 09:53 PST (History)
6 users (show)

See Also:


Attachments
Patch (22.92 KB, patch)
2018-11-12 04:43 PST, Antoine Quint
simon.fraser: review+
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews104 for mac-sierra-wk2 (3.04 MB, application/zip)
2018-11-12 06:09 PST, EWS Watchlist
no flags Details
Archive of layout-test-results from ews200 for win-future (12.83 MB, application/zip)
2018-11-12 08:21 PST, EWS Watchlist
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2018-11-12 04:13:50 PST
In case all the relevant animations for the document are accelerated, we should not be calling updateAnimationsAndSendEvents().
Comment 1 Antoine Quint 2018-11-12 04:14:01 PST
<rdar://problem/45356027>
Comment 2 Antoine Quint 2018-11-12 04:43:19 PST
Created attachment 354546 [details]
Patch
Comment 3 EWS Watchlist 2018-11-12 06:09:49 PST
Comment on attachment 354546 [details]
Patch

Attachment 354546 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/9957516

New failing tests:
fast/layers/no-clipping-overflow-hidden-added-after-transform.html
Comment 4 EWS Watchlist 2018-11-12 06:09:50 PST
Created attachment 354553 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 5 Simon Fraser (smfr) 2018-11-12 07:56:49 PST
Comment on attachment 354546 [details]
Patch

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

> Source/WebCore/animation/DocumentTimeline.cpp:316
> +    m_waitingOnVMIdle = true;
> +    if (!m_currentTimeClearingTaskQueue.hasPendingTasks())
> +        m_currentTimeClearingTaskQueue.enqueueTask(std::bind(&DocumentTimeline::maybeClearCachedCurrentTime, this));
> +    m_document->vm().whenIdle([this, protectedThis = makeRefPtr(this)]() {
> +        m_waitingOnVMIdle = false;
> +        maybeClearCachedCurrentTime();
> +    });

Is the VM going idle correct in terms of HTMLEventLoop behaviors, or is this a stop-gap until we have that?
Comment 6 EWS Watchlist 2018-11-12 08:20:57 PST
Comment on attachment 354546 [details]
Patch

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

New failing tests:
animations/transform-non-accelerated.html
animations/play-state-paused.html
webanimations/accelerated-animation-with-delay.html
animations/animation-direction-reverse.html
webanimations/accelerated-transition-by-removing-property.html
http/wpt/css/css-animations/set-animation-play-state-to-paused-001.html
transitions/start-transform-transition.html
fast/animation/css-animation-resuming-when-visible-with-style-change.html
animations/animation-direction-normal.html
animations/dynamic-stylesheet-loading.html
Comment 7 EWS Watchlist 2018-11-12 08:21:08 PST
Created attachment 354561 [details]
Archive of layout-test-results from ews200 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews200  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 8 Antoine Quint 2018-11-12 08:41:03 PST
(In reply to Simon Fraser (smfr) from comment #5)
> Comment on attachment 354546 [details]
> 
> Is the VM going idle correct in terms of HTMLEventLoop behaviors, or is this
> a stop-gap until we have that?

The latter. This is just so that I can tell for sure that we always return the same current time for the given animation frame. This works well in practice.
Comment 9 Antoine Quint 2018-11-13 02:19:31 PST
Tracking the Windows failures in https://bugs.webkit.org/show_bug.cgi?id=191584.
Comment 10 Antoine Quint 2018-11-13 02:23:04 PST
Committed r238128: <https://trac.webkit.org/changeset/238128>
Comment 11 Truitt Savell 2018-11-16 09:53:50 PST
It looks like https://trac.webkit.org/changeset/238128/webkit has caused 

imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events.html

to become flakey. Confirmed using command:
run-webkit-tests --root debug-238128 imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events.html --iterations 1000 -f --debug -1

The test fails around 15 times out of the 1000. test does not fail at all on 238127.

Diff:
--- /Volumes/Data/slave/highsierra-debug-tests-wk1/build/layout-test-results/imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events-expected.txt
+++ /Volumes/Data/slave/highsierra-debug-tests-wk1/build/layout-test-results/imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events-actual.txt
@@ -1,7 +1,7 @@
 
 PASS Fires cancel event before requestAnimationFrame 
 PASS Fires finish event before requestAnimationFrame 
-FAIL Sorts finish events by composite order assert_array_equals: finish events for various animation type should be sorted by composite order property 0, expected "CSSTransition:finish" but got "ScriptAnimation:finish"
+PASS Sorts finish events by composite order 
 FAIL Sorts cancel events by composite order assert_array_equals: cancel events should be sorted by composite order lengths differ, expected 5 got 3
 FAIL Sorts events for the same transition assert_array_equals: Playback and CSS events for the same transition should be sorted by schedule event time and composite order lengths differ, expected 2 got 1
 PASS Playback events with the same timeline retain the order in which they arequeued