Crash when accessing an item in SVGLengthList and then replacing it with a previous...
authorsaid@apple.com <said@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Feb 2015 00:13:16 +0000 (00:13 +0000)
committersaid@apple.com <said@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Feb 2015 00:13:16 +0000 (00:13 +0000)
https://bugs.webkit.org/show_bug.cgi?id=141552.

Reviewed by Darin Adler.

Source/WebCore:

Tests: LayoutTests/svg/dom/SVGLengthList-basics.xhtml: This test is modified to
include a new test case.

* svg/properties/SVGListPropertyTearOff.h: Commit the removal of the replacing item
before trying to detach the wrapper of the item which going to be replaced.

LayoutTests:

* svg/dom/SVGLengthList-basics-expected.txt:
* svg/dom/SVGLengthList-basics.xhtml: Add a new test case to this test. Have a
reference to an SVGLength in an SVGLengthList and then replace this SVGLength
with another one which comes before it in the SVGLengthList.

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

LayoutTests/ChangeLog
LayoutTests/svg/dom/SVGLengthList-basics-expected.txt
LayoutTests/svg/dom/SVGLengthList-basics.xhtml
Source/WebCore/ChangeLog
Source/WebCore/svg/properties/SVGListPropertyTearOff.h

index 3407e0c..bb8d7cc 100644 (file)
@@ -1,3 +1,15 @@
+2015-02-15  Said Abou-Hallawa  <sabouhallawa@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.
+
+        Reviewed by Darin Adler.
+
+        * svg/dom/SVGLengthList-basics-expected.txt:
+        * svg/dom/SVGLengthList-basics.xhtml: Add a new test case to this test. Have a
+        reference to an SVGLength in an SVGLengthList and then replace this SVGLength
+        with another one which comes before it in the SVGLengthList.
+
 2015-02-14  Benjamin Poulain  <benjamin@webkit.org>
 
         Add the initial matching implementation for attribute selectors with case-insensitive value
index 5facd0d..de5c1f3 100644 (file)
@@ -119,6 +119,22 @@ PASS text1.x.baseVal.replaceItem(text1.x.baseVal.getItem(3), 4) threw exception
 Set x='1 2 3 4' for text1
 PASS text1.setAttribute('x', '1 2 3 4') is undefined.
 
+Test overlapping edge cases for replaceItem()
+PASS text1.x.baseVal.replaceItem(text1.x.baseVal.getItem(0), 3) is text1.x.baseVal.getItem(2)
+PASS text1.x.baseVal.numberOfItems is 3
+PASS text1.x.baseVal.getItem(2).value is 2
+PASS text1.x.baseVal.replaceItem(text1.x.baseVal.getItem(0), 2) is text1.x.baseVal.getItem(1)
+PASS text1.x.baseVal.numberOfItems is 2
+PASS text1.x.baseVal.getItem(1).value is 4
+PASS text1.x.baseVal.replaceItem(text1.x.baseVal.getItem(0), 1) is text1.x.baseVal.getItem(0)
+PASS text1.x.baseVal.numberOfItems is 1
+PASS text1.x.baseVal.getItem(0).value is 6
+PASS text1.x.baseVal.replaceItem(text1.x.baseVal.getItem(0), 0) is text1.x.baseVal.getItem(0)
+PASS text1.x.baseVal.numberOfItems is 1
+
+Set x='1 2 3 4' for text1
+PASS text1.setAttribute('x', '1 2 3 4') is undefined.
+
 Test uncommon arguments for removeItem()
 PASS text1.x.baseVal.removeItem(30) threw exception Error: IndexSizeError: DOM Exception 1.
 PASS text1.x.baseVal.removeItem(0).value is 1
index 76499a6..415ed90 100644 (file)
     shouldBeUndefined("text1.setAttribute('x', '1 2 3 4')");
 
     debug("");
+    debug("Test overlapping edge cases for replaceItem()");
+    var item = text1.x.baseVal.getItem(3);
+    shouldBe("text1.x.baseVal.replaceItem(text1.x.baseVal.getItem(0), 3)", "text1.x.baseVal.getItem(2)");
+    shouldBe("text1.x.baseVal.numberOfItems", "3");    
+    item = text1.x.baseVal.getItem(2);
+    item.newValueSpecifiedUnits(item.unitType, item.value * 2);
+    shouldBe("text1.x.baseVal.getItem(2).value", "2");
+    shouldBe("text1.x.baseVal.replaceItem(text1.x.baseVal.getItem(0), 2)", "text1.x.baseVal.getItem(1)");
+    shouldBe("text1.x.baseVal.numberOfItems", "2");
+    item = text1.x.baseVal.getItem(1);
+    item.newValueSpecifiedUnits(item.unitType, item.value * 2);
+    shouldBe("text1.x.baseVal.getItem(1).value", "4");
+    shouldBe("text1.x.baseVal.replaceItem(text1.x.baseVal.getItem(0), 1)", "text1.x.baseVal.getItem(0)");
+    shouldBe("text1.x.baseVal.numberOfItems", "1");
+    item = text1.x.baseVal.getItem(0);
+    item.newValueSpecifiedUnits(item.unitType, item.value * 2);
+    shouldBe("text1.x.baseVal.getItem(0).value", "6");
+    shouldBe("text1.x.baseVal.replaceItem(text1.x.baseVal.getItem(0), 0)", "text1.x.baseVal.getItem(0)");
+    shouldBe("text1.x.baseVal.numberOfItems", "1");
+
+    debug("");
+    debug("Set x='1 2 3 4' for text1");
+    shouldBeUndefined("text1.setAttribute('x', '1 2 3 4')");
+
+    debug("");
     debug("Test uncommon arguments for removeItem()");
     shouldThrow("text1.x.baseVal.removeItem(30)");
 
index 0e020dd..f7f5a5b 100644 (file)
@@ -1,3 +1,16 @@
+2015-02-15  Said Abou-Hallawa  <sabouhallawa@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.
+
+        Reviewed by Darin Adler.
+
+        Tests: LayoutTests/svg/dom/SVGLengthList-basics.xhtml: This test is modified to
+        include a new test case.
+
+        * svg/properties/SVGListPropertyTearOff.h: Commit the removal of the replacing item
+        before trying to detach the wrapper of the item which going to be replaced.
+
 2015-02-15  David Kilzer  <ddkilzer@apple.com>
 
         CoreText only needs to be soft-linked on Windows
index 13bb3fe..e2639a1 100644 (file)
@@ -186,7 +186,7 @@ protected:
         if (!livesInOtherList && indexToModify && static_cast<unsigned>(indexToRemove) == *indexToModify)
             return false;
 
-        propertyTearOff->removeItemFromList(indexToRemove, livesInOtherList);
+        propertyTearOff->removeItemFromList(indexToRemove, true);
 
         if (!indexToModify)
             return true;