Prepare SVGSMILElement for oilpan

Source/core/svg/animation/SVGSMILElement.cpp

 /*
  * Copyright (C) 2008 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 187 matching lines @@
 SVGSMILElement::~SVGSMILElement()
 {
 #if !ENABLE(OILPAN)
     clearResourceAndEventBaseReferences();
 #endif
     smilEndEventSender().cancelEvent(this);
     smilBeginEventSender().cancelEvent(this);
     smilRepeatEventSender().cancelEvent(this);
     smilRepeatNEventSender().cancelEvent(this);
 #if !ENABLE(OILPAN)
-    // FIXME: Oilpan: Clear |m_syncBaseDependents| using weak processing.
     clearConditions();
 
     if (m_timeContainer && m_targetElement && hasValidAttributeName())
         m_timeContainer->unschedule(this, m_targetElement, m_attributeName);
 #endif
 }
 
 void SVGSMILElement::clearResourceAndEventBaseReferences()
 {
     document().accessSVGExtensions().removeAllTargetReferencesForElement(this);
 }
 
 void SVGSMILElement::clearConditions()
 {
     disconnectSyncBaseConditions();
     disconnectEventBaseConditions();

[haraken, 2014/05/29 04:58:58]

Making m_syncBaseDependents weak will allow us to remove
disconnectSyncBaseConditions(), but is it OK to remove
disconnectEventBaseConditions() as well?

[kouhei (in TOK), 2014/05/29 05:08:10]

|disconnectEventBaseConditions()| can also be removed as ~RefPtr will be handled
by |~Condition| which is now GCFinalized.

     m_conditions.clear();
 }
 
 void SVGSMILElement::buildPendingResource()
 {
     clearResourceAndEventBaseReferences();
 
     if (!inDocument()) {
         // Reset the target element if we are no longer in the document.
         setTargetElement(0);
@@ skipping 360 matching lines @@
 
 void SVGSMILElement::connectSyncBaseConditions()
 {
     if (m_syncBaseConditionsConnected)
         disconnectSyncBaseConditions();
     m_syncBaseConditionsConnected = true;
     for (unsigned n = 0; n < m_conditions.size(); ++n) {
         Condition* condition = m_conditions[n].get();
         if (condition->type() == Condition::Syncbase) {
             ASSERT(!condition->baseID().isEmpty());
-            condition->setSyncBase(treeScope().getElementById(AtomicString(condition->baseID())));
-            if (!condition->syncBase() || !isSVGSMILElement(*condition->syncBase())) {
+            Element* element = treeScope().getElementById(AtomicString(condition->baseID()));
+            if (!element || !isSVGSMILElement(*element)) {
                 condition->setSyncBase(0);
                 continue;
             }
-            toSVGSMILElement(condition->syncBase())->addSyncBaseDependent(this);
+            SVGSMILElement* svgSMILElement = toSVGSMILElement(element);
+            condition->setSyncBase(svgSMILElement);
+            svgSMILElement->addSyncBaseDependent(this);
         }
     }
 }
 
 void SVGSMILElement::disconnectSyncBaseConditions()
 {
     if (!m_syncBaseConditionsConnected)
         return;
     m_syncBaseConditionsConnected = false;
     for (unsigned n = 0; n < m_conditions.size(); ++n) {
         Condition* condition = m_conditions[n].get();
         if (condition->type() == Condition::Syncbase) {
             if (condition->syncBase())
-                toSVGSMILElement(condition->syncBase())->removeSyncBaseDependent(this);
+                condition->syncBase()->removeSyncBaseDependent(this);
             condition->setSyncBase(0);
         }
     }
 }
 
 void SVGSMILElement::connectEventBaseConditions()
 {
     disconnectEventBaseConditions();
     for (unsigned n = 0; n < m_conditions.size(); ++n) {
         Condition* condition = m_conditions[n].get();
@@ skipping 19 matching lines @@
         Condition* condition = m_conditions[n].get();
         if (condition->type() == Condition::EventBase) {
             ASSERT(!condition->syncBase());
             if (!condition->eventListener())
                 continue;
             // Note: It's a memory optimization to try to remove our condition
             // event listener, but it's not guaranteed to work, since we have
             // no guarantee that eventBaseFor() will be able to find our condition's
             // original eventBase. So, we also have to disconnect ourselves from
             // our condition event listener, in case it later fires.
             SVGElement* eventBase = eventBaseFor(*condition);

[haraken, 2014/05/29 06:14:10]

Is it guaranteed that the eventBase exists in the same document as the
SVGSMILElement? If the eventBase and the SVGSMILElement are in the same tree,
they die together, so I agree that we don't need to clean up the eventBase when
the SVGSMILElement dies.

             if (eventBase)
                 eventBase->removeEventListener(AtomicString(condition->name()), condition->eventListener(), false);
             condition->eventListener()->disconnectAnimation();
             condition->setEventListener(nullptr);

[haraken, 2014/05/29 06:14:10]

I don't fully understand this. Would you elaborate on why you don't need to run
line 668 and 669 when the SVGSMILElement dies?

         }
     }
 }
 
 void SVGSMILElement::setAttributeName(const QualifiedName& attributeName)
 {
     if (m_timeContainer && m_targetElement && m_attributeName != attributeName) {
         if (hasValidAttributeName())
             m_timeContainer->unschedule(this, m_targetElement, m_attributeName);
         m_attributeName = attributeName;
@@ skipping 560 matching lines @@
             smilEndEventSender().dispatchEventSoon(this);
     }
 
     m_nextProgressTime = calculateNextProgressTime(elapsed);
     return animationIsContributing;
 }
 
 void SVGSMILElement::notifyDependentsIntervalChanged()
 {
     ASSERT(m_intervalBegin.isFinite());
-    DEFINE_STATIC_LOCAL(HashSet<SVGSMILElement*>, loopBreaker, ());
-    if (!loopBreaker.add(this).isNewEntry)
+    DEFINE_STATIC_LOCAL(OwnPtrWillBePersistent<WillBeHeapHashSet<RawPtrWillBeMember<SVGSMILElement> > >, loopBreaker, (adoptPtrWillBeNoop(new WillBeHeapHashSet<RawPtrWillBeMember<SVGSMILElement> >())));
+    if (!loopBreaker->add(this).isNewEntry)
         return;
 
     TimeDependentSet::iterator end = m_syncBaseDependents.end();
     for (TimeDependentSet::iterator it = m_syncBaseDependents.begin(); it != end; ++it) {
         SVGSMILElement* dependent = *it;
         dependent->createInstanceTimesFromSyncbase(this);
     }
 
-    loopBreaker.remove(this);
+    loopBreaker->remove(this);
 }
 
 void SVGSMILElement::createInstanceTimesFromSyncbase(SVGSMILElement* syncBase)
 {
     // FIXME: To be really correct, this should handle updating exising interval by changing
     // the associated times instead of creating new ones.
     for (unsigned n = 0; n < m_conditions.size(); ++n) {
         Condition* condition = m_conditions[n].get();
         if (condition->type() == Condition::Syncbase && condition->syncBase() == syncBase) {
             ASSERT(condition->name() == "begin" || condition->name() == "end");
@@ skipping 81 matching lines @@
 void SVGSMILElement::trace(Visitor* visitor)
 {
     visitor->trace(m_targetElement);
     visitor->trace(m_timeContainer);
     visitor->trace(m_conditions);
     visitor->trace(m_syncBaseDependents);
     SVGElement::trace(visitor);
 }
 
 }