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/bindings/scripts/CodeGeneratorJS.pm | 6 +- Source/WebCore/svg/SVGPathElement.cpp | 23 +++--- Source/WebCore/svg/SVGPathElement.h | 8 +- Source/WebCore/svg/SVGPathSegWithContext.h | 2 +- Source/WebCore/svg/SVGPolyElement.cpp | 12 +-- Source/WebCore/svg/SVGPolyElement.h | 4 +- Source/WebCore/svg/SVGViewSpec.cpp | 8 +- Source/WebCore/svg/SVGViewSpec.h | 2 +- .../properties/SVGAnimatedListPropertyTearOff.h | 86 ++++++++++++++-------- .../SVGAnimatedPathSegListPropertyTearOff.h | 30 ++++---- .../WebCore/svg/properties/SVGAnimatedProperty.h | 22 +++--- .../svg/properties/SVGAnimatedPropertyMacros.h | 4 +- .../SVGAnimatedTransformListPropertyTearOff.h | 22 ++++-- .../svg/properties/SVGListPropertyTearOff.h | 6 ++ .../properties/SVGPathSegListPropertyTearOff.cpp | 4 +- 15 files changed, 145 insertions(+), 94 deletions(-) (limited to 'Source/WebCore') diff --git a/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm b/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm index df4234e23..2cca58703 100644 --- a/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm +++ b/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm @@ -3624,7 +3624,11 @@ sub NativeToJSValue return "toJSNewlyCreated(exec, $globalObject, WTF::getPtr($value))"; } - if ($codeGenerator->IsSVGAnimatedType($interfaceName) or $interfaceName eq "SVGViewSpec") { + # $type has to be used here because SVGViewSpec.transform is of type SVGTransformList. + if ($codeGenerator->IsSVGTypeNeedingTearOff($type) and $type =~ /(? to real type, so the right toJS() method can be invoked. + $value = "static_cast<" . GetNativeType($type) . ">($value" . ".get())"; + } elsif ($codeGenerator->IsSVGAnimatedType($interfaceName) or $interfaceName eq "SVGViewSpec") { # Convert from abstract SVGProperty to real type, so the right toJS() method can be invoked. $value = "static_cast<" . GetNativeType($type) . ">($value)"; } elsif ($codeGenerator->IsSVGTypeNeedingTearOff($type) and not $interfaceName =~ /List$/) { diff --git a/Source/WebCore/svg/SVGPathElement.cpp b/Source/WebCore/svg/SVGPathElement.cpp index 3324d0b5d..b030c9a83 100644 --- a/Source/WebCore/svg/SVGPathElement.cpp +++ b/Source/WebCore/svg/SVGPathElement.cpp @@ -310,10 +310,15 @@ void SVGPathElement::removedFrom(ContainerNode* rootParent) SVGPathByteStream* SVGPathElement::pathByteStream() const { - SVGAnimatedProperty* property = SVGAnimatedProperty::lookupWrapper(this, dPropertyInfo()); + RefPtr property = SVGAnimatedProperty::lookupWrapper(this, dPropertyInfo()); if (!property || !property->isAnimating()) return m_pathByteStream.get(); - return static_cast(property)->animatedPathByteStream(); + + SVGPathByteStream* animatedPathByteStream = static_cast(property.get())->animatedPathByteStream(); + if (!animatedPathByteStream) + return m_pathByteStream.get(); + + return animatedPathByteStream; } PassRefPtr SVGPathElement::lookupOrCreateDWrapper(SVGElement* contextElement) @@ -321,7 +326,7 @@ PassRefPtr SVGPathElement::lookupOrCreateDWrapper(SVGElemen ASSERT(contextElement); SVGPathElement* ownerType = toSVGPathElement(contextElement); - if (SVGAnimatedProperty* property = SVGAnimatedProperty::lookupWrapper(ownerType, dPropertyInfo())) + if (RefPtr property = SVGAnimatedProperty::lookupWrapper(ownerType, dPropertyInfo())) return property; // Build initial SVGPathSegList. @@ -340,26 +345,26 @@ void SVGPathElement::synchronizeD(SVGElement* contextElement) ownerType->m_pathSegList.synchronize(ownerType, dPropertyInfo()->attributeName, ownerType->m_pathSegList.value.valueAsString()); } -SVGPathSegListPropertyTearOff* SVGPathElement::pathSegList() +RefPtr SVGPathElement::pathSegList() { m_pathSegList.shouldSynchronize = true; - return static_cast(static_pointer_cast(lookupOrCreateDWrapper(this))->baseVal()); + return static_cast(static_pointer_cast(lookupOrCreateDWrapper(this))->baseVal().get()); } -SVGPathSegListPropertyTearOff* SVGPathElement::normalizedPathSegList() +RefPtr SVGPathElement::normalizedPathSegList() { // FIXME: https://bugs.webkit.org/show_bug.cgi?id=15412 - Implement normalized path segment lists! return 0; } -SVGPathSegListPropertyTearOff* SVGPathElement::animatedPathSegList() +RefPtr SVGPathElement::animatedPathSegList() { m_pathSegList.shouldSynchronize = true; m_isAnimValObserved = true; - return static_cast(static_pointer_cast(lookupOrCreateDWrapper(this))->animVal()); + return static_cast(static_pointer_cast(lookupOrCreateDWrapper(this))->animVal().get()); } -SVGPathSegListPropertyTearOff* SVGPathElement::animatedNormalizedPathSegList() +RefPtr SVGPathElement::animatedNormalizedPathSegList() { // FIXME: https://bugs.webkit.org/show_bug.cgi?id=15412 - Implement normalized path segment lists! return 0; diff --git a/Source/WebCore/svg/SVGPathElement.h b/Source/WebCore/svg/SVGPathElement.h index c81ead198..0ca66ac11 100644 --- a/Source/WebCore/svg/SVGPathElement.h +++ b/Source/WebCore/svg/SVGPathElement.h @@ -83,10 +83,10 @@ public: PassRefPtr createSVGPathSegCurvetoQuadraticSmoothRel(float x, float y, SVGPathSegRole role = PathSegUndefinedRole); // Used in the bindings only. - SVGPathSegListPropertyTearOff* pathSegList(); - SVGPathSegListPropertyTearOff* animatedPathSegList(); - SVGPathSegListPropertyTearOff* normalizedPathSegList(); - SVGPathSegListPropertyTearOff* animatedNormalizedPathSegList(); + RefPtr pathSegList(); + RefPtr animatedPathSegList(); + RefPtr normalizedPathSegList(); + RefPtr animatedNormalizedPathSegList(); SVGPathByteStream* pathByteStream() const; diff --git a/Source/WebCore/svg/SVGPathSegWithContext.h b/Source/WebCore/svg/SVGPathSegWithContext.h index 701067227..4ba1447bf 100644 --- a/Source/WebCore/svg/SVGPathSegWithContext.h +++ b/Source/WebCore/svg/SVGPathSegWithContext.h @@ -33,7 +33,7 @@ public: { } - SVGAnimatedProperty* animatedProperty() const + RefPtr animatedProperty() const { switch (m_role) { case PathSegUndefinedRole: 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()); } } diff --git a/Source/WebCore/svg/SVGPolyElement.h b/Source/WebCore/svg/SVGPolyElement.h index bd0dd49aa..f399a427e 100644 --- a/Source/WebCore/svg/SVGPolyElement.h +++ b/Source/WebCore/svg/SVGPolyElement.h @@ -33,8 +33,8 @@ namespace WebCore { class SVGPolyElement : public SVGGraphicsElement , public SVGExternalResourcesRequired { public: - SVGListPropertyTearOff* points(); - SVGListPropertyTearOff* animatedPoints(); + RefPtr > points(); + RefPtr > animatedPoints(); SVGPointList& pointList() const { return m_points.value; } diff --git a/Source/WebCore/svg/SVGViewSpec.cpp b/Source/WebCore/svg/SVGViewSpec.cpp index 253c082fb..5ac02dbdd 100644 --- a/Source/WebCore/svg/SVGViewSpec.cpp +++ b/Source/WebCore/svg/SVGViewSpec.cpp @@ -117,8 +117,8 @@ void SVGViewSpec::setTransformString(const String& transform) SVGTransformList newList; newList.parse(transform); - if (SVGAnimatedProperty* wrapper = SVGAnimatedProperty::lookupWrapper(m_contextElement, transformPropertyInfo())) - static_cast(wrapper)->detachListWrappers(newList.size()); + if (RefPtr wrapper = SVGAnimatedProperty::lookupWrapper(m_contextElement, transformPropertyInfo())) + static_pointer_cast(wrapper)->detachListWrappers(newList.size()); m_transform = newList; } @@ -148,12 +148,12 @@ SVGElement* SVGViewSpec::viewTarget() const return toSVGElement(element); } -SVGTransformListPropertyTearOff* SVGViewSpec::transform() +RefPtr SVGViewSpec::transform() { if (!m_contextElement) return 0; // Return the animVal here, as its readonly by default - which is exactly what we want here. - return static_cast(static_pointer_cast(lookupOrCreateTransformWrapper(this))->animVal()); + return static_cast(static_pointer_cast(lookupOrCreateTransformWrapper(this))->animVal().get()); } PassRefPtr SVGViewSpec::viewBoxAnimated() diff --git a/Source/WebCore/svg/SVGViewSpec.h b/Source/WebCore/svg/SVGViewSpec.h index f8634afd6..3887db7f5 100644 --- a/Source/WebCore/svg/SVGViewSpec.h +++ b/Source/WebCore/svg/SVGViewSpec.h @@ -69,7 +69,7 @@ public: void resetContextElement() { m_contextElement = 0; } // Custom non-animated 'transform' property. - SVGTransformListPropertyTearOff* transform(); + RefPtr transform(); SVGTransformList transformBaseValue() const { return m_transform; } // Custom animated 'viewBox' property. diff --git a/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h b/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h index 65ee9e58d..136ba8253 100644 --- a/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h +++ b/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h @@ -40,35 +40,49 @@ public: typedef SVGListPropertyTearOff ListPropertyTearOff; typedef PropertyType ContentType; - virtual ListProperty* baseVal() + virtual PassRefPtr baseVal() { - if (!m_baseVal) - m_baseVal = ListPropertyTearOff::create(this, BaseValRole, m_values, m_wrappers); - return static_cast(m_baseVal.get()); + if (m_baseVal) + return m_baseVal; + + RefPtr property = ListPropertyTearOff::create(this, BaseValRole, m_values, m_wrappers).get(); + m_baseVal = property.get(); + return property.release(); + } + + virtual PassRefPtr animVal() + { + if (m_animVal) + return m_animVal; + + RefPtr property = ListPropertyTearOff::create(this, AnimValRole, m_values, m_wrappers).get(); + m_animVal = property.get(); + return property.release(); } - virtual ListProperty* animVal() + void propertyWillBeDeleted(const ListProperty& property) { - if (!m_animVal) - m_animVal = ListPropertyTearOff::create(this, AnimValRole, m_values, m_wrappers); - return static_cast(m_animVal.get()); + if (&property == m_baseVal) + m_baseVal = 0; + else if (&property == m_animVal) + m_animVal = 0; } virtual bool isAnimatedListTearOff() const { return true; } - int findItem(SVGProperty* property) const + int findItem(SVGProperty* property) { // This should ever be called for our baseVal, as animVal can't modify the list. // It's safe to cast to ListPropertyTearOff here as all classes inheriting from us supply their own removeItemFromList() method. typedef SVGPropertyTearOff::ListItemType> ListItemTearOff; - return static_cast(m_baseVal.get())->findItem(static_cast(property)); + return static_pointer_cast(baseVal())->findItem(static_cast(property)); } void removeItemFromList(size_t itemIndex, bool shouldSynchronizeWrappers) { // This should ever be called for our baseVal, as animVal can't modify the list. // It's safe to cast to ListPropertyTearOff here as all classes inheriting from us supply their own removeItemFromList() method. - static_cast(m_baseVal.get())->removeItemFromList(itemIndex, shouldSynchronizeWrappers); + static_pointer_cast(baseVal())->removeItemFromList(itemIndex, shouldSynchronizeWrappers); } void detachListWrappers(unsigned newListSize) @@ -79,8 +93,8 @@ public: PropertyType& currentAnimatedValue() { ASSERT(m_isAnimating); - ASSERT(m_animVal); - return static_cast(m_animVal.get())->values(); + ASSERT(m_animatingAnimVal); + return static_pointer_cast(m_animatingAnimVal)->values(); } const PropertyType& currentBaseValue() const @@ -91,6 +105,7 @@ public: void animationStarted(PropertyType* newAnimVal, bool shouldOwnValues = false) { ASSERT(!m_isAnimating); + ASSERT(!m_animatingAnimVal); ASSERT(newAnimVal); ASSERT(m_values.size() == m_wrappers.size()); ASSERT(m_animatedWrappers.isEmpty()); @@ -99,49 +114,49 @@ public: if (!newAnimVal->isEmpty()) m_animatedWrappers.fill(0, newAnimVal->size()); - ListProperty* animVal = static_cast(this->animVal()); - animVal->setValuesAndWrappers(newAnimVal, &m_animatedWrappers, shouldOwnValues); - ASSERT(animVal->values().size() == animVal->wrappers().size()); - ASSERT(animVal->wrappers().size() == m_animatedWrappers.size()); + m_animatingAnimVal = animVal(); + m_animatingAnimVal->setValuesAndWrappers(newAnimVal, &m_animatedWrappers, shouldOwnValues); + ASSERT(m_animatingAnimVal->values().size() == m_animatingAnimVal->wrappers().size()); + ASSERT(m_animatingAnimVal->wrappers().size() == m_animatedWrappers.size()); m_isAnimating = true; } void animationEnded() { ASSERT(m_isAnimating); - ASSERT(m_animVal); + ASSERT(m_animatingAnimVal); ASSERT(m_values.size() == m_wrappers.size()); - ListProperty* animVal = static_cast(m_animVal.get()); - ASSERT(animVal->values().size() == animVal->wrappers().size()); - ASSERT(animVal->wrappers().size() == m_animatedWrappers.size()); + ASSERT(m_animatingAnimVal->values().size() == m_animatingAnimVal->wrappers().size()); + ASSERT(m_animatingAnimVal->wrappers().size() == m_animatedWrappers.size()); - animVal->setValuesAndWrappers(&m_values, &m_wrappers, false); - ASSERT(animVal->values().size() == animVal->wrappers().size()); - ASSERT(animVal->wrappers().size() == m_wrappers.size()); + m_animatingAnimVal->setValuesAndWrappers(&m_values, &m_wrappers, false); + ASSERT(m_animatingAnimVal->values().size() == m_animatingAnimVal->wrappers().size()); + ASSERT(m_animatingAnimVal->wrappers().size() == m_wrappers.size()); m_animatedWrappers.clear(); + m_animatingAnimVal = 0; m_isAnimating = false; } void synchronizeWrappersIfNeeded() { + ASSERT(m_isAnimating); + ASSERT(m_animatingAnimVal); + // Eventually the wrapper list needs synchronization because any SVGAnimateLengthList::calculateAnimatedValue() call may // mutate the length of our values() list, and thus the wrapper() cache needs synchronization, to have the same size. // Also existing wrappers which point directly at elements in the existing SVGLengthList have to be detached (so a copy // of them is created, so existing animVal variables in JS are kept-alive). If we'd detach them later the underlying // SVGLengthList was already mutated, and our list item wrapper tear offs would point nowhere. Assertions would fire. - ListProperty* animVal = static_cast(m_animVal.get()); - animVal->detachListWrappers(animVal->values().size()); + m_animatingAnimVal->detachListWrappers(m_animatingAnimVal->values().size()); - ASSERT(animVal->values().size() == animVal->wrappers().size()); - ASSERT(animVal->wrappers().size() == m_animatedWrappers.size()); + ASSERT(m_animatingAnimVal->values().size() == m_animatingAnimVal->wrappers().size()); + ASSERT(m_animatingAnimVal->wrappers().size() == m_animatedWrappers.size()); } void animValWillChange() { - ASSERT(m_isAnimating); - ASSERT(m_animVal); ASSERT(m_values.size() == m_wrappers.size()); synchronizeWrappersIfNeeded(); } @@ -164,6 +179,8 @@ protected: SVGAnimatedListPropertyTearOff(SVGElement* contextElement, const QualifiedName& attributeName, AnimatedPropertyType animatedPropertyType, PropertyType& values) : SVGAnimatedProperty(contextElement, attributeName, animatedPropertyType) , m_values(values) + , m_baseVal(0) + , m_animVal(0) { if (!values.isEmpty()) m_wrappers.fill(0, values.size()); @@ -174,8 +191,13 @@ protected: ListWrapperCache m_wrappers; ListWrapperCache m_animatedWrappers; - RefPtr m_baseVal; - RefPtr m_animVal; + // Cache the raw pointer but return a RefPtr<>. This will break the cyclic reference + // between SVGListPropertyTearOff and SVGAnimatedListPropertyTearOff once the property + // pointer is not needed. + ListProperty* m_baseVal; + ListProperty* m_animVal; + + RefPtr m_animatingAnimVal; }; } diff --git a/Source/WebCore/svg/properties/SVGAnimatedPathSegListPropertyTearOff.h b/Source/WebCore/svg/properties/SVGAnimatedPathSegListPropertyTearOff.h index eb6dd19e3..1cfe0230e 100644 --- a/Source/WebCore/svg/properties/SVGAnimatedPathSegListPropertyTearOff.h +++ b/Source/WebCore/svg/properties/SVGAnimatedPathSegListPropertyTearOff.h @@ -32,32 +32,36 @@ namespace WebCore { class SVGAnimatedPathSegListPropertyTearOff : public SVGAnimatedListPropertyTearOff { public: - virtual SVGListProperty* baseVal() + virtual PassRefPtr baseVal() OVERRIDE { - if (!m_baseVal) - m_baseVal = SVGPathSegListPropertyTearOff::create(this, BaseValRole, PathSegUnalteredRole, m_values, m_wrappers); - return static_cast*>(m_baseVal.get()); + if (m_baseVal) + return m_baseVal; + + RefPtr property = SVGPathSegListPropertyTearOff::create(this, BaseValRole, PathSegUnalteredRole, m_values, m_wrappers); + m_baseVal = property.get(); + return property.release(); } - virtual SVGListProperty* animVal() + virtual PassRefPtr animVal() OVERRIDE { - if (!m_animVal) - m_animVal = SVGPathSegListPropertyTearOff::create(this, AnimValRole, PathSegUnalteredRole, m_values, m_wrappers); - return static_cast*>(m_animVal.get()); + if (m_animVal) + return m_animVal; + + RefPtr property = SVGPathSegListPropertyTearOff::create(this, AnimValRole, PathSegUnalteredRole, m_values, m_wrappers); + m_animVal = property.get(); + return property.release(); } - int findItem(const RefPtr& segment) const + int findItem(const RefPtr& segment) { // This should ever be called for our baseVal, as animVal can't modify the list. - ASSERT(m_baseVal); - return static_cast(m_baseVal.get())->findItem(segment); + return static_cast(baseVal().get())->findItem(segment); } void removeItemFromList(size_t itemIndex, bool shouldSynchronizeWrappers) { // This should ever be called for our baseVal, as animVal can't modify the list. - ASSERT(m_baseVal); - static_cast(m_baseVal.get())->removeItemFromList(itemIndex, shouldSynchronizeWrappers); + static_cast(baseVal().get())->removeItemFromList(itemIndex, shouldSynchronizeWrappers); } static PassRefPtr create(SVGElement* contextElement, const QualifiedName& attributeName, AnimatedPropertyType animatedPropertyType, SVGPathSegList& values) diff --git a/Source/WebCore/svg/properties/SVGAnimatedProperty.h b/Source/WebCore/svg/properties/SVGAnimatedProperty.h index 93f076b2b..d40a50434 100644 --- a/Source/WebCore/svg/properties/SVGAnimatedProperty.h +++ b/Source/WebCore/svg/properties/SVGAnimatedProperty.h @@ -53,18 +53,22 @@ public: { ASSERT(info); SVGAnimatedPropertyDescription key(element, info->propertyIdentifier); - RefPtr wrapper = animatedPropertyCache()->get(key); - if (!wrapper) { - wrapper = TearOffType::create(element, info->attributeName, info->animatedPropertyType, property); - if (info->animatedPropertyState == PropertyIsReadOnly) - wrapper->setIsReadOnly(); - animatedPropertyCache()->set(key, wrapper.get()); - } + Cache::AddResult result = animatedPropertyCache()->add(key, 0); + if (!result.isNewEntry) + return static_cast(result.iterator->value); + + RefPtr wrapper = TearOffType::create(element, info->attributeName, info->animatedPropertyType, property); + if (info->animatedPropertyState == PropertyIsReadOnly) + wrapper->setIsReadOnly(); + + // Cache the raw pointer but return a RefPtr<>. This will break the cyclic reference + // between SVGAnimatedProperty and SVGElement once the property pointer is not needed. + result.iterator->value = wrapper.get(); return static_pointer_cast(wrapper); } template - static TearOffType* lookupWrapper(OwnerType* element, const SVGPropertyInfo* info) + static PassRefPtr lookupWrapper(OwnerType* element, const SVGPropertyInfo* info) { ASSERT(info); SVGAnimatedPropertyDescription key(element, info->propertyIdentifier); @@ -72,7 +76,7 @@ public: } template - static TearOffType* lookupWrapper(const OwnerType* element, const SVGPropertyInfo* info) + static PassRefPtr lookupWrapper(const OwnerType* element, const SVGPropertyInfo* info) { return lookupWrapper(const_cast(element), info); } diff --git a/Source/WebCore/svg/properties/SVGAnimatedPropertyMacros.h b/Source/WebCore/svg/properties/SVGAnimatedPropertyMacros.h index 1ddbc5c6c..e31015e52 100644 --- a/Source/WebCore/svg/properties/SVGAnimatedPropertyMacros.h +++ b/Source/WebCore/svg/properties/SVGAnimatedPropertyMacros.h @@ -118,7 +118,7 @@ public: \ static const SVGPropertyInfo* LowerProperty##PropertyInfo(); \ PropertyType& LowerProperty() const \ { \ - if (TearOffType* wrapper = SVGAnimatedProperty::lookupWrapper(this, LowerProperty##PropertyInfo())) { \ + if (RefPtr wrapper = SVGAnimatedProperty::lookupWrapper(this, LowerProperty##PropertyInfo())) { \ if (wrapper->isAnimating()) \ return wrapper->currentAnimatedValue(); \ } \ @@ -179,7 +179,7 @@ private: \ DECLARE_ANIMATED_PROPERTY(TearOffType, PropertyType, UpperProperty, LowerProperty) \ void detachAnimated##UpperProperty##ListWrappers(unsigned newListSize) \ { \ - if (TearOffType* wrapper = SVGAnimatedProperty::lookupWrapper(this, LowerProperty##PropertyInfo())) \ + if (RefPtr wrapper = SVGAnimatedProperty::lookupWrapper(this, LowerProperty##PropertyInfo())) \ wrapper->detachListWrappers(newListSize); \ } diff --git a/Source/WebCore/svg/properties/SVGAnimatedTransformListPropertyTearOff.h b/Source/WebCore/svg/properties/SVGAnimatedTransformListPropertyTearOff.h index 6f643e87c..2953f2ff6 100644 --- a/Source/WebCore/svg/properties/SVGAnimatedTransformListPropertyTearOff.h +++ b/Source/WebCore/svg/properties/SVGAnimatedTransformListPropertyTearOff.h @@ -29,18 +29,24 @@ namespace WebCore { class SVGAnimatedTransformListPropertyTearOff : public SVGAnimatedListPropertyTearOff { public: - virtual SVGListPropertyTearOff* baseVal() + virtual PassRefPtr baseVal() OVERRIDE { - if (!m_baseVal) - m_baseVal = SVGTransformListPropertyTearOff::create(this, BaseValRole, m_values, m_wrappers); - return static_cast*>(m_baseVal.get()); + if (m_baseVal) + return m_baseVal; + + RefPtr property = SVGTransformListPropertyTearOff::create(this, BaseValRole, m_values, m_wrappers); + m_baseVal = property.get(); + return property.release(); } - virtual SVGListPropertyTearOff* animVal() + virtual PassRefPtr animVal() OVERRIDE { - if (!m_animVal) - m_animVal = SVGTransformListPropertyTearOff::create(this, AnimValRole, m_values, m_wrappers); - return static_cast*>(m_animVal.get()); + if (m_animVal) + return m_animVal; + + RefPtr property = SVGTransformListPropertyTearOff::create(this, AnimValRole, m_values, m_wrappers); + m_animVal = property.get(); + return property.release(); } static PassRefPtr create(SVGElement* contextElement, const QualifiedName& attributeName, AnimatedPropertyType animatedPropertyType, SVGTransformList& values) diff --git a/Source/WebCore/svg/properties/SVGListPropertyTearOff.h b/Source/WebCore/svg/properties/SVGListPropertyTearOff.h index 3c6552467..326a89a74 100644 --- a/Source/WebCore/svg/properties/SVGListPropertyTearOff.h +++ b/Source/WebCore/svg/properties/SVGListPropertyTearOff.h @@ -121,6 +121,12 @@ protected: { } + virtual ~SVGListPropertyTearOff() + { + if (m_animatedProperty) + m_animatedProperty->propertyWillBeDeleted(*this); + } + virtual bool isReadOnly() const { if (m_role == AnimValRole) diff --git a/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.cpp b/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.cpp index 573d453a4..51c48b4c3 100644 --- a/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.cpp +++ b/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.cpp @@ -95,7 +95,7 @@ SVGPathElement* SVGPathSegListPropertyTearOff::contextElement() const bool SVGPathSegListPropertyTearOff::processIncomingListItemValue(const ListItemType& newItem, unsigned* indexToModify) { SVGPathSegWithContext* newItemWithContext = static_cast(newItem.get()); - SVGAnimatedProperty* animatedPropertyOfItem = newItemWithContext->animatedProperty(); + RefPtr animatedPropertyOfItem = newItemWithContext->animatedProperty(); // Alter the role, after calling animatedProperty(), as that may influence the returned animated property. newItemWithContext->setContextAndRole(contextElement(), m_pathSegRole); @@ -111,7 +111,7 @@ bool SVGPathSegListPropertyTearOff::processIncomingListItemValue(const ListItemT // Spec: If newItem is already in a list, it is removed from its previous list before it is inserted into this list. // 'newItem' is already living in another list. If it's not our list, synchronize the other lists wrappers after the removal. bool livesInOtherList = animatedPropertyOfItem != m_animatedProperty; - SVGAnimatedPathSegListPropertyTearOff* propertyTearOff = static_cast(animatedPropertyOfItem); + RefPtr propertyTearOff = static_pointer_cast(animatedPropertyOfItem); int indexToRemove = propertyTearOff->findItem(newItem.get()); ASSERT(indexToRemove != -1); -- cgit v1.2.1 From 21a11f7197da091eae93e987e73fbf5537dfaf3a Mon Sep 17 00:00:00 2001 From: Said Abou-Hallawa Date: Tue, 8 Mar 2016 16:13:36 +0100 Subject: Fixed crashes on SVG path animation use cases. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Based on upstream fix by said@apple.com at http://trac.webkit.org/changeset/196670 A destructor was added to SVGListPropertyTearOff that notifies its wrapper (the SVGAnimatedListPropertyTearoff) about its deletion. This allows the wrapper to nullify any references to the wrapped content. We needed to do the same thing for SVGPathSegListPropertyTearOff. Both SVGPathSegListPropertyTearOff and SVGListPropertyTearOff inherit from SVGListProperty and both hold pointers to SVGAnimatedListPropertyTearOff which needs to be notified. Change-Id: I1873825c7bdc07bf06cd5c300156ebe084f2607e Reviewed-by: Konstantin Tokarev Reviewed-by: Michael Brüning --- Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.h | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'Source/WebCore') diff --git a/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.h b/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.h index ee704d681..4c1a30c4b 100644 --- a/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.h +++ b/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.h @@ -111,6 +111,12 @@ public: return Base::appendItemValues(newItem, ec); } + virtual ~SVGPathSegListPropertyTearOff() + { + if (m_animatedProperty) + m_animatedProperty->propertyWillBeDeleted(*this); + } + private: SVGPathSegListPropertyTearOff(AnimatedListPropertyTearOff* animatedProperty, SVGPropertyRole role, SVGPathSegRole pathSegRole, SVGPathSegList& values, ListWrapperCache& wrappers) : SVGListProperty(role, values, &wrappers) -- cgit v1.2.1 From 7205faf1a546a690f68176989100109e9a3335b7 Mon Sep 17 00:00:00 2001 From: Javier Fernandez Date: Mon, 7 Mar 2016 23:56:13 +0100 Subject: Many assertion failures and crashes on SVG path animation cases when JS garbage collection happens quickly. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Based on upstream fix by said@apple.com at http://trac.webkit.org/changeset/197125 Since the whole document was leaking once an SVGAnimatedProperty was created so there was no way to produce this bug. After fixing the leak, one crash and one assert got uncovered. Both of them happen because of the fact: "if an SVGAnimatedProperty is not referenced it will be deleted." * svg/SVGPathElement.cpp: (WebCore::SVGPathElement::lookupOrCreateDWrapper): The code in this function was assuming that the wrapper will be created only once which happens when SVGAnimatedProperty::lookupOrCreateWrapper() is called. Before making this single call, lookupOrCreateDWrapper() was building an initial SVGPathSegList from byte stream. But now SVGAnimatedProperty::lookupWrapper() can return false even after creating the SVGAnimatedProperty because it was deleted later. Calling buildSVGPathSegListFromByteStream() more than once was causing SVGAnimatedListPropertyTearOff::animationStarted() to fire the assertion ASSERT(m_values.size() == m_wrappers.size()) because the path segments were appended twice to m_values which is in fact SVGPathElement::m_pathSegList.value. The fix is to build the initial SVGPathSegList only once which should happen when m_pathSegList.value.isEmpty(). (WebCore::SVGPathElement::animatedPropertyWillBeDeleted): * svg/SVGPathElement.h: * svg/properties/SVGAnimatedPathSegListPropertyTearOff.h: (WebCore::SVGAnimatedPathSegListPropertyTearOff::~SVGAnimatedPathSegListPropertyTearOff): SVGPathElement is assuming the following equivalence relation: m_pathSegList.shouldSynchronize ~ SVGAnimatedProperty_is_created_and_not_null. SVGPathElement::animatedPathSegList() and animatedNormalizedPathSegList() set m_pathSegList.shouldSynchronize to true when SVGAnimatedProperty is created but nothing sets m_pathSegList.shouldSynchronize back to false. This was not a problem when the SVGAnimatedProperty was leaking but after ensuring it is deleted when it is not referenced this equivalence relation becomes untrue sometimes. This caused SVGPathElement::svgAttributeChanged() to crash when we check m_pathSegList.shouldSynchronize and if it is true we assume that SVGAnimatedProperty::lookupWrapper() will return a non-null pointer and therefore we deference this pointer and call SVGAnimatedProperty::isAnimating(). To fix this crash we need to set m_pathSegList.shouldSynchronize back to false when the associated SVGAnimatedProperty is deleted. Change-Id: I05be755635b02d0d76105fc2eb21c2f013298c4e Reviewed-by: Michael Brüning --- Source/WebCore/svg/SVGPathElement.cpp | 13 +++++++++++-- Source/WebCore/svg/SVGPathElement.h | 4 +++- .../svg/properties/SVGAnimatedPathSegListPropertyTearOff.h | 7 +++++++ 3 files changed, 21 insertions(+), 3 deletions(-) (limited to 'Source/WebCore') diff --git a/Source/WebCore/svg/SVGPathElement.cpp b/Source/WebCore/svg/SVGPathElement.cpp index b030c9a83..c34c476f4 100644 --- a/Source/WebCore/svg/SVGPathElement.cpp +++ b/Source/WebCore/svg/SVGPathElement.cpp @@ -329,13 +329,22 @@ PassRefPtr SVGPathElement::lookupOrCreateDWrapper(SVGElemen if (RefPtr property = SVGAnimatedProperty::lookupWrapper(ownerType, dPropertyInfo())) return property; - // Build initial SVGPathSegList. - buildSVGPathSegListFromByteStream(ownerType->m_pathByteStream.get(), ownerType, ownerType->m_pathSegList.value, UnalteredParsing); + if (ownerType->m_pathSegList.value.isEmpty()) + buildSVGPathSegListFromByteStream(ownerType->m_pathByteStream.get(), ownerType, ownerType->m_pathSegList.value, UnalteredParsing); return SVGAnimatedProperty::lookupOrCreateWrapper (ownerType, dPropertyInfo(), ownerType->m_pathSegList.value); } +void SVGPathElement::animatedPropertyWillBeDeleted() +{ + // m_pathSegList.shouldSynchronize is set to true when the 'd' wrapper for m_pathSegList + // is created and cached. We need to reset it back to false when this wrapper is deleted + // so we can be sure if shouldSynchronize is true, SVGAnimatedProperty::lookupWrapper() + // will return a valid cached 'd' wrapper for the m_pathSegList. + m_pathSegList.shouldSynchronize = false; +} + void SVGPathElement::synchronizeD(SVGElement* contextElement) { ASSERT(contextElement); diff --git a/Source/WebCore/svg/SVGPathElement.h b/Source/WebCore/svg/SVGPathElement.h index 0ca66ac11..e9db8a56e 100644 --- a/Source/WebCore/svg/SVGPathElement.h +++ b/Source/WebCore/svg/SVGPathElement.h @@ -98,7 +98,9 @@ public: bool isAnimValObserved() const { return m_isAnimValObserved; } -private: + void animatedPropertyWillBeDeleted(); + + private: SVGPathElement(const QualifiedName&, Document*); virtual bool isValid() const { return SVGTests::isValid(); } diff --git a/Source/WebCore/svg/properties/SVGAnimatedPathSegListPropertyTearOff.h b/Source/WebCore/svg/properties/SVGAnimatedPathSegListPropertyTearOff.h index 1cfe0230e..dd5396aff 100644 --- a/Source/WebCore/svg/properties/SVGAnimatedPathSegListPropertyTearOff.h +++ b/Source/WebCore/svg/properties/SVGAnimatedPathSegListPropertyTearOff.h @@ -113,6 +113,13 @@ private: : SVGAnimatedListPropertyTearOff(contextElement, attributeName, animatedPropertyType, values) , m_animatedPathByteStream(0) { + ASSERT(contextElement); + ASSERT(toSVGPathElement(contextElement)); + } + + virtual ~SVGAnimatedPathSegListPropertyTearOff() + { + static_cast(contextElement())->animatedPropertyWillBeDeleted(); } SVGPathByteStream* m_animatedPathByteStream; -- cgit v1.2.1 From 3b4c850361abbc2aae556dbf99c8e4c8086ae569 Mon Sep 17 00:00:00 2001 From: Konstantin Tokarev Date: Tue, 22 Mar 2016 15:58:19 +0300 Subject: Allow using system SQLite without pkg-config Change-Id: Ifff0f8877a2d2d77a04468c205c3353f043e7738 Reviewed-by: Allan Sandfeld Jensen --- Source/WebCore/WebCore.pri | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'Source/WebCore') diff --git a/Source/WebCore/WebCore.pri b/Source/WebCore/WebCore.pri index ffe389d46..01893948e 100644 --- a/Source/WebCore/WebCore.pri +++ b/Source/WebCore/WebCore.pri @@ -233,7 +233,7 @@ use?(GRAPHICS_SURFACE) { } have?(sqlite3) { - mac { + osx|contains(QT_CONFIG, no-pkg-config) { LIBS += -lsqlite3 } else { PKGCONFIG += sqlite3 -- cgit v1.2.1 From b555ab7169f23d47fe980aa0c76c0eb9d18d6ab1 Mon Sep 17 00:00:00 2001 From: Konstantin Tokarev Date: Fri, 18 Mar 2016 21:57:07 +0300 Subject: Allow building QtWebKit on Windows with non-ICU Qt build. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also, from now use_wchar_unicode is never silently enabled when ICU config test fails, and requires qmake argument WEBKIT_CONFIG+=use_wchar_unicode. Change-Id: I434f5245c796b723a3bb116f62f8d53d05c3b4f7 Reviewed-by: Michael Brüning --- Source/WebCore/Target.pri | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'Source/WebCore') diff --git a/Source/WebCore/Target.pri b/Source/WebCore/Target.pri index d1aad9f9b..e525aa1fc 100644 --- a/Source/WebCore/Target.pri +++ b/Source/WebCore/Target.pri @@ -1122,7 +1122,6 @@ SOURCES += \ platform/text/TextCodecUTF8.cpp \ platform/text/TextCodecICU.cpp \ platform/text/TextEncoding.cpp \ - platform/text/TextEncodingDetectorICU.cpp \ platform/text/TextEncodingRegistry.cpp \ platform/text/TextStream.cpp \ platform/ThreadGlobalData.cpp \ @@ -2978,11 +2977,12 @@ mac { platform/text/cf/StringImplCF.cpp } -contains(QT_CONFIG,icu)|mac: SOURCES += platform/text/TextBreakIteratorICU.cpp use?(wchar_unicode): { SOURCES += platform/text/wchar/TextBreakIteratorWchar.cpp \ platform/text/TextEncodingDetectorNone.cpp - SOURCES -= platform/text/TextEncodingDetectorICU.cpp +} else { + SOURCES += platform/text/TextBreakIteratorICU.cpp \ + platform/text/TextEncodingDetectorICU.cpp } mac { -- cgit v1.2.1 From 4d435a7c294703e7a4a48cc962bc4f2a8db3f9c0 Mon Sep 17 00:00:00 2001 From: Konstantin Tokarev Date: Sun, 10 Apr 2016 01:46:27 +0000 Subject: Fixed compilation of JPEGImageDecoder with libjpeg v9. https://bugs.webkit.org/show_bug.cgi?id=156445 Patch by Konstantin Tokarev on 2016-04-09 Reviewed by Michael Catanzaro. ICU defines TRUE and FALSE macros, breaking libjpeg v9 headers. No new tests needed. * platform/image-decoders/jpeg/JPEGImageDecoder.h: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@199278 268f45cc-cd09-0410-ab3c-d52691b4dbfc Change-Id: I82db8bae210f8b03bd472a82925bd308fa01b6ca Reviewed-by: Allan Sandfeld Jensen --- Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.h | 3 +++ 1 file changed, 3 insertions(+) (limited to 'Source/WebCore') diff --git a/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.h b/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.h index b1f61a14d..3231eedba 100644 --- a/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.h +++ b/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.h @@ -40,6 +40,9 @@ #define XMD_H #endif +// ICU defines TRUE and FALSE macros, breaking libjpeg v9 headers +#undef TRUE +#undef FALSE extern "C" { #include "jpeglib.h" } -- cgit v1.2.1 From 71136c9621103522e85654c8e144d5f1c961de1c Mon Sep 17 00:00:00 2001 From: Maurice van der Pot Date: Mon, 31 Mar 2014 10:21:27 +0000 Subject: Fix mixed use of booleans in JPEGImageDecoder.cpp https://bugs.webkit.org/show_bug.cgi?id=122412 Patch by Maurice van der Pot on 2014-03-31 Reviewed by Darin Adler. Trivial fix for compilation error; no new tests. * platform/image-decoders/jpeg/JPEGImageDecoder.cpp: (WebCore::JPEGImageReader::decode): (WebCore::fill_input_buffer): Use TRUE/FALSE defined by libjpeg for libjpeg booleans git-svn-id: http://svn.webkit.org/repository/webkit/trunk@166490 268f45cc-cd09-0410-ab3c-d52691b4dbfc Change-Id: I6c669c951fa4bc87862b261ad1a9dd05016086e3 Reviewed-by: Allan Sandfeld Jensen --- .../WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'Source/WebCore') diff --git a/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp b/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp index 7da75c874..38aa71950 100644 --- a/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp +++ b/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp @@ -334,7 +334,7 @@ public: switch (m_state) { case JPEG_HEADER: // Read file parameters with jpeg_read_header(). - if (jpeg_read_header(&m_info, true) == JPEG_SUSPENDED) + if (jpeg_read_header(&m_info, TRUE) == JPEG_SUSPENDED) return false; // I/O suspension. switch (m_info.jpeg_color_space) { @@ -420,9 +420,9 @@ public: // of progressive JPEG. m_info.dct_method = dctMethod(); m_info.dither_mode = ditherMode(); - m_info.do_fancy_upsampling = doFancyUpsampling(); - m_info.enable_2pass_quant = false; - m_info.do_block_smoothing = true; + m_info.do_fancy_upsampling = doFancyUpsampling() ? TRUE : FALSE; + m_info.enable_2pass_quant = FALSE; + m_info.do_block_smoothing = TRUE; // Start decompressor. if (!jpeg_start_decompress(&m_info)) @@ -573,7 +573,7 @@ boolean fill_input_buffer(j_decompress_ptr) // Our decode step always sets things up properly, so if this method is ever // called, then we have hit the end of the buffer. A return value of false // indicates that we have no data to supply yet. - return false; + return FALSE; } void term_source(j_decompress_ptr jd) -- cgit v1.2.1 From c1b8d4bf2a36cd59e31758a9e6af872c17c4cfb8 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Tue, 3 May 2016 13:34:22 +0200 Subject: Only load QImageIO plugins from white-listed formats Not all QImage plugins are safe to load from the internet. We should only load formats that are well-used on the internet and we can be reasonably sure are safe. [ChangeLog][WebKit][Behavior Change] QtWebkit will no longer support any QImage plugin with the Size option, but instead only decode formats that have been whitelisted. If you are using QtWebKit for controlled content and wish to override the white-listed it can now be done with the environment variable QTWEBKIT_IMAGEFORMAT_WHITELIST which takes a comma-separated list of QImageIO formats. Change-Id: Ifc4f1a3addfa4ec117697a12000db3c265422314 Reviewed-by: Richard J. Moore --- .../platform/graphics/qt/ImageDecoderQt.cpp | 25 ++++++++++++++++++++++ 1 file changed, 25 insertions(+) (limited to 'Source/WebCore') diff --git a/Source/WebCore/platform/graphics/qt/ImageDecoderQt.cpp b/Source/WebCore/platform/graphics/qt/ImageDecoderQt.cpp index 2917815bd..74696c23d 100644 --- a/Source/WebCore/platform/graphics/qt/ImageDecoderQt.cpp +++ b/Source/WebCore/platform/graphics/qt/ImageDecoderQt.cpp @@ -31,6 +31,7 @@ #include #include +#include #include namespace WebCore { @@ -45,6 +46,25 @@ ImageDecoderQt::~ImageDecoderQt() { } +static const char* s_formatWhiteList[] = {"png", "jpeg", "gif", "webp", "bmp", "svg", "ico", 0}; + +static bool isFormatWhiteListed(const QByteArray &format) +{ + static QSet whiteListSet; + if (whiteListSet.isEmpty()) { + QByteArray whiteListEnv = qgetenv("QTWEBKIT_IMAGEFORMAT_WHITELIST"); + if (!whiteListEnv.isEmpty()) + whiteListSet = QSet::fromList(whiteListEnv.split(',')); + + const char **formatIt = s_formatWhiteList; + while (*formatIt) { + whiteListSet.insert(QByteArray(*formatIt)); + ++formatIt; + } + } + return whiteListSet.contains(format); +} + void ImageDecoderQt::setData(SharedBuffer* data, bool allDataReceived) { if (failed()) @@ -73,6 +93,11 @@ void ImageDecoderQt::setData(SharedBuffer* data, bool allDataReceived) // QImageReader only allows retrieving the format before reading the image m_format = m_reader->format(); + if (!isFormatWhiteListed(m_format)) { + qWarning("Image of format '%s' blocked because it is not considered safe. If you are sure it is safe to do so, you can white-list the format by setting the environment variable QTWEBKIT_IMAGEFORMAT_WHITELIST=%s", m_format.constData(), m_format.constData()); + setFailed(); + m_reader.clear(); + } } bool ImageDecoderQt::isSizeAvailable() -- cgit v1.2.1