Commit 19fb5cb9 authored by kouhei@chromium.org's avatar kouhei@chromium.org

Prepare SVGDocumentExtensions::m_elementDependencies for oilpan

|m_elementDependencies| had raw ptr to |SVGElement|s.
This was to avoid reference cycles, but we can convert this to strong references in oilpan.

If the whole document is dead, rebuild code is not called. This means that
rebuildElementReference is not called for referencing element, but the
referencing element must be in the same document and is going to die anyway.
If the whole document isn't dead (== the part of the document tree is going
away), the unregister/rebuild code is guaranteed to be called from removedFrom,
and rebuildElementReference is guaranteed to be triggered from there.

BUG=357163, 370834

Review URL: https://codereview.chromium.org/327473002

git-svn-id: svn://svn.chromium.org/blink/trunk@175880 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 4b47c355
...@@ -183,24 +183,27 @@ static inline void removeFromCacheAndInvalidateDependencies(RenderObject* object ...@@ -183,24 +183,27 @@ static inline void removeFromCacheAndInvalidateDependencies(RenderObject* object
if (!object->node() || !object->node()->isSVGElement()) if (!object->node() || !object->node()->isSVGElement())
return; return;
HashSet<SVGElement*>* dependencies = object->document().accessSVGExtensions().setOfElementsReferencingTarget(toSVGElement(object->node())); SVGElementSet* dependencies = object->document().accessSVGExtensions().setOfElementsReferencingTarget(toSVGElement(object->node()));
if (!dependencies) if (!dependencies)
return; return;
// We allow cycles in SVGDocumentExtensions reference sets in order to avoid expensive // We allow cycles in SVGDocumentExtensions reference sets in order to avoid expensive
// reference graph adjustments on changes, so we need to break possible cycles here. // reference graph adjustments on changes, so we need to break possible cycles here.
DEFINE_STATIC_LOCAL(HashSet<SVGElement*>, invalidatingDependencies, ()); // This strong reference is safe, as it is guaranteed that this set will be emptied
// at the end of recursion.
typedef WillBeHeapHashSet<RawPtrWillBeMember<SVGElement> > SVGElementSet;
DEFINE_STATIC_LOCAL(OwnPtrWillBePersistent<SVGElementSet>, invalidatingDependencies, (adoptPtrWillBeNoop(new SVGElementSet)));
HashSet<SVGElement*>::iterator end = dependencies->end(); SVGElementSet::iterator end = dependencies->end();
for (HashSet<SVGElement*>::iterator it = dependencies->begin(); it != end; ++it) { for (SVGElementSet::iterator it = dependencies->begin(); it != end; ++it) {
if (RenderObject* renderer = (*it)->renderer()) { if (RenderObject* renderer = (*it)->renderer()) {
if (UNLIKELY(!invalidatingDependencies.add(*it).isNewEntry)) { if (UNLIKELY(!invalidatingDependencies->add(*it).isNewEntry)) {
// Reference cycle: we are in process of invalidating this dependant. // Reference cycle: we are in process of invalidating this dependant.
continue; continue;
} }
RenderSVGResource::markForLayoutAndParentResourceInvalidation(renderer, needsLayout); RenderSVGResource::markForLayoutAndParentResourceInvalidation(renderer, needsLayout);
invalidatingDependencies.remove(*it); invalidatingDependencies->remove(*it);
} }
} }
} }
......
...@@ -307,10 +307,10 @@ Element* SVGDocumentExtensions::removeElementFromPendingResourcesForRemoval(cons ...@@ -307,10 +307,10 @@ Element* SVGDocumentExtensions::removeElementFromPendingResourcesForRemoval(cons
return element; return element;
} }
HashSet<SVGElement*>* SVGDocumentExtensions::setOfElementsReferencingTarget(SVGElement* referencedElement) const SVGElementSet* SVGDocumentExtensions::setOfElementsReferencingTarget(SVGElement* referencedElement) const
{ {
ASSERT(referencedElement); ASSERT(referencedElement);
const HashMap<SVGElement*, OwnPtr<HashSet<SVGElement*> > >::const_iterator it = m_elementDependencies.find(referencedElement); const ElementDependenciesMap::const_iterator it = m_elementDependencies.find(referencedElement);
if (it == m_elementDependencies.end()) if (it == m_elementDependencies.end())
return 0; return 0;
return it->value.get(); return it->value.get();
...@@ -321,25 +321,25 @@ void SVGDocumentExtensions::addElementReferencingTarget(SVGElement* referencingE ...@@ -321,25 +321,25 @@ void SVGDocumentExtensions::addElementReferencingTarget(SVGElement* referencingE
ASSERT(referencingElement); ASSERT(referencingElement);
ASSERT(referencedElement); ASSERT(referencedElement);
if (HashSet<SVGElement*>* elements = m_elementDependencies.get(referencedElement)) { if (SVGElementSet* elements = m_elementDependencies.get(referencedElement)) {
elements->add(referencingElement); elements->add(referencingElement);
return; return;
} }
OwnPtr<HashSet<SVGElement*> > elements = adoptPtr(new HashSet<SVGElement*>); OwnPtrWillBeRawPtr<SVGElementSet> elements = adoptPtrWillBeNoop(new SVGElementSet);
elements->add(referencingElement); elements->add(referencingElement);
m_elementDependencies.set(referencedElement, elements.release()); m_elementDependencies.set(referencedElement, elements.release());
} }
void SVGDocumentExtensions::removeAllTargetReferencesForElement(SVGElement* referencingElement) void SVGDocumentExtensions::removeAllTargetReferencesForElement(SVGElement* referencingElement)
{ {
Vector<SVGElement*> toBeRemoved; WillBeHeapVector<RawPtrWillBeMember<SVGElement> > toBeRemoved;
HashMap<SVGElement*, OwnPtr<HashSet<SVGElement*> > >::iterator end = m_elementDependencies.end(); ElementDependenciesMap::iterator end = m_elementDependencies.end();
for (HashMap<SVGElement*, OwnPtr<HashSet<SVGElement*> > >::iterator it = m_elementDependencies.begin(); it != end; ++it) { for (ElementDependenciesMap::iterator it = m_elementDependencies.begin(); it != end; ++it) {
SVGElement* referencedElement = it->key; SVGElement* referencedElement = it->key;
HashSet<SVGElement*>* referencingElements = it->value.get(); SVGElementSet* referencingElements = it->value.get();
HashSet<SVGElement*>::iterator setIt = referencingElements->find(referencingElement); SVGElementSet::iterator setIt = referencingElements->find(referencingElement);
if (setIt == referencingElements->end()) if (setIt == referencingElements->end())
continue; continue;
...@@ -354,22 +354,20 @@ void SVGDocumentExtensions::removeAllTargetReferencesForElement(SVGElement* refe ...@@ -354,22 +354,20 @@ void SVGDocumentExtensions::removeAllTargetReferencesForElement(SVGElement* refe
void SVGDocumentExtensions::rebuildAllElementReferencesForTarget(SVGElement* referencedElement) void SVGDocumentExtensions::rebuildAllElementReferencesForTarget(SVGElement* referencedElement)
{ {
ASSERT(referencedElement); ASSERT(referencedElement);
HashMap<SVGElement*, OwnPtr<HashSet<SVGElement*> > >::iterator it = m_elementDependencies.find(referencedElement); ElementDependenciesMap::iterator it = m_elementDependencies.find(referencedElement);
if (it == m_elementDependencies.end()) if (it == m_elementDependencies.end())
return; return;
ASSERT(it->key == referencedElement); ASSERT(it->key == referencedElement);
Vector<SVGElement*> toBeNotified;
HashSet<SVGElement*>* referencingElements = it->value.get(); WillBeHeapVector<RawPtrWillBeMember<SVGElement> > toBeNotified;
HashSet<SVGElement*>::iterator setEnd = referencingElements->end(); SVGElementSet* referencingElements = it->value.get();
for (HashSet<SVGElement*>::iterator setIt = referencingElements->begin(); setIt != setEnd; ++setIt) copyToVector(*referencingElements, toBeNotified);
toBeNotified.append(*setIt);
// Force rebuilding the referencingElement so it knows about this change. // Force rebuilding the referencingElement so it knows about this change.
Vector<SVGElement*>::iterator vectorEnd = toBeNotified.end(); WillBeHeapVector<RawPtrWillBeMember<SVGElement> >::iterator vectorEnd = toBeNotified.end();
for (Vector<SVGElement*>::iterator vectorIt = toBeNotified.begin(); vectorIt != vectorEnd; ++vectorIt) { for (WillBeHeapVector<RawPtrWillBeMember<SVGElement> >::iterator vectorIt = toBeNotified.begin(); vectorIt != vectorEnd; ++vectorIt) {
// Before rebuilding referencingElement ensure it was not removed from under us. // Before rebuilding referencingElement ensure it was not removed from under us.
if (HashSet<SVGElement*>* referencingElements = setOfElementsReferencingTarget(referencedElement)) { if (SVGElementSet* referencingElements = setOfElementsReferencingTarget(referencedElement)) {
if (referencingElements->contains(*vectorIt)) if (referencingElements->contains(*vectorIt))
(*vectorIt)->svgAttributeChanged(XLinkNames::hrefAttr); (*vectorIt)->svgAttributeChanged(XLinkNames::hrefAttr);
} }
...@@ -379,7 +377,7 @@ void SVGDocumentExtensions::rebuildAllElementReferencesForTarget(SVGElement* ref ...@@ -379,7 +377,7 @@ void SVGDocumentExtensions::rebuildAllElementReferencesForTarget(SVGElement* ref
void SVGDocumentExtensions::removeAllElementReferencesForTarget(SVGElement* referencedElement) void SVGDocumentExtensions::removeAllElementReferencesForTarget(SVGElement* referencedElement)
{ {
ASSERT(referencedElement); ASSERT(referencedElement);
HashMap<SVGElement*, OwnPtr<HashSet<SVGElement*> > >::iterator it = m_elementDependencies.find(referencedElement); ElementDependenciesMap::iterator it = m_elementDependencies.find(referencedElement);
if (it == m_elementDependencies.end()) if (it == m_elementDependencies.end())
return; return;
ASSERT(it->key == referencedElement); ASSERT(it->key == referencedElement);
...@@ -480,9 +478,11 @@ SVGSVGElement* SVGDocumentExtensions::rootElement() const ...@@ -480,9 +478,11 @@ SVGSVGElement* SVGDocumentExtensions::rootElement() const
void SVGDocumentExtensions::trace(Visitor* visitor) void SVGDocumentExtensions::trace(Visitor* visitor)
{ {
visitor->trace(m_document);
visitor->trace(m_timeContainers); visitor->trace(m_timeContainers);
visitor->trace(m_svgFontFaceElements); visitor->trace(m_svgFontFaceElements);
visitor->trace(m_pendingSVGFontFaceElementsForRemoval); visitor->trace(m_pendingSVGFontFaceElementsForRemoval);
visitor->trace(m_elementDependencies);
visitor->trace(m_relativeLengthSVGRoots); visitor->trace(m_relativeLengthSVGRoots);
} }
......
...@@ -42,6 +42,8 @@ class SVGSMILElement; ...@@ -42,6 +42,8 @@ class SVGSMILElement;
class SVGSVGElement; class SVGSVGElement;
class Element; class Element;
typedef WillBeHeapHashSet<RawPtrWillBeMember<SVGElement> > SVGElementSet;
class SVGDocumentExtensions : public NoBaseWillBeGarbageCollectedFinalized<SVGDocumentExtensions> { class SVGDocumentExtensions : public NoBaseWillBeGarbageCollectedFinalized<SVGDocumentExtensions> {
WTF_MAKE_NONCOPYABLE(SVGDocumentExtensions); WTF_MAKE_FAST_ALLOCATED_WILL_BE_REMOVED; WTF_MAKE_NONCOPYABLE(SVGDocumentExtensions); WTF_MAKE_FAST_ALLOCATED_WILL_BE_REMOVED;
public: public:
...@@ -67,7 +69,7 @@ public: ...@@ -67,7 +69,7 @@ public:
SVGResourcesCache* resourcesCache() const { return m_resourcesCache.get(); } SVGResourcesCache* resourcesCache() const { return m_resourcesCache.get(); }
HashSet<SVGElement*>* setOfElementsReferencingTarget(SVGElement* referencedElement) const; SVGElementSet* setOfElementsReferencingTarget(SVGElement* referencedElement) const;
void addElementReferencingTarget(SVGElement* referencingElement, SVGElement* referencedElement); void addElementReferencingTarget(SVGElement* referencingElement, SVGElement* referencedElement);
void removeAllTargetReferencesForElement(SVGElement*); void removeAllTargetReferencesForElement(SVGElement*);
void rebuildAllElementReferencesForTarget(SVGElement*); void rebuildAllElementReferencesForTarget(SVGElement*);
...@@ -98,7 +100,7 @@ public: ...@@ -98,7 +100,7 @@ public:
void trace(Visitor*); void trace(Visitor*);
private: private:
Document* m_document; // weak reference RawPtrWillBeMember<Document> m_document;
WillBeHeapHashSet<RawPtrWillBeMember<SVGSVGElement> > m_timeContainers; // For SVG 1.2 support this will need to be made more general. WillBeHeapHashSet<RawPtrWillBeMember<SVGSVGElement> > m_timeContainers; // For SVG 1.2 support this will need to be made more general.
#if ENABLE(SVG_FONTS) #if ENABLE(SVG_FONTS)
WillBeHeapHashSet<RawPtrWillBeMember<SVGFontFaceElement> > m_svgFontFaceElements; WillBeHeapHashSet<RawPtrWillBeMember<SVGFontFaceElement> > m_svgFontFaceElements;
...@@ -108,7 +110,8 @@ private: ...@@ -108,7 +110,8 @@ private:
HashMap<AtomicString, RenderSVGResourceContainer*> m_resources; HashMap<AtomicString, RenderSVGResourceContainer*> m_resources;
HashMap<AtomicString, OwnPtr<SVGPendingElements> > m_pendingResources; // Resources that are pending. HashMap<AtomicString, OwnPtr<SVGPendingElements> > m_pendingResources; // Resources that are pending.
HashMap<AtomicString, OwnPtr<SVGPendingElements> > m_pendingResourcesForRemoval; // Resources that are pending and scheduled for removal. HashMap<AtomicString, OwnPtr<SVGPendingElements> > m_pendingResourcesForRemoval; // Resources that are pending and scheduled for removal.
HashMap<SVGElement*, OwnPtr<HashSet<SVGElement*> > > m_elementDependencies; typedef WillBeHeapHashMap<RawPtrWillBeMember<SVGElement>, OwnPtrWillBeMember<SVGElementSet> > ElementDependenciesMap;
ElementDependenciesMap m_elementDependencies;
OwnPtr<SVGResourcesCache> m_resourcesCache; OwnPtr<SVGResourcesCache> m_resourcesCache;
WillBeHeapHashSet<RawPtrWillBeMember<SVGSVGElement> > m_relativeLengthSVGRoots; // Root SVG elements with relative length descendants. WillBeHeapHashSet<RawPtrWillBeMember<SVGSVGElement> > m_relativeLengthSVGRoots; // Root SVG elements with relative length descendants.
FloatPoint m_translate; FloatPoint m_translate;
......
...@@ -101,7 +101,7 @@ SVGElement::~SVGElement() ...@@ -101,7 +101,7 @@ SVGElement::~SVGElement()
} }
// With Oilpan, either removedFrom has been called or the document is dead // With Oilpan, either removedFrom has been called or the document is dead
// as well and there is no reason to clear out the extensions. // as well and there is no reason to clear out the references.
document().accessSVGExtensions().rebuildAllElementReferencesForTarget(this); document().accessSVGExtensions().rebuildAllElementReferencesForTarget(this);
document().accessSVGExtensions().removeAllElementReferencesForTarget(this); document().accessSVGExtensions().removeAllElementReferencesForTarget(this);
#endif #endif
......
...@@ -236,9 +236,9 @@ void SVGPathElement::invalidateMPathDependencies() ...@@ -236,9 +236,9 @@ void SVGPathElement::invalidateMPathDependencies()
{ {
// <mpath> can only reference <path> but this dependency is not handled in // <mpath> can only reference <path> but this dependency is not handled in
// markForLayoutAndParentResourceInvalidation so we update any mpath dependencies manually. // markForLayoutAndParentResourceInvalidation so we update any mpath dependencies manually.
if (HashSet<SVGElement*>* dependencies = document().accessSVGExtensions().setOfElementsReferencingTarget(this)) { if (SVGElementSet* dependencies = document().accessSVGExtensions().setOfElementsReferencingTarget(this)) {
HashSet<SVGElement*>::iterator end = dependencies->end(); SVGElementSet::iterator end = dependencies->end();
for (HashSet<SVGElement*>::iterator it = dependencies->begin(); it != end; ++it) { for (SVGElementSet::iterator it = dependencies->begin(); it != end; ++it) {
if (isSVGMPathElement(**it)) if (isSVGMPathElement(**it))
toSVGMPathElement(*it)->targetPathChanged(); toSVGMPathElement(*it)->targetPathChanged();
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment