Don't register excessive pending SVG resources

When an SVG resource was destroyed, it would notify the
SVGResourcesCache, which would walk the entire cache, notify the layout
object and add a "pending" entry for the id referring to the
corresponding element.
This would mean that every layout object which had any kind of resource
would get a "pending" reference from every id to itself - regardless of
if it ever referred to a resource with the given id. In the particular
test, this resulted in a fairly large (ever-growing) "pending" element
sets because there was persistent resource references in the document.

Fix by only adding "pending" entries for the current clients of the
resource that's being destroyed.
SVGResourcesCache::resourceDestroyed is removed in favor of new method
detachAllClients() in LayoutSVGResourceContainer. The part that
unregistered the resource itself as a client is removed in favor of the
pre-existing call to clientDestroyed() already existing in
LayoutSVGModelObject::willBeDestroyed (delegated to from the resource.)
SVGResources::resourceDestroyed is changed to not call
removeAllClientsFromCache() on the resource being passed - this is
instead done once after having cleared the references in all the
clients.

With this change, the "cycle time" of the test in the bug changes from
linearly increasing to constant.

BUG=521334

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

git-svn-id: svn://svn.chromium.org/blink/trunk@200840 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp

@@ -31,7 +31,7 @@
 
 namespace blink {
 
-static inline SVGDocumentExtensions& svgExtensionsFromElement(SVGElement* element)
+static inline SVGDocumentExtensions& svgExtensionsFromElement(Element* element)
 {
     ASSERT(element);
     return element->document().accessSVGExtensions();
@@ -68,7 +68,11 @@ void LayoutSVGResourceContainer::layout()
 
 void LayoutSVGResourceContainer::willBeDestroyed()
 {
-    SVGResourcesCache::resourceDestroyed(this);
+    // Detach all clients referring to this resource. If the resource itself is
+    // a client, it will be detached from any such resources by the call to
+    // LayoutSVGHiddenContainer::willBeDestroyed() below.
+    detachAllClients();
+
     LayoutSVGHiddenContainer::willBeDestroyed();
     if (m_registered)
         svgExtensionsFromElement(element()).removeResource(m_id);
@@ -84,6 +88,23 @@ void LayoutSVGResourceContainer::styleDidChange(StyleDifference diff, const Comp
     }
 }
 
+void LayoutSVGResourceContainer::detachAllClients()
+{
+    for (auto* client : m_clients) {
+        // Unlink the resource from the client's SVGResources. (The actual
+        // removal will be signaled after processing all the clients.)
+        SVGResources* resources = SVGResourcesCache::cachedResourcesForLayoutObject(client);
+        ASSERT(resources); // Or else the client wouldn't be in the list in the first place.
+        resources->resourceDestroyed(this);
+
+        // Add a pending resolution based on the id of the old resource.
+        Element* clientElement = toElement(client->node());
+        svgExtensionsFromElement(clientElement).addPendingResource(m_id, clientElement);
+    }
+
+    removeAllClientsFromCache();
+}
+
 void LayoutSVGResourceContainer::idChanged()
 {
     // Invalidate all our current clients.

third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.h

@@ -92,6 +92,7 @@ private:
     friend class SVGResourcesCache;
     void addClient(LayoutObject*);
     void removeClient(LayoutObject*);
+    void detachAllClients();
 
     void registerResource();
 

third_party/WebKit/Source/core/layout/svg/SVGResources.cpp

@@ -374,56 +374,40 @@ void SVGResources::resourceDestroyed(LayoutSVGResourceContainer* resource)
     case MaskerResourceType:
         if (!m_clipperFilterMaskerData)
             break;
-        if (m_clipperFilterMaskerData->masker == resource) {
-            m_clipperFilterMaskerData->masker->removeAllClientsFromCache();
+        if (m_clipperFilterMaskerData->masker == resource)
             m_clipperFilterMaskerData->masker = nullptr;
-        }
         break;
     case MarkerResourceType:
         if (!m_markerData)
             break;
-        if (m_markerData->markerStart == resource) {
-            m_markerData->markerStart->removeAllClientsFromCache();
+        if (m_markerData->markerStart == resource)
             m_markerData->markerStart = nullptr;
-        }
-        if (m_markerData->markerMid == resource) {
-            m_markerData->markerMid->removeAllClientsFromCache();
+        if (m_markerData->markerMid == resource)
             m_markerData->markerMid = nullptr;
-        }
-        if (m_markerData->markerEnd == resource) {
-            m_markerData->markerEnd->removeAllClientsFromCache();
+        if (m_markerData->markerEnd == resource)
             m_markerData->markerEnd = nullptr;
-        }
         break;
     case PatternResourceType:
     case LinearGradientResourceType:
     case RadialGradientResourceType:
         if (!m_fillStrokeData)
             break;
-        if (m_fillStrokeData->fill == resource) {
-            m_fillStrokeData->fill->removeAllClientsFromCache();
+        if (m_fillStrokeData->fill == resource)
             m_fillStrokeData->fill = nullptr;
-        }
-        if (m_fillStrokeData->stroke == resource) {
-            m_fillStrokeData->stroke->removeAllClientsFromCache();
+        if (m_fillStrokeData->stroke == resource)
             m_fillStrokeData->stroke = nullptr;
-        }
         break;
     case FilterResourceType:
         if (!m_clipperFilterMaskerData)
             break;
-        if (m_clipperFilterMaskerData->filter == resource) {
-            m_clipperFilterMaskerData->filter->removeAllClientsFromCache();
+        if (m_clipperFilterMaskerData->filter == resource)
             m_clipperFilterMaskerData->filter = nullptr;
-        }
         break;
     case ClipperResourceType:
         if (!m_clipperFilterMaskerData)
             break;
-        if (m_clipperFilterMaskerData->clipper == resource) {
-            m_clipperFilterMaskerData->clipper->removeAllClientsFromCache();
+        if (m_clipperFilterMaskerData->clipper == resource)
             m_clipperFilterMaskerData->clipper = nullptr;
-        }
         break;
     default:
         ASSERT_NOT_REACHED();

third_party/WebKit/Source/core/layout/svg/SVGResourcesCache.cpp

@@ -169,24 +169,4 @@ void SVGResourcesCache::clientDestroyed(LayoutObject* layoutObject)
     cache.removeResourcesFromLayoutObject(layoutObject);
 }
 
-void SVGResourcesCache::resourceDestroyed(LayoutSVGResourceContainer* resource)
-{
-    ASSERT(resource);
-    SVGResourcesCache& cache = resourcesCache(resource->document());
-
-    // The resource itself may have clients, that need to be notified.
-    cache.removeResourcesFromLayoutObject(resource);
-
-    for (auto& objectResources : cache.m_cache) {
-        objectResources.value->resourceDestroyed(resource);
-
-        // Mark users of destroyed resources as pending resolution based on the id of the old resource.
-        Element* resourceElement = resource->element();
-        Element* clientElement = toElement(objectResources.key->node());
-        SVGDocumentExtensions& extensions = clientElement->document().accessSVGExtensions();
-
-        extensions.addPendingResource(resourceElement->fastGetAttribute(HTMLNames::idAttr), clientElement);
-    }
-}
-
 }

third_party/WebKit/Source/core/layout/svg/SVGResourcesCache.h

@@ -56,9 +56,6 @@ public:
     // Called from all SVG layoutObjects styleDidChange() methods.
     static void clientStyleChanged(LayoutObject*, StyleDifference, const ComputedStyle& newStyle);
 
-    // Called from LayoutSVGResourceContainer::willBeDestroyed().
-    static void resourceDestroyed(LayoutSVGResourceContainer*);
-
 private:
     void addResourcesFromLayoutObject(LayoutObject*, const ComputedStyle&);
     void removeResourcesFromLayoutObject(LayoutObject*);