Encapsulate globals in CSSPropertyAnimation.cpp
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 12 Sep 2013 20:52:13 +0000 (20:52 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 12 Sep 2013 20:52:13 +0000 (20:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=121205

Reviewed by Antti Koivisto.

Encapsulated the globals inside a newly added CSSPropertyAnimationWrapperMap. Also removed the circular
dependency from ShorthandPropertyWrapper's constructor to CSSPropertyAnimationWrapperMap::instance().
The circular dependency still exists in ensurePropertyMap but I'm going to remove it in the bug 121199.

* page/animation/CSSPropertyAnimation.cpp:
(WebCore::ShorthandPropertyWrapper::ShorthandPropertyWrapper): Takes a Vector of longhand wrappers instead of
calling wrapperForProperty in the middle of constructing the very table. This circular dependency is now
encapsulated in CSSPropertyAnimationWrapperMap::ensurePropertyMap.
(WebCore::CSSPropertyAnimationWrapperMap::instance): Added.
(WebCore::CSSPropertyAnimationWrapperMap::wrapperForProperty): Renamed from WebCore::wrapperForProperty.
(WebCore::CSSPropertyAnimationWrapperMap::wrapperForIndex): Added.
(WebCore::CSSPropertyAnimationWrapperMap::size): Added.
(WebCore::CSSPropertyAnimationWrapperMap::addPropertyWrapper): Renamed from WebCore::addPropertyWrapper. Also
cleaned up boolean logics to use early exits instead of nested ifs.
(WebCore::CSSPropertyAnimationWrapperMap::addShorthandProperties): Renamed from WebCore::addShorthandProperties.
(WebCore::CSSPropertyAnimationWrapperMap::ensurePropertyMap): Renamed from WebCore::ensurePropertyMap.
Added an alias gPropertyWrappers for m_propertyWrappers; this aliasing will be removed in the bug 121199.
(WebCore::CSSPropertyAnimation::blendProperties):
(WebCore::CSSPropertyAnimation::animationOfPropertyIsAccelerated):
(WebCore::CSSPropertyAnimation::animatableShorthandsAffectingProperty):
(WebCore::CSSPropertyAnimation::propertiesEqual):
(WebCore::CSSPropertyAnimation::getPropertyAtIndex):
(WebCore::CSSPropertyAnimation::getNumProperties):
* page/animation/CSSPropertyAnimation.h:
* rendering/style/RenderStyle.h:

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

Source/WebCore/ChangeLog
Source/WebCore/page/animation/CSSPropertyAnimation.cpp
Source/WebCore/page/animation/CSSPropertyAnimation.h
Source/WebCore/rendering/style/RenderStyle.h

index 5df943dca9a78e119c44113c5e0ba57890975564..2dedf4a7b0d653ce121c968dbee4be8751a7f4a1 100644 (file)
@@ -1,3 +1,36 @@
+2013-09-11  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Encapsulate globals in CSSPropertyAnimation.cpp
+        https://bugs.webkit.org/show_bug.cgi?id=121205
+
+        Reviewed by Antti Koivisto.
+
+        Encapsulated the globals inside a newly added CSSPropertyAnimationWrapperMap. Also removed the circular
+        dependency from ShorthandPropertyWrapper's constructor to CSSPropertyAnimationWrapperMap::instance().
+        The circular dependency still exists in ensurePropertyMap but I'm going to remove it in the bug 121199.
+
+        * page/animation/CSSPropertyAnimation.cpp:
+        (WebCore::ShorthandPropertyWrapper::ShorthandPropertyWrapper): Takes a Vector of longhand wrappers instead of
+        calling wrapperForProperty in the middle of constructing the very table. This circular dependency is now
+        encapsulated in CSSPropertyAnimationWrapperMap::ensurePropertyMap.
+        (WebCore::CSSPropertyAnimationWrapperMap::instance): Added.
+        (WebCore::CSSPropertyAnimationWrapperMap::wrapperForProperty): Renamed from WebCore::wrapperForProperty.
+        (WebCore::CSSPropertyAnimationWrapperMap::wrapperForIndex): Added.
+        (WebCore::CSSPropertyAnimationWrapperMap::size): Added.
+        (WebCore::CSSPropertyAnimationWrapperMap::addPropertyWrapper): Renamed from WebCore::addPropertyWrapper. Also
+        cleaned up boolean logics to use early exits instead of nested ifs.
+        (WebCore::CSSPropertyAnimationWrapperMap::addShorthandProperties): Renamed from WebCore::addShorthandProperties.
+        (WebCore::CSSPropertyAnimationWrapperMap::ensurePropertyMap): Renamed from WebCore::ensurePropertyMap.
+        Added an alias gPropertyWrappers for m_propertyWrappers; this aliasing will be removed in the bug 121199.
+        (WebCore::CSSPropertyAnimation::blendProperties):
+        (WebCore::CSSPropertyAnimation::animationOfPropertyIsAccelerated):
+        (WebCore::CSSPropertyAnimation::animatableShorthandsAffectingProperty):
+        (WebCore::CSSPropertyAnimation::propertiesEqual):
+        (WebCore::CSSPropertyAnimation::getPropertyAtIndex):
+        (WebCore::CSSPropertyAnimation::getNumProperties):
+        * page/animation/CSSPropertyAnimation.h:
+        * rendering/style/RenderStyle.h:
+
 2013-09-12  Anders Carlsson  <andersca@apple.com>
 
         SharedBuffer::createNSData should return a RetainPtr<NSData>
index 4b7cbdd267fbac088d1f1cecd979fab5736e9e8e..30eb23002c56306a85b9dcb3631544988b52de0b 100644 (file)
@@ -401,32 +401,6 @@ private:
     CSSPropertyID m_prop;
 };
 
-static int gPropertyWrapperMap[numCSSProperties];
-static const int cInvalidPropertyWrapperIndex = -1;
-static Vector<AnimationPropertyWrapperBase*>* gPropertyWrappers = 0;
-
-static void addPropertyWrapper(CSSPropertyID propertyID, AnimationPropertyWrapperBase* wrapper)
-{
-    int propIndex = propertyID - firstCSSProperty;
-
-    ASSERT(gPropertyWrapperMap[propIndex] == cInvalidPropertyWrapperIndex);
-
-    unsigned wrapperIndex = gPropertyWrappers->size();
-    gPropertyWrappers->append(wrapper);
-    gPropertyWrapperMap[propIndex] = wrapperIndex;
-}
-
-static AnimationPropertyWrapperBase* wrapperForProperty(CSSPropertyID propertyID)
-{
-    int propIndex = propertyID - firstCSSProperty;
-    if (propIndex >= 0 && propIndex < numCSSProperties) {
-        int wrapperIndex = gPropertyWrapperMap[propIndex];
-        if (wrapperIndex >= 0)
-            return (*gPropertyWrappers)[wrapperIndex];
-    }
-    return 0;
-}
-
 template <typename T>
 class PropertyWrapperGetter : public AnimationPropertyWrapperBase {
 public:
@@ -982,14 +956,10 @@ private:
 
 class ShorthandPropertyWrapper : public AnimationPropertyWrapperBase {
 public:
-    ShorthandPropertyWrapper(CSSPropertyID property, const StylePropertyShorthand& shorthand)
+    ShorthandPropertyWrapper(CSSPropertyID property, Vector<AnimationPropertyWrapperBase*> longhandWrappers)
         : AnimationPropertyWrapperBase(property)
     {
-        for (unsigned i = 0; i < shorthand.length(); ++i) {
-            AnimationPropertyWrapperBase* wrapper = wrapperForProperty(shorthand.properties()[i]);
-            if (wrapper)
-                m_propertyWrappers.append(wrapper);
-        }
+        m_propertyWrappers.swap(longhandWrappers);
     }
 
     virtual bool isShorthandWrapper() const { return true; }
@@ -1105,7 +1075,66 @@ private:
 };
 #endif
 
-static void addShorthandProperties()
+class CSSPropertyAnimationWrapperMap {
+public:
+    static CSSPropertyAnimationWrapperMap& instance()
+    {
+        // FIXME: This data is never destroyed. Maybe we should ref count it and toss it when the last AnimationController is destroyed?
+        DEFINE_STATIC_LOCAL(OwnPtr<CSSPropertyAnimationWrapperMap>, map, ());
+        if (!map) {
+            map = adoptPtr(new CSSPropertyAnimationWrapperMap);
+            map->ensurePropertyMap(); // FIXME: ensurePropertyMap() calls instance() inside addShorthandProperties().
+        }
+        return *map;
+    }
+
+    AnimationPropertyWrapperBase* wrapperForProperty(CSSPropertyID propertyID)
+    {
+        int propIndex = propertyID - firstCSSProperty;
+        if (propertyID < firstCSSProperty || propertyID > lastCSSProperty)
+            return 0;
+
+        unsigned wrapperIndex = m_propertyToIdMap[propIndex];
+        if (wrapperIndex == cInvalidPropertyWrapperIndex)
+            return 0;
+
+        return m_propertyWrappers[wrapperIndex];
+    }
+
+    AnimationPropertyWrapperBase* wrapperForIndex(unsigned index)
+    {
+        ASSERT(index < m_propertyWrappers.size());
+        return m_propertyWrappers[index];
+    }
+
+    unsigned size()
+    {
+        return m_propertyWrappers.size();
+    }
+
+
+private:
+    void addShorthandProperties();
+    void ensurePropertyMap();
+
+    void addPropertyWrapper(CSSPropertyID propertyID, AnimationPropertyWrapperBase* wrapper)
+    {
+        int propIndex = propertyID - firstCSSProperty;
+
+        ASSERT(m_propertyToIdMap[propIndex] == cInvalidPropertyWrapperIndex);
+
+        unsigned wrapperIndex = m_propertyWrappers.size();
+        m_propertyWrappers.append(wrapper);
+        m_propertyToIdMap[propIndex] = wrapperIndex;
+    }
+
+    Vector<AnimationPropertyWrapperBase*> m_propertyWrappers;
+    unsigned m_propertyToIdMap[numCSSProperties];
+
+    const unsigned cInvalidPropertyWrapperIndex = UINT_MAX;
+};
+
+void CSSPropertyAnimationWrapperMap::addShorthandProperties()
 {
     static const CSSPropertyID animatableShorthandProperties[] = {
         CSSPropertyBackground, // for background-color, background-position, background-image
@@ -1130,21 +1159,27 @@ static void addShorthandProperties()
         CSSPropertyWebkitTransformOrigin
     };
 
+    CSSPropertyAnimationWrapperMap& map = CSSPropertyAnimationWrapperMap::instance();
+
     for (size_t i = 0; i < WTF_ARRAY_LENGTH(animatableShorthandProperties); ++i) {
         CSSPropertyID propertyID = animatableShorthandProperties[i];
         StylePropertyShorthand shorthand = shorthandForProperty(propertyID);
-        if (shorthand.length() > 0)
-            addPropertyWrapper(propertyID, new ShorthandPropertyWrapper(propertyID, shorthand));
+        if (!shorthand.length())
+            continue;
+
+        Vector<AnimationPropertyWrapperBase*> longhandWrappers;
+        for (unsigned i = 0; i < shorthand.length(); ++i) {
+            if (AnimationPropertyWrapperBase* wrapper = map.wrapperForProperty(shorthand.properties()[i]))
+                longhandWrappers.append(wrapper);
+        }
+
+        map.addPropertyWrapper(propertyID, new ShorthandPropertyWrapper(propertyID, longhandWrappers));
     }
 }
 
-void CSSPropertyAnimation::ensurePropertyMap()
+void CSSPropertyAnimationWrapperMap::ensurePropertyMap()
 {
-    // FIXME: This data is never destroyed. Maybe we should ref count it and toss it when the last AnimationController is destroyed?
-    if (gPropertyWrappers)
-        return;
-
-    gPropertyWrappers = new Vector<AnimationPropertyWrapperBase*>();
+    Vector<AnimationPropertyWrapperBase*>* gPropertyWrappers = &m_propertyWrappers; // FIXME: Remove this aliasing.
 
     // build the list of property wrappers to do the comparisons and blends
     gPropertyWrappers->append(new PropertyWrapper<Length>(CSSPropertyLeft, &RenderStyle::left, &RenderStyle::setLeft));
@@ -1307,14 +1342,14 @@ void CSSPropertyAnimation::ensurePropertyMap()
 
     // Make sure unused slots have a value
     for (unsigned int i = 0; i < static_cast<unsigned int>(numCSSProperties); ++i)
-        gPropertyWrapperMap[i] = cInvalidPropertyWrapperIndex;
+        m_propertyToIdMap[i] = cInvalidPropertyWrapperIndex;
 
     // First we put the non-shorthand property wrappers into the map, so the shorthand-building
     // code can find them.
     size_t n = gPropertyWrappers->size();
     for (unsigned int i = 0; i < n; ++i) {
-        ASSERT((*gPropertyWrappers)[i]->property() - firstCSSProperty < numCSSProperties);
-        gPropertyWrapperMap[(*gPropertyWrappers)[i]->property() - firstCSSProperty] = i;
+        ASSERT(m_propertyWrappers[i]->property() - firstCSSProperty < numCSSProperties);
+        m_propertyToIdMap[m_propertyWrappers[i]->property() - firstCSSProperty] = i;
     }
 
     // Now add the shorthand wrappers.
@@ -1347,9 +1382,7 @@ bool CSSPropertyAnimation::blendProperties(const AnimationBase* anim, CSSPropert
 {
     ASSERT(prop != CSSPropertyInvalid);
 
-    ensurePropertyMap();
-
-    AnimationPropertyWrapperBase* wrapper = wrapperForProperty(prop);
+    AnimationPropertyWrapperBase* wrapper = CSSPropertyAnimationWrapperMap::instance().wrapperForProperty(prop);
     if (wrapper) {
         wrapper->blend(anim, dst, a, b, progress);
 #if USE(ACCELERATED_COMPOSITING)
@@ -1365,8 +1398,7 @@ bool CSSPropertyAnimation::blendProperties(const AnimationBase* anim, CSSPropert
 #if USE(ACCELERATED_COMPOSITING)
 bool CSSPropertyAnimation::animationOfPropertyIsAccelerated(CSSPropertyID prop)
 {
-    ensurePropertyMap();
-    AnimationPropertyWrapperBase* wrapper = wrapperForProperty(prop);
+    AnimationPropertyWrapperBase* wrapper = CSSPropertyAnimationWrapperMap::instance().wrapperForProperty(prop);
     return wrapper ? wrapper->animationIsAccelerated() : false;
 }
 #endif
@@ -1374,20 +1406,18 @@ bool CSSPropertyAnimation::animationOfPropertyIsAccelerated(CSSPropertyID prop)
 // Note: this is inefficient. It's only called from pauseTransitionAtTime().
 HashSet<CSSPropertyID> CSSPropertyAnimation::animatableShorthandsAffectingProperty(CSSPropertyID property)
 {
-    ensurePropertyMap();
+    CSSPropertyAnimationWrapperMap& map = CSSPropertyAnimationWrapperMap::instance();
 
     HashSet<CSSPropertyID> foundProperties;
-    for (int i = 0; i < getNumProperties(); ++i)
-        gatherEnclosingShorthandProperties(property, (*gPropertyWrappers)[i], foundProperties);
+    for (unsigned i = 0; i < map.size(); ++i)
+        gatherEnclosingShorthandProperties(property, map.wrapperForIndex(i), foundProperties);
 
     return foundProperties;
 }
 
 bool CSSPropertyAnimation::propertiesEqual(CSSPropertyID prop, const RenderStyle* a, const RenderStyle* b)
 {
-    ensurePropertyMap();
-
-    AnimationPropertyWrapperBase* wrapper = wrapperForProperty(prop);
+    AnimationPropertyWrapperBase* wrapper = CSSPropertyAnimationWrapperMap::instance().wrapperForProperty(prop);
     if (wrapper)
         return wrapper->equals(a, b);
     return true;
@@ -1395,21 +1425,19 @@ bool CSSPropertyAnimation::propertiesEqual(CSSPropertyID prop, const RenderStyle
 
 CSSPropertyID CSSPropertyAnimation::getPropertyAtIndex(int i, bool& isShorthand)
 {
-    ensurePropertyMap();
+    CSSPropertyAnimationWrapperMap& map = CSSPropertyAnimationWrapperMap::instance();
 
-    if (i < 0 || i >= getNumProperties())
+    if (i < 0 || static_cast<unsigned>(i) >= map.size())
         return CSSPropertyInvalid;
 
-    AnimationPropertyWrapperBase* wrapper = (*gPropertyWrappers)[i];
+    AnimationPropertyWrapperBase* wrapper = map.wrapperForIndex(i);
     isShorthand = wrapper->isShorthandWrapper();
     return wrapper->property();
 }
 
 int CSSPropertyAnimation::getNumProperties()
 {
-    ensurePropertyMap();
-
-    return gPropertyWrappers->size();
+    return CSSPropertyAnimationWrapperMap::instance().size();
 }
 
 }
index 5290557bbc2f43a3a830650fa6a86c9727e0cec0..c41c7dd90c3927ca4e28aab5c33f550337c8c441 100644 (file)
@@ -51,8 +51,6 @@ public:
 
     // Return true if we need to start software animation timers
     static bool blendProperties(const AnimationBase*, CSSPropertyID, RenderStyle* dst, const RenderStyle* a, const RenderStyle* b, double progress);
-private:
-    static void ensurePropertyMap();
 };
 
 } // namespace WebCore
index 9a2ab3bd450b8ffe5d16b56db7f9ca38d35c320b..04adc85d1c14fc82159d1f6876c6e61565138792 100644 (file)
@@ -127,7 +127,7 @@ class ContentData;
 typedef Vector<RefPtr<RenderStyle>, 4> PseudoStyleCache;
 
 class RenderStyle: public RefCounted<RenderStyle> {
-    friend class CSSPropertyAnimation; // Used by CSS animations. We can't allow them to animate based off visited colors.
+    friend class CSSPropertyAnimationWrapperMap; // Used by CSS animations. We can't allow them to animate based off visited colors.
     friend class ApplyStyleCommand; // Editing has to only reveal unvisited info.
     friend class DeprecatedStyleBuilder; // Sets members directly.
     friend class EditingStyle; // Editing has to only reveal unvisited info.