- Revision
- 187827
- Author
- [email protected]
- Date
- 2015-08-04 02:15:30 -0700 (Tue, 04 Aug 2015)
Log Message
Merge r186541 - Crash when appending an SVG <use> element dynamically which has animated SVG <path> element
https://bugs.webkit.org/show_bug.cgi?id=146690
<rdar://problem/20790376>
Patch by Said Abou-Hallawa <[email protected]> on 2015-07-08
Reviewed by Dean Jackson.
Source/WebCore:
Test: svg/animations/insert-animate-use-path-while-animation.svg
The crashing call stack shows that
SVGAnimatedListPropertyTearOff<SVGPathSegList>::m_animVal is null when
trying to access it in synchronizeWrappersIfNeeded(). This happens because
animationStarted() was not called for this animatedType.
SVGAnimateElementBase::resetAnimatedType() calls
SVGAnimatedPathAnimator::startAnimValAnimation() at the beginning of the
animation. For the target element and all its instances, this function calls
SVGAnimatedPathSegListPropertyTearOff::animationStarted() which calls
SVGAnimatedListPropertyTearOff<SVGPathSegList>::animationStarted(). This
last function allocates the member m_animVal when calling
SVGAnimatedListPropertyTearOff<SVGPathSegList>::animVal().
When adding a new instance of the same animating target element,
SVGAnimateElementBase::resetAnimatedType() just keeps calling
SVGAnimatedPathAnimator::animValDidChange() for all the instances of the
targetElement without ensuring that all of them have started their
animations.
The fix is to make SVGAnimatedPathAnimator::resetAnimValToBaseVal() ensure
that animationStarted() is called for the targetElement and all its instances.
* svg/SVGAnimatedPath.cpp:
(WebCore::SVGAnimatedPathAnimator::startAnimValAnimation): Move resetting
the animation value and starting the animatedTypes code to a new overriding
function which is named resetAnimValToBaseVal().
(WebCore::SVGAnimatedPathAnimator::resetAnimValToBaseVal): Call the overriding
function which calls buildSVGPathByteStreamFromSVGPathSegList() as before
and ensure that all the animatedTypes have started their animations.
* svg/SVGAnimatedPath.h:
LayoutTests:
When adding dynamically a new <use> element which references an animated
SVG path after the animation starts, ensure that WebKit is not crashing.
* svg/animations/insert-animate-use-path-while-animation-expected.txt: Added.
* svg/animations/insert-animate-use-path-while-animation.svg: Added.
Modified Paths
Added Paths
Diff
Modified: releases/WebKitGTK/webkit-2.8/LayoutTests/ChangeLog (187826 => 187827)
--- releases/WebKitGTK/webkit-2.8/LayoutTests/ChangeLog 2015-08-04 08:47:19 UTC (rev 187826)
+++ releases/WebKitGTK/webkit-2.8/LayoutTests/ChangeLog 2015-08-04 09:15:30 UTC (rev 187827)
@@ -1,3 +1,17 @@
+2015-07-08 Said Abou-Hallawa <[email protected]>
+
+ Crash when appending an SVG <use> element dynamically which has animated SVG <path> element
+ https://bugs.webkit.org/show_bug.cgi?id=146690
+ <rdar://problem/20790376>
+
+ Reviewed by Dean Jackson.
+
+ When adding dynamically a new <use> element which references an animated
+ SVG path after the animation starts, ensure that WebKit is not crashing.
+
+ * svg/animations/insert-animate-use-path-while-animation-expected.txt: Added.
+ * svg/animations/insert-animate-use-path-while-animation.svg: Added.
+
2015-07-07 Zalan Bujtas <[email protected]>
Crash when parent iframe is set to display none and the child frame is mutated the same time.
Added: releases/WebKitGTK/webkit-2.8/LayoutTests/svg/animations/insert-animate-use-path-while-animation-expected.txt (0 => 187827)
--- releases/WebKitGTK/webkit-2.8/LayoutTests/svg/animations/insert-animate-use-path-while-animation-expected.txt (rev 0)
+++ releases/WebKitGTK/webkit-2.8/LayoutTests/svg/animations/insert-animate-use-path-while-animation-expected.txt 2015-08-04 09:15:30 UTC (rev 187827)
@@ -0,0 +1 @@
+Passed
Added: releases/WebKitGTK/webkit-2.8/LayoutTests/svg/animations/insert-animate-use-path-while-animation.svg (0 => 187827)
--- releases/WebKitGTK/webkit-2.8/LayoutTests/svg/animations/insert-animate-use-path-while-animation.svg (rev 0)
+++ releases/WebKitGTK/webkit-2.8/LayoutTests/svg/animations/insert-animate-use-path-while-animation.svg 2015-08-04 09:15:30 UTC (rev 187827)
@@ -0,0 +1,41 @@
+<svg width="600" height="400" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+ <path id="animatedRect" fill="green">
+ <animate
+ attributeType="XML"
+ attributeName="d"
+ from="M20 60 L20 110 L120 110 L120 60 Z"
+ to="M45 35 L45 135 L95 135 L95 35 Z"
+ dur="1s"
+ repeatCount="indefinite"/>
+ </path>
+ <script><![CDATA[
+ if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+ }
+
+ // Wait until the only instance of 'animatedRect' starts animation.
+ setTimeout(function () {
+ var svgns = 'http://www.w3.org/2000/svg';
+ var xlinkns = 'http://www.w3.org/1999/xlink';
+
+ for (var i = 1; i <= 4; i++) {
+ var shape = document.createElementNS(svgns, "use");
+ shape.setAttributeNS(xlinkns, 'href', '#animatedRect');
+ shape.setAttributeNS(null, 'transform', 'translate(' + 100 * i + ', 0)');
+ document.documentElement.appendChild(shape);
+ }
+
+ if (window.testRunner) {
+ // Wait until the next animation cycle starts to make sure
+ // all of the instances of 'animatedRect' are animating.
+ setTimeout(function () {
+ var text = document.createElementNS(svgns, "text");
+ text.appendChild(document.createTextNode("Passed"));
+ document.documentElement.appendChild(text);
+ testRunner.notifyDone();
+ }, 50);
+ }
+ }, 50);
+ ]]></script>
+</svg>
Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog (187826 => 187827)
--- releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog 2015-08-04 08:47:19 UTC (rev 187826)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog 2015-08-04 09:15:30 UTC (rev 187827)
@@ -1,3 +1,46 @@
+2015-07-08 Said Abou-Hallawa <[email protected]>
+
+ Crash when appending an SVG <use> element dynamically which has animated SVG <path> element
+ https://bugs.webkit.org/show_bug.cgi?id=146690
+ <rdar://problem/20790376>
+
+ Reviewed by Dean Jackson.
+
+ Test: svg/animations/insert-animate-use-path-while-animation.svg
+
+ The crashing call stack shows that
+ SVGAnimatedListPropertyTearOff<SVGPathSegList>::m_animVal is null when
+ trying to access it in synchronizeWrappersIfNeeded(). This happens because
+ animationStarted() was not called for this animatedType.
+
+ SVGAnimateElementBase::resetAnimatedType() calls
+ SVGAnimatedPathAnimator::startAnimValAnimation() at the beginning of the
+ animation. For the target element and all its instances, this function calls
+ SVGAnimatedPathSegListPropertyTearOff::animationStarted() which calls
+ SVGAnimatedListPropertyTearOff<SVGPathSegList>::animationStarted(). This
+ last function allocates the member m_animVal when calling
+ SVGAnimatedListPropertyTearOff<SVGPathSegList>::animVal().
+
+ When adding a new instance of the same animating target element,
+ SVGAnimateElementBase::resetAnimatedType() just keeps calling
+ SVGAnimatedPathAnimator::animValDidChange() for all the instances of the
+ targetElement without ensuring that all of them have started their
+ animations.
+
+ The fix is to make SVGAnimatedPathAnimator::resetAnimValToBaseVal() ensure
+ that animationStarted() is called for the targetElement and all its instances.
+
+ * svg/SVGAnimatedPath.cpp:
+ (WebCore::SVGAnimatedPathAnimator::startAnimValAnimation): Move resetting
+ the animation value and starting the animatedTypes code to a new overriding
+ function which is named resetAnimValToBaseVal().
+
+ (WebCore::SVGAnimatedPathAnimator::resetAnimValToBaseVal): Call the overriding
+ function which calls buildSVGPathByteStreamFromSVGPathSegList() as before
+ and ensure that all the animatedTypes have started their animations.
+
+ * svg/SVGAnimatedPath.h:
+
2015-07-07 Zalan Bujtas <[email protected]>
Crash when parent iframe is set to display none and the child frame is mutated the same time.
Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/svg/SVGAnimatedPath.cpp (187826 => 187827)
--- releases/WebKitGTK/webkit-2.8/Source/WebCore/svg/SVGAnimatedPath.cpp 2015-08-04 08:47:19 UTC (rev 187826)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/svg/SVGAnimatedPath.cpp 2015-08-04 09:15:30 UTC (rev 187827)
@@ -41,25 +41,10 @@
std::unique_ptr<SVGAnimatedType> SVGAnimatedPathAnimator::startAnimValAnimation(const SVGElementAnimatedPropertyList& animatedTypes)
{
ASSERT(animatedTypes.size() >= 1);
- SVGAnimatedPathSegListPropertyTearOff* property = castAnimatedPropertyToActualType<SVGAnimatedPathSegListPropertyTearOff>(animatedTypes[0].properties[0].get());
- const SVGPathSegList& baseValue = property->currentBaseValue();
// Build initial path byte stream.
auto byteStream = std::make_unique<SVGPathByteStream>();
- buildSVGPathByteStreamFromSVGPathSegList(baseValue, byteStream.get(), UnalteredParsing);
-
- Vector<RefPtr<SVGAnimatedPathSegListPropertyTearOff>> result;
-
- SVGElementAnimatedPropertyList::const_iterator end = animatedTypes.end();
- for (SVGElementAnimatedPropertyList::const_iterator it = animatedTypes.begin(); it != end; ++it)
- result.append(castAnimatedPropertyToActualType<SVGAnimatedPathSegListPropertyTearOff>(it->properties[0].get()));
-
- SVGElement::InstanceUpdateBlocker blocker(*property->contextElement());
-
- size_t resultSize = result.size();
- for (size_t i = 0; i < resultSize; ++i)
- result[i]->animationStarted(byteStream.get(), &baseValue);
-
+ resetAnimValToBaseVal(animatedTypes, byteStream.get());
return SVGAnimatedType::createPath(WTF::move(byteStream));
}
@@ -68,13 +53,35 @@
stopAnimValAnimationForType<SVGAnimatedPathSegListPropertyTearOff>(animatedTypes);
}
+void SVGAnimatedPathAnimator::resetAnimValToBaseVal(const SVGElementAnimatedPropertyList& animatedTypes, SVGPathByteStream* byteStream)
+{
+ SVGAnimatedPathSegListPropertyTearOff* property = castAnimatedPropertyToActualType<SVGAnimatedPathSegListPropertyTearOff>(animatedTypes[0].properties[0].get());
+ const SVGPathSegList& baseValue = property->currentBaseValue();
+
+ buildSVGPathByteStreamFromSVGPathSegList(baseValue, byteStream, UnalteredParsing);
+
+ Vector<RefPtr<SVGAnimatedPathSegListPropertyTearOff>> result;
+
+ for (auto& type : animatedTypes) {
+ auto* segment = castAnimatedPropertyToActualType<SVGAnimatedPathSegListPropertyTearOff>(type.properties[0].get());
+ if (segment->isAnimating())
+ continue;
+ result.append(segment);
+ }
+
+ if (!result.isEmpty()) {
+ SVGElement::InstanceUpdateBlocker blocker(*property->contextElement());
+ for (auto& segment : result)
+ segment->animationStarted(byteStream, &baseValue);
+ }
+}
+
void SVGAnimatedPathAnimator::resetAnimValToBaseVal(const SVGElementAnimatedPropertyList& animatedTypes, SVGAnimatedType* type)
{
ASSERT(animatedTypes.size() >= 1);
ASSERT(type);
ASSERT(type->type() == m_type);
- const SVGPathSegList& baseValue = castAnimatedPropertyToActualType<SVGAnimatedPathSegListPropertyTearOff>(animatedTypes[0].properties[0].get())->currentBaseValue();
- buildSVGPathByteStreamFromSVGPathSegList(baseValue, type->path(), UnalteredParsing);
+ resetAnimValToBaseVal(animatedTypes, type->path());
}
void SVGAnimatedPathAnimator::animValWillChange(const SVGElementAnimatedPropertyList& animatedTypes)
Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/svg/SVGAnimatedPath.h (187826 => 187827)
--- releases/WebKitGTK/webkit-2.8/Source/WebCore/svg/SVGAnimatedPath.h 2015-08-04 08:47:19 UTC (rev 187826)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/svg/SVGAnimatedPath.h 2015-08-04 09:15:30 UTC (rev 187827)
@@ -40,6 +40,9 @@
virtual void addAnimatedTypes(SVGAnimatedType*, SVGAnimatedType*) override;
virtual void calculateAnimatedValue(float percentage, unsigned repeatCount, SVGAnimatedType*, SVGAnimatedType*, SVGAnimatedType*, SVGAnimatedType*) override;
virtual float calculateDistance(const String& fromString, const String& toString) override;
+
+private:
+ void resetAnimValToBaseVal(const SVGElementAnimatedPropertyList&, SVGPathByteStream*);
};
} // namespace WebCore
