[Web Animations] Make WPT test at timing-model/timelines/document-timelines.html...
authorgraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 30 Jun 2018 13:43:26 +0000 (13:43 +0000)
committergraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 30 Jun 2018 13:43:26 +0000 (13:43 +0000)
https://bugs.webkit.org/show_bug.cgi?id=186507
<rdar://problem/41000257>

Reviewed by Dean Jackson.

LayoutTests/imported/w3c:

Record WPT test progressions.

* web-platform-tests/web-animations/timing-model/timelines/document-timelines-expected.txt:

Source/WebCore:

The Web Animations spec, along with the HTML5 event loop spec, specify some assumptions on the time reported by
document.timeline.currentTime:

- it should only increase once per frame
- it should have the same value as the timestamp passed to requestAnimationFrame() callbacks

The WPT test at web-platform-tests/web-animations/timing-model/timelines/document-timelines.html relies on these
assumptions to be true so that we check that the start time of a new animation is not the same as the timeline time
when it was created, since it will be in the "play-pending" state for a frame.

In order to support this, we add two new methods on DocumentAnimationScheduler. First, when a scheduled display update
fires, we record the timestamp and expose it via lastTimestamp() such that DocumentTimeline and ScriptedAnimationController
can use the same value when updating animations. Then, to know whether code is run as a result of a display update, we
expose isFiring().

Now, within DocumentTimeline::currentTime(), we can cache the current time this way:

- if we're in the middle of a display update, use the value returned by lastTimestamp().
- otherwise, compute what would have been the ideal number of frames (at 60fps or less if throttled) and add those to
the lastTimestamp() value.

Then, we remove this cached current time when both currently-running JavaScript has completed and all animation update
code has completed by waiting on the invalidation task to run.

* animation/DocumentAnimationScheduler.cpp:
(WebCore::DocumentAnimationScheduler::displayRefreshFired):
* animation/DocumentAnimationScheduler.h:
(WebCore::DocumentAnimationScheduler::lastTimestamp):
(WebCore::DocumentAnimationScheduler::isFiring const):
* animation/DocumentTimeline.cpp:
(WebCore::DocumentTimeline::currentTime):
(WebCore::DocumentTimeline::performInvalidationTask):
(WebCore::DocumentTimeline::maybeClearCachedCurrentTime):
* animation/DocumentTimeline.h:
* dom/ScriptedAnimationController.cpp:
(WebCore::ScriptedAnimationController::serviceScriptedAnimations):
(WebCore::ScriptedAnimationController::documentAnimationSchedulerDidFire):

LayoutTests:

This test now passes reliably.

* TestExpectations:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@233394 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/TestExpectations
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/web-animations/timing-model/timelines/document-timelines-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/animation/DocumentAnimationScheduler.cpp
Source/WebCore/animation/DocumentAnimationScheduler.h
Source/WebCore/animation/DocumentTimeline.cpp
Source/WebCore/animation/DocumentTimeline.h
Source/WebCore/dom/ScriptedAnimationController.cpp

index bf57ed3..f07c879 100644 (file)
@@ -1,3 +1,15 @@
+2018-06-29  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Make WPT test at timing-model/timelines/document-timelines.html pass reliably
+        https://bugs.webkit.org/show_bug.cgi?id=186507
+        <rdar://problem/41000257>
+
+        Reviewed by Dean Jackson.
+
+        This test now passes reliably.
+
+        * TestExpectations:
+
 2018-06-30  Antoine Quint  <graouts@apple.com>
 
         [Web Animations] Make imported/mozilla/css-transitions/test_event-dispatch.html pass reliably
index 5e0b19c..e4fd0cf 100644 (file)
@@ -1955,7 +1955,6 @@ webkit.org/b/177997 webgl/1.0.2/conformance/textures/copy-tex-image-2d-formats.h
 
 webkit.org/b/179069 imported/w3c/web-platform-tests/html/semantics/embedded-content/the-iframe-element/sandbox_032.htm [ Pass Failure ]
 
-webkit.org/b/179287 imported/w3c/web-platform-tests/web-animations/timing-model/timelines/document-timelines.html [ Pass Failure ]
 webkit.org/b/179287 imported/w3c/web-platform-tests/web-animations/timing-model/timelines/timelines.html [ Pass Failure ]
 webkit.org/b/181116 imported/w3c/web-platform-tests/web-animations/interfaces/Animation/finished.html [ Pass Failure ]
 webkit.org/b/181120 imported/w3c/web-platform-tests/web-animations/interfaces/Animation/onfinish.html [ Pass Failure ]
index 8e16dd4..a23453a 100644 (file)
@@ -1,3 +1,15 @@
+2018-06-29  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Make WPT test at timing-model/timelines/document-timelines.html pass reliably
+        https://bugs.webkit.org/show_bug.cgi?id=186507
+        <rdar://problem/41000257>
+
+        Reviewed by Dean Jackson.
+
+        Record WPT test progressions.
+
+        * web-platform-tests/web-animations/timing-model/timelines/document-timelines-expected.txt:
+
 2018-06-27  Youenn Fablet  <youenn@apple.com>
 
         Add Cross-Origin-Resource-Policy tests for workers and service workers
index 3850af1..3f7472e 100644 (file)
@@ -1,3 +1,3 @@
 
-FAIL Document timelines report current time relative to navigationStart assert_equals: The current time matches requestAnimationFrame time expected 96 but got 89
+PASS Document timelines report current time relative to navigationStart 
 
index 4c90eb0..0b77617 100644 (file)
@@ -1,3 +1,49 @@
+2018-06-29  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Make WPT test at timing-model/timelines/document-timelines.html pass reliably
+        https://bugs.webkit.org/show_bug.cgi?id=186507
+        <rdar://problem/41000257>
+
+        Reviewed by Dean Jackson.
+
+        The Web Animations spec, along with the HTML5 event loop spec, specify some assumptions on the time reported by
+        document.timeline.currentTime:
+
+        - it should only increase once per frame
+        - it should have the same value as the timestamp passed to requestAnimationFrame() callbacks
+
+        The WPT test at web-platform-tests/web-animations/timing-model/timelines/document-timelines.html relies on these
+        assumptions to be true so that we check that the start time of a new animation is not the same as the timeline time
+        when it was created, since it will be in the "play-pending" state for a frame.
+
+        In order to support this, we add two new methods on DocumentAnimationScheduler. First, when a scheduled display update
+        fires, we record the timestamp and expose it via lastTimestamp() such that DocumentTimeline and ScriptedAnimationController
+        can use the same value when updating animations. Then, to know whether code is run as a result of a display update, we
+        expose isFiring().
+
+        Now, within DocumentTimeline::currentTime(), we can cache the current time this way:
+
+        - if we're in the middle of a display update, use the value returned by lastTimestamp().
+        - otherwise, compute what would have been the ideal number of frames (at 60fps or less if throttled) and add those to
+        the lastTimestamp() value.
+
+        Then, we remove this cached current time when both currently-running JavaScript has completed and all animation update
+        code has completed by waiting on the invalidation task to run.
+
+        * animation/DocumentAnimationScheduler.cpp:
+        (WebCore::DocumentAnimationScheduler::displayRefreshFired):
+        * animation/DocumentAnimationScheduler.h:
+        (WebCore::DocumentAnimationScheduler::lastTimestamp):
+        (WebCore::DocumentAnimationScheduler::isFiring const):
+        * animation/DocumentTimeline.cpp:
+        (WebCore::DocumentTimeline::currentTime):
+        (WebCore::DocumentTimeline::performInvalidationTask):
+        (WebCore::DocumentTimeline::maybeClearCachedCurrentTime):
+        * animation/DocumentTimeline.h:
+        * dom/ScriptedAnimationController.cpp:
+        (WebCore::ScriptedAnimationController::serviceScriptedAnimations):
+        (WebCore::ScriptedAnimationController::documentAnimationSchedulerDidFire):
+
 2018-06-29  Nan Wang  <n_wang@apple.com>
 
         Crash under WebCore::AXObjectCache::handleMenuItemSelected
index 57cbb23..ef59cd0 100644 (file)
@@ -72,6 +72,12 @@ bool DocumentAnimationScheduler::scheduleScriptedAnimationResolution()
 
 void DocumentAnimationScheduler::displayRefreshFired()
 {
+    if (!m_document || !m_document->domWindow())
+        return;
+
+    m_isFiring = true;
+    m_lastTimestamp = Seconds(m_document->domWindow()->nowTimestamp());
+
     if (m_scheduledWebAnimationsResolution) {
         m_scheduledWebAnimationsResolution = false;
         m_document->timeline().documentAnimationSchedulerDidFire();
@@ -82,6 +88,8 @@ void DocumentAnimationScheduler::displayRefreshFired()
         if (auto* scriptedAnimationController = m_document->scriptedAnimationController())
             scriptedAnimationController->documentAnimationSchedulerDidFire();
     }
+
+    m_isFiring = false;
 }
 
 void DocumentAnimationScheduler::windowScreenDidChange(PlatformDisplayID displayID)
index ff75355..258a692 100644 (file)
@@ -32,6 +32,7 @@
 #include <wtf/Ref.h>
 #include <wtf/RefCounted.h>
 #include <wtf/RefPtr.h>
+#include <wtf/Seconds.h>
 
 namespace WebCore {
 
@@ -49,12 +50,17 @@ public:
     bool scheduleWebAnimationsResolution();
     bool scheduleScriptedAnimationResolution();
 
+    Seconds lastTimestamp() { return m_lastTimestamp; }
+    bool isFiring() const { return m_isFiring; }
+
 private:
     DocumentAnimationScheduler(Document&, PlatformDisplayID);
 
     RefPtr<Document> m_document;
     bool m_scheduledWebAnimationsResolution { false };
     bool m_scheduledScriptedAnimationResolution { false };
+    bool m_isFiring { false };
+    Seconds m_lastTimestamp { 0_s };
 
     void displayRefreshFired() override;
     RefPtr<DisplayRefreshMonitor> createDisplayRefreshMonitor(PlatformDisplayID) const override;
index 3f08837..9619d94 100644 (file)
@@ -130,9 +130,36 @@ std::optional<Seconds> DocumentTimeline::currentTime()
     if (m_paused || m_isSuspended || !m_document || !m_document->domWindow())
         return AnimationTimeline::currentTime();
 
+#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
+    // If we're in the middle of firing a frame, either due to a requestAnimationFrame callback
+    // or scheduling an animation update, we want to ensure we use the same time we're using as
+    // the timestamp for requestAnimationFrame() callbacks.
+    if (m_document->animationScheduler().isFiring())
+        m_cachedCurrentTime = m_document->animationScheduler().lastTimestamp();
+#endif
+
     if (!m_cachedCurrentTime) {
+#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
+        // If we're not in the middle of firing a frame, let's make our best guess at what the currentTime should
+        // be since the last time a frame fired by increment of our update interval. This way code using something
+        // like setTimeout() or handling events will get a time that's only updating at around 60fps, or less if
+        // we're throttled.
+        auto lastAnimationSchedulerTimestamp = m_document->animationScheduler().lastTimestamp();
+        auto delta = Seconds(m_document->domWindow()->nowTimestamp()) - lastAnimationSchedulerTimestamp;
+        int frames = std::floor(delta.seconds() / animationInterval().seconds());
+        m_cachedCurrentTime = lastAnimationSchedulerTimestamp + Seconds(frames * animationInterval().seconds());
+#else
         m_cachedCurrentTime = Seconds(m_document->domWindow()->nowTimestamp());
+#endif
+        // We want to be sure to keep this time cached until we've both finished running JS and finished updating
+        // animations, so we schedule the invalidation task and register a whenIdle callback on the VM, which will
+        // fire syncronously if no JS is running.
         scheduleInvalidationTaskIfNeeded();
+        m_waitingOnVMIdle = true;
+        m_document->vm().whenIdle([this]() {
+            m_waitingOnVMIdle = false;
+            maybeClearCachedCurrentTime();
+        });
     }
     return m_cachedCurrentTime;
 }
@@ -175,7 +202,17 @@ void DocumentTimeline::performInvalidationTask()
     applyPendingAcceleratedAnimations();
 
     updateAnimationSchedule();
-    m_cachedCurrentTime = std::nullopt;
+    maybeClearCachedCurrentTime();
+}
+
+void DocumentTimeline::maybeClearCachedCurrentTime()
+{
+    // We want to make sure we only clear the cached current time if we're not currently running
+    // JS or waiting on all current animation updating code to have completed. This is so that
+    // we're guaranteed to have a consistent current time reported for all work happening in a given
+    // JS frame or throughout updating animations in WebCore.
+    if (!m_waitingOnVMIdle && !m_invalidationTaskQueue.hasPendingTasks())
+        m_cachedCurrentTime = std::nullopt;
 }
 
 void DocumentTimeline::updateAnimationSchedule()
index d5f4e85..940d786 100644 (file)
@@ -83,10 +83,12 @@ private:
     void scheduleAnimationResolution();
     void updateAnimations();
     void performEventDispatchTask();
+    void maybeClearCachedCurrentTime();
 
     RefPtr<Document> m_document;
     bool m_paused { false };
     bool m_isSuspended { false };
+    bool m_waitingOnVMIdle { false };
     std::optional<Seconds> m_cachedCurrentTime;
     GenericTaskQueue<Timer> m_invalidationTaskQueue;
     GenericTaskQueue<Timer> m_eventDispatchTaskQueue;
index 86e83fd..bfcf460 100644 (file)
@@ -196,7 +196,8 @@ void ScriptedAnimationController::serviceScriptedAnimations(double timestamp)
 
     TraceScope tracingScope(RAFCallbackStart, RAFCallbackEnd);
 
-    double highResNowMs = 1000 * timestamp;
+    // We round this to the nearest microsecond so that we can return a time that matches what is returned by document.timeline.currentTime.
+    double highResNowMs = std::round(1000 * timestamp);
     double legacyHighResNowMs = 1000 * (timestamp + m_document->loader()->timing().referenceWallTime().secondsSinceEpoch().seconds());
 
     // First, generate a list of callbacks to consider.  Callbacks registered from this point
@@ -297,7 +298,8 @@ void ScriptedAnimationController::animationTimerFired()
 #if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
 void ScriptedAnimationController::documentAnimationSchedulerDidFire()
 {
-    serviceScriptedAnimations(m_document->domWindow()->nowTimestamp());
+    // We obtain the time from the animation scheduler so that we use the same timestamp as the DocumentTimeline.
+    serviceScriptedAnimations(m_document->animationScheduler().lastTimestamp().seconds());
 }
 #endif