- Revision
- 180129
- Author
- s...@apple.com
- Date
- 2015-02-15 18:08:39 -0800 (Sun, 15 Feb 2015)
Log Message
Crash when accessing an item in SVGTransformList and then removing a previous item from this list.
https://bugs.webkit.org/show_bug.cgi?id=141550.
Reviewed by Darin Adler.
Source/WebCore:
Tests: LayoutTests/svg/dom/SVGTransformList-basics.xhtml: This test is modified to
include a new test case.
* svg/properties/SVGMatrixTearOff.h: m_value of SVGMatrixTearOff will be a null
pointer. There is no point in having SVGMatrixTearOff points to the parent and
the property of the parent at the same time.
(WebCore::SVGMatrixTearOff::create): SVGMatrixTearOff will hold a pointer to
the parent SVGPropertyTearOff<SVGTransform>. But it should overrides setValue()
and propertyReference() so it can set and get the SVGMatrix from the SVGTransform
parent.
(WebCore::SVGMatrixTearOff::SVGMatrixTearOff): Pass a nullptr to the base class.
SVGMatrixTearOff will act as a proxy of the parent. It does not hold any data by
itself but it knows what property to set and get from the parent.
* svg/properties/SVGPropertyTearOff.h:
(WebCore::SVGPropertyTearOff::create): Add a create method which can take a pointer value.
(WebCore::SVGPropertyTearOff::propertyReference):
(WebCore::SVGPropertyTearOff::setValue): Make these functions virtual so concrete classes
like SVGMatrixTearOff can override them.
(WebCore::SVGPropertyTearOff::SVGPropertyTearOff): Add a new constructor.
LayoutTests:
* svg/dom/SVGTransformList-basics-expected.txt:
* svg/dom/SVGTransformList-basics.xhtml: Add a new test case to this test. Have a
reference to an SVGMatrix of the last SVGTransform of an SVGTransformList and then
remove the items of this list from the beginning till the end.
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (180128 => 180129)
--- trunk/LayoutTests/ChangeLog 2015-02-16 00:13:16 UTC (rev 180128)
+++ trunk/LayoutTests/ChangeLog 2015-02-16 02:08:39 UTC (rev 180129)
@@ -1,5 +1,17 @@
2015-02-15 Said Abou-Hallawa <sabouhall...@apple.com>
+ Crash when accessing an item in SVGTransformList and then removing a previous item from this list.
+ https://bugs.webkit.org/show_bug.cgi?id=141550.
+
+ Reviewed by Darin Adler.
+
+ * svg/dom/SVGTransformList-basics-expected.txt:
+ * svg/dom/SVGTransformList-basics.xhtml: Add a new test case to this test. Have a
+ reference to an SVGMatrix of the last SVGTransform of an SVGTransformList and then
+ remove the items of this list from the beginning till the end.
+
+2015-02-15 Said Abou-Hallawa <sabouhall...@apple.com>
+
Crash when accessing an item in SVGLengthList and then replacing it with a previous item in the list.
https://bugs.webkit.org/show_bug.cgi?id=141552.
Modified: trunk/LayoutTests/svg/dom/SVGTransformList-basics-expected.txt (180128 => 180129)
--- trunk/LayoutTests/svg/dom/SVGTransformList-basics-expected.txt 2015-02-16 00:13:16 UTC (rev 180128)
+++ trunk/LayoutTests/svg/dom/SVGTransformList-basics-expected.txt 2015-02-16 02:08:39 UTC (rev 180129)
@@ -51,6 +51,13 @@
PASS circle1.transform.baseVal.insertItemBefore(circle1, 0) threw exception TypeError: Argument 1 ('item') to SVGTransformList.insertItemBefore must be an instance of SVGTransform.
PASS circle1.transform.baseVal.insertItemBefore(null, 0) threw exception Error: SVG_WRONG_TYPE_ERR: DOM SVG Exception 0.
+Test overlapping edge cases for removeItem()
+PASS circle1.setAttribute('transform', 'scale(2 2) translate(10 10)') is undefined.
+PASS dumpTransform(circle1.transform.baseVal.getItem(1)) is "type=SVG_TRANSFORM_MATRIX matrix=[1.0 0.0 0.0 1.0 20.0 20.0]"
+PASS dumpTransform(circle1.transform.baseVal.getItem(1)) is "type=SVG_TRANSFORM_MATRIX matrix=[1.0 0.0 0.0 1.0 20.0 20.0]"
+PASS dumpTransform(circle1.transform.baseVal.getItem(1)) is "type=SVG_TRANSFORM_MATRIX matrix=[1.0 0.0 0.0 1.0 40.0 40.0]"
+PASS circle1.transform.baseVal.numberOfItems is 0
+
Set transform='rotate(90) scale(2 2) translate(10 10) skewX(45)' for circle1
PASS circle1.setAttribute('transform', 'rotate(90) scale(2 2) translate(10 10) skewX(45)') is undefined.
PASS circle1.transform.baseVal.numberOfItems is 4
Modified: trunk/LayoutTests/svg/dom/SVGTransformList-basics.xhtml (180128 => 180129)
--- trunk/LayoutTests/svg/dom/SVGTransformList-basics.xhtml 2015-02-16 00:13:16 UTC (rev 180128)
+++ trunk/LayoutTests/svg/dom/SVGTransformList-basics.xhtml 2015-02-16 02:08:39 UTC (rev 180129)
@@ -103,6 +103,21 @@
shouldThrow("circle1.transform.baseVal.insertItemBefore(null, 0)");
debug("");
+ debug("Test overlapping edge cases for removeItem()");
+ shouldBeUndefined("circle1.setAttribute('transform', 'scale(2 2) translate(10 10)')");
+ var matrix = circle1.transform.baseVal.getItem(1).matrix;
+ matrix.e *= 2;
+ matrix.f *= 2;
+ shouldBeEqualToString("dumpTransform(circle1.transform.baseVal.getItem(1))", "type=SVG_TRANSFORM_MATRIX matrix=[1.0 0.0 0.0 1.0 20.0 20.0]");
+ matrix = matrix.translate(20, 20);
+ shouldBeEqualToString("dumpTransform(circle1.transform.baseVal.getItem(1))", "type=SVG_TRANSFORM_MATRIX matrix=[1.0 0.0 0.0 1.0 20.0 20.0]");
+ circle1.transform.baseVal.getItem(1).setMatrix(matrix);
+ shouldBeEqualToString("dumpTransform(circle1.transform.baseVal.getItem(1))", "type=SVG_TRANSFORM_MATRIX matrix=[1.0 0.0 0.0 1.0 40.0 40.0]");
+ circle1.transform.baseVal.removeItem(0);
+ circle1.transform.baseVal.removeItem(0);
+ shouldBe("circle1.transform.baseVal.numberOfItems", "0");
+
+ debug("");
debug("Set transform='rotate(90) scale(2 2) translate(10 10) skewX(45)' for circle1");
shouldBeUndefined("circle1.setAttribute('transform', 'rotate(90) scale(2 2) translate(10 10) skewX(45)')");
shouldBe("circle1.transform.baseVal.numberOfItems", "4");
Modified: trunk/Source/WebCore/ChangeLog (180128 => 180129)
--- trunk/Source/WebCore/ChangeLog 2015-02-16 00:13:16 UTC (rev 180128)
+++ trunk/Source/WebCore/ChangeLog 2015-02-16 02:08:39 UTC (rev 180129)
@@ -1,5 +1,37 @@
2015-02-15 Said Abou-Hallawa <sabouhall...@apple.com>
+ Crash when accessing an item in SVGTransformList and then removing a previous item from this list.
+ https://bugs.webkit.org/show_bug.cgi?id=141550.
+
+ Reviewed by Darin Adler.
+
+ Tests: LayoutTests/svg/dom/SVGTransformList-basics.xhtml: This test is modified to
+ include a new test case.
+
+ * svg/properties/SVGMatrixTearOff.h: m_value of SVGMatrixTearOff will be a null
+ pointer. There is no point in having SVGMatrixTearOff points to the parent and
+ the property of the parent at the same time.
+
+ (WebCore::SVGMatrixTearOff::create): SVGMatrixTearOff will hold a pointer to
+ the parent SVGPropertyTearOff<SVGTransform>. But it should overrides setValue()
+ and propertyReference() so it can set and get the SVGMatrix from the SVGTransform
+ parent.
+
+ (WebCore::SVGMatrixTearOff::SVGMatrixTearOff): Pass a nullptr to the base class.
+ SVGMatrixTearOff will act as a proxy of the parent. It does not hold any data by
+ itself but it knows what property to set and get from the parent.
+
+ * svg/properties/SVGPropertyTearOff.h:
+ (WebCore::SVGPropertyTearOff::create): Add a create method which can take a pointer value.
+
+ (WebCore::SVGPropertyTearOff::propertyReference):
+ (WebCore::SVGPropertyTearOff::setValue): Make these functions virtual so concrete classes
+ like SVGMatrixTearOff can override them.
+
+ (WebCore::SVGPropertyTearOff::SVGPropertyTearOff): Add a new constructor.
+
+2015-02-15 Said Abou-Hallawa <sabouhall...@apple.com>
+
Crash when accessing an item in SVGLengthList and then replacing it with a previous item in the list.
https://bugs.webkit.org/show_bug.cgi?id=141552.
Modified: trunk/Source/WebCore/svg/properties/SVGMatrixTearOff.h (180128 => 180129)
--- trunk/Source/WebCore/svg/properties/SVGMatrixTearOff.h 2015-02-16 00:13:16 UTC (rev 180128)
+++ trunk/Source/WebCore/svg/properties/SVGMatrixTearOff.h 2015-02-16 02:08:39 UTC (rev 180129)
@@ -30,13 +30,18 @@
// Used for non-animated POD types that are not associated with a SVGAnimatedProperty object, nor with a XML DOM attribute
// and that contain a parent type that's exposed to the bindings via a SVGStaticPropertyTearOff object
// (for example: SVGTransform::matrix).
- static PassRefPtr<SVGMatrixTearOff> create(SVGPropertyTearOff<SVGTransform>& parent, SVGMatrix& value)
+ static Ref<SVGMatrixTearOff> create(SVGPropertyTearOff<SVGTransform>& parent, SVGMatrix& value)
{
- RefPtr<SVGMatrixTearOff> result = adoptRef(new SVGMatrixTearOff(&parent, value));
+ ASSERT(&parent.propertyReference().svgMatrix() == &value);
+ Ref<SVGMatrixTearOff> result = adoptRef(*new SVGMatrixTearOff(&parent));
parent.addChild(result->m_weakFactory.createWeakPtr());
- return result.release();
+ return result;
}
+ virtual SVGMatrix& propertyReference() override { return m_parent->propertyReference().svgMatrix(); }
+
+ virtual void setValue(SVGMatrix& value) override { m_parent->propertyReference().setMatrix(value); }
+
virtual void commitChange()
{
m_parent->propertyReference().updateSVGMatrix();
@@ -44,8 +49,8 @@
}
private:
- SVGMatrixTearOff(SVGPropertyTearOff<SVGTransform>* parent, SVGMatrix& value)
- : SVGPropertyTearOff<SVGMatrix>(0, UndefinedRole, value)
+ SVGMatrixTearOff(SVGPropertyTearOff<SVGTransform>* parent)
+ : SVGPropertyTearOff<SVGMatrix>(nullptr)
, m_parent(parent)
, m_weakFactory(this)
{
Modified: trunk/Source/WebCore/svg/properties/SVGPropertyTearOff.h (180128 => 180129)
--- trunk/Source/WebCore/svg/properties/SVGPropertyTearOff.h 2015-02-16 00:13:16 UTC (rev 180128)
+++ trunk/Source/WebCore/svg/properties/SVGPropertyTearOff.h 2015-02-16 02:08:39 UTC (rev 180129)
@@ -39,22 +39,27 @@
// Used for child types (baseVal/animVal) of a SVGAnimated* property (for example: SVGAnimatedLength::baseVal()).
// Also used for list tear offs (for example: text.x.baseVal.getItem(0)).
- static PassRefPtr<Self> create(SVGAnimatedProperty* animatedProperty, SVGPropertyRole role, PropertyType& value)
+ static Ref<Self> create(SVGAnimatedProperty* animatedProperty, SVGPropertyRole role, PropertyType& value)
{
ASSERT(animatedProperty);
- return adoptRef(new Self(animatedProperty, role, value));
+ return adoptRef(*new Self(animatedProperty, role, value));
}
// Used for non-animated POD types (for example: SVGSVGElement::createSVGLength()).
- static PassRefPtr<Self> create(const PropertyType& initialValue)
+ static Ref<Self> create(const PropertyType& initialValue)
{
- return adoptRef(new Self(initialValue));
+ return adoptRef(*new Self(initialValue));
}
- PropertyType& propertyReference() { return *m_value; }
+ static Ref<Self> create(const PropertyType* initialValue)
+ {
+ return adoptRef(*new Self(initialValue));
+ }
+
+ virtual PropertyType& propertyReference() { return *m_value; }
SVGAnimatedProperty* animatedProperty() const { return m_animatedProperty; }
- void setValue(PropertyType& value)
+ virtual void setValue(PropertyType& value)
{
if (m_valueIsCopy) {
detachChildren();
@@ -134,9 +139,14 @@
}
SVGPropertyTearOff(const PropertyType& initialValue)
+ : SVGPropertyTearOff(&initialValue)
+ {
+ }
+
+ SVGPropertyTearOff(const PropertyType* initialValue)
: m_animatedProperty(0)
, m_role(UndefinedRole)
- , m_value(new PropertyType(initialValue))
+ , m_value(initialValue ? new PropertyType(*initialValue) : nullptr)
, m_valueIsCopy(true)
{
}
