Attachment 343269 Details for Bug 183826 – Patch

[patch] Patch

bug-183826-20180621224859.patch (text/plain), 38.60 KB, created by Antoine Quint on 2018-06-21 13:49:00 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Antoine Quint

Created: 2018-06-21 13:49:00 PDT

Size: 38.60 KB

patch

obsolete

>Subversion Revision: 233051
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 0eea90d725deeebf7a726f11be7045b7bf177064..0fb818810593ce8e792293bc501ee43b730e1c87 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,79 @@
>+2018-06-21  Antoine Quint  <graouts@apple.com>
>+
>+        [Web Animations] Make imported/mozilla/css-animations/test_animation-pausing.html pass reliably
>+        https://bugs.webkit.org/show_bug.cgi?id=183826
>+        <rdar://problem/40997412>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        There were two issues preventing the sub-tests in this test file to pass.
>+
>+        The first issue is that the CSS Animations Level 2 specification defines that calling pause() on a CSSAnimation
>+        object is "sticky" until a call to play() is made, meaning that any changes to the running state via the CSS
>+        animation-play-state property is overridden by the stickiness of the pause() call. In this patch we add an
>+        m_stickyPaused flag which is set in API calls to pause() and play(). While this flag is true, changes to the
>+        animation-play-state property to the "running" value are ignored.
>+
>+        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.
>+
>+        In this patch we add a new DocumentAnimationScheduler class which is created by a Document to manage this specific
>+        situation. Now DocumentTimeline and ScriptedAnimationController use this supporting object instead of being their
>+        own DisplayRefreshMonitorClient and call scheduleWebAnimationsResolution() and scheduleScriptedAnimationResolution()
>+        respectively to indicate the need to schedule an animation through the DisplayRefreshMonitorManager to serve the specific
>+        needs of either, or both, classes. Then DocumentAnimationScheduler ensures that Web Animations resolution happens
>+        prior to requestAnimationFrame callbacks when both are scheduled.
>+
>+        In the future we should be able to move more code from DocumentTimeline and ScriptedAnimationController over to
>+        DocumentAnimationScheduler, such as support for throttling and using a timer-based fallback, but this patch provides
>+        the minimal functionality required to provide a sounder foundation.
>+
>+        * Modules/webvr/VRDisplay.cpp:
>+        (WebCore::VRDisplay::requestAnimationFrame):
>+        * Sources.txt:
>+        * WebCore.xcodeproj/project.pbxproj:
>+        * animation/CSSAnimation.cpp:
>+        (WebCore::CSSAnimation::syncPropertiesWithBackingAnimation):
>+        (WebCore::CSSAnimation::bindingsPlay):
>+        (WebCore::CSSAnimation::bindingsPause):
>+        * animation/CSSAnimation.h:
>+        * animation/DocumentAnimationScheduler.cpp: Added.
>+        (WebCore::DocumentAnimationScheduler::create):
>+        (WebCore::DocumentAnimationScheduler::DocumentAnimationScheduler):
>+        (WebCore::DocumentAnimationScheduler::detachFromDocument):
>+        (WebCore::DocumentAnimationScheduler::scheduleWebAnimationsResolution):
>+        (WebCore::DocumentAnimationScheduler::scheduleScriptedAnimationResolution):
>+        (WebCore::DocumentAnimationScheduler::displayRefreshFired):
>+        (WebCore::DocumentAnimationScheduler::windowScreenDidChange):
>+        (WebCore::DocumentAnimationScheduler::createDisplayRefreshMonitor const):
>+        * animation/DocumentAnimationScheduler.h: Copied from Source/WebCore/animation/CSSAnimation.h.
>+        * animation/DocumentTimeline.cpp:
>+        (WebCore::DocumentTimeline::create):
>+        (WebCore::DocumentTimeline::DocumentTimeline):
>+        (WebCore::DocumentTimeline::scheduleAnimationResolution):
>+        (WebCore::DocumentTimeline::windowScreenDidChange): Deleted.
>+        (WebCore::DocumentTimeline::createDisplayRefreshMonitor const): Deleted.
>+        * animation/DocumentTimeline.h:
>+        * dom/Document.cpp:
>+        (WebCore::Document::prepareForDestruction):
>+        (WebCore::Document::windowScreenDidChange):
>+        (WebCore::Document::requestAnimationFrame):
>+        (WebCore::Document::animationScheduler):
>+        (WebCore::Document::timeline):
>+        * dom/Document.h:
>+        * dom/ScriptedAnimationController.cpp:
>+        (WebCore::ScriptedAnimationController::ScriptedAnimationController):
>+        (WebCore::ScriptedAnimationController::scheduleAnimation):
>+        (WebCore::ScriptedAnimationController::documentAnimationSchedulerDidFire):
>+        (WebCore::ScriptedAnimationController::windowScreenDidChange): Deleted.
>+        (WebCore::ScriptedAnimationController::displayRefreshFired): Deleted.
>+        (WebCore::ScriptedAnimationController::createDisplayRefreshMonitor const): Deleted.
>+        * dom/ScriptedAnimationController.h:
>+        (WebCore::ScriptedAnimationController::create):
>+
> 2018-06-20  Antoine Quint  <graouts@apple.com>
> 
>         [Web Animations] Make imported/mozilla/css-animations/test_animation-ready.html pass reliably
>diff --git a/Source/WebCore/Modules/webvr/VRDisplay.cpp b/Source/WebCore/Modules/webvr/VRDisplay.cpp
>index 127cc5c838214a282d30797a76b518cd929192cf..3e4f32e5c2b20b38949092d793c0567eee340913 100644
>--- a/Source/WebCore/Modules/webvr/VRDisplay.cpp
>+++ b/Source/WebCore/Modules/webvr/VRDisplay.cpp
>@@ -110,13 +110,7 @@ uint32_t VRDisplay::requestAnimationFrame(Ref<RequestAnimationFrameCallback>&& c
> {
>     if (!m_scriptedAnimationController) {
>         auto* document = downcast<Document>(scriptExecutionContext());
>-#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>-        // FIXME: Get the display id of the HMD as it should use the HMD native refresh rate.
>-        PlatformDisplayID displayID = document->page() ? document->page()->chrome().displayID() : 0;
>-        m_scriptedAnimationController = ScriptedAnimationController::create(*document, displayID);
>-#else
>-        m_scriptedAnimationController = ScriptedAnimationController::create(*document, 0);
>-#endif
>+        m_scriptedAnimationController = ScriptedAnimationController::create(*document);
>     }
> 
>     return m_scriptedAnimationController->registerCallback(WTFMove(callback));
>diff --git a/Source/WebCore/Sources.txt b/Source/WebCore/Sources.txt
>index 8b02601cb890da9fc4e1d0b9b7d8c34a590e29f3..6210ff2b1ab76aecb91616f67fb7e1736a781afc 100644
>--- a/Source/WebCore/Sources.txt
>+++ b/Source/WebCore/Sources.txt
>@@ -339,6 +339,7 @@ animation/AnimationTimeline.cpp
> animation/CSSAnimation.cpp
> animation/CSSTransition.cpp
> animation/DeclarativeAnimation.cpp
>+animation/DocumentAnimationScheduler.cpp
> animation/DocumentTimeline.cpp
> animation/KeyframeEffect.cpp
> animation/KeyframeEffectReadOnly.cpp
>diff --git a/Source/WebCore/WebCore.xcodeproj/project.pbxproj b/Source/WebCore/WebCore.xcodeproj/project.pbxproj
>index 4566d7cd34498c159d2454d92160e2d72eeb6d7b..703bd6aa4408ed39b437d4e00f765c236716157a 100644
>--- a/Source/WebCore/WebCore.xcodeproj/project.pbxproj
>+++ b/Source/WebCore/WebCore.xcodeproj/project.pbxproj
>@@ -2025,6 +2025,7 @@
> 		71556CBE1F9F0A4900E78D08 /* JSKeyframeEffect.h in Headers */ = {isa = PBXBuildFile; fileRef = 71556CB71F9F09FC00E78D08 /* JSKeyframeEffect.h */; };
> 		715AD7202050513200D592DC /* DeclarativeAnimation.h in Headers */ = {isa = PBXBuildFile; fileRef = 715AD71D2050512400D592DC /* DeclarativeAnimation.h */; settings = {ATTRIBUTES = (Private, ); }; };
> 		715AD7212050513F00D592DC /* CSSTransition.h in Headers */ = {isa = PBXBuildFile; fileRef = 7123C186204739BA00789392 /* CSSTransition.h */; };
>+		716E55B020DBABF100F0CF29 /* DocumentAnimationScheduler.h in Headers */ = {isa = PBXBuildFile; fileRef = 716E55AD20DBABDC00F0CF29 /* DocumentAnimationScheduler.h */; settings = {ATTRIBUTES = (Private, ); }; };
> 		71A1B6081DEE5AD70073BCFB /* modern-media-controls-localized-strings.js in Resources */ = {isa = PBXBuildFile; fileRef = 71A1B6061DEE5A820073BCFB /* modern-media-controls-localized-strings.js */; };
> 		71A57DF2154BE25C0009D120 /* SVGPathUtilities.h in Headers */ = {isa = PBXBuildFile; fileRef = 71A57DF0154BE25C0009D120 /* SVGPathUtilities.h */; };
> 		71B28427203CEC4C0036AA5D /* JSCSSAnimation.h in Headers */ = {isa = PBXBuildFile; fileRef = 71B28426203CEC0D0036AA5D /* JSCSSAnimation.h */; settings = {ATTRIBUTES = (Private, ); }; };
>@@ -9058,6 +9059,8 @@
> 		716C8DF61E48B2B5005BD0DA /* volume-down-fullscreen@2x.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "volume-down-fullscreen@2x.png"; sourceTree = "<group>"; };
> 		716C8DF71E48B2B5005BD0DA /* volume-up-fullscreen@1x.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "volume-up-fullscreen@1x.png"; sourceTree = "<group>"; };
> 		716C8DF81E48B2B5005BD0DA /* volume-up-fullscreen@2x.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "volume-up-fullscreen@2x.png"; sourceTree = "<group>"; };
>+		716E55AD20DBABDC00F0CF29 /* DocumentAnimationScheduler.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DocumentAnimationScheduler.h; sourceTree = "<group>"; };
>+		716E55AF20DBABDD00F0CF29 /* DocumentAnimationScheduler.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = DocumentAnimationScheduler.cpp; sourceTree = "<group>"; };
> 		716FA0D81DB26591007323CC /* airplay-button.css */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.css; path = "airplay-button.css"; sourceTree = "<group>"; };
> 		716FA0D91DB26591007323CC /* airplay-button.js */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.javascript; path = "airplay-button.js"; sourceTree = "<group>"; };
> 		716FA0DA1DB26591007323CC /* airplay-placard.js */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.javascript; path = "airplay-placard.js"; sourceTree = "<group>"; };
>@@ -19506,6 +19509,8 @@
> 				7123C185204739B900789392 /* CSSTransition.idl */,
> 				715AD71F2050512400D592DC /* DeclarativeAnimation.cpp */,
> 				715AD71D2050512400D592DC /* DeclarativeAnimation.h */,
>+				716E55AF20DBABDD00F0CF29 /* DocumentAnimationScheduler.cpp */,
>+				716E55AD20DBABDC00F0CF29 /* DocumentAnimationScheduler.h */,
> 				71025EC41F99F096004A250C /* DocumentTimeline.cpp */,
> 				71025EC51F99F096004A250C /* DocumentTimeline.h */,
> 				71025ECA1F99F096004A250C /* DocumentTimeline.idl */,
>@@ -27678,6 +27683,7 @@
> 				B2F34FE60E82F81400F627CD /* DNS.h in Headers */,
> 				7EE6846F12D26E3800E73215 /* DNSResolveQueueCFNet.h in Headers */,
> 				A8185F4009765766005826D9 /* Document.h in Headers */,
>+				716E55B020DBABF100F0CF29 /* DocumentAnimationScheduler.h in Headers */,
> 				A3BB59F41457A40D00AC56FE /* DocumentEventQueue.h in Headers */,
> 				A8185F3D09765766005826D9 /* DocumentFragment.h in Headers */,
> 				46E1666E1FCC86A200C9710B /* DocumentIdentifier.h in Headers */,
>diff --git a/Source/WebCore/animation/CSSAnimation.cpp b/Source/WebCore/animation/CSSAnimation.cpp
>index abb6dad408008c85e69d1d61b1c6344b637e688b..931b3bfc9fb2218bd07c79d68a3a97c5e12e3abd 100644
>--- a/Source/WebCore/animation/CSSAnimation.cpp
>+++ b/Source/WebCore/animation/CSSAnimation.cpp
>@@ -91,9 +91,10 @@ void CSSAnimation::syncPropertiesWithBackingAnimation()
>     timing->setIterationDuration(Seconds(animation.duration()));
> 
>     // Synchronize the play state
>-    if (animation.playState() == AnimationPlayState::Playing && playState() == WebAnimation::PlayState::Paused)
>-        play();
>-    else if (animation.playState() == AnimationPlayState::Paused && playState() == WebAnimation::PlayState::Running)
>+    if (animation.playState() == AnimationPlayState::Playing && playState() == WebAnimation::PlayState::Paused) {
>+        if (!m_stickyPaused)
>+            play();
>+    } else if (animation.playState() == AnimationPlayState::Paused && playState() == WebAnimation::PlayState::Running)
>         pause();
> 
>     unsuspendEffectInvalidation();
>@@ -153,12 +154,14 @@ WebAnimation::FinishedPromise& CSSAnimation::bindingsFinished()
> ExceptionOr<void> CSSAnimation::bindingsPlay()
> {
>     flushPendingStyleChanges();
>+    m_stickyPaused = false;
>     return DeclarativeAnimation::bindingsPlay();
> }
> 
> ExceptionOr<void> CSSAnimation::bindingsPause()
> {
>     flushPendingStyleChanges();
>+    m_stickyPaused = true;
>     return DeclarativeAnimation::bindingsPause();
> }
> 
>diff --git a/Source/WebCore/animation/CSSAnimation.h b/Source/WebCore/animation/CSSAnimation.h
>index 3db1053db5360e2b382a278b8681b60453e2071d..07a42e188d6515f8a97f7ce473dfc5bcc050b01e 100644
>--- a/Source/WebCore/animation/CSSAnimation.h
>+++ b/Source/WebCore/animation/CSSAnimation.h
>@@ -64,6 +64,7 @@ private:
> 
>     String m_animationName;
>     std::unique_ptr<RenderStyle> m_unanimatedStyle;
>+    bool m_stickyPaused { false };
> };
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/animation/DocumentAnimationScheduler.cpp b/Source/WebCore/animation/DocumentAnimationScheduler.cpp
>new file mode 100644
>index 0000000000000000000000000000000000000000..57cbb2326a2968b9e92024a4a6732bab713ce2aa
>--- /dev/null
>+++ b/Source/WebCore/animation/DocumentAnimationScheduler.cpp
>@@ -0,0 +1,105 @@
>+/*
>+ * Copyright (C) 2018 Apple Inc. All rights reserved.
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ * 1. Redistributions of source code must retain the above copyright
>+ *    notice, this list of conditions and the following disclaimer.
>+ * 2. Redistributions in binary form must reproduce the above copyright
>+ *    notice, this list of conditions and the following disclaimer in the
>+ *    documentation and/or other materials provided with the distribution.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
>+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
>+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
>+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
>+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
>+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
>+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>+ */
>+
>+#include "config.h"
>+#include "DocumentAnimationScheduler.h"
>+
>+#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>+
>+#include "Chrome.h"
>+#include "ChromeClient.h"
>+#include "DOMWindow.h"
>+#include "DisplayRefreshMonitor.h"
>+#include "DisplayRefreshMonitorManager.h"
>+#include "Document.h"
>+#include "DocumentTimeline.h"
>+#include "Page.h"
>+#include "ScriptedAnimationController.h"
>+
>+namespace WebCore {
>+
>+Ref<DocumentAnimationScheduler> DocumentAnimationScheduler::create(Document& document, PlatformDisplayID displayID)
>+{
>+    return adoptRef(*new DocumentAnimationScheduler(document, displayID));
>+}
>+
>+DocumentAnimationScheduler::DocumentAnimationScheduler(Document& document, PlatformDisplayID displayID)
>+    : m_document(&document)
>+{
>+    windowScreenDidChange(displayID);
>+}
>+
>+DocumentAnimationScheduler::~DocumentAnimationScheduler() = default;
>+
>+void DocumentAnimationScheduler::detachFromDocument()
>+{
>+    m_document = nullptr;
>+}
>+
>+bool DocumentAnimationScheduler::scheduleWebAnimationsResolution()
>+{
>+    m_scheduledWebAnimationsResolution = true;
>+    return DisplayRefreshMonitorManager::sharedManager().scheduleAnimation(*this);
>+}
>+
>+bool DocumentAnimationScheduler::scheduleScriptedAnimationResolution()
>+{
>+    m_scheduledScriptedAnimationResolution = true;
>+    return DisplayRefreshMonitorManager::sharedManager().scheduleAnimation(*this);
>+}
>+
>+void DocumentAnimationScheduler::displayRefreshFired()
>+{
>+    if (m_scheduledWebAnimationsResolution) {
>+        m_scheduledWebAnimationsResolution = false;
>+        m_document->timeline().documentAnimationSchedulerDidFire();
>+    }
>+
>+    if (m_scheduledScriptedAnimationResolution) {
>+        m_scheduledScriptedAnimationResolution = false;
>+        if (auto* scriptedAnimationController = m_document->scriptedAnimationController())
>+            scriptedAnimationController->documentAnimationSchedulerDidFire();
>+    }
>+}
>+
>+void DocumentAnimationScheduler::windowScreenDidChange(PlatformDisplayID displayID)
>+{
>+    DisplayRefreshMonitorManager::sharedManager().windowScreenDidChange(displayID, *this);
>+}
>+
>+RefPtr<DisplayRefreshMonitor> DocumentAnimationScheduler::createDisplayRefreshMonitor(PlatformDisplayID displayID) const
>+{
>+    if (!m_document || !m_document->page())
>+        return nullptr;
>+
>+    if (auto monitor = m_document->page()->chrome().client().createDisplayRefreshMonitor(displayID))
>+        return monitor;
>+
>+    return DisplayRefreshMonitor::createDefaultDisplayRefreshMonitor(displayID);
>+}
>+
>+} // namespace WebCore
>+
>+#endif // USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>diff --git a/Source/WebCore/animation/DocumentAnimationScheduler.h b/Source/WebCore/animation/DocumentAnimationScheduler.h
>new file mode 100644
>index 0000000000000000000000000000000000000000..ff753552e68c739b01ff51c24b2e6df373d7b5e5
>--- /dev/null
>+++ b/Source/WebCore/animation/DocumentAnimationScheduler.h
>@@ -0,0 +1,65 @@
>+/*
>+ * Copyright (C) 2018 Apple Inc. All rights reserved.
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ * 1. Redistributions of source code must retain the above copyright
>+ *    notice, this list of conditions and the following disclaimer.
>+ * 2. Redistributions in binary form must reproduce the above copyright
>+ *    notice, this list of conditions and the following disclaimer in the
>+ *    documentation and/or other materials provided with the distribution.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
>+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
>+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
>+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
>+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
>+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
>+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>+ */
>+
>+#pragma once
>+
>+#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>+
>+#include "DisplayRefreshMonitorClient.h"
>+#include "PlatformScreen.h"
>+#include <wtf/Ref.h>
>+#include <wtf/RefCounted.h>
>+#include <wtf/RefPtr.h>
>+
>+namespace WebCore {
>+
>+class Document;
>+
>+class DocumentAnimationScheduler : public RefCounted<DocumentAnimationScheduler>
>+    , public DisplayRefreshMonitorClient {
>+public:
>+    static Ref<DocumentAnimationScheduler> create(Document&, PlatformDisplayID);
>+    ~DocumentAnimationScheduler();
>+
>+    void detachFromDocument();
>+    void windowScreenDidChange(PlatformDisplayID);
>+
>+    bool scheduleWebAnimationsResolution();
>+    bool scheduleScriptedAnimationResolution();
>+
>+private:
>+    DocumentAnimationScheduler(Document&, PlatformDisplayID);
>+
>+    RefPtr<Document> m_document;
>+    bool m_scheduledWebAnimationsResolution { false };
>+    bool m_scheduledScriptedAnimationResolution { false };
>+
>+    void displayRefreshFired() override;
>+    RefPtr<DisplayRefreshMonitor> createDisplayRefreshMonitor(PlatformDisplayID) const override;
>+};
>+
>+} // namespace WebCore
>+
>+#endif // USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>diff --git a/Source/WebCore/animation/DocumentTimeline.cpp b/Source/WebCore/animation/DocumentTimeline.cpp
>index 0649696616ad0410a49351a69facb72c5b3efeb7..5ab209827fa661488f936043abad56b8b92522aa 100644
>--- a/Source/WebCore/animation/DocumentTimeline.cpp
>+++ b/Source/WebCore/animation/DocumentTimeline.cpp
>@@ -27,12 +27,8 @@
> #include "DocumentTimeline.h"
> 
> #include "AnimationPlaybackEvent.h"
>-#include "Chrome.h"
>-#include "ChromeClient.h"
> #include "DOMWindow.h"
> #include "DeclarativeAnimation.h"
>-#include "DisplayRefreshMonitor.h"
>-#include "DisplayRefreshMonitorManager.h"
> #include "Document.h"
> #include "KeyframeEffect.h"
> #include "Page.h"
>@@ -43,12 +39,12 @@ static const Seconds throttledAnimationInterval { 30_ms };
> 
> namespace WebCore {
> 
>-Ref<DocumentTimeline> DocumentTimeline::create(Document& document, PlatformDisplayID displayID)
>+Ref<DocumentTimeline> DocumentTimeline::create(Document& document)
> {
>-    return adoptRef(*new DocumentTimeline(document, displayID));
>+    return adoptRef(*new DocumentTimeline(document));
> }
> 
>-DocumentTimeline::DocumentTimeline(Document& document, PlatformDisplayID displayID)
>+DocumentTimeline::DocumentTimeline(Document& document)
>     : AnimationTimeline(DocumentTimelineClass)
>     , m_document(&document)
>     , m_animationScheduleTimer(*this, &DocumentTimeline::animationScheduleTimerFired)
>@@ -56,7 +52,6 @@ DocumentTimeline::DocumentTimeline(Document& document, PlatformDisplayID display
>     , m_animationResolutionTimer(*this, &DocumentTimeline::animationResolutionTimerFired)
> #endif
> {
>-    windowScreenDidChange(displayID);
> }
> 
> DocumentTimeline::~DocumentTimeline()
>@@ -218,7 +213,7 @@ void DocumentTimeline::animationScheduleTimerFired()
> void DocumentTimeline::scheduleAnimationResolution()
> {
> #if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>-    DisplayRefreshMonitorManager::sharedManager().scheduleAnimation(*this);
>+    m_document->animationScheduler().scheduleWebAnimationsResolution();
> #else
>     // FIXME: We need to use the same logic as ScriptedAnimationController here,
>     // which will be addressed by the refactor tracked by webkit.org/b/179293.
>@@ -227,7 +222,7 @@ void DocumentTimeline::scheduleAnimationResolution()
> }
> 
> #if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>-void DocumentTimeline::displayRefreshFired()
>+void DocumentTimeline::documentAnimationSchedulerDidFire()
> #else
> void DocumentTimeline::animationResolutionTimerFired()
> #endif
>@@ -392,26 +387,4 @@ void DocumentTimeline::performEventDispatchTask()
>         pendingEvent->target()->dispatchEvent(pendingEvent);
> }
> 
>-void DocumentTimeline::windowScreenDidChange(PlatformDisplayID displayID)
>-{
>-#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>-    DisplayRefreshMonitorManager::sharedManager().windowScreenDidChange(displayID, *this);
>-#else
>-    UNUSED_PARAM(displayID);
>-#endif
>-}
>-
>-#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>-RefPtr<DisplayRefreshMonitor> DocumentTimeline::createDisplayRefreshMonitor(PlatformDisplayID displayID) const
>-{
>-    if (!m_document || !m_document->page())
>-        return nullptr;
>-
>-    if (auto monitor = m_document->page()->chrome().client().createDisplayRefreshMonitor(displayID))
>-        return monitor;
>-
>-    return DisplayRefreshMonitor::createDefaultDisplayRefreshMonitor(displayID);
>-}
>-#endif
>-
> } // namespace WebCore
>diff --git a/Source/WebCore/animation/DocumentTimeline.h b/Source/WebCore/animation/DocumentTimeline.h
>index 1ba448b2e8c73e1402b27bc91948139b6e5b5c98..d5f4e851d7c09693aeb91ef22544f0ee3b1be093 100644
>--- a/Source/WebCore/animation/DocumentTimeline.h
>+++ b/Source/WebCore/animation/DocumentTimeline.h
>@@ -27,26 +27,18 @@
> 
> #include "AnimationTimeline.h"
> #include "GenericTaskQueue.h"
>-#include "PlatformScreen.h"
> #include "Timer.h"
> #include <wtf/Ref.h>
> 
>-#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>-#include "DisplayRefreshMonitorClient.h"
>-#endif
>-
> namespace WebCore {
> 
> class AnimationPlaybackEvent;
> class RenderElement;
> 
> class DocumentTimeline final : public AnimationTimeline
>-#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>-    , public DisplayRefreshMonitorClient
>-#endif
> {
> public:
>-    static Ref<DocumentTimeline> create(Document&, PlatformDisplayID);
>+    static Ref<DocumentTimeline> create(Document&);
>     ~DocumentTimeline();
> 
>     Document* document() const { return m_document.get(); }
>@@ -55,7 +47,6 @@ public:
>     void pause() override;
> 
>     void timingModelDidChange() override;
>-    void windowScreenDidChange(PlatformDisplayID);
> 
>     // If possible, compute the visual extent of any transform animation on the given renderer
>     // using the given rect, returning the result in the rect. Return false if there is some
>@@ -71,6 +62,10 @@ public:
> 
>     void enqueueAnimationPlaybackEvent(AnimationPlaybackEvent&);
> 
>+#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>+    void documentAnimationSchedulerDidFire();
>+#endif
>+
>     void updateThrottlingState();
>     WEBCORE_EXPORT Seconds animationInterval() const;
>     WEBCORE_EXPORT void suspendAnimations();
>@@ -79,7 +74,7 @@ public:
>     WEBCORE_EXPORT unsigned numberOfActiveAnimationsForTesting() const;
> 
> private:
>-    DocumentTimeline(Document&, PlatformDisplayID);
>+    DocumentTimeline(Document&);
> 
>     void scheduleInvalidationTaskIfNeeded();
>     void performInvalidationTask();
>@@ -100,11 +95,7 @@ private:
>     HashSet<RefPtr<WebAnimation>> m_acceleratedAnimationsPendingRunningStateChange;
>     Vector<Ref<AnimationPlaybackEvent>> m_pendingAnimationEvents;
> 
>-#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>-    // Override for DisplayRefreshMonitorClient
>-    void displayRefreshFired() override;
>-    RefPtr<DisplayRefreshMonitor> createDisplayRefreshMonitor(PlatformDisplayID) const override;
>-#else
>+#if !USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>     void animationResolutionTimerFired();
>     Timer m_animationResolutionTimer;
> #endif
>diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
>index 6873146b6e3f03bf453beb7b0aa09fb0c75fc381..2269f150acdaf1ceb0ee3b30a300fdb3bb1170f5 100644
>--- a/Source/WebCore/dom/Document.cpp
>+++ b/Source/WebCore/dom/Document.cpp
>@@ -56,6 +56,7 @@
> #include "DOMWindow.h"
> #include "DateComponents.h"
> #include "DebugPageOverlays.h"
>+#include "DocumentAnimationScheduler.h"
> #include "DocumentLoader.h"
> #include "DocumentMarkerController.h"
> #include "DocumentSharedObjectPool.h"
>@@ -2443,6 +2444,11 @@ void Document::prepareForDestruction()
>         m_timeline = nullptr;
>     }
> 
>+    if (m_animationScheduler) {
>+        m_animationScheduler->detachFromDocument();
>+        m_animationScheduler = nullptr;
>+    }
>+
>     m_hasPreparedForDestruction = true;
> 
>     // Note that m_pageCacheState can be Document::AboutToEnterPageCache if our frame
>@@ -5910,11 +5916,8 @@ void Document::resumeScriptedAnimationControllerCallbacks()
> 
> void Document::windowScreenDidChange(PlatformDisplayID displayID)
> {
>-    if (m_scriptedAnimationController)
>-        m_scriptedAnimationController->windowScreenDidChange(displayID);
>-
>-    if (m_timeline)
>-        m_timeline->windowScreenDidChange(displayID);
>+    if (m_animationScheduler)
>+        m_animationScheduler->windowScreenDidChange(displayID);
> 
>     if (RenderView* view = renderView()) {
>         if (view->usesCompositing())
>@@ -6500,11 +6503,8 @@ double Document::monotonicTimestamp() const
> int Document::requestAnimationFrame(Ref<RequestAnimationFrameCallback>&& callback)
> {
>     if (!m_scriptedAnimationController) {
>-#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>-        m_scriptedAnimationController = ScriptedAnimationController::create(*this, page() ? page()->chrome().displayID() : 0);
>-#else
>-        m_scriptedAnimationController = ScriptedAnimationController::create(*this, 0);
>-#endif
>+        m_scriptedAnimationController = ScriptedAnimationController::create(*this);
>+
>         // It's possible that the Page may have suspended scripted animations before
>         // we were created. We need to make sure that we don't start up the animation
>         // controller on a background tab, for example.
>@@ -7691,10 +7691,18 @@ void Document::setConsoleMessageListener(RefPtr<StringCallback>&& listener)
>     m_consoleMessageListener = listener;
> }
> 
>+DocumentAnimationScheduler& Document::animationScheduler()
>+{
>+    if (!m_animationScheduler)
>+        m_animationScheduler = DocumentAnimationScheduler::create(*this, page() ? page()->chrome().displayID() : 0);
>+
>+    return *m_animationScheduler;
>+}
>+
> DocumentTimeline& Document::timeline()
> {
>     if (!m_timeline)
>-        m_timeline = DocumentTimeline::create(*this, page() ? page()->chrome().displayID() : 0);
>+        m_timeline = DocumentTimeline::create(*this);
> 
>     return *m_timeline;
> }
>diff --git a/Source/WebCore/dom/Document.h b/Source/WebCore/dom/Document.h
>index 6ed1e6879ca1e1d89af89b21a2dd0ff65fe3c990..95626f9e4da138274439c38ba14629e9fea8c5d4 100644
>--- a/Source/WebCore/dom/Document.h
>+++ b/Source/WebCore/dom/Document.h
>@@ -80,6 +80,7 @@ class InputCursor;
> 
> namespace WebCore {
> 
>+class DocumentAnimationScheduler;
> class ApplicationStateChangeListener;
> class AXObjectCache;
> class Attr;
>@@ -1400,6 +1401,8 @@ public:
> 
>     WEBCORE_EXPORT void setConsoleMessageListener(RefPtr<StringCallback>&&); // For testing.
> 
>+    DocumentAnimationScheduler& animationScheduler();
>+
>     WEBCORE_EXPORT DocumentTimeline& timeline();
>     DocumentTimeline* existingTimeline() const { return m_timeline.get(); }
>     Vector<RefPtr<WebAnimation>> getAnimations();
>@@ -1912,6 +1915,7 @@ private:
>     bool m_hasFrameSpecificStorageAccess { false };
>     bool m_grantStorageAccessOverride { false };
> 
>+    RefPtr<DocumentAnimationScheduler> m_animationScheduler;
>     RefPtr<DocumentTimeline> m_timeline;
>     DocumentIdentifier m_identifier;
> 
>diff --git a/Source/WebCore/dom/ScriptedAnimationController.cpp b/Source/WebCore/dom/ScriptedAnimationController.cpp
>index c4d85d83494deadf88f01d4a5c71c17bde9f6484..93b1765700ab165883f4fdf78d7a53f91e46a136 100644
>--- a/Source/WebCore/dom/ScriptedAnimationController.cpp
>+++ b/Source/WebCore/dom/ScriptedAnimationController.cpp
>@@ -29,9 +29,8 @@
> #include "Chrome.h"
> #include "ChromeClient.h"
> #include "DOMWindow.h"
>-#include "DisplayRefreshMonitor.h"
>-#include "DisplayRefreshMonitorManager.h"
> #include "Document.h"
>+#include "DocumentAnimationScheduler.h"
> #include "DocumentLoader.h"
> #include "Frame.h"
> #include "FrameView.h"
>@@ -55,11 +54,10 @@ static const Seconds aggressiveThrottlingAnimationInterval { 10_s };
> 
> namespace WebCore {
> 
>-ScriptedAnimationController::ScriptedAnimationController(Document& document, PlatformDisplayID displayID)
>+ScriptedAnimationController::ScriptedAnimationController(Document& document)
>     : m_document(&document)
>     , m_animationTimer(*this, &ScriptedAnimationController::animationTimerFired)
> {
>-    windowScreenDidChange(displayID);
> }
> 
> ScriptedAnimationController::~ScriptedAnimationController() = default;
>@@ -234,17 +232,6 @@ void ScriptedAnimationController::serviceScriptedAnimations(double timestamp)
>         scheduleAnimation();
> }
> 
>-void ScriptedAnimationController::windowScreenDidChange(PlatformDisplayID displayID)
>-{
>-    if (!requestAnimationFrameEnabled())
>-        return;
>-#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>-    DisplayRefreshMonitorManager::sharedManager().windowScreenDidChange(displayID, *this);
>-#else
>-    UNUSED_PARAM(displayID);
>-#endif
>-}
>-
> Seconds ScriptedAnimationController::interval() const
> {
> #if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>@@ -274,7 +261,7 @@ void ScriptedAnimationController::scheduleAnimation()
> 
> #if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>     if (!m_isUsingTimer && !isThrottled()) {
>-        if (DisplayRefreshMonitorManager::sharedManager().scheduleAnimation(*this))
>+        if (m_document->animationScheduler().scheduleScriptedAnimationResolution())
>             return;
> 
>         m_isUsingTimer = true;
>@@ -308,21 +295,10 @@ void ScriptedAnimationController::animationTimerFired()
> }
> 
> #if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>-void ScriptedAnimationController::displayRefreshFired()
>+void ScriptedAnimationController::documentAnimationSchedulerDidFire()
> {
>     serviceScriptedAnimations(m_document->domWindow()->nowTimestamp());
> }
>-
>-RefPtr<DisplayRefreshMonitor> ScriptedAnimationController::createDisplayRefreshMonitor(PlatformDisplayID displayID) const
>-{
>-    if (!m_document->page())
>-        return nullptr;
>-
>-    if (auto monitor = m_document->page()->chrome().client().createDisplayRefreshMonitor(displayID))
>-        return monitor;
>-
>-    return DisplayRefreshMonitor::createDefaultDisplayRefreshMonitor(displayID);
>-}
> #endif
> 
> }
>diff --git a/Source/WebCore/dom/ScriptedAnimationController.h b/Source/WebCore/dom/ScriptedAnimationController.h
>index 06d8aa8a499f43cb46d5800ba4dab6a5843747f8..7ab8ec14a007b735f850834cc4dc123e9e6a9f91 100644
>--- a/Source/WebCore/dom/ScriptedAnimationController.h
>+++ b/Source/WebCore/dom/ScriptedAnimationController.h
>@@ -25,17 +25,12 @@
> 
> #pragma once
> 
>-#include "PlatformScreen.h"
> #include "Timer.h"
> #include <wtf/OptionSet.h>
> #include <wtf/RefCounted.h>
> #include <wtf/RefPtr.h>
> #include <wtf/Vector.h>
> 
>-#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>-#include "DisplayRefreshMonitorClient.h"
>-#endif
>-
> namespace WebCore {
> 
> class Document;
>@@ -43,14 +38,11 @@ class Page;
> class RequestAnimationFrameCallback;
> 
> class ScriptedAnimationController : public RefCounted<ScriptedAnimationController>
>-#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>-    , public DisplayRefreshMonitorClient
>-#endif
> {
> public:
>-    static Ref<ScriptedAnimationController> create(Document& document, PlatformDisplayID displayID)
>+    static Ref<ScriptedAnimationController> create(Document& document)
>     {
>-        return adoptRef(*new ScriptedAnimationController(document, displayID));
>+        return adoptRef(*new ScriptedAnimationController(document));
>     }
>     ~ScriptedAnimationController();
>     void clearDocumentPointer() { m_document = nullptr; }
>@@ -77,19 +69,16 @@ public:
>     WEBCORE_EXPORT bool isThrottled() const;
>     WEBCORE_EXPORT Seconds interval() const;
> 
>-    void windowScreenDidChange(PlatformDisplayID);
>+#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>+    void documentAnimationSchedulerDidFire();
>+#endif
> 
> private:
>-    ScriptedAnimationController(Document&, PlatformDisplayID);
>+    ScriptedAnimationController(Document&);
> 
>     void scheduleAnimation();
>     void animationTimerFired();
> 
>-#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>-    // Override for DisplayRefreshMonitorClient
>-    void displayRefreshFired() override;
>-#endif
>-
>     Page* page() const;
> 
>     typedef Vector<RefPtr<RequestAnimationFrameCallback>> CallbackList;
>@@ -103,7 +92,6 @@ private:
>     double m_lastAnimationFrameTimestamp { 0 };
> 
> #if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
>-    RefPtr<DisplayRefreshMonitor> createDisplayRefreshMonitor(PlatformDisplayID) const override;
>     OptionSet<ThrottlingReason> m_throttlingReasons;
>     bool m_isUsingTimer { false };
> #endif
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 6af8aebb7ed621fa0f250d43940c98db161b419e..03e73f32dd8ae6a2b01140490aa9c9d3963039c0 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,15 @@
>+2018-06-21  Antoine Quint  <graouts@apple.com>
>+
>+        [Web Animations] Make imported/mozilla/css-animations/test_animation-pausing.html pass reliably
>+        https://bugs.webkit.org/show_bug.cgi?id=183826
>+        <rdar://problem/40997412>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This test now passes reliably.
>+
>+        * TestExpectations:
>+
> 2018-06-20  Antoine Quint  <graouts@apple.com>
> 
>         [Web Animations] Make imported/mozilla/css-animations/test_animation-ready.html pass reliably
>diff --git a/LayoutTests/imported/mozilla/ChangeLog b/LayoutTests/imported/mozilla/ChangeLog
>index a1e687993176b5ae0d3942a8da65fe3d5abeee2c..7dc985df9eaa8ef0c0dee3e3ff32abf02857ab36 100644
>--- a/LayoutTests/imported/mozilla/ChangeLog
>+++ b/LayoutTests/imported/mozilla/ChangeLog
>@@ -1,3 +1,15 @@
>+2018-06-21  Antoine Quint  <graouts@apple.com>
>+
>+        [Web Animations] Make imported/mozilla/css-animations/test_animation-pausing.html pass reliably
>+        https://bugs.webkit.org/show_bug.cgi?id=183826
>+        <rdar://problem/40997412>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Mark progressions in the Mozilla CSS Animations tests.
>+
>+        * css-animations/test_animation-pausing-expected.txt:
>+
> 2018-06-20  Antoine Quint  <graouts@apple.com>
> 
>         [Web Animations] Make imported/mozilla/css-animations/test_animation-ready.html pass reliably
>diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
>index 68cce6c6b09ddab8a8d6ee760af8380ea9691d26..dbc32b297fc1e37c65d6f39a44d214b89b5a0a2b 100644
>--- a/LayoutTests/TestExpectations
>+++ b/LayoutTests/TestExpectations
>@@ -1926,7 +1926,6 @@ webkit.org/b/181122 imported/w3c/web-platform-tests/web-animations/interfaces/An
> webkit.org/b/181123 imported/w3c/web-platform-tests/web-animations/interfaces/Animation/finish.html [ Pass Failure ]
> webkit.org/b/181888 imported/w3c/web-platform-tests/web-animations/timing-model/animation-effects/current-iteration.html [ Pass Failure ]
> 
>-webkit.org/b/183826 imported/mozilla/css-animations/test_animation-pausing.html [ Pass Failure Timeout ]
> webkit.org/b/183834 imported/mozilla/css-animations/test_animation-starttime.html [ Pass Failure Timeout ]
> webkit.org/b/183836 imported/mozilla/css-animations/test_animations-dynamic-changes.html [ Pass Failure Timeout ]
> webkit.org/b/183837 imported/mozilla/css-transitions/test_document-get-animations.html [ Pass Failure Timeout ]
>diff --git a/LayoutTests/imported/mozilla/css-animations/test_animation-pausing-expected.txt b/LayoutTests/imported/mozilla/css-animations/test_animation-pausing-expected.txt
>index da2838c9ab2296173df868df7e9e7daca6834166..3534e7bf05c6569100eb118236c032ebb6046629 100644
>--- a/LayoutTests/imported/mozilla/css-animations/test_animation-pausing-expected.txt
>+++ b/LayoutTests/imported/mozilla/css-animations/test_animation-pausing-expected.txt
>@@ -1,8 +1,8 @@
> 
> PASS play() overrides animation-play-state 
>-FAIL pause() overrides animation-play-state undefined is not an object (evaluating 'animation.pause')
>-FAIL play() is overridden by later setting "animation-play-state: paused" undefined is not an object (evaluating 'animation.play')
>-FAIL play() flushes pending changes to animation-play-state first assert_greater_than: Playing value of margin-left is increasing expected a number greater than 0 but got 0
>-FAIL pause() applies pending changes to animation-play-state first undefined is not an object (evaluating 'animation.pause')
>-FAIL Setting the current time completes a pending pause assert_true: Animation is pause-pending expected true got false
>+PASS pause() overrides animation-play-state 
>+PASS play() is overridden by later setting "animation-play-state: paused" 
>+PASS play() flushes pending changes to animation-play-state first 
>+PASS pause() applies pending changes to animation-play-state first 
>+PASS Setting the current time completes a pending pause 
>

Flags:

dino: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 183826: 343269