Animations in an AnimationList are never null
authorandersca@apple.com <andersca@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 5 Sep 2013 15:15:59 +0000 (15:15 +0000)
committerandersca@apple.com <andersca@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 5 Sep 2013 15:15:59 +0000 (15:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=120720

Reviewed by Andreas Kling.

Change AnimationList::operator[] to return an Animation& and fix up related code to also take references.

* css/CSSComputedStyleDeclaration.cpp:
* css/DeprecatedStyleBuilder.cpp:
* page/animation/CompositeAnimation.cpp:
* page/animation/KeyframeAnimation.cpp:
* platform/animation/Animation.h:
* platform/animation/AnimationList.cpp:
* platform/animation/AnimationList.h:
* rendering/RenderLayerBacking.cpp:

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

Source/WebCore/ChangeLog
Source/WebCore/css/CSSComputedStyleDeclaration.cpp
Source/WebCore/css/DeprecatedStyleBuilder.cpp
Source/WebCore/page/animation/CompositeAnimation.cpp
Source/WebCore/page/animation/KeyframeAnimation.cpp
Source/WebCore/platform/animation/Animation.h
Source/WebCore/platform/animation/AnimationList.cpp
Source/WebCore/platform/animation/AnimationList.h
Source/WebCore/platform/graphics/GraphicsLayerAnimation.cpp
Source/WebCore/rendering/RenderLayerBacking.cpp
Source/WebCore/rendering/style/RenderStyle.cpp

index 2709965..a11c727 100644 (file)
@@ -1,3 +1,21 @@
+2013-09-04  Anders Carlsson  <andersca@apple.com>
+
+        Animations in an AnimationList are never null
+        https://bugs.webkit.org/show_bug.cgi?id=120720
+
+        Reviewed by Andreas Kling.
+
+        Change AnimationList::operator[] to return an Animation& and fix up related code to also take references.
+
+        * css/CSSComputedStyleDeclaration.cpp:
+        * css/DeprecatedStyleBuilder.cpp:
+        * page/animation/CompositeAnimation.cpp:
+        * page/animation/KeyframeAnimation.cpp:
+        * platform/animation/Animation.h:
+        * platform/animation/AnimationList.cpp:
+        * platform/animation/AnimationList.h:
+        * rendering/RenderLayerBacking.cpp:
+
 2013-09-05  Antti Koivisto  <antti@apple.com>
 
         Call createTextRenderersForSiblingsAfterAttachIfNeeded only for the attach root
index 57cf0de..ce70e57 100644 (file)
@@ -1165,17 +1165,18 @@ static PassRefPtr<CSSValue> valueForGridPosition(const GridPosition& position)
     return list.release();
 }
 
-static PassRefPtr<CSSValue> createTransitionPropertyValue(const Animation* animation)
+static PassRefPtr<CSSValue> createTransitionPropertyValue(const Animation& animation)
 {
     RefPtr<CSSValue> propertyValue;
-    if (animation->animationMode() == Animation::AnimateNone)
+    if (animation.animationMode() == Animation::AnimateNone)
         propertyValue = cssValuePool().createIdentifierValue(CSSValueNone);
-    else if (animation->animationMode() == Animation::AnimateAll)
+    else if (animation.animationMode() == Animation::AnimateAll)
         propertyValue = cssValuePool().createIdentifierValue(CSSValueAll);
     else
-        propertyValue = cssValuePool().createValue(getPropertyNameString(animation->property()), CSSPrimitiveValue::CSS_STRING);
+        propertyValue = cssValuePool().createValue(getPropertyNameString(animation.property()), CSSPrimitiveValue::CSS_STRING);
     return propertyValue.release();
 }
+
 static PassRefPtr<CSSValue> getTransitionPropertyValue(const AnimationList* animList)
 {
     RefPtr<CSSValueList> list = CSSValueList::createCommaSeparated();
@@ -1192,7 +1193,7 @@ static PassRefPtr<CSSValue> getDelayValue(const AnimationList* animList)
     RefPtr<CSSValueList> list = CSSValueList::createCommaSeparated();
     if (animList) {
         for (size_t i = 0; i < animList->size(); ++i)
-            list->append(cssValuePool().createValue(animList->animation(i)->delay(), CSSPrimitiveValue::CSS_S));
+            list->append(cssValuePool().createValue(animList->animation(i).delay(), CSSPrimitiveValue::CSS_S));
     } else {
         // Note that initialAnimationDelay() is used for both transitions and animations
         list->append(cssValuePool().createValue(Animation::initialAnimationDelay(), CSSPrimitiveValue::CSS_S));
@@ -1205,7 +1206,7 @@ static PassRefPtr<CSSValue> getDurationValue(const AnimationList* animList)
     RefPtr<CSSValueList> list = CSSValueList::createCommaSeparated();
     if (animList) {
         for (size_t i = 0; i < animList->size(); ++i)
-            list->append(cssValuePool().createValue(animList->animation(i)->duration(), CSSPrimitiveValue::CSS_S));
+            list->append(cssValuePool().createValue(animList->animation(i).duration(), CSSPrimitiveValue::CSS_S));
     } else {
         // Note that initialAnimationDuration() is used for both transitions and animations
         list->append(cssValuePool().createValue(Animation::initialAnimationDuration(), CSSPrimitiveValue::CSS_S));
@@ -1254,7 +1255,7 @@ static PassRefPtr<CSSValue> getTimingFunctionValue(const AnimationList* animList
     RefPtr<CSSValueList> list = CSSValueList::createCommaSeparated();
     if (animList) {
         for (size_t i = 0; i < animList->size(); ++i)
-            list->append(createTimingFunctionValue(animList->animation(i)->timingFunction().get()));
+            list->append(createTimingFunctionValue(animList->animation(i).timingFunction().get()));
     } else
         // Note that initialAnimationTimingFunction() is used for both transitions and animations
         list->append(createTimingFunctionValue(Animation::initialAnimationTimingFunction().get()));
@@ -2523,7 +2524,7 @@ PassRefPtr<CSSValue> ComputedStyleExtractor::propertyValue(CSSPropertyID propert
             const AnimationList* t = style->animations();
             if (t) {
                 for (size_t i = 0; i < t->size(); ++i) {
-                    if (t->animation(i)->direction())
+                    if (t->animation(i).direction())
                         list->append(cssValuePool().createIdentifierValue(CSSValueAlternate));
                     else
                         list->append(cssValuePool().createIdentifierValue(CSSValueNormal));
@@ -2539,7 +2540,7 @@ PassRefPtr<CSSValue> ComputedStyleExtractor::propertyValue(CSSPropertyID propert
             const AnimationList* t = style->animations();
             if (t) {
                 for (size_t i = 0; i < t->size(); ++i) {
-                    switch (t->animation(i)->fillMode()) {
+                    switch (t->animation(i).fillMode()) {
                     case AnimationFillModeNone:
                         list->append(cssValuePool().createIdentifierValue(CSSValueNone));
                         break;
@@ -2563,7 +2564,7 @@ PassRefPtr<CSSValue> ComputedStyleExtractor::propertyValue(CSSPropertyID propert
             const AnimationList* t = style->animations();
             if (t) {
                 for (size_t i = 0; i < t->size(); ++i) {
-                    double iterationCount = t->animation(i)->iterationCount();
+                    double iterationCount = t->animation(i).iterationCount();
                     if (iterationCount == Animation::IterationCountInfinite)
                         list->append(cssValuePool().createIdentifierValue(CSSValueInfinite));
                     else
@@ -2578,7 +2579,7 @@ PassRefPtr<CSSValue> ComputedStyleExtractor::propertyValue(CSSPropertyID propert
             const AnimationList* t = style->animations();
             if (t) {
                 for (size_t i = 0; i < t->size(); ++i)
-                    list->append(cssValuePool().createValue(t->animation(i)->name(), CSSPrimitiveValue::CSS_STRING));
+                    list->append(cssValuePool().createValue(t->animation(i).name(), CSSPrimitiveValue::CSS_STRING));
             } else
                 list->append(cssValuePool().createIdentifierValue(CSSValueNone));
             return list.release();
@@ -2588,7 +2589,7 @@ PassRefPtr<CSSValue> ComputedStyleExtractor::propertyValue(CSSPropertyID propert
             const AnimationList* t = style->animations();
             if (t) {
                 for (size_t i = 0; i < t->size(); ++i) {
-                    int prop = t->animation(i)->playState();
+                    int prop = t->animation(i).playState();
                     if (prop == AnimPlayStatePlaying)
                         list->append(cssValuePool().createIdentifierValue(CSSValueRunning));
                     else
@@ -2742,11 +2743,11 @@ PassRefPtr<CSSValue> ComputedStyleExtractor::propertyValue(CSSPropertyID propert
                 RefPtr<CSSValueList> transitionsList = CSSValueList::createCommaSeparated();
                 for (size_t i = 0; i < animList->size(); ++i) {
                     RefPtr<CSSValueList> list = CSSValueList::createSpaceSeparated();
-                    const Animation* animation = animList->animation(i);
+                    const Animation& animation = animList->animation(i);
                     list->append(createTransitionPropertyValue(animation));
-                    list->append(cssValuePool().createValue(animation->duration(), CSSPrimitiveValue::CSS_S));
-                    list->append(createTimingFunctionValue(animation->timingFunction().get()));
-                    list->append(cssValuePool().createValue(animation->delay(), CSSPrimitiveValue::CSS_S));
+                    list->append(cssValuePool().createValue(animation.duration(), CSSPrimitiveValue::CSS_S));
+                    list->append(createTimingFunctionValue(animation.timingFunction().get()));
+                    list->append(cssValuePool().createValue(animation.delay(), CSSPrimitiveValue::CSS_S));
                     transitionsList->append(list);
                 }
                 return transitionsList.release();
index a946ded..21fc166 100644 (file)
@@ -1635,12 +1635,12 @@ template <typename T,
           const AnimationList* (RenderStyle::*immutableAnimationGetterFunction)() const>
 class ApplyPropertyAnimation {
 public:
-    static void setValue(Animation* animation, T value) { (animation->*setterFunction)(value); }
-    static T value(const Animation* animation) { return (animation->*getterFunction)(); }
-    static bool test(const Animation* animation) { return (animation->*testFunction)(); }
-    static void clear(Animation* animation) { (animation->*clearFunction)(); }
+    static void setValue(Animation& animation, T value) { (animation.*setterFunction)(value); }
+    static T value(const Animation& animation) { return (animation.*getterFunction)(); }
+    static bool test(const Animation& animation) { return (animation.*testFunction)(); }
+    static void clear(Animation& animation) { (animation.*clearFunction)(); }
     static T initial() { return (*initialFunction)(); }
-    static void map(StyleResolver* styleResolver, Animation* animation, CSSValue* value) { (styleResolver->styleMap()->*mapFunction)(animation, value); }
+    static void map(StyleResolver* styleResolver, Animation& animation, CSSValue* value) { (styleResolver->styleMap()->*mapFunction)(&animation, value); }
     static AnimationList* accessAnimations(RenderStyle* style) { return (style->*animationGetterFunction)(); }
     static const AnimationList* animations(RenderStyle* style) { return (style->*immutableAnimationGetterFunction)(); }
 
@@ -1653,7 +1653,7 @@ public:
             if (list->size() <= i)
                 list->append(Animation::create());
             setValue(list->animation(i), value(parentList->animation(i)));
-            list->animation(i)->setAnimationMode(parentList->animation(i)->animationMode());
+            list->animation(i).setAnimationMode(parentList->animation(i).animationMode());
         }
 
         /* Reset any remaining animations to not have the property set. */
@@ -1668,7 +1668,7 @@ public:
             list->append(Animation::create());
         setValue(list->animation(0), initial());
         if (propertyID == CSSPropertyWebkitTransitionProperty)
-            list->animation(0)->setAnimationMode(Animation::AnimateAll);
+            list->animation(0).setAnimationMode(Animation::AnimateAll);
         for (size_t i = 1; i < list->size(); ++i)
             clear(list->animation(i));
     }
index a3641ea..7f0435a 100644 (file)
@@ -97,14 +97,14 @@ void CompositeAnimation::updateTransitions(RenderObject* renderer, RenderStyle*
     // Check to see if we need to update the active transitions
     if (targetStyle->transitions()) {
         for (size_t i = 0; i < targetStyle->transitions()->size(); ++i) {
-            const Animation* anim = targetStyle->transitions()->animation(i);
-            bool isActiveTransition = !m_suspended && (anim->duration() || anim->delay() > 0);
+            const Animation& animation = targetStyle->transitions()->animation(i);
+            bool isActiveTransition = !m_suspended && (animation.duration() || animation.delay() > 0);
 
-            Animation::AnimationMode mode = anim->animationMode();
+            Animation::AnimationMode mode = animation.animationMode();
             if (mode == Animation::AnimateNone)
                 continue;
 
-            CSSPropertyID prop = anim->property();
+            CSSPropertyID prop = animation.property();
 
             bool all = mode == Animation::AnimateAll;
 
@@ -174,9 +174,9 @@ void CompositeAnimation::updateTransitions(RenderObject* renderer, RenderStyle*
                 // <https://bugs.webkit.org/show_bug.cgi?id=24787>
                 if (!equal && isActiveTransition) {
                     // Add the new transition
-                    RefPtr<ImplicitAnimation> animation = ImplicitAnimation::create(const_cast<Animation*>(anim), prop, renderer, this, modifiedCurrentStyle ? modifiedCurrentStyle.get() : fromStyle);
-                    LOG(Animations, "Created ImplicitAnimation %p for property %s duration %.2f delay %.2f", animation.get(), getPropertyName(prop), anim->duration(), anim->delay());
-                    m_transitions.set(prop, animation.release());
+                    RefPtr<ImplicitAnimation> implicitAnimation = ImplicitAnimation::create(const_cast<Animation*>(&animation), prop, renderer, this, modifiedCurrentStyle ? modifiedCurrentStyle.get() : fromStyle);
+                    LOG(Animations, "Created ImplicitAnimation %p for property %s duration %.2f delay %.2f", implicitAnimation.get(), getPropertyName(prop), animation.duration(), animation.delay());
+                    m_transitions.set(prop, implicitAnimation.release());
                 }
                 
                 // We only need one pass for the single prop case
@@ -234,10 +234,10 @@ void CompositeAnimation::updateKeyframeAnimations(RenderObject* renderer, Render
         if (targetStyle->animations()) {
             int numAnims = targetStyle->animations()->size();
             for (int i = 0; i < numAnims; ++i) {
-                const Animation* anim = targetStyle->animations()->animation(i);
-                AtomicString animationName(anim->name());
+                const Animation& animation = targetStyle->animations()->animation(i);
+                AtomicString animationName(animation.name());
 
-                if (!anim->isValidAnimation())
+                if (!animation.isValidAnimation())
                     continue;
                 
                 // See if there is a current animation for this name.
@@ -251,14 +251,14 @@ void CompositeAnimation::updateKeyframeAnimations(RenderObject* renderer, Render
                     // This one is still active.
 
                     // Animations match, but play states may differ. Update if needed.
-                    keyframeAnim->updatePlayState(anim->playState());
+                    keyframeAnim->updatePlayState(animation.playState());
                                 
                     // Set the saved animation to this new one, just in case the play state has changed.
-                    keyframeAnim->setAnimation(anim);
+                    keyframeAnim->setAnimation(&animation);
                     keyframeAnim->setIndex(i);
-                } else if ((anim->duration() || anim->delay()) && anim->iterationCount() && animationName != none) {
-                    keyframeAnim = KeyframeAnimation::create(const_cast<Animation*>(anim), renderer, i, this, targetStyle);
-                    LOG(Animations, "Creating KeyframeAnimation %p with keyframes %s, duration %.2f, delay %.2f, iterations %.2f", keyframeAnim.get(), anim->name().utf8().data(), anim->duration(), anim->delay(), anim->iterationCount());
+                } else if ((animation.duration() || animation.delay()) && animation.iterationCount() && animationName != none) {
+                    keyframeAnim = KeyframeAnimation::create(const_cast<Animation*>(&animation), renderer, i, this, targetStyle);
+                    LOG(Animations, "Creating KeyframeAnimation %p with keyframes %s, duration %.2f, delay %.2f, iterations %.2f", keyframeAnim.get(), animation.name().utf8().data(), animation.duration(), animation.delay(), animation.iterationCount());
                     if (m_suspended) {
                         keyframeAnim->updatePlayState(AnimPlayStatePaused);
                         LOG(Animations, "  (created in suspended/paused state)");
index 305b6b4..ef068e9 100644 (file)
@@ -73,8 +73,8 @@ static const Animation* getAnimationFromStyleByName(const RenderStyle* style, co
         return 0;
 
     for (size_t i = 0; i < style->animations()->size(); i++) {
-        if (name == style->animations()->animation(i)->name())
-            return style->animations()->animation(i);
+        if (name == style->animations()->animation(i).name())
+            return &style->animations()->animation(i);
     }
 
     return 0;
index 8106efd..bc23996 100644 (file)
@@ -39,7 +39,7 @@ public:
     ~Animation();
 
     static PassRefPtr<Animation> create() { return adoptRef(new Animation); }
-    static PassRefPtr<Animation> create(const Animation* o) { return adoptRef(new Animation(*o)); }
+    static PassRefPtr<Animation> create(const Animation& other) { return adoptRef(new Animation(other)); }
 
     bool isDelaySet() const { return m_delaySet; }
     bool isDirectionSet() const { return m_directionSet; }
index 58a40aa..625d790 100644 (file)
 namespace WebCore {
 
 #define FILL_UNSET_PROPERTY(test, propGet, propSet) \
-for (i = 0; i < size() && animation(i)->test(); ++i) { } \
+for (i = 0; i < size() && animation(i).test(); ++i) { } \
 if (i < size() && i != 0) { \
     for (size_t j = 0; i < size(); ++i, ++j) \
-        animation(i)->propSet(animation(j)->propGet()); \
+        animation(i).propSet(animation(j).propGet()); \
 }
 
-AnimationList::AnimationList(const AnimationList& o)
+AnimationList::AnimationList(const AnimationList& other)
 {
-    for (size_t i = 0; i < o.size(); ++i)
-        m_animations.append(Animation::create(o.animation(i)));
+    for (size_t i = 0; i < other.size(); ++i)
+        m_animations.append(Animation::create(other.animation(i)));
 }
 
 void AnimationList::fillUnsetProperties()
@@ -51,12 +51,12 @@ void AnimationList::fillUnsetProperties()
     FILL_UNSET_PROPERTY(isPropertySet, property, setProperty);
 }
 
-bool AnimationList::operator==(const AnimationList& o) const
+bool AnimationList::operator==(const AnimationList& other) const
 {
-    if (size() != o.size())
+    if (size() != other.size())
         return false;
     for (size_t i = 0; i < size(); ++i)
-        if (*animation(i) != *o.animation(i))
+        if (animation(i) != other.animation(i))
             return false;
     return true;
 }
index bf8ff9f..c84cb9b 100644 (file)
@@ -38,10 +38,10 @@ public:
     AnimationList(const AnimationList&);
 
     void fillUnsetProperties();
-    bool operator==(const AnimationList& o) const;
-    bool operator!=(const AnimationList& o) const
+    bool operator==(const AnimationList&) const;
+    bool operator!=(const AnimationList& other) const
     {
-        return !(*this == o);
+        return !(*this == other);
     }
     
     size_t size() const { return m_animations.size(); }
@@ -49,15 +49,19 @@ public:
     
     void resize(size_t n) { m_animations.resize(n); }
     void remove(size_t i) { m_animations.remove(i); }
-    void append(PassRefPtr<Animation> anim) { m_animations.append(anim); }
+    void append(PassRefPtr<Animation> animation)
+    {
+        ASSERT(animation);
+        m_animations.append(animation);
+    }
     
-    Animation* animation(size_t i) { return m_animations[i].get(); }
-    const Animation* animation(size_t i) const { return m_animations[i].get(); }
+    Animation& animation(size_t i) { return *m_animations[i]; }
+    const Animation& animation(size_t i) const { return *m_animations[i]; }
     
 private:
     AnimationList& operator=(const AnimationList&);
 
-    Vector<RefPtr<Animation> > m_animations;
+    Vector<RefPtr<Animation>> m_animations;
 };    
 
 
index 093bf16..add686f 100644 (file)
@@ -216,7 +216,7 @@ static const TimingFunction* timingFunctionForAnimationValue(const AnimationValu
 GraphicsLayerAnimation::GraphicsLayerAnimation(const String& name, const KeyframeValueList& keyframes, const IntSize& boxSize, const Animation* animation, double startTime, bool listsMatch)
     : m_keyframes(keyframes)
     , m_boxSize(boxSize)
-    , m_animation(Animation::create(animation))
+    , m_animation(Animation::create(*animation))
     , m_name(name)
     , m_listsMatch(listsMatch)
     , m_startTime(startTime)
index 97591d6..39911c1 100644 (file)
@@ -2118,7 +2118,7 @@ bool RenderLayerBacking::startAnimation(double timeOffset, const Animation* anim
             continue;
             
         // Get timing function.
-        RefPtr<TimingFunction> tf = keyframeStyle->hasAnimations() ? (*keyframeStyle->animations()).animation(0)->timingFunction() : 0;
+        RefPtr<TimingFunction> tf = keyframeStyle->hasAnimations() ? (*keyframeStyle->animations()).animation(0).timingFunction() : 0;
         
         bool isFirstOrLastKeyframe = key == 0 || key == 1;
         if ((hasTransform && isFirstOrLastKeyframe) || currentKeyframe.containsProperty(CSSPropertyWebkitTransform))
index 8db3614..3f31ff4 100644 (file)
@@ -1258,7 +1258,7 @@ void RenderStyle::adjustAnimations()
 
     // Get rid of empty animations and anything beyond them
     for (size_t i = 0; i < animationList->size(); ++i) {
-        if (animationList->animation(i)->isEmpty()) {
+        if (animationList->animation(i).isEmpty()) {
             animationList->resize(i);
             break;
         }
@@ -1281,7 +1281,7 @@ void RenderStyle::adjustTransitions()
 
     // Get rid of empty transitions and anything beyond them
     for (size_t i = 0; i < transitionList->size(); ++i) {
-        if (transitionList->animation(i)->isEmpty()) {
+        if (transitionList->animation(i).isEmpty()) {
             transitionList->resize(i);
             break;
         }
@@ -1299,7 +1299,7 @@ void RenderStyle::adjustTransitions()
     // but the lists tend to be very short, so it is probably ok
     for (size_t i = 0; i < transitionList->size(); ++i) {
         for (size_t j = i+1; j < transitionList->size(); ++j) {
-            if (transitionList->animation(i)->property() == transitionList->animation(j)->property()) {
+            if (transitionList->animation(i).property() == transitionList->animation(j).property()) {
                 // toss i
                 transitionList->remove(i);
                 j = i;
@@ -1326,9 +1326,9 @@ const Animation* RenderStyle::transitionForProperty(CSSPropertyID property) cons
 {
     if (transitions()) {
         for (size_t i = 0; i < transitions()->size(); ++i) {
-            const Animation* p = transitions()->animation(i);
-            if (p->animationMode() == Animation::AnimateAll || p->property() == property) {
-                return p;
+            const Animation& p = transitions()->animation(i);
+            if (p.animationMode() == Animation::AnimateAll || p.property() == property) {
+                return &p;
             }
         }
     }