Attachment 343153 Details for Bug 183830 – Patch

[patch] Patch

bug-183830-20180620143143.patch (text/plain), 13.80 KB, created by Antoine Quint on 2018-06-20 05:31:44 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Antoine Quint

Created: 2018-06-20 05:31:44 PDT

Size: 13.80 KB

patch

obsolete

>Subversion Revision: 233010
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index d9d658fb7c5726201ca62cd2170c8345e5dbe6ac..7853d9a0faddc9ef4b363cf083b26e004bf9eec1 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,37 @@
>+2018-06-20  Antoine Quint  <graouts@apple.com>
>+
>+        [Web Animations] Make imported/mozilla/css-animations/test_animation-ready.html pass reliably
>+        https://bugs.webkit.org/show_bug.cgi?id=183830
>+        <rdar://problem/40997539>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The CSS Animations Level 2 spec explains how the Web Animations API and CSS Animations interact and requires
>+        pending styles changes on the target element of a CSS Animation to be flushed when using a Web Animations API
>+        on it in order to ensure that any animation- CSS property is accounted for.
>+
>+        * animation/CSSAnimation.cpp:
>+        (WebCore::CSSAnimation::bindingsStartTime const):
>+        (WebCore::CSSAnimation::setBindingsStartTime):
>+        (WebCore::CSSAnimation::bindingsCurrentTime const):
>+        (WebCore::CSSAnimation::setBindingsCurrentTime):
>+        (WebCore::CSSAnimation::bindingsPlayState const):
>+        (WebCore::CSSAnimation::bindingsPending const):
>+        (WebCore::CSSAnimation::bindingsReady):
>+        (WebCore::CSSAnimation::bindingsFinished):
>+        (WebCore::CSSAnimation::bindingsPlay):
>+        (WebCore::CSSAnimation::bindingsPause):
>+        (WebCore::CSSAnimation::flushPendingStyleChanges const):
>+        * animation/CSSAnimation.h:
>+        * animation/WebAnimation.h:
>+        (WebCore::WebAnimation::bindingsPlayState const):
>+        (WebCore::WebAnimation::bindingsPending const):
>+        (WebCore::WebAnimation::bindingsReady):
>+        (WebCore::WebAnimation::bindingsFinished):
>+        (WebCore::WebAnimation::bindingsPlay):
>+        (WebCore::WebAnimation::bindingsPause):
>+        * animation/WebAnimation.idl:
>+
> 2018-06-20  Antoine Quint  <graouts@apple.com>
> 
>         [Web Animations] Make imported/mozilla/css-transitions/test_element-get-animations.html pass reliably
>diff --git a/Source/WebCore/animation/CSSAnimation.cpp b/Source/WebCore/animation/CSSAnimation.cpp
>index 8d5dfc7c35d798101b9a3915acd3c5be4800b44e..abb6dad408008c85e69d1d61b1c6344b637e688b 100644
>--- a/Source/WebCore/animation/CSSAnimation.cpp
>+++ b/Source/WebCore/animation/CSSAnimation.cpp
>@@ -99,25 +99,77 @@ void CSSAnimation::syncPropertiesWithBackingAnimation()
>     unsuspendEffectInvalidation();
> }
> 
>+std::optional<double> CSSAnimation::bindingsStartTime() const
>+{
>+    flushPendingStyleChanges();
>+    return DeclarativeAnimation::bindingsStartTime();
>+}
>+
>+void CSSAnimation::setBindingsStartTime(std::optional<double> startTime)
>+{
>+    flushPendingStyleChanges();
>+    return DeclarativeAnimation::setBindingsStartTime(startTime);
>+}
>+
> std::optional<double> CSSAnimation::bindingsCurrentTime() const
> {
>-    auto currentTime = WebAnimation::bindingsCurrentTime();
>+    flushPendingStyleChanges();
>+    auto currentTime = DeclarativeAnimation::bindingsCurrentTime();
>     if (currentTime)
>         return std::max(0.0, std::min(currentTime.value(), effect()->timing()->activeDuration().milliseconds()));
>     return currentTime;
> }
> 
>+ExceptionOr<void> CSSAnimation::setBindingsCurrentTime(std::optional<double> currentTime)
>+{
>+    flushPendingStyleChanges();
>+    return DeclarativeAnimation::setBindingsCurrentTime(currentTime);
>+}
>+
> WebAnimation::PlayState CSSAnimation::bindingsPlayState() const
> {
>-    // Since an animation's play state can be set via the animation-play-state property,
>-    // we need to account for any pending CSS changes first.
>+    flushPendingStyleChanges();
>+    return DeclarativeAnimation::bindingsPlayState();
>+}
>+
>+bool CSSAnimation::bindingsPending() const
>+{
>+    flushPendingStyleChanges();
>+    return DeclarativeAnimation::bindingsPending();
>+}
>+
>+WebAnimation::ReadyPromise& CSSAnimation::bindingsReady()
>+{
>+    flushPendingStyleChanges();
>+    return DeclarativeAnimation::bindingsReady();
>+}
>+
>+WebAnimation::FinishedPromise& CSSAnimation::bindingsFinished()
>+{
>+    flushPendingStyleChanges();
>+    return DeclarativeAnimation::bindingsFinished();
>+}
>+
>+ExceptionOr<void> CSSAnimation::bindingsPlay()
>+{
>+    flushPendingStyleChanges();
>+    return DeclarativeAnimation::bindingsPlay();
>+}
>+
>+ExceptionOr<void> CSSAnimation::bindingsPause()
>+{
>+    flushPendingStyleChanges();
>+    return DeclarativeAnimation::bindingsPause();
>+}
>+
>+void CSSAnimation::flushPendingStyleChanges() const
>+{
>     if (auto* animationEffect = effect()) {
>         if (is<KeyframeEffectReadOnly>(animationEffect)) {
>             if (auto* target = downcast<KeyframeEffectReadOnly>(animationEffect)->target())
>                 target->document().updateStyleIfNeeded();
>         }
>     }
>-    return DeclarativeAnimation::bindingsPlayState();
> }
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/animation/CSSAnimation.h b/Source/WebCore/animation/CSSAnimation.h
>index d4ec5adfed727e574357860af39d5d8dd326deaa..3db1053db5360e2b382a278b8681b60453e2071d 100644
>--- a/Source/WebCore/animation/CSSAnimation.h
>+++ b/Source/WebCore/animation/CSSAnimation.h
>@@ -43,8 +43,16 @@ public:
>     const String& animationName() const { return m_animationName; }
>     const RenderStyle& unanimatedStyle() const { return *m_unanimatedStyle; }
> 
>+    std::optional<double> bindingsStartTime() const final;
>+    void setBindingsStartTime(std::optional<double>) final;
>     std::optional<double> bindingsCurrentTime() const final;
>+    ExceptionOr<void> setBindingsCurrentTime(std::optional<double>) final;
>     WebAnimation::PlayState bindingsPlayState() const final;
>+    bool bindingsPending() const final;
>+    WebAnimation::ReadyPromise& bindingsReady() final;
>+    WebAnimation::FinishedPromise& bindingsFinished() final;
>+    ExceptionOr<void> bindingsPlay() final;
>+    ExceptionOr<void> bindingsPause() final;
> 
> protected:
>     void syncPropertiesWithBackingAnimation() final;
>@@ -52,6 +60,8 @@ protected:
> private:
>     CSSAnimation(Element&, const Animation&, const RenderStyle&);
> 
>+    void flushPendingStyleChanges() const;
>+
>     String m_animationName;
>     std::unique_ptr<RenderStyle> m_unanimatedStyle;
> };
>diff --git a/Source/WebCore/animation/WebAnimation.h b/Source/WebCore/animation/WebAnimation.h
>index 9079aceb8b734824292f7d55eb9d6ffae28ba58f..052b94af779259c68aa81069ff6d9c54e83e111d 100644
>--- a/Source/WebCore/animation/WebAnimation.h
>+++ b/Source/WebCore/animation/WebAnimation.h
>@@ -67,13 +67,9 @@ public:
>     AnimationTimeline* timeline() const { return m_timeline.get(); }
>     virtual void setTimeline(RefPtr<AnimationTimeline>&&);
> 
>-    std::optional<double> bindingsStartTime() const;
>-    void setBindingsStartTime(std::optional<double>);
>     std::optional<Seconds> startTime() const;
>     void setStartTime(std::optional<Seconds>);
> 
>-    virtual std::optional<double> bindingsCurrentTime() const;
>-    ExceptionOr<void> setBindingsCurrentTime(std::optional<double>);
>     std::optional<Seconds> currentTime() const;
>     ExceptionOr<void> setCurrentTime(std::optional<Seconds>);
> 
>@@ -83,7 +79,6 @@ public:
> 
>     enum class PlayState { Idle, Running, Paused, Finished };
>     PlayState playState() const;
>-    virtual PlayState bindingsPlayState() const { return playState(); }
> 
>     bool pending() const { return hasPendingPauseTask() || hasPendingPlayTask(); }
> 
>@@ -99,6 +94,17 @@ public:
>     ExceptionOr<void> pause();
>     ExceptionOr<void> reverse();
> 
>+    virtual std::optional<double> bindingsStartTime() const;
>+    virtual void setBindingsStartTime(std::optional<double>);
>+    virtual std::optional<double> bindingsCurrentTime() const;
>+    virtual ExceptionOr<void> setBindingsCurrentTime(std::optional<double>);
>+    virtual PlayState bindingsPlayState() const { return playState(); }
>+    virtual bool bindingsPending() const { return pending(); }
>+    virtual ReadyPromise& bindingsReady() { return ready(); }
>+    virtual FinishedPromise& bindingsFinished() { return finished(); }
>+    virtual ExceptionOr<void> bindingsPlay() { return play(); }
>+    virtual ExceptionOr<void> bindingsPause() { return pause(); }
>+
>     Seconds timeToNextRequiredTick() const;
>     virtual void resolve(RenderStyle&);
>     void effectTargetDidChange(Element* previousTarget, Element* newTarget);
>diff --git a/Source/WebCore/animation/WebAnimation.idl b/Source/WebCore/animation/WebAnimation.idl
>index 88e30d1fd1f29dccc5e2fd2d249207bc7b5d0289..b8454178061baf85cf68c6c68a2f32f01426674e 100644
>--- a/Source/WebCore/animation/WebAnimation.idl
>+++ b/Source/WebCore/animation/WebAnimation.idl
>@@ -44,14 +44,14 @@ enum AnimationPlayState {
>     [MayThrowException, ImplementedAs=bindingsCurrentTime] attribute double? currentTime;
>     attribute double playbackRate;
>     [ImplementedAs=bindingsPlayState] readonly attribute AnimationPlayState playState;
>-    readonly attribute boolean pending;
>+    [ImplementedAs=bindingsPending] readonly attribute boolean pending;
>     attribute EventHandler onfinish;
>     attribute EventHandler oncancel;
>-    readonly attribute Promise<WebAnimation> ready;
>-    readonly attribute Promise<WebAnimation> finished;
>+    [ImplementedAs=bindingsReady] readonly attribute Promise<WebAnimation> ready;
>+    [ImplementedAs=bindingsFinished] readonly attribute Promise<WebAnimation> finished;
>     void cancel();
>     [MayThrowException] void finish();
>-    [MayThrowException] void play();
>-    [MayThrowException] void pause();
>+    [MayThrowException, ImplementedAs=bindingsPlay] void play();
>+    [MayThrowException, ImplementedAs=bindingsPause] void pause();
>     [MayThrowException] void reverse();
> };
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 366eddce08fd0cde6f800e723d00d44e8e5d42ea..8c8b73739f94c8c5586d2b888f5f9c0168de38f3 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,15 @@
>+2018-06-20  Antoine Quint  <graouts@apple.com>
>+
>+        [Web Animations] Make imported/mozilla/css-animations/test_animation-ready.html pass reliably
>+        https://bugs.webkit.org/show_bug.cgi?id=183830
>+        <rdar://problem/40997539>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This test now passes reliably.
>+
>+        * TestExpectations:
>+
> 2018-06-20  Antoine Quint  <graouts@apple.com>
> 
>         [Web Animations] Make imported/mozilla/css-transitions/test_element-get-animations.html pass reliably
>diff --git a/LayoutTests/imported/mozilla/ChangeLog b/LayoutTests/imported/mozilla/ChangeLog
>index 773ccc17d34d7f4d7da84675361a40095e2d19c5..b4e6b62d29fb68a1f47a5e92f244a0ef15ec7a76 100644
>--- a/LayoutTests/imported/mozilla/ChangeLog
>+++ b/LayoutTests/imported/mozilla/ChangeLog
>@@ -1,3 +1,15 @@
>+2018-06-20  Antoine Quint  <graouts@apple.com>
>+
>+        [Web Animations] Make imported/mozilla/css-animations/test_animation-ready.html pass reliably
>+        https://bugs.webkit.org/show_bug.cgi?id=183830
>+        <rdar://problem/40997539>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Mark progressions in the Mozilla CSS Animations tests.
>+
>+        * css-animations/test_animation-ready-expected.txt:
>+
> 2018-06-20  Antoine Quint  <graouts@apple.com>
> 
>         [Web Animations] Make imported/mozilla/css-transitions/test_element-get-animations.html pass reliably
>diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
>index dc471b2c8bd09f9a118578656989138ec3846bbc..851aee3f9842ba183f6b859af408413c50b92388 100644
>--- a/LayoutTests/TestExpectations
>+++ b/LayoutTests/TestExpectations
>@@ -1927,7 +1927,6 @@ webkit.org/b/181123 imported/w3c/web-platform-tests/web-animations/interfaces/An
> 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/183830 imported/mozilla/css-animations/test_animation-ready.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-ready-expected.txt b/LayoutTests/imported/mozilla/css-animations/test_animation-ready-expected.txt
>index 437a7b17a74b39bb0edbff058f36e34db14a9640..eed2afc1b1edeaf25a418c3ad1fbef8ae9c15b2a 100644
>--- a/LayoutTests/imported/mozilla/css-animations/test_animation-ready-expected.txt
>+++ b/LayoutTests/imported/mozilla/css-animations/test_animation-ready-expected.txt
>@@ -1,9 +1,9 @@
> 
>-FAIL A new ready promise is created when setting animation-play-state: running assert_not_equals: After updating animation-play-state a new ready promise object is created got disallowed value object "[object Promise]"
>+PASS A new ready promise is created when setting animation-play-state: running 
> PASS ready promise is rejected when an animation is canceled by resetting the animation property 
> PASS ready promise is rejected when an animation is cancelled by updating the animation property 
>-FAIL A new ready promise is created when setting animation-play-state: paused assert_not_equals: A new Promise object is generated when setting animation-play-state: paused got disallowed value object "[object Promise]"
>-FAIL Pausing twice re-uses the same Promise assert_equals: Ready promise objects are identical after redundant pause expected object "[object Promise]" but got object "[object Promise]"
>-FAIL If a pause operation is interrupted, the ready promise is reused assert_true: Animation is pending expected true got false
>+PASS A new ready promise is created when setting animation-play-state: paused 
>+PASS Pausing twice re-uses the same Promise 
>+PASS If a pause operation is interrupted, the ready promise is reused 
> PASS When a pause is complete the Promise callback gets the correct animation 
>

Flags:

dino: review+
ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 183830: 343150 | 343153 | 343159 | 343202