Accumulate SVG animations into first contributing element
authorpdr@google.com <pdr@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 20 May 2012 20:58:04 +0000 (20:58 +0000)
committerpdr@google.com <pdr@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 20 May 2012 20:58:04 +0000 (20:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=86385

Reviewed by Nikolas Zimmermann.

Source/WebCore:

Previously we were accumulating animations into the first animation element when
there were multiple animations for a single element. This crashed if the first
animation ended first because the first animation was prematurely cleaned up.
This change accumulates animations into the first _contributing_ animation element.

Tests: svg/animations/multiple-animations-ending.html
       svg/animations/svg-two-animate-elements-crash-expected.svg
       svg/animations/svg-two-animate-elements-crash.svg

* svg/animation/SMILTimeContainer.cpp:
(WebCore::SMILTimeContainer::updateAnimations):
* svg/animation/SVGSMILElement.cpp:
(WebCore::SVGSMILElement::progress):

LayoutTests:

* svg/animations/multiple-animations-ending-expected.txt: Added.
* svg/animations/multiple-animations-ending.html: Added.
* svg/animations/resources/multiple-animations-ending.svg: Added.
* svg/animations/script-tests/multiple-animations-ending.js: Added.
(sample1):
(sample2):
(sample3):
(sample4):
(sample5):
(sample6):
(sample7):
(sample8):
(sample9):
(sample10):
(sample11):
(sample12):
(sample13):
(sample14):
(sample15):
(sample16):
(sample17):
(sample18):
(sample19):
(sample20):
(sample21):
(sample22):
(sample23):
(executeTest):
* svg/animations/svg-two-animate-elements-crash-expected.svg: Added.
* svg/animations/svg-two-animate-elements-crash.svg: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@117711 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/svg/animations/multiple-animations-ending-expected.txt [new file with mode: 0644]
LayoutTests/svg/animations/multiple-animations-ending.html [new file with mode: 0644]
LayoutTests/svg/animations/resources/multiple-animations-ending.svg [new file with mode: 0644]
LayoutTests/svg/animations/script-tests/multiple-animations-ending.js [new file with mode: 0644]
LayoutTests/svg/animations/svg-two-animate-elements-crash-expected.svg [new file with mode: 0644]
LayoutTests/svg/animations/svg-two-animate-elements-crash.svg [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/svg/animation/SMILTimeContainer.cpp
Source/WebCore/svg/animation/SVGSMILElement.cpp

index 74eba29..49096f4 100644 (file)
@@ -1,5 +1,43 @@
 2012-05-20  Philip Rogers  <pdr@google.com>
 
+        Accumulate SVG animations into first contributing element
+        https://bugs.webkit.org/show_bug.cgi?id=86385
+
+        Reviewed by Nikolas Zimmermann.
+
+        * svg/animations/multiple-animations-ending-expected.txt: Added.
+        * svg/animations/multiple-animations-ending.html: Added.
+        * svg/animations/resources/multiple-animations-ending.svg: Added.
+        * svg/animations/script-tests/multiple-animations-ending.js: Added.
+        (sample1):
+        (sample2):
+        (sample3):
+        (sample4):
+        (sample5):
+        (sample6):
+        (sample7):
+        (sample8):
+        (sample9):
+        (sample10):
+        (sample11):
+        (sample12):
+        (sample13):
+        (sample14):
+        (sample15):
+        (sample16):
+        (sample17):
+        (sample18):
+        (sample19):
+        (sample20):
+        (sample21):
+        (sample22):
+        (sample23):
+        (executeTest):
+        * svg/animations/svg-two-animate-elements-crash-expected.svg: Added.
+        * svg/animations/svg-two-animate-elements-crash.svg: Added.
+
+2012-05-20  Philip Rogers  <pdr@google.com>
+
         Fix hit testing on non-scaling strokes
         https://bugs.webkit.org/show_bug.cgi?id=82628
 
diff --git a/LayoutTests/svg/animations/multiple-animations-ending-expected.txt b/LayoutTests/svg/animations/multiple-animations-ending-expected.txt
new file mode 100644 (file)
index 0000000..675c0b4
--- /dev/null
@@ -0,0 +1,196 @@
+Test the effect of multiple animations ending.
+
+
+This checks the effect on multiple animations ending on one target
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS rect1.x.animVal.value is 0
+PASS rect1.x.baseVal.value is 0
+PASS rect2.x.animVal.value is 100
+PASS rect2.x.baseVal.value is 200
+PASS rect3.x.animVal.value is 200
+PASS rect3.x.baseVal.value is 0
+PASS rect4.x.animVal.value is 10
+PASS rect4.x.baseVal.value is 0
+PASS rect1.x.animVal.value is 50
+PASS rect1.x.baseVal.value is 0
+PASS rect2.x.animVal.value is 80
+PASS rect2.x.baseVal.value is 200
+PASS rect3.x.animVal.value is 210
+PASS rect3.x.baseVal.value is 0
+PASS rect4.x.animVal.value is 30
+PASS rect4.x.baseVal.value is 0
+PASS rect1.x.animVal.value is 50
+PASS rect1.x.baseVal.value is 0
+PASS rect2.x.animVal.value is 80
+PASS rect2.x.baseVal.value is 200
+PASS rect3.x.animVal.value is 210
+PASS rect3.x.baseVal.value is 0
+PASS rect4.x.animVal.value is 30
+PASS rect4.x.baseVal.value is 0
+PASS rect1.x.animVal.value is 50
+PASS rect1.x.baseVal.value is 0
+PASS rect2.x.animVal.value is 80
+PASS rect2.x.baseVal.value is 200
+PASS rect3.x.animVal.value is 210
+PASS rect3.x.baseVal.value is 0
+PASS rect4.x.animVal.value is 30
+PASS rect4.x.baseVal.value is 0
+PASS rect1.x.animVal.value is 100
+PASS rect1.x.baseVal.value is 0
+PASS rect2.x.animVal.value is 60
+PASS rect2.x.baseVal.value is 200
+PASS rect3.x.animVal.value is 220
+PASS rect3.x.baseVal.value is 0
+PASS rect4.x.animVal.value is 30
+PASS rect4.x.baseVal.value is 0
+PASS rect1.x.animVal.value is 0
+PASS rect1.x.baseVal.value is 0
+PASS rect2.x.animVal.value is 0
+PASS rect2.x.baseVal.value is 200
+PASS rect3.x.animVal.value is 0
+PASS rect3.x.baseVal.value is 0
+PASS rect4.x.animVal.value is 60
+PASS rect4.x.baseVal.value is 0
+PASS rect1.x.animVal.value is 0
+PASS rect1.x.baseVal.value is 0
+PASS rect2.x.animVal.value is 0
+PASS rect2.x.baseVal.value is 200
+PASS rect3.x.animVal.value is 0
+PASS rect3.x.baseVal.value is 0
+PASS rect4.x.animVal.value is 60
+PASS rect4.x.baseVal.value is 0
+PASS rect1.x.animVal.value is 0
+PASS rect1.x.baseVal.value is 0
+PASS rect2.x.animVal.value is 5
+PASS rect2.x.baseVal.value is 200
+PASS rect3.x.animVal.value is 5
+PASS rect3.x.baseVal.value is 0
+PASS rect4.x.animVal.value is 80
+PASS rect4.x.baseVal.value is 0
+PASS rect1.x.animVal.value is 200
+PASS rect1.x.baseVal.value is 0
+PASS rect2.x.animVal.value is 5
+PASS rect2.x.baseVal.value is 200
+PASS rect3.x.animVal.value is 5
+PASS rect3.x.baseVal.value is 0
+PASS rect4.x.animVal.value is 80
+PASS rect4.x.baseVal.value is 0
+PASS rect1.x.animVal.value is 200
+PASS rect1.x.baseVal.value is 0
+PASS rect2.x.animVal.value is 5
+PASS rect2.x.baseVal.value is 200
+PASS rect3.x.animVal.value is 5
+PASS rect3.x.baseVal.value is 0
+PASS rect4.x.animVal.value is 80
+PASS rect4.x.baseVal.value is 0
+PASS rect1.x.animVal.value is 225
+PASS rect1.x.baseVal.value is 0
+PASS rect2.x.animVal.value is 10
+PASS rect2.x.baseVal.value is 200
+PASS rect3.x.animVal.value is 5
+PASS rect3.x.baseVal.value is 0
+PASS rect4.x.animVal.value is 80
+PASS rect4.x.baseVal.value is 0
+PASS rect1.x.animVal.value is 225
+PASS rect1.x.baseVal.value is 0
+PASS rect2.x.animVal.value is 20
+PASS rect2.x.baseVal.value is 200
+PASS rect3.x.animVal.value is 5
+PASS rect3.x.baseVal.value is 0
+PASS rect4.x.animVal.value is 110
+PASS rect4.x.baseVal.value is 0
+PASS rect1.x.animVal.value is 225
+PASS rect1.x.baseVal.value is 0
+PASS rect2.x.animVal.value is 20
+PASS rect2.x.baseVal.value is 200
+PASS rect3.x.animVal.value is 5
+PASS rect3.x.baseVal.value is 0
+PASS rect4.x.animVal.value is 110
+PASS rect4.x.baseVal.value is 0
+PASS rect1.x.animVal.value is 250
+PASS rect1.x.baseVal.value is 0
+PASS rect2.x.animVal.value is 0
+PASS rect2.x.baseVal.value is 200
+PASS rect3.x.animVal.value is 5
+PASS rect3.x.baseVal.value is 0
+PASS rect4.x.animVal.value is 130
+PASS rect4.x.baseVal.value is 0
+PASS rect1.x.animVal.value is 50
+PASS rect1.x.baseVal.value is 0
+PASS rect2.x.animVal.value is 200
+PASS rect2.x.baseVal.value is 200
+PASS rect3.x.animVal.value is 5
+PASS rect3.x.baseVal.value is 0
+PASS rect4.x.animVal.value is 130
+PASS rect4.x.baseVal.value is 0
+PASS rect1.x.animVal.value is 50
+PASS rect1.x.baseVal.value is 0
+PASS rect2.x.animVal.value is 200
+PASS rect2.x.baseVal.value is 200
+PASS rect3.x.animVal.value is 5
+PASS rect3.x.baseVal.value is 0
+PASS rect4.x.animVal.value is 130
+PASS rect4.x.baseVal.value is 0
+PASS rect1.x.animVal.value is 0
+PASS rect1.x.baseVal.value is 0
+PASS rect2.x.animVal.value is 200
+PASS rect2.x.baseVal.value is 200
+PASS rect3.x.animVal.value is 5
+PASS rect3.x.baseVal.value is 0
+PASS rect4.x.animVal.value is 130
+PASS rect4.x.baseVal.value is 0
+PASS rect1.x.animVal.value is 250
+PASS rect1.x.baseVal.value is 0
+PASS rect2.x.animVal.value is 200
+PASS rect2.x.baseVal.value is 200
+PASS rect3.x.animVal.value is 250
+PASS rect3.x.baseVal.value is 0
+PASS rect4.x.animVal.value is 160
+PASS rect4.x.baseVal.value is 0
+PASS rect1.x.animVal.value is 250
+PASS rect1.x.baseVal.value is 0
+PASS rect2.x.animVal.value is 200
+PASS rect2.x.baseVal.value is 200
+PASS rect3.x.animVal.value is 250
+PASS rect3.x.baseVal.value is 0
+PASS rect4.x.animVal.value is 160
+PASS rect4.x.baseVal.value is 0
+PASS rect1.x.animVal.value is 250
+PASS rect1.x.baseVal.value is 0
+PASS rect2.x.animVal.value is 200
+PASS rect2.x.baseVal.value is 200
+PASS rect3.x.animVal.value is 200
+PASS rect3.x.baseVal.value is 0
+PASS rect4.x.animVal.value is 180
+PASS rect4.x.baseVal.value is 0
+PASS rect1.x.animVal.value is 250
+PASS rect1.x.baseVal.value is 0
+PASS rect2.x.animVal.value is 200
+PASS rect2.x.baseVal.value is 200
+PASS rect3.x.animVal.value is 200
+PASS rect3.x.baseVal.value is 0
+PASS rect4.x.animVal.value is 180
+PASS rect4.x.baseVal.value is 0
+PASS rect1.x.animVal.value is 250
+PASS rect1.x.baseVal.value is 0
+PASS rect2.x.animVal.value is 200
+PASS rect2.x.baseVal.value is 200
+PASS rect3.x.animVal.value is 150
+PASS rect3.x.baseVal.value is 0
+PASS rect4.x.animVal.value is 180
+PASS rect4.x.baseVal.value is 0
+PASS rect1.x.animVal.value is 250
+PASS rect1.x.baseVal.value is 0
+PASS rect2.x.animVal.value is 200
+PASS rect2.x.baseVal.value is 200
+PASS rect3.x.animVal.value is 150
+PASS rect3.x.baseVal.value is 0
+PASS rect4.x.animVal.value is 180
+PASS rect4.x.baseVal.value is 0
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/svg/animations/multiple-animations-ending.html b/LayoutTests/svg/animations/multiple-animations-ending.html
new file mode 100644 (file)
index 0000000..4a663f5
--- /dev/null
@@ -0,0 +1,14 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../../fast/js/resources/js-test-pre.js"></script>
+<script src="../dynamic-updates/resources/SVGTestCase.js"></script>
+<script src="resources/SVGAnimationTestCase.js"></script>
+</head>
+<body onload="runSMILTest()">
+<h1>Test the effect of multiple animations ending.</h1>
+<p id="description"></p>
+<div id="console"></div>
+<script src="script-tests/multiple-animations-ending.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/svg/animations/resources/multiple-animations-ending.svg b/LayoutTests/svg/animations/resources/multiple-animations-ending.svg
new file mode 100644 (file)
index 0000000..ca57c2f
--- /dev/null
@@ -0,0 +1,45 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1 Tiny//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11-tiny.dtd">
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+
+<!-- Test that the first element can end while others continue without crashing, and the second
+     can end and remain frozen. Also test that a third element can animate after the second has ended
+     but that the result is still to return to the second animation's freeze position. -->
+<rect x='0' y='0' width='50' height='50' fill='green'>
+    <animate id="an1" attributeName='x' from='0' to='100' begin='0s' dur='1s' />
+    <animate id="an2" attributeName='x' from='200' to='250' begin='1.5s' dur='1s' fill='freeze' />
+    <animate id="an3" attributeName='x' from='50' to='0' begin='2.5s' dur='0.5s' />
+</rect>
+
+<!-- Test that a second element can take priority over the first from 0-1s, then
+     test that the first element can animate for 1s, and finally test that the
+     second element can once again animate after the first has ended. After all
+     animations end, test that they are removed and the rect returns to its home. -->
+<rect x='200' y='75' width='50' height='50' fill='green'>
+    <animate id="an4" attributeName='x' from='0' to='10' begin='1s' dur='1s'/>
+    <animate id="an5" attributeName='x' from='100' to='0' begin='0s' dur='2.5s'/>
+</rect>
+
+<!-- Test that a repeating animation can take priority over another animation, and that the
+     end state is the second animation's freeze value. Also test that, after a pause, a third
+     animation can take over and have its freeze value satisfied at the end. -->
+<rect x='0' y='150' width='50' height='50' fill='green'>
+    <animate id="an6" attributeName='x' from='200' to='240' begin='0s' dur='2s' fill='freeze'/>
+    <animate id="an7" attributeName='x' from='0' to='5' begin='1s' dur='0.1s' repeatCount="5" fill='freeze'/>
+    <animate id="an8" attributeName='x' from='250' to='150' begin='3s' dur='1s' fill='freeze'/>
+</rect>
+
+<!-- Test that 4 animations can animate a rect in 20px 'steps' and that correct freeze values are
+     honored even though the animation elements are specified in non-sequential order. Also test
+     that two repeating animations (active for only a short duration) only momentarily
+     affect the overall animation and are correctly removed. -->
+<rect x='0' y='225' width='50' height='50' fill='green'>
+    <animate id="an9" attributeName='x' from='200' to='250' begin='1.6s' dur='0.1s' repeatCount="2" fill='remove'/>
+    <animate id="anA" attributeName='x' from='160' to='180' begin='3s' dur='0.5s' fill='freeze'/>
+    <animate id="anB" attributeName='x' from='110' to='130' begin='2s' dur='0.5s' fill='freeze'/>
+    <animate id="anC" attributeName='x' from='10' to='30' begin='0s' dur='0.5s' fill='freeze'/>
+    <animate id="anD" attributeName='x' from='60' to='80' begin='1s' dur='0.5s' fill='freeze'/>
+    <animate id="anE" attributeName='x' from='200' to='250' begin='3.6s' dur='0.1s' repeatCount="2" fill='remove'/>
+</rect>
+
+</svg>
diff --git a/LayoutTests/svg/animations/script-tests/multiple-animations-ending.js b/LayoutTests/svg/animations/script-tests/multiple-animations-ending.js
new file mode 100644 (file)
index 0000000..d143126
--- /dev/null
@@ -0,0 +1,365 @@
+description("This checks the effect on multiple animations ending on one target");
+embedSVGTestCase("resources/multiple-animations-ending.svg");
+
+// Setup animation test
+function sample1() {
+    shouldBeCloseEnough("rect1.x.animVal.value", "0");
+    shouldBe("rect1.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect2.x.animVal.value", "100");
+    shouldBe("rect2.x.baseVal.value", "200");
+
+    shouldBeCloseEnough("rect3.x.animVal.value", "200");
+    shouldBe("rect3.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect4.x.animVal.value", "10");
+    shouldBe("rect4.x.baseVal.value", "0");
+}
+
+function sample2() {
+    shouldBeCloseEnough("rect1.x.animVal.value", "50");
+    shouldBe("rect1.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect2.x.animVal.value", "80");
+    shouldBe("rect2.x.baseVal.value", "200");
+
+    shouldBeCloseEnough("rect3.x.animVal.value", "210");
+    shouldBe("rect3.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect4.x.animVal.value", "30");
+    shouldBe("rect4.x.baseVal.value", "0");
+}
+
+function sample3() {
+    shouldBeCloseEnough("rect1.x.animVal.value", "50");
+    shouldBe("rect1.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect2.x.animVal.value", "80");
+    shouldBe("rect2.x.baseVal.value", "200");
+
+    shouldBeCloseEnough("rect3.x.animVal.value", "210");
+    shouldBe("rect3.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect4.x.animVal.value", "30");
+    shouldBe("rect4.x.baseVal.value", "0");
+}
+
+function sample4() {
+    shouldBeCloseEnough("rect1.x.animVal.value", "50");
+    shouldBe("rect1.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect2.x.animVal.value", "80");
+    shouldBe("rect2.x.baseVal.value", "200");
+
+    shouldBeCloseEnough("rect3.x.animVal.value", "210");
+    shouldBe("rect3.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect4.x.animVal.value", "30");
+    shouldBe("rect4.x.baseVal.value", "0");
+}
+
+function sample5() {
+    shouldBeCloseEnough("rect1.x.animVal.value", "100");
+    shouldBe("rect1.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect2.x.animVal.value", "60");
+    shouldBe("rect2.x.baseVal.value", "200");
+
+    shouldBeCloseEnough("rect3.x.animVal.value", "220");
+    shouldBe("rect3.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect4.x.animVal.value", "30");
+    shouldBe("rect4.x.baseVal.value", "0");
+}
+
+function sample6() {
+    shouldBeCloseEnough("rect1.x.animVal.value", "0");
+    shouldBe("rect1.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect2.x.animVal.value", "0");
+    shouldBe("rect2.x.baseVal.value", "200");
+
+    shouldBeCloseEnough("rect3.x.animVal.value", "0");
+    shouldBe("rect3.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect4.x.animVal.value", "60");
+    shouldBe("rect4.x.baseVal.value", "0");
+}
+
+function sample7() {
+    shouldBeCloseEnough("rect1.x.animVal.value", "0");
+    shouldBe("rect1.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect2.x.animVal.value", "0");
+    shouldBe("rect2.x.baseVal.value", "200");
+
+    shouldBeCloseEnough("rect3.x.animVal.value", "0");
+    shouldBe("rect3.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect4.x.animVal.value", "60");
+    shouldBe("rect4.x.baseVal.value", "0");
+}
+
+function sample8() {
+    shouldBeCloseEnough("rect1.x.animVal.value", "0");
+    shouldBe("rect1.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect2.x.animVal.value", "5");
+    shouldBe("rect2.x.baseVal.value", "200");
+
+    shouldBeCloseEnough("rect3.x.animVal.value", "5");
+    shouldBe("rect3.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect4.x.animVal.value", "80");
+    shouldBe("rect4.x.baseVal.value", "0");
+}
+
+function sample9() {
+    shouldBeCloseEnough("rect1.x.animVal.value", "200");
+    shouldBe("rect1.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect2.x.animVal.value", "5");
+    shouldBe("rect2.x.baseVal.value", "200");
+
+    shouldBeCloseEnough("rect3.x.animVal.value", "5");
+    shouldBe("rect3.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect4.x.animVal.value", "80");
+    shouldBe("rect4.x.baseVal.value", "0");
+}
+
+function sample10() {
+    shouldBeCloseEnough("rect1.x.animVal.value", "200");
+    shouldBe("rect1.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect2.x.animVal.value", "5");
+    shouldBe("rect2.x.baseVal.value", "200");
+
+    shouldBeCloseEnough("rect3.x.animVal.value", "5");
+    shouldBe("rect3.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect4.x.animVal.value", "80");
+    shouldBe("rect4.x.baseVal.value", "0");
+}
+
+function sample11() {
+    shouldBeCloseEnough("rect1.x.animVal.value", "225");
+    shouldBe("rect1.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect2.x.animVal.value", "10");
+    shouldBe("rect2.x.baseVal.value", "200");
+
+    shouldBeCloseEnough("rect3.x.animVal.value", "5");
+    shouldBe("rect3.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect4.x.animVal.value", "80");
+    shouldBe("rect4.x.baseVal.value", "0");
+}
+
+function sample12() {
+    shouldBeCloseEnough("rect1.x.animVal.value", "225");
+    shouldBe("rect1.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect2.x.animVal.value", "20");
+    shouldBe("rect2.x.baseVal.value", "200");
+
+    shouldBeCloseEnough("rect3.x.animVal.value", "5");
+    shouldBe("rect3.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect4.x.animVal.value", "110");
+    shouldBe("rect4.x.baseVal.value", "0");
+}
+
+function sample13() {
+    shouldBeCloseEnough("rect1.x.animVal.value", "225");
+    shouldBe("rect1.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect2.x.animVal.value", "20");
+    shouldBe("rect2.x.baseVal.value", "200");
+
+    shouldBeCloseEnough("rect3.x.animVal.value", "5");
+    shouldBe("rect3.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect4.x.animVal.value", "110");
+    shouldBe("rect4.x.baseVal.value", "0");
+}
+
+function sample14() {
+    shouldBeCloseEnough("rect1.x.animVal.value", "250");
+    shouldBe("rect1.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect2.x.animVal.value", "0");
+    shouldBe("rect2.x.baseVal.value", "200");
+
+    shouldBeCloseEnough("rect3.x.animVal.value", "5");
+    shouldBe("rect3.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect4.x.animVal.value", "130");
+    shouldBe("rect4.x.baseVal.value", "0");
+}
+
+function sample15() {
+    shouldBeCloseEnough("rect1.x.animVal.value", "50");
+    shouldBe("rect1.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect2.x.animVal.value", "200");
+    shouldBe("rect2.x.baseVal.value", "200");
+
+    shouldBeCloseEnough("rect3.x.animVal.value", "5");
+    shouldBe("rect3.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect4.x.animVal.value", "130");
+    shouldBe("rect4.x.baseVal.value", "0");
+}
+
+function sample16() {
+    shouldBeCloseEnough("rect1.x.animVal.value", "50");
+    shouldBe("rect1.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect2.x.animVal.value", "200");
+    shouldBe("rect2.x.baseVal.value", "200");
+
+    shouldBeCloseEnough("rect3.x.animVal.value", "5");
+    shouldBe("rect3.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect4.x.animVal.value", "130");
+    shouldBe("rect4.x.baseVal.value", "0");
+}
+
+function sample17() {
+    shouldBeCloseEnough("rect1.x.animVal.value", "0");
+    shouldBe("rect1.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect2.x.animVal.value", "200");
+    shouldBe("rect2.x.baseVal.value", "200");
+
+    shouldBeCloseEnough("rect3.x.animVal.value", "5");
+    shouldBe("rect3.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect4.x.animVal.value", "130");
+    shouldBe("rect4.x.baseVal.value", "0");
+}
+
+function sample18() {
+    shouldBeCloseEnough("rect1.x.animVal.value", "250");
+    shouldBe("rect1.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect2.x.animVal.value", "200");
+    shouldBe("rect2.x.baseVal.value", "200");
+
+    shouldBeCloseEnough("rect3.x.animVal.value", "250");
+    shouldBe("rect3.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect4.x.animVal.value", "160");
+    shouldBe("rect4.x.baseVal.value", "0");
+}
+
+function sample19() {
+    shouldBeCloseEnough("rect1.x.animVal.value", "250");
+    shouldBe("rect1.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect2.x.animVal.value", "200");
+    shouldBe("rect2.x.baseVal.value", "200");
+
+    shouldBeCloseEnough("rect3.x.animVal.value", "250");
+    shouldBe("rect3.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect4.x.animVal.value", "160");
+    shouldBe("rect4.x.baseVal.value", "0");
+}
+
+function sample20() {
+    shouldBeCloseEnough("rect1.x.animVal.value", "250");
+    shouldBe("rect1.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect2.x.animVal.value", "200");
+    shouldBe("rect2.x.baseVal.value", "200");
+
+    shouldBeCloseEnough("rect3.x.animVal.value", "200");
+    shouldBe("rect3.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect4.x.animVal.value", "180");
+    shouldBe("rect4.x.baseVal.value", "0");
+}
+
+function sample21() {
+    shouldBeCloseEnough("rect1.x.animVal.value", "250");
+    shouldBe("rect1.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect2.x.animVal.value", "200");
+    shouldBe("rect2.x.baseVal.value", "200");
+
+    shouldBeCloseEnough("rect3.x.animVal.value", "200");
+    shouldBe("rect3.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect4.x.animVal.value", "180");
+    shouldBe("rect4.x.baseVal.value", "0");
+}
+
+function sample22() {
+    shouldBeCloseEnough("rect1.x.animVal.value", "250");
+    shouldBe("rect1.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect2.x.animVal.value", "200");
+    shouldBe("rect2.x.baseVal.value", "200");
+
+    shouldBeCloseEnough("rect3.x.animVal.value", "150");
+    shouldBe("rect3.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect4.x.animVal.value", "180");
+    shouldBe("rect4.x.baseVal.value", "0");
+}
+
+function sample23() {
+    shouldBeCloseEnough("rect1.x.animVal.value", "250");
+    shouldBe("rect1.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect2.x.animVal.value", "200");
+    shouldBe("rect2.x.baseVal.value", "200");
+
+    shouldBeCloseEnough("rect3.x.animVal.value", "150");
+    shouldBe("rect3.x.baseVal.value", "0");
+
+    shouldBeCloseEnough("rect4.x.animVal.value", "180");
+    shouldBe("rect4.x.baseVal.value", "0");
+}
+
+function executeTest() {
+    var rects = rootSVGElement.ownerDocument.getElementsByTagName("rect");
+    rect1 = rects[0];
+    rect2 = rects[1];
+    rect3 = rects[2];
+    rect4 = rects[3];
+
+    const expectedValues = [
+        // [animationId, time, sampleCallback]
+        ["an1", 0.0,   sample1],
+        ["an1", 0.499, sample2],
+        ["an1", 0.5,   sample3],
+        ["an1", 0.501, sample4],
+        ["an1", 0.999, sample5],
+        ["an1", 1.0,   sample6],
+        ["an1", 1.001, sample7],
+        ["an1", 1.499, sample8],
+        ["an1", 1.5,   sample9],
+        ["an1", 1.501, sample10],
+        ["an1", 1.999, sample11],
+        ["an1", 2.0,   sample12],
+        ["an1", 2.001, sample13],
+        ["an1", 2.499, sample14],
+        ["an1", 2.5,   sample15],
+        ["an1", 2.501, sample16],
+        ["an1", 2.999, sample17],
+        ["an1", 3.0,   sample18],
+        ["an1", 3.001, sample19],
+        ["an1", 3.499, sample20],
+        ["an1", 3.5,   sample21],
+        ["an1", 4.0,   sample22],
+        ["an1", 9.0,   sample23]
+    ];
+
+    runAnimationTest(expectedValues);
+}
+
+window.animationStartsImmediately = true;
+var successfullyParsed = true;
diff --git a/LayoutTests/svg/animations/svg-two-animate-elements-crash-expected.svg b/LayoutTests/svg/animations/svg-two-animate-elements-crash-expected.svg
new file mode 100644 (file)
index 0000000..a3e32af
--- /dev/null
@@ -0,0 +1,11 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+  <!-- 
+    Test for WK86385 - This test passes if it doesn't crash.
+    This test has two animations running simultaneously where
+    the first animation ends before the second. At the end of
+    the test the green circle should be hidden behind a 100x100
+    green square in the top-left corner.
+  -->
+  <ellipse cx="50" cy="50" rx="20" ry="20" fill="green"/>
+  <rect width="100" height="100" x="0" y="0" fill="green"/>
+</svg>
diff --git a/LayoutTests/svg/animations/svg-two-animate-elements-crash.svg b/LayoutTests/svg/animations/svg-two-animate-elements-crash.svg
new file mode 100644 (file)
index 0000000..b75ccc0
--- /dev/null
@@ -0,0 +1,31 @@
+<svg xmlns="http://www.w3.org/2000/svg" onload="load()">
+  <!-- 
+    Test for WK86385 - This test passes if it doesn't crash.
+    This test has two animations running simultaneously where
+    the first animation ends before the second. At the end of
+    the test the green circle should be hidden behind a 100x100
+    green square in the top-left corner.
+  -->
+  <ellipse cx="50" cy="50" rx="20" ry="20" fill="green">
+    <animate attributeName="cx" from="0" to="500" dur="0.1"/>
+    <animate attributeName="cx" from="0" to="200" dur="0.2"/>
+  </ellipse>
+  <rect width="100" height="100" fill="green">
+    <animate attributeName="width" from="10" to="50" dur="0.1"/>
+    <animate attributeName="width" additive="sum" fill="freeze" from="10" to="0" dur="0.2"/>
+  </rect>
+<script>
+<![CDATA[
+function load() {
+  if (window.layoutTestController) {
+      layoutTestController.waitUntilDone();
+  }
+
+  window.setTimeout(function() {
+    if (window.layoutTestController)
+      layoutTestController.notifyDone();
+  }, 500);
+}
+]]>
+</script>
+</svg>
index bfeeff5..fedc67b 100644 (file)
@@ -1,5 +1,26 @@
 2012-05-20  Philip Rogers  <pdr@google.com>
 
+        Accumulate SVG animations into first contributing element
+        https://bugs.webkit.org/show_bug.cgi?id=86385
+
+        Reviewed by Nikolas Zimmermann.
+
+        Previously we were accumulating animations into the first animation element when
+        there were multiple animations for a single element. This crashed if the first
+        animation ended first because the first animation was prematurely cleaned up.
+        This change accumulates animations into the first _contributing_ animation element.
+
+        Tests: svg/animations/multiple-animations-ending.html
+               svg/animations/svg-two-animate-elements-crash-expected.svg
+               svg/animations/svg-two-animate-elements-crash.svg
+
+        * svg/animation/SMILTimeContainer.cpp:
+        (WebCore::SMILTimeContainer::updateAnimations):
+        * svg/animation/SVGSMILElement.cpp:
+        (WebCore::SVGSMILElement::progress):
+
+2012-05-20  Philip Rogers  <pdr@google.com>
+
         Fix hit testing on non-scaling strokes
         https://bugs.webkit.org/show_bug.cgi?id=82628
 
index f421c8c..1566533 100644 (file)
@@ -241,19 +241,23 @@ void SMILTimeContainer::updateAnimations(SMILTime elapsed, bool seekToTime)
                 continue;
         }
         
-        // Results are accumulated to the first animation that animates a particular element/attribute pair.
+        // Results are accumulated to the first animation that animates and contributes to a particular element/attribute pair.
         ElementAttributePair key(targetElement, attributeName); 
         SVGSMILElement* resultElement = resultsElements.get(key).get();
+        bool accumulatedResultElement = false;
         if (!resultElement) {
             if (!animation->hasValidAttributeType())
                 continue;
             resultElement = animation;
             resultsElements.add(key, resultElement);
+            accumulatedResultElement = true;
         }
 
         // This will calculate the contribution from the animation and add it to the resultsElement.
         if (animation->progress(elapsed, resultElement, seekToTime))
             contributingElements.add(resultElement);
+        else if (accumulatedResultElement)
+            resultsElements.remove(key);
 
         SMILTime nextFireTime = animation->nextProgressTime();
         if (nextFireTime.isFinite())
index 8695974..353e7b9 100644 (file)
@@ -1055,8 +1055,8 @@ bool SVGSMILElement::progress(SMILTime elapsed, SVGSMILElement* resultElement, b
     m_activeState = determineActiveState(elapsed);
     bool animationIsContributing = isContributing(elapsed);
 
-    // Only reset the animated type to the base value once for the lowest priority animation that animates a particular element/attribute pair.
-    if (this == resultElement)
+    // Only reset the animated type to the base value once for the lowest priority animation that animates and contributes to a particular element/attribute pair.
+    if (this == resultElement && animationIsContributing)
         resetAnimatedType();
 
     if (animationIsContributing) {