Prepare SMILTimeContainer for oilpan

Source/core/svg/animation/SMILTimeContainer.cpp

Index: Source/core/svg/animation/SMILTimeContainer.cpp
diff --git a/Source/core/svg/animation/SMILTimeContainer.cpp b/Source/core/svg/animation/SMILTimeContainer.cpp
index 43db4f317fafcdceec09fdbfac5177de78ba175b..711f07034210a679a4a61b1ed25bfb48e317d5f3 100644
--- a/Source/core/svg/animation/SMILTimeContainer.cpp
+++ b/Source/core/svg/animation/SMILTimeContainer.cpp
@@ -87,11 +87,11 @@ void SMILTimeContainer::schedule(SVGSMILElement* animation, SVGElement* target,
 #endif
 
     ElementAttributePair key(target, attributeName);
-    OwnPtr<AnimationsVector>& scheduled = m_scheduledAnimations.add(key, nullptr).storedValue->value;
+    OwnPtrWillBeMember<AnimationsLinkedHashSet>& scheduled = m_scheduledAnimations.add(key, nullptr).storedValue->value;
     if (!scheduled)
-        scheduled = adoptPtr(new AnimationsVector);
+        scheduled = adoptPtrWillBeNoop(new AnimationsLinkedHashSet);
     ASSERT(!scheduled->contains(animation));
-    scheduled->append(animation);
+    scheduled->add(animation);
 
     SMILTime nextFireTime = animation->nextProgressTime();
     if (nextFireTime.isFinite())
@@ -107,11 +107,16 @@ void SMILTimeContainer::unschedule(SVGSMILElement* animation, SVGElement* target
 #endif
 
     ElementAttributePair key(target, attributeName);
-    AnimationsVector* scheduled = m_scheduledAnimations.get(key);
+    GroupedAnimationsMap::iterator it = m_scheduledAnimations.find(key);
+    ASSERT(it != m_scheduledAnimations.end());
+    AnimationsLinkedHashSet* scheduled = it->value;
     ASSERT(scheduled);
-    size_t idx = scheduled->find(animation);
-    ASSERT(idx != kNotFound);
-    scheduled->remove(idx);

[Erik Corry, 2014/05/28 08:10:05]

This was a remove from a vector, which is an n squared operation if you do it n
times.  It's nice to get rid of this (disclaimer: I haven't measured whether it
matters).

+    AnimationsLinkedHashSet::iterator itAnimation = scheduled->find(animation);
+    ASSERT(itAnimation != scheduled->end());
+    scheduled->remove(itAnimation);
+
+    if (scheduled->isEmpty())
+        m_scheduledAnimations.remove(it);
 }
 
 bool SMILTimeContainer::hasAnimations() const
@@ -235,10 +240,12 @@ void SMILTimeContainer::setElapsed(SMILTime time)
 #endif
     GroupedAnimationsMap::iterator end = m_scheduledAnimations.end();
     for (GroupedAnimationsMap::iterator it = m_scheduledAnimations.begin(); it != end; ++it) {
-        AnimationsVector* scheduled = it->value.get();
-        unsigned size = scheduled->size();
-        for (unsigned n = 0; n < size; n++)
-            scheduled->at(n)->reset();
+        if (!it->key.first)
+            continue;

[haraken, 2014/05/28 01:43:56]

(If this code is not performance-sensitive,) rather than having the continue
statement in the loop, I'd prefer calling a helper method that cleans up
m_scheduledAnimations before starting the loop.

cleanUpDeadEntries();  // Cleans up m_scheduledAnimations
for (...) {
  ...;
}

+
+        AnimationsLinkedHashSet* scheduled = it->value.get();
+        for (AnimationsLinkedHashSet::const_iterator itAnimation = scheduled->begin(), itAnimationEnd = scheduled->end(); itAnimation != itAnimationEnd; ++itAnimation)
+            (*itAnimation)->reset();
     }
 #ifndef NDEBUG
     m_preventScheduledAnimationsChanges = false;
@@ -376,21 +383,28 @@ SMILTime SMILTimeContainer::updateAnimations(SMILTime elapsed, bool seekToTime)
     if (m_documentOrderIndexesDirty)
         updateDocumentOrderIndexes();
 
-    WillBeHeapVector<RefPtrWillBeMember<SVGSMILElement> >  animationsToApply;
-    GroupedAnimationsMap::iterator end = m_scheduledAnimations.end();
-    for (GroupedAnimationsMap::iterator it = m_scheduledAnimations.begin(); it != end; ++it) {
-        AnimationsVector* scheduled = it->value.get();
+    WillBeHeapHashSet<ElementAttributePair> invalidKeys;
+    typedef WillBeHeapVector<RefPtrWillBeMember<SVGSMILElement> > AnimationsVector;
+    AnimationsVector animationsToApply;
+    for (GroupedAnimationsMap::iterator it = m_scheduledAnimations.begin(), end = m_scheduledAnimations.end(); it != end; ++it) {
+        if (!it->key.first || it->value->isEmpty()) {
+            invalidKeys.add(it->key);

[haraken, 2014/05/28 01:43:56]

Ditto.

cleanUpDeadEntries();
for (...) {
  ...;
}

[kouhei (in TOK), 2014/05/28 01:50:14]

I'd prefer current code, as |updateAnimations| is to be called on every frame.

+            continue;
+        }
+
+        AnimationsLinkedHashSet* scheduled = it->value.get();
 
         // Sort according to priority. Elements with later begin time have higher priority.
         // In case of a tie, document order decides.
         // FIXME: This should also consider timing relationships between the elements. Dependents
         // have higher priority.
-        std::sort(scheduled->begin(), scheduled->end(), PriorityCompare(elapsed));
+        AnimationsVector scheduledAnimations;
+        copyToVector(*scheduled, scheduledAnimations);
+        std::sort(scheduledAnimations.begin(), scheduledAnimations.end(), PriorityCompare(elapsed));
 
         SVGSMILElement* resultElement = 0;
-        unsigned size = scheduled->size();
-        for (unsigned n = 0; n < size; n++) {
-            SVGSMILElement* animation = scheduled->at(n);
+        for (AnimationsVector::const_iterator itAnimation = scheduledAnimations.begin(), itAnimationEnd = scheduledAnimations.end(); itAnimation != itAnimationEnd; ++itAnimation) {
+            SVGSMILElement* animation = *itAnimation;
             ASSERT(animation->timeContainer() == this);
             ASSERT(animation->targetElement());
             ASSERT(animation->hasValidAttributeName());
@@ -415,6 +429,7 @@ SMILTime SMILTimeContainer::updateAnimations(SMILTime elapsed, bool seekToTime)
         if (resultElement)
             animationsToApply.append(resultElement);
     }
+    m_scheduledAnimations.removeAll(invalidKeys);
 
     std::sort(animationsToApply.begin(), animationsToApply.end(), PriorityCompare(elapsed));
 
@@ -452,4 +467,9 @@ SMILTime SMILTimeContainer::updateAnimations(SMILTime elapsed, bool seekToTime)
     return earliestFireTime;
 }
 
+void SMILTimeContainer::trace(Visitor* visitor)
+{
+    visitor->trace(m_scheduledAnimations);
+}
+
 }