[Web Animations] CSSTransition objects should have fill: backwards to allow seeking...
authorgraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Mar 2018 18:45:54 +0000 (18:45 +0000)
committergraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Mar 2018 18:45:54 +0000 (18:45 +0000)
https://bugs.webkit.org/show_bug.cgi?id=184129

Reviewed by Dean Jackson.

Source/WebCore:

In order to allow a CSS Transition to be seeked prior to its start time, it needs to have its fill mode set
to backwards. Adding code to set the fill mode in CSSTransition::initialize() yields early timing model
invalidation and we could get in a situation where stylesWouldYieldNewCSSTransitionsBlendingKeyframes()
was called before we had a chance to create blending keyframes for a CSS transitions, since the call
to create blending keyframes is made after the call to initialize(), so we now cater for this case.

* animation/CSSTransition.cpp:
(WebCore::CSSTransition::initialize):
* animation/CSSTransition.h:
* animation/KeyframeEffectReadOnly.cpp:
(WebCore::KeyframeEffectReadOnly::stylesWouldYieldNewCSSTransitionsBlendingKeyframes const):

LayoutTests:

Make one test opt into CSS Animations and CSS Transitions as Web Animations and fix expectations for a CSSTransition
test which mistakenly assumes the fill to be "none".

* transitions/transition-in-delay-phase.html:
* webanimations/css-transitions.html:

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

LayoutTests/ChangeLog
LayoutTests/transitions/transition-in-delay-phase.html
LayoutTests/webanimations/css-transitions.html
Source/WebCore/ChangeLog
Source/WebCore/animation/CSSTransition.cpp
Source/WebCore/animation/CSSTransition.h
Source/WebCore/animation/KeyframeEffectReadOnly.cpp

index 69a46a3..35c1952 100644 (file)
@@ -1,3 +1,16 @@
+2018-03-29  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] CSSTransition objects should have fill: backwards to allow seeking prior to start time
+        https://bugs.webkit.org/show_bug.cgi?id=184129
+
+        Reviewed by Dean Jackson.
+
+        Make one test opt into CSS Animations and CSS Transitions as Web Animations and fix expectations for a CSSTransition
+        test which mistakenly assumes the fill to be "none".
+
+        * transitions/transition-in-delay-phase.html:
+        * webanimations/css-transitions.html:
+
 2018-03-28  Ryan Haddad  <ryanhaddad@apple.com>
 
         Mark imported/w3c/web-platform-tests/IndexedDB/idbobjectstore_createIndex7-event_order.htm as flaky.
index da3d3b7..903a81e 100644 (file)
@@ -1,3 +1,4 @@
+<!-- webkit-test-runner [ enableCSSAnimationsAndCSSTransitionsBackedByWebAnimations=true ] -->
 <html>
 <head>
   <style>
index 3c452aa..9acddba 100644 (file)
@@ -64,7 +64,7 @@ targetTest(target => {
     assert_equals(computedTiming.iterations, 1, "The animations's computed timing iterations property matches the properties set by CSS");
     assert_equals(computedTiming.localTime, 0, "The animations's computed timing localTime property matches the properties set by CSS");
     assert_equals(computedTiming.progress, 0.5, "The animations's computed timing progress property matches the properties set by CSS");
-    assert_equals(computedTiming.fill, "none", "The animations's computed timing fill property matches the properties set by CSS");
+    assert_equals(computedTiming.fill, "backwards", "The animations's computed timing fill property matches the default value set for transitions");
     assert_equals(computedTiming.easing, "linear", "The animations's computed timing easing property matches the properties set by CSS");
     assert_equals(computedTiming.direction, "normal", "The animations's computed timing direction property matches the properties set by CSS");
 
index 375f1ee..3bae427 100644 (file)
@@ -1,3 +1,22 @@
+2018-03-29  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] CSSTransition objects should have fill: backwards to allow seeking prior to start time
+        https://bugs.webkit.org/show_bug.cgi?id=184129
+
+        Reviewed by Dean Jackson.
+
+        In order to allow a CSS Transition to be seeked prior to its start time, it needs to have its fill mode set
+        to backwards. Adding code to set the fill mode in CSSTransition::initialize() yields early timing model
+        invalidation and we could get in a situation where stylesWouldYieldNewCSSTransitionsBlendingKeyframes()
+        was called before we had a chance to create blending keyframes for a CSS transitions, since the call
+        to create blending keyframes is made after the call to initialize(), so we now cater for this case.
+
+        * animation/CSSTransition.cpp:
+        (WebCore::CSSTransition::initialize):
+        * animation/CSSTransition.h:
+        * animation/KeyframeEffectReadOnly.cpp:
+        (WebCore::KeyframeEffectReadOnly::stylesWouldYieldNewCSSTransitionsBlendingKeyframes const):
+
 2018-03-30  Daniel Bates  <dabates@apple.com>
 
         Remove unused MIMETypeRegistry::getSupportedImageMIMETypesForEncoding()
index 4d1d79b..e95479b 100644 (file)
@@ -46,6 +46,19 @@ CSSTransition::CSSTransition(Element& element, CSSPropertyID property, const Ani
 {
 }
 
+void CSSTransition::initialize(const Element& target)
+{
+    DeclarativeAnimation::initialize(target);
+
+    suspendEffectInvalidation();
+
+    // In order for CSS Transitions to be seeked backwards, they need to have their fill mode set to backwards
+    // such that the original CSS value applied prior to the transition is used for a negative current time.
+    effect()->timing()->setFill(FillMode::Backwards);
+
+    unsuspendEffectInvalidation();
+}
+
 bool CSSTransition::matchesBackingAnimationAndStyles(const Animation& newBackingAnimation, const RenderStyle* oldStyle, const RenderStyle& newStyle) const
 {
     // See if the animations match, excluding the property since we can move from an "all" transition to an explicit property transition.
index 7d3eeee..fae1bec 100644 (file)
@@ -47,6 +47,9 @@ public:
     bool matchesBackingAnimationAndStyles(const Animation&, const RenderStyle* oldStyle, const RenderStyle& newStyle) const;
     bool canBeListed() const final;
 
+protected:
+    void initialize(const Element&) final;
+
 private:
     CSSTransition(Element&, CSSPropertyID, const Animation&);
 
index 016be0a..5c1f886 100644 (file)
@@ -870,6 +870,11 @@ void KeyframeEffectReadOnly::computeCSSTransitionBlendingKeyframes(const RenderS
 bool KeyframeEffectReadOnly::stylesWouldYieldNewCSSTransitionsBlendingKeyframes(const RenderStyle& oldStyle, const RenderStyle& newStyle) const
 {
     ASSERT(is<CSSTransition>(animation()));
+
+    // We don't yet have blending keyframes to compare with, so these wouldn't be new keyframes, but the fisrt ones.
+    if (!hasBlendingKeyframes())
+        return false;
+
     auto property = downcast<CSSTransition>(animation())->backingAnimation().property();
 
     // There cannot be new keyframes if the start and to values are the same.