From 49ad697403f7031f292924096851b498f788bbfd Mon Sep 17 00:00:00 2001 From: Javier Fernandez Date: Mon, 7 Mar 2016 15:05:43 +0100 Subject: Breaking several cyclic references between SVG animated properties. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Based on upstream fix by sabouhallawa@apple.com at http://trac.webkit.org/changeset/196268 The leak happens because of cyclic reference between SVGListPropertyTearOff and SVGAnimatedListPropertyTearOff which is derived from SVGAnimatedProperty. There is also cyclic reference between SVGAnimatedProperty and SVGElement and this causes the whole document to be leaked. So if the JS requests, for example, an instance of SVGPolylineElement.points, the whole document will be leaked. The fix depends on having the cyclic reference as is since the owning and the owned classes have to live together if any of them is referenced. But the owning class caches a raw 'ref-counted' pointer of the owned class. If it is requested for an instance of the owned class it returned a RefPtr<> of it. Once the owned class is not used, it can delete itself. The only thing needed here is to notify the owner class of the deletion so it cleans its caches and be able to create a new pointer if it is requested for an instance of the owned class later. Revert the change of r181345 in SVGAnimatedProperty::lookupOrCreateWrapper() to break the cyclic reference between SVGElement and SVGAnimatedProperty. Also apply the same approach in SVGAnimatedListPropertyTearOff::baseVal() and animVal() to break cyclic reference between SVGListPropertyTearOff and SVGAnimatedListPropertyTearOff. Change-Id: Ied6a077299e47855feb235a1c9310f1a58aad91b Reviewed-by: Konstantin Tokarev Reviewed-by: Michael Brüning --- Source/WebCore/svg/SVGPolyElement.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'Source/WebCore/svg/SVGPolyElement.cpp') diff --git a/Source/WebCore/svg/SVGPolyElement.cpp b/Source/WebCore/svg/SVGPolyElement.cpp index fd5c652f1..977234d5e 100644 --- a/Source/WebCore/svg/SVGPolyElement.cpp +++ b/Source/WebCore/svg/SVGPolyElement.cpp @@ -88,8 +88,8 @@ void SVGPolyElement::parseAttribute(const QualifiedName& name, const AtomicStrin if (!pointsListFromSVGData(newList, value)) document()->accessSVGExtensions()->reportError("Problem parsing points=\"" + value + "\""); - if (SVGAnimatedProperty* wrapper = SVGAnimatedProperty::lookupWrapper(this, pointsPropertyInfo())) - static_cast(wrapper)->detachListWrappers(newList.size()); + if (RefPtr wrapper = SVGAnimatedProperty::lookupWrapper(this, pointsPropertyInfo())) + static_pointer_cast(wrapper)->detachListWrappers(newList.size()); m_points.value = newList; return; @@ -147,16 +147,16 @@ PassRefPtr SVGPolyElement::lookupOrCreatePointsWrapper(SVGE (ownerType, pointsPropertyInfo(), ownerType->m_points.value); } -SVGListPropertyTearOff* SVGPolyElement::points() +RefPtr > SVGPolyElement::points() { m_points.shouldSynchronize = true; - return static_cast*>(static_pointer_cast(lookupOrCreatePointsWrapper(this))->baseVal()); + return static_cast*>(static_pointer_cast(lookupOrCreatePointsWrapper(this))->baseVal().get()); } -SVGListPropertyTearOff* SVGPolyElement::animatedPoints() +RefPtr > SVGPolyElement::animatedPoints() { m_points.shouldSynchronize = true; - return static_cast*>(static_pointer_cast(lookupOrCreatePointsWrapper(this))->animVal()); + return static_cast*>(static_pointer_cast(lookupOrCreatePointsWrapper(this))->animVal().get()); } } -- cgit v1.2.1