2008-08-11 Simon Fraser <simon.fraser@apple.com>
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 11 Aug 2008 22:44:06 +0000 (22:44 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 11 Aug 2008 22:44:06 +0000 (22:44 +0000)
        Reviewed by Dave Hyatt

        https://bugs.webkit.org/show_bug.cgi?id=20328
        Fix a problem when an 'all' transition transition with more than
        one property changing is interrupted, and did some AnimationController
        cleanup.

        Test: transitions/interrupted-all-transition.html

        * page/AnimationController.cpp:
        (WebCore::ImplicitAnimation::ImplicitAnimation):
        (WebCore::AnimationControllerPrivate::blendProperties):
        (WebCore::CompositeAnimation::updateTransitions):
        (WebCore::CompositeAnimation::cleanupFinishedAnimations):
        (WebCore::CompositeAnimation::setTransitionStartTime):
        (WebCore::CompositeAnimation::overrideImplicitAnimations):
        (WebCore::CompositeAnimation::resumeOverriddenImplicitAnimations):
        (WebCore::ImplicitAnimation::animate):
        (WebCore::ImplicitAnimation::onAnimationEnd):
        (WebCore::ImplicitAnimation::sendTransitionEvent):
        (WebCore::ImplicitAnimation::affectsProperty):
        (WebCore::KeyframeAnimation::endAnimation):
        (WebCore::KeyframeAnimation::onAnimationEnd):

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

LayoutTests/ChangeLog
LayoutTests/transitions/interrupted-all-transition-expected.txt [new file with mode: 0644]
LayoutTests/transitions/interrupted-all-transition.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/page/AnimationController.cpp

index d0f8569a3ba7428e81ba12528ca93fb03fc4a34a..63804972401e5f48f5ba4e6cc3f9748161002adb 100644 (file)
@@ -1,3 +1,14 @@
+2008-08-11  Simon Fraser  <simon.fraser@apple.com>
+
+        Reviewed by Dave Hyatt
+
+        https://bugs.webkit.org/show_bug.cgi?id=20328
+        Add testcase for interrupted 'all' transition with more than
+        one property changing.
+        
+        * transitions/interrupted-all-transition-expected.txt: Added.
+        * transitions/interrupted-all-transition.html: Added.
+
 2008-08-11  Dan Bernstein  <mitz@apple.com>
 
         Reviewed by Darin Adler.
diff --git a/LayoutTests/transitions/interrupted-all-transition-expected.txt b/LayoutTests/transitions/interrupted-all-transition-expected.txt
new file mode 100644 (file)
index 0000000..586e8a2
--- /dev/null
@@ -0,0 +1,3 @@
+Box should start moving left after left style is reset after 500ms
+
+PASS
diff --git a/LayoutTests/transitions/interrupted-all-transition.html b/LayoutTests/transitions/interrupted-all-transition.html
new file mode 100644 (file)
index 0000000..c2dad64
--- /dev/null
@@ -0,0 +1,59 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"
+   "http://www.w3.org/TR/html4/loose.dtd">
+
+<html lang="en">
+<head>
+  <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
+  <title>Interrupted All Transition</title>
+  <style type="text/css" media="screen">
+    #container {
+      position: relative;
+      width: 400px;
+      height: 100px;
+      border: 1px solid black;
+    }
+    #box {
+      position: absolute;
+      height: 100px;
+      width: 100px;
+      background-color: blue;
+      -webkit-transition-duration: 1s;
+      -webkit-transition-timing-function: linear;
+    }
+  </style>
+  <script type="text/javascript" charset="utf-8">
+    if (window.layoutTestController) {
+      layoutTestController.dumpAsText();
+      layoutTestController.waitUntilDone();
+    }
+
+    function startTransition()
+    {
+      var box = document.getElementById('box');
+      box.style.left = '300px';
+      box.style.opacity = 0.5;
+      window.setTimeout(function() {
+        box.style.left = '0px';
+        
+        window.setTimeout(function() {
+          var boxPos = parseInt(window.getComputedStyle(box).left);
+          document.getElementById('result').innerHTML = (boxPos < 200) ? "PASS" : "FAIL";
+          if (window.layoutTestController)
+              layoutTestController.notifyDone();
+        }, 250);
+      }, 500);
+    }
+    window.addEventListener('load', startTransition, false)
+  </script>
+</head>
+<body>
+
+<p>Box should start moving left after left style is reset after 500ms</p>
+<div id="container">
+  <div id="box">
+  </div>
+</div>
+<div id="result">
+</div>
+</body>
+</html>
index 75576abb0ec2c66067c786e8c6a5114a38ab1f11..80535edd1ae8d495461addebe3fcfb5a108ddc21 100644 (file)
@@ -1,3 +1,29 @@
+2008-08-11  Simon Fraser  <simon.fraser@apple.com>
+
+        Reviewed by Dave Hyatt
+
+        https://bugs.webkit.org/show_bug.cgi?id=20328
+        Fix a problem when an 'all' transition transition with more than
+        one property changing is interrupted, and did some AnimationController
+        cleanup.
+
+        Test: transitions/interrupted-all-transition.html
+
+        * page/AnimationController.cpp:
+        (WebCore::ImplicitAnimation::ImplicitAnimation):
+        (WebCore::AnimationControllerPrivate::blendProperties):
+        (WebCore::CompositeAnimation::updateTransitions):
+        (WebCore::CompositeAnimation::cleanupFinishedAnimations):
+        (WebCore::CompositeAnimation::setTransitionStartTime):
+        (WebCore::CompositeAnimation::overrideImplicitAnimations):
+        (WebCore::CompositeAnimation::resumeOverriddenImplicitAnimations):
+        (WebCore::ImplicitAnimation::animate):
+        (WebCore::ImplicitAnimation::onAnimationEnd):
+        (WebCore::ImplicitAnimation::sendTransitionEvent):
+        (WebCore::ImplicitAnimation::affectsProperty):
+        (WebCore::KeyframeAnimation::endAnimation):
+        (WebCore::KeyframeAnimation::onAnimationEnd):
+
 2008-08-11  Kevin McCullough  <kmccullough@apple.com>
 
         Reviewed by Tim.
index 0326ca7902f6db5bf44e555c27f313d2f0708166..599af614ae1010cd8b85926d8b4f3aeaacb5a7ff 100644 (file)
@@ -160,9 +160,8 @@ class ImplicitAnimation;
 class KeyframeAnimation;
 class AnimationControllerPrivate;
 
-// A CompositeAnimation represents a collection of animations that
-// are running, such as a number of properties transitioning at once.
-
+// A CompositeAnimation represents a collection of animations that are running
+// on a single RenderObject, such as a number of properties transitioning at once.
 class CompositeAnimation : public Noncopyable {
 public:
     CompositeAnimation(AnimationControllerPrivate* animationController)
@@ -177,7 +176,7 @@ public:
         deleteAllValues(m_keyframeAnimations);
     }
     
-    RenderStyle* animate(RenderObject*, RenderStyle* currentStyle, RenderStyle* targetStyle);
+    RenderStyle* animate(RenderObject*, const RenderStyle* currentStyle, RenderStyle* targetStyle);
     
     void setAnimating(bool inAnimating);
     bool animating();
@@ -206,8 +205,8 @@ public:
     void setWaitingForStyleAvailable(bool waiting);
 
 protected:
-    void updateTransitions(RenderObject* renderer, RenderStyle* currentStyle, RenderStyle* targetStyle);
-    void updateKeyframeAnimations(RenderObject* renderer, RenderStyle* currentStyle, RenderStyle* targetStyle);
+    void updateTransitions(RenderObject* renderer, const RenderStyle* currentStyle, RenderStyle* targetStyle);
+    void updateKeyframeAnimations(RenderObject* renderer, const RenderStyle* currentStyle, RenderStyle* targetStyle);
 
     KeyframeAnimation* findKeyframeAnimation(const AtomicString& name);
     
@@ -377,15 +376,19 @@ protected:
     bool m_waitingForEndEvent;
 };
 
+// An ImplicitAnimation tracks the state of a transition of a specific CSS property
+// for a single RenderObject.
 class ImplicitAnimation : public AnimationBase {
 public:
-    ImplicitAnimation(const Animation* transition, RenderObject* renderer, CompositeAnimation* compAnim)
+    ImplicitAnimation(const Animation* transition, int animatingProperty, RenderObject* renderer, CompositeAnimation* compAnim)
     : AnimationBase(transition, renderer, compAnim)
-    , m_property(transition->property())
+    , m_transitionProperty(transition->property())
+    , m_animatingProperty(animatingProperty)
     , m_overridden(false)
     , m_fromStyle(0)
     , m_toStyle(0)
     {
+        ASSERT(animatingProperty != cAnimateAll);
     }
     
     virtual ~ImplicitAnimation()
@@ -402,7 +405,8 @@ public:
             updateStateMachine(STATE_INPUT_END_ANIMATION, -1);     
     }
     
-    int property() const { return m_property; }
+    int transitionProperty() const { return m_transitionProperty; }
+    int animatingProperty() const { return m_animatingProperty; }
     
     virtual void onAnimationEnd(double inElapsedTime);
     
@@ -419,7 +423,7 @@ public:
     
     bool hasStyle() const { return m_fromStyle && m_toStyle; }
     
-    bool isTargetPropertyEqual(int prop, RenderStyle* targetStyle);
+    bool isTargetPropertyEqual(int prop, const RenderStyle* targetStyle);
 
     void blendPropertyValueInStyle(int prop, RenderStyle* currentStyle);
     
@@ -432,8 +436,9 @@ protected:
     bool sendTransitionEvent(const AtomicString& inEventType, double inElapsedTime);
     
 private:
-    int m_property;
-    bool m_overridden;
+    int m_transitionProperty;   // Transition property as specified in the RenderStyle. May be cAnimateAll
+    int m_animatingProperty;    // Specific property for this ImplicitAnimation
+    bool m_overridden;          // true when there is a keyframe animation that overrides the transitioning property
     
     // The two styles that we are blending.
     RenderStyle* m_fromStyle;
@@ -450,6 +455,8 @@ void AnimationEventDispatcher::timerFired(Timer<AnimationTimerBase>*)
     m_anim->animationEventDispatcherFired(m_element.get(), m_name, m_property, m_reset, m_eventType, m_elapsedTime);
 }
 
+// An KeyframeAnimation tracks the state of an explicit animation
+// for a single RenderObject.
 class KeyframeAnimation : public AnimationBase {
 public:
     KeyframeAnimation(const Animation* animation, RenderObject* renderer, int index, CompositeAnimation* compAnim)
@@ -1235,6 +1242,7 @@ int AnimationControllerPrivate::getPropertyAtIndex(int i)
 bool AnimationControllerPrivate::blendProperties(int prop, RenderStyle* dst, const RenderStyle* a, const RenderStyle* b, double prog)
 {
     if (prop == cAnimateAll) {
+        ASSERT(0);
         bool needsTimer = false;
     
         size_t n = gPropertyWrappers->size();
@@ -1389,7 +1397,7 @@ void AnimationControllerPrivate::resumeAnimations(Document* document)
     updateAnimationTimer();
 }
 
-void CompositeAnimation::updateTransitions(RenderObject* renderer, RenderStyle* currentStyle, RenderStyle* targetStyle)
+void CompositeAnimation::updateTransitions(RenderObject* renderer, const RenderStyle* currentStyle, RenderStyle* targetStyle)
 {
     // If currentStyle is null, we don't do transitions
     if (!currentStyle || !targetStyle->transitions())
@@ -1410,14 +1418,17 @@ void CompositeAnimation::updateTransitions(RenderObject* renderer, RenderStyle*
         
         // Handle both the 'all' and single property cases. For the single prop case, we make only one pass
         // through the loop
-        for (int i = 0; ; ++i) {
+        for (int propertyIndex = 0; ; ++propertyIndex) {
             if (all) {
-                if (i >= AnimationControllerPrivate::getNumProperties())
+                if (propertyIndex >= AnimationControllerPrivate::getNumProperties())
                     break;
                 // get the next prop
-                prop = AnimationControllerPrivate::getPropertyAtIndex(i);
+                prop = AnimationControllerPrivate::getPropertyAtIndex(propertyIndex);
             }
 
+            // ImplicitAnimations are always hashed by actual properties, never cAnimateAll
+            ASSERT(prop > firstCSSProperty && prop < (firstCSSProperty + numCSSProperties));
+
             // See if there is a current transition for this prop
             ImplicitAnimation* implAnim = m_transitions.get(prop);
             bool equal = true;
@@ -1425,28 +1436,19 @@ void CompositeAnimation::updateTransitions(RenderObject* renderer, RenderStyle*
             if (implAnim) {
                 // There is one, has our target changed?
                 if (!implAnim->isTargetPropertyEqual(prop, targetStyle)) {
-                    // It has changed - toss it and start over
-                    // Opacity is special since it can pop in and out of RenderLayers. We need to compute
-                    // the blended opacity value between the previous from and to styles and put that in the currentStyle, which
-                    // will become the new fromStyle. This is changing a const RenderStyle, but we know what we are doing, really :-)
-                    if (prop == CSSPropertyOpacity) {
-                        // get the blended value of opacity into the currentStyle (which will be the new fromStyle)
-                        implAnim->blendPropertyValueInStyle(CSSPropertyOpacity, currentStyle);
-                    }
-
                     implAnim->reset(renderer);
                     delete implAnim;
                     m_transitions.remove(prop);
                     equal = false;
                 }
-            }
-            else
+            } else {
                 // See if we need to start a new transition
                 equal = AnimationControllerPrivate::propertiesEqual(prop, currentStyle, targetStyle);
+            }
             
             if (!equal) {
-                // AAdd the new transition
-                ImplicitAnimation* animation = new ImplicitAnimation(const_cast<Animation*>(anim), renderer, this);
+                // Add the new transition
+                ImplicitAnimation* animation = new ImplicitAnimation(const_cast<Animation*>(anim), prop, renderer, this);
                 m_transitions.set(prop, animation);
             }
             
@@ -1457,7 +1459,7 @@ void CompositeAnimation::updateTransitions(RenderObject* renderer, RenderStyle*
     }
 }
 
-void CompositeAnimation::updateKeyframeAnimations(RenderObject* renderer, RenderStyle* currentStyle, RenderStyle* targetStyle)
+void CompositeAnimation::updateKeyframeAnimations(RenderObject* renderer, const RenderStyle* currentStyle, RenderStyle* targetStyle)
 {
     // Nothing to do if we don't have any animations, and didn't have any before
     if (m_keyframeAnimations.isEmpty() && !targetStyle->hasAnimations())
@@ -1529,7 +1531,7 @@ KeyframeAnimation* CompositeAnimation::findKeyframeAnimation(const AtomicString&
     return m_keyframeAnimations.get(name.impl());
 }
 
-RenderStyle* CompositeAnimation::animate(RenderObject* renderer, RenderStyle* currentStyle, RenderStyle* targetStyle)
+RenderStyle* CompositeAnimation::animate(RenderObject* renderer, const RenderStyle* currentStyle, RenderStyle* targetStyle)
 {
     RenderStyle* resultStyle = 0;
     
@@ -1619,11 +1621,7 @@ void CompositeAnimation::resetTransitions(RenderObject* renderer)
 
 void CompositeAnimation::resetAnimations(RenderObject* renderer)
 {
-    AnimationNameMap::const_iterator kfend = m_keyframeAnimations.end();
-    for (AnimationNameMap::const_iterator it = m_keyframeAnimations.begin(); it != kfend; ++it) {
-        KeyframeAnimation* anim = it->second;
-        delete anim;
-    }
+    deleteAllValues(m_keyframeAnimations);
     m_keyframeAnimations.clear();
 }
 
@@ -1641,7 +1639,7 @@ void CompositeAnimation::cleanupFinishedAnimations(RenderObject* renderer)
         if (!anim)
             continue;
         if (anim->postactive() && !anim->waitingForEndEvent())
-            finishedTransitions.append(anim->property());
+            finishedTransitions.append(anim->animatingProperty());
     }
     
     // Delete them
@@ -1694,8 +1692,7 @@ void CompositeAnimation::setTransitionStartTime(int property, double t)
     CSSPropertyTransitionsMap::const_iterator end = m_transitions.end();
     for (CSSPropertyTransitionsMap::const_iterator it = m_transitions.begin(); it != end; ++it) {
         ImplicitAnimation* anim = it->second;
-        if (anim && anim->waitingForStartTime() && 
-                    (anim->property() == property || anim->property() == cAnimateAll))
+        if (anim && anim->waitingForStartTime() && anim->animatingProperty() == property)
             anim->updateStateMachine(AnimationBase::STATE_INPUT_START_TIME_SET, t);
     }
 }
@@ -1749,7 +1746,7 @@ void CompositeAnimation::overrideImplicitAnimations(int property)
     CSSPropertyTransitionsMap::const_iterator end = m_transitions.end();
     for (CSSPropertyTransitionsMap::const_iterator it = m_transitions.begin(); it != end; ++it) {
         ImplicitAnimation* anim = it->second;
-        if (anim && (anim->property() == property || anim->property() == cAnimateAll))
+        if (anim && anim->animatingProperty() == property)
             anim->setOverridden(true);
     }
 }
@@ -1759,7 +1756,7 @@ void CompositeAnimation::resumeOverriddenImplicitAnimations(int property)
     CSSPropertyTransitionsMap::const_iterator end = m_transitions.end();
     for (CSSPropertyTransitionsMap::const_iterator it = m_transitions.begin(); it != end; ++it) {
         ImplicitAnimation* anim = it->second;
-        if (anim && (anim->property() == property || anim->property() == cAnimateAll))
+        if (anim && anim->animatingProperty() == property)
             anim->setOverridden(false);
     }
 }
@@ -1828,12 +1825,9 @@ void ImplicitAnimation::animate(CompositeAnimation* animation, RenderObject* ren
         return;
     
     // If we get this far and the animation is done, it means we are cleaning up a just finished animation.
-    // If so, send back the targetStyle (it will get tossed later)
-    if (postactive()) {
-        if (!animatedStyle)
-            animatedStyle = const_cast<RenderStyle*>(targetStyle);
+    // So just return. Everything is already all cleaned up
+    if (postactive())
         return;
-    }
 
     // Reset to start the transition if we are new
     if (isnew())
@@ -1845,17 +1839,15 @@ void ImplicitAnimation::animate(CompositeAnimation* animation, RenderObject* ren
         animatedStyle = new (renderer->renderArena()) RenderStyle(*targetStyle);
     
     double prog = progress(1, 0);
-    bool needsAnim = AnimationControllerPrivate::blendProperties(m_property, animatedStyle, m_fromStyle, m_toStyle, prog);
+    bool needsAnim = AnimationControllerPrivate::blendProperties(m_animatingProperty, animatedStyle, m_fromStyle, m_toStyle, prog);
     if (needsAnim)
         setAnimating();
 }
 
 void ImplicitAnimation::onAnimationEnd(double inElapsedTime)
 {
-    // we're converting the animation into a transition here
     if (!sendTransitionEvent(EventNames::webkitTransitionEndEvent, inElapsedTime)) {
-        // we didn't dispatch an event, which would call endAnimation(), so we'll just end
-        // it here.
+        // We didn't dispatch an event, which would call endAnimation(), so we'll just call it here.
         endAnimation(true);
     }
 }
@@ -1869,12 +1861,10 @@ bool ImplicitAnimation::sendTransitionEvent(const AtomicString& inEventType, dou
             Element* element = elementForEventDispatch();
             if (element) {
                 String propertyName;
-                if (m_property == cAnimateAll)
-                    propertyName = String("");
-                else
-                    propertyName = String(getPropertyName((CSSPropertyID)m_property));
+                if (m_transitionProperty != cAnimateAll)
+                    propertyName = String(getPropertyName((CSSPropertyID)m_transitionProperty));
                 m_waitingForEndEvent = true;
-                m_animationEventDispatcher.startTimer(element, propertyName, m_property, true, inEventType, inElapsedTime);
+                m_animationEventDispatcher.startTimer(element, propertyName, m_transitionProperty, true, inEventType, inElapsedTime);
                 return true; // Did dispatch an event
             }
         }
@@ -1915,11 +1905,10 @@ void ImplicitAnimation::setOverridden(bool b)
 
 bool ImplicitAnimation::affectsProperty(int property) const
 {
-    return m_property == property ||
-    (m_property == cAnimateAll && !AnimationControllerPrivate::propertiesEqual(property, m_fromStyle, m_toStyle));
+    return (m_animatingProperty == property);
 }
 
-bool ImplicitAnimation::isTargetPropertyEqual(int prop, RenderStyle* targetStyle)
+bool ImplicitAnimation::isTargetPropertyEqual(int prop, const RenderStyle* targetStyle)
 {
     return AnimationControllerPrivate::propertiesEqual(prop, m_toStyle, targetStyle);
 }
@@ -2008,10 +1997,9 @@ void KeyframeAnimation::animate(CompositeAnimation* animation, RenderObject* ren
 
 void KeyframeAnimation::endAnimation(bool reset)
 {
-    if (m_object) {
-        // restore the original (unanimated) style
+    // Restore the original (unanimated) style
+    if (m_object)
         setChanged(m_object->element());
-    }
 }
 
 void KeyframeAnimation::onAnimationStart(double inElapsedTime)
@@ -2026,10 +2014,8 @@ void KeyframeAnimation::onAnimationIteration(double inElapsedTime)
 
 void KeyframeAnimation::onAnimationEnd(double inElapsedTime)
 {
-    // FIXME: set the unanimated style on the element
     if (!sendAnimationEvent(EventNames::webkitAnimationEndEvent, inElapsedTime)) {
-        // we didn't dispatch an event, which would call endAnimation(), so we'll just end
-        // it here.
+        // We didn't dispatch an event, which would call endAnimation(), so we'll just call it here.
         endAnimation(true);
     }
 }
@@ -2058,7 +2044,7 @@ bool KeyframeAnimation::sendAnimationEvent(const AtomicString& inEventType, doub
 
 void KeyframeAnimation::overrideAnimations()
 {
-    // this will override implicit animations that match the properties in the keyframe animation
+    // This will override implicit animations that match the properties in the keyframe animation
     HashSet<int>::const_iterator end = m_keyframes->endProperties();
     for (HashSet<int>::const_iterator it = m_keyframes->beginProperties(); it != end; ++it)
         compositeAnimation()->overrideImplicitAnimations(*it);
@@ -2066,7 +2052,7 @@ void KeyframeAnimation::overrideAnimations()
 
 void KeyframeAnimation::resumeOverriddenAnimations()
 {
-    // this will resume overridden implicit animations
+    // This will resume overridden implicit animations
     HashSet<int>::const_iterator end = m_keyframes->endProperties();
     for (HashSet<int>::const_iterator it = m_keyframes->beginProperties(); it != end; ++it)
         compositeAnimation()->resumeOverriddenImplicitAnimations(*it);