[webkit-changes] [143454] trunk

Wed, 20 Feb 2013 05:36:02 -0800

Title: [143454] trunk
Revision
143454
Author
fmal...@chromium.org
Date
2013-02-20 05:38:02 -0800 (Wed, 20 Feb 2013)

Log Message

Clear SVGPathSeg role on removal.
https://bugs.webkit.org/show_bug.cgi?id=110058

Reviewed by Dirk Schulze.

Source/WebCore:

SVGPathSegListPropertyTearOff::initialize() and SVGPathSegListPropertyTearOff::replaceItem()
need to clear the context and role for segments being expunged from the list, similarly to
removeItem(). Otherwise, processIncomingListItemValue() can get confused and attempt to
remove stale segments.

Test: svg/dom/SVGPathSegList-crash.html

* svg/properties/SVGPathSegListPropertyTearOff.cpp:
(WebCore::SVGPathSegListPropertyTearOff::clearContextAndRoles):
(WebCore::SVGPathSegListPropertyTearOff::clear):
(WebCore::SVGPathSegListPropertyTearOff::replaceItem):
(WebCore):
* svg/properties/SVGPathSegListPropertyTearOff.h:
(WebCore::SVGPathSegListPropertyTearOff::initialize):
(SVGPathSegListPropertyTearOff):

LayoutTests:

* svg/dom/SVGPathSegList-crash-expected.txt: Added.
* svg/dom/SVGPathSegList-crash.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (143453 => 143454)


--- trunk/LayoutTests/ChangeLog	2013-02-20 12:45:58 UTC (rev 143453)
+++ trunk/LayoutTests/ChangeLog	2013-02-20 13:38:02 UTC (rev 143454)
@@ -1,3 +1,13 @@
+2013-02-20  Florin Malita  <fmal...@chromium.org>
+
+        Clear SVGPathSeg role on removal.
+        https://bugs.webkit.org/show_bug.cgi?id=110058
+
+        Reviewed by Dirk Schulze.
+
+        * svg/dom/SVGPathSegList-crash-expected.txt: Added.
+        * svg/dom/SVGPathSegList-crash.html: Added.
+
 2013-02-20  Sheriff Bot  <webkit.review....@gmail.com>
 
         Unreviewed, rolling out r143434.

Added: trunk/LayoutTests/svg/dom/SVGPathSegList-crash-expected.txt (0 => 143454)


--- trunk/LayoutTests/svg/dom/SVGPathSegList-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/svg/dom/SVGPathSegList-crash-expected.txt	2013-02-20 13:38:02 UTC (rev 143454)
@@ -0,0 +1 @@
+PASS: Did not crash.

Added: trunk/LayoutTests/svg/dom/SVGPathSegList-crash.html (0 => 143454)


--- trunk/LayoutTests/svg/dom/SVGPathSegList-crash.html	                        (rev 0)
+++ trunk/LayoutTests/svg/dom/SVGPathSegList-crash.html	2013-02-20 13:38:02 UTC (rev 143454)
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+  <body>
+    <script>
+        if (window.testRunner)
+            testRunner.dumpAsText();
+
+        var path = document.createElementNS("http://www.w3.org/2000/svg", "path");
+        var seg1 = path.createSVGPathSegLinetoAbs(10, 10);
+        var seg2 = path.createSVGPathSegLinetoAbs(100, 100);
+
+        path.pathSegList.initialize(seg1);
+        path.pathSegList.initialize(seg2);
+        path.pathSegList.initialize(seg1);
+        path.pathSegList.replaceItem(seg2, 0);
+        path.pathSegList.initialize(seg1);
+    </script>
+  </body>
+  <div>PASS: Did not crash.</div>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (143453 => 143454)


--- trunk/Source/WebCore/ChangeLog	2013-02-20 12:45:58 UTC (rev 143453)
+++ trunk/Source/WebCore/ChangeLog	2013-02-20 13:38:02 UTC (rev 143454)
@@ -1,3 +1,26 @@
+2013-02-20  Florin Malita  <fmal...@chromium.org>
+
+        Clear SVGPathSeg role on removal.
+        https://bugs.webkit.org/show_bug.cgi?id=110058
+
+        Reviewed by Dirk Schulze.
+
+        SVGPathSegListPropertyTearOff::initialize() and SVGPathSegListPropertyTearOff::replaceItem()
+        need to clear the context and role for segments being expunged from the list, similarly to
+        removeItem(). Otherwise, processIncomingListItemValue() can get confused and attempt to
+        remove stale segments.
+
+        Test: svg/dom/SVGPathSegList-crash.html
+
+        * svg/properties/SVGPathSegListPropertyTearOff.cpp:
+        (WebCore::SVGPathSegListPropertyTearOff::clearContextAndRoles):
+        (WebCore::SVGPathSegListPropertyTearOff::clear):
+        (WebCore::SVGPathSegListPropertyTearOff::replaceItem):
+        (WebCore):
+        * svg/properties/SVGPathSegListPropertyTearOff.h:
+        (WebCore::SVGPathSegListPropertyTearOff::initialize):
+        (SVGPathSegListPropertyTearOff):
+
 2013-02-20  Sheriff Bot  <webkit.review....@gmail.com>
 
         Unreviewed, rolling out r143434.

Modified: trunk/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.cpp (143453 => 143454)


--- trunk/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.cpp	2013-02-20 12:45:58 UTC (rev 143453)
+++ trunk/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.cpp	2013-02-20 13:38:02 UTC (rev 143454)
@@ -29,18 +29,23 @@
 
 namespace WebCore {
 
-void SVGPathSegListPropertyTearOff::clear(ExceptionCode& ec)
+void SVGPathSegListPropertyTearOff::clearContextAndRoles()
 {
     ASSERT(m_values);
-    if (m_values->isEmpty())
-        return;
-
     unsigned size = m_values->size();
     for (unsigned i = 0; i < size; ++i) {
         ListItemType item = m_values->at(i);
         static_cast<SVGPathSegWithContext*>(item.get())->setContextAndRole(0, PathSegUndefinedRole);
     }
+}
 
+void SVGPathSegListPropertyTearOff::clear(ExceptionCode& ec)
+{
+    ASSERT(m_values);
+    if (m_values->isEmpty())
+        return;
+
+    clearContextAndRoles();
     SVGPathSegListPropertyTearOff::Base::clearValues(ec);
 }
 
@@ -54,6 +59,24 @@
     return returnedItem.release();
 }
 
+SVGPathSegListPropertyTearOff::PassListItemType SVGPathSegListPropertyTearOff::replaceItem(PassListItemType passNewItem, unsigned index, ExceptionCode& ec)
+{
+    // Not specified, but FF/Opera do it this way, and it's just sane.
+    if (!passNewItem) {
+        ec = SVGException::SVG_WRONG_TYPE_ERR;
+        return 0;
+    }
+
+    if (index < m_values->size()) {
+        ListItemType replacedItem = m_values->at(index);
+        ASSERT(replacedItem);
+        static_cast<SVGPathSegWithContext*>(replacedItem.get())->setContextAndRole(0, PathSegUndefinedRole);
+    }
+
+    ListItemType newItem = passNewItem;
+    return Base::replaceItemValues(newItem, index, ec);
+}
+
 SVGPathSegListPropertyTearOff::PassListItemType SVGPathSegListPropertyTearOff::removeItem(unsigned index, ExceptionCode& ec)
 {
     SVGPathSegListPropertyTearOff::ListItemType removedItem = SVGPathSegListPropertyTearOff::Base::removeItemValues(index, ec);

Modified: trunk/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.h (143453 => 143454)


--- trunk/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.h	2013-02-20 12:45:58 UTC (rev 143453)
+++ trunk/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.h	2013-02-20 13:38:02 UTC (rev 143454)
@@ -76,6 +76,7 @@
             return 0;
         }
 
+        clearContextAndRoles();
         ListItemType newItem = passNewItem;
         return Base::initializeValues(newItem, ec);
     }
@@ -94,18 +95,8 @@
         return Base::insertItemBeforeValues(newItem, index, ec);
     }
 
-    PassListItemType replaceItem(PassListItemType passNewItem, unsigned index, ExceptionCode& ec)
-    {
-        // Not specified, but FF/Opera do it this way, and it's just sane.
-        if (!passNewItem) {
-            ec = SVGException::SVG_WRONG_TYPE_ERR;
-            return 0;
-        }
+    PassListItemType replaceItem(PassListItemType, unsigned index, ExceptionCode&);
 
-        ListItemType newItem = passNewItem;
-        return Base::replaceItemValues(newItem, index, ec);
-    }
-
     PassListItemType removeItem(unsigned index, ExceptionCode&);
 
     PassListItemType appendItem(PassListItemType passNewItem, ExceptionCode& ec)
@@ -130,6 +121,8 @@
 
     SVGPathElement* contextElement() const;
 
+    void clearContextAndRoles();
+
     using Base::m_role;
 
     virtual bool isReadOnly() const

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to