Title: [230233] branches/safari-605-branch
Revision
230233
Author
jmarc...@apple.com
Date
2018-04-03 20:27:17 -0700 (Tue, 03 Apr 2018)

Log Message

Cherry-pick r229830. rdar://problem/39155360

    Disconnect the SVGPathSegList items from their SVGPathElement before rebuilding a new list
    https://bugs.webkit.org/show_bug.cgi?id=183723
    <rdar://problem/38517871>

    Patch by Said Abou-Hallawa <sabouhall...@apple.com> on 2018-03-21
    Reviewed by Daniel Bates.

    Source/WebCore:

    When setting the "d" attribute directly on a path, we rebuild the list
    of path segments held for creating the property tear off. The old path
    segments need to get disconnected from the path element. We already do
    that when a path segment is replaced or removed.

    Test: svg/dom/reuse-pathseg-after-changing-d.html

    * svg/SVGPathElement.cpp:
    (WebCore::SVGPathElement::svgAttributeChanged):
    * svg/SVGPathSegList.cpp:
    (WebCore::SVGPathSegList::clear): SVGPathSegListValues::clearContextAndRoles()
    will now be called from SVGPathSegListValues::clear() via SVGListProperty::clearValues().
    (WebCore::SVGPathSegList::replaceItem):
    (WebCore::SVGPathSegList::removeItem):
    (WebCore::SVGPathSegList::clearContextAndRoles): Deleted.
    * svg/SVGPathSegList.h: SVGPathSegListValues::clearContextAndRoles() will
    now be called from SVGPathSegListValues::clear() via SVGListProperty::initializeValues().
    * svg/SVGPathSegListValues.cpp:
    (WebCore::SVGPathSegListValues::clearItemContextAndRole):
    (WebCore::SVGPathSegListValues::clearContextAndRoles):
    * svg/SVGPathSegListValues.h:
    (WebCore::SVGPathSegListValues::operator=):
    (WebCore::SVGPathSegListValues::clear):

    LayoutTests:

    * svg/dom/reuse-pathseg-after-changing-d-expected.txt: Added.
    * svg/dom/reuse-pathseg-after-changing-d.html: Added.

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

Modified Paths

Added Paths

Diff

Modified: branches/safari-605-branch/LayoutTests/ChangeLog (230232 => 230233)


--- branches/safari-605-branch/LayoutTests/ChangeLog	2018-04-04 03:03:42 UTC (rev 230232)
+++ branches/safari-605-branch/LayoutTests/ChangeLog	2018-04-04 03:27:17 UTC (rev 230233)
@@ -1,3 +1,58 @@
+2018-04-03  Jason Marcell  <jmarc...@apple.com>
+
+        Cherry-pick r229830. rdar://problem/39155360
+
+    Disconnect the SVGPathSegList items from their SVGPathElement before rebuilding a new list
+    https://bugs.webkit.org/show_bug.cgi?id=183723
+    <rdar://problem/38517871>
+    
+    Patch by Said Abou-Hallawa <sabouhall...@apple.com> on 2018-03-21
+    Reviewed by Daniel Bates.
+    
+    Source/WebCore:
+    
+    When setting the "d" attribute directly on a path, we rebuild the list
+    of path segments held for creating the property tear off. The old path
+    segments need to get disconnected from the path element. We already do
+    that when a path segment is replaced or removed.
+    
+    Test: svg/dom/reuse-pathseg-after-changing-d.html
+    
+    * svg/SVGPathElement.cpp:
+    (WebCore::SVGPathElement::svgAttributeChanged):
+    * svg/SVGPathSegList.cpp:
+    (WebCore::SVGPathSegList::clear): SVGPathSegListValues::clearContextAndRoles()
+    will now be called from SVGPathSegListValues::clear() via SVGListProperty::clearValues().
+    (WebCore::SVGPathSegList::replaceItem):
+    (WebCore::SVGPathSegList::removeItem):
+    (WebCore::SVGPathSegList::clearContextAndRoles): Deleted.
+    * svg/SVGPathSegList.h: SVGPathSegListValues::clearContextAndRoles() will
+    now be called from SVGPathSegListValues::clear() via SVGListProperty::initializeValues().
+    * svg/SVGPathSegListValues.cpp:
+    (WebCore::SVGPathSegListValues::clearItemContextAndRole):
+    (WebCore::SVGPathSegListValues::clearContextAndRoles):
+    * svg/SVGPathSegListValues.h:
+    (WebCore::SVGPathSegListValues::operator=):
+    (WebCore::SVGPathSegListValues::clear):
+    
+    LayoutTests:
+    
+    * svg/dom/reuse-pathseg-after-changing-d-expected.txt: Added.
+    * svg/dom/reuse-pathseg-after-changing-d.html: Added.
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@229830 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2018-03-21  Said Abou-Hallawa  <sabouhall...@apple.com>
+
+            Disconnect the SVGPathSegList items from their SVGPathElement before rebuilding a new list
+            https://bugs.webkit.org/show_bug.cgi?id=183723
+            <rdar://problem/38517871>
+
+            Reviewed by Daniel Bates.
+
+            * svg/dom/reuse-pathseg-after-changing-d-expected.txt: Added.
+            * svg/dom/reuse-pathseg-after-changing-d.html: Added.
+
 2018-03-21  Jason Marcell  <jmarc...@apple.com>
 
         Cherry-pick r229297. rdar://problem/38682578

Added: branches/safari-605-branch/LayoutTests/svg/dom/reuse-pathseg-after-changing-d-expected.txt (0 => 230233)


--- branches/safari-605-branch/LayoutTests/svg/dom/reuse-pathseg-after-changing-d-expected.txt	                        (rev 0)
+++ branches/safari-605-branch/LayoutTests/svg/dom/reuse-pathseg-after-changing-d-expected.txt	2018-04-04 03:27:17 UTC (rev 230233)
@@ -0,0 +1 @@
+This should not crash.

Added: branches/safari-605-branch/LayoutTests/svg/dom/reuse-pathseg-after-changing-d.html (0 => 230233)


--- branches/safari-605-branch/LayoutTests/svg/dom/reuse-pathseg-after-changing-d.html	                        (rev 0)
+++ branches/safari-605-branch/LayoutTests/svg/dom/reuse-pathseg-after-changing-d.html	2018-04-04 03:27:17 UTC (rev 230233)
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<body>
+	<p>This should not crash.</p>
+	<script>
+    	if (window.testRunner)
+        	testRunner.dumpAsText();
+    	const svgns = "http://www.w3.org/2000/svg";
+    	let path = document.createElementNS(svgns, "path");
+    	let pathSegList = path.pathSegList;
+    	let pathSeg = path.createSVGPathSegCurvetoCubicSmoothAbs(0, 0, 1, 1);
+    	pathSegList.insertItemBefore(pathSeg, 1);
+    	path.setAttribute("d", "M 0 0");
+    	pathSegList.insertItemBefore(pathSeg, 0);
+	</script>
+</body>

Modified: branches/safari-605-branch/Source/WebCore/ChangeLog (230232 => 230233)


--- branches/safari-605-branch/Source/WebCore/ChangeLog	2018-04-04 03:03:42 UTC (rev 230232)
+++ branches/safari-605-branch/Source/WebCore/ChangeLog	2018-04-04 03:27:17 UTC (rev 230233)
@@ -1,3 +1,79 @@
+2018-04-03  Jason Marcell  <jmarc...@apple.com>
+
+        Cherry-pick r229830. rdar://problem/39155360
+
+    Disconnect the SVGPathSegList items from their SVGPathElement before rebuilding a new list
+    https://bugs.webkit.org/show_bug.cgi?id=183723
+    <rdar://problem/38517871>
+    
+    Patch by Said Abou-Hallawa <sabouhall...@apple.com> on 2018-03-21
+    Reviewed by Daniel Bates.
+    
+    Source/WebCore:
+    
+    When setting the "d" attribute directly on a path, we rebuild the list
+    of path segments held for creating the property tear off. The old path
+    segments need to get disconnected from the path element. We already do
+    that when a path segment is replaced or removed.
+    
+    Test: svg/dom/reuse-pathseg-after-changing-d.html
+    
+    * svg/SVGPathElement.cpp:
+    (WebCore::SVGPathElement::svgAttributeChanged):
+    * svg/SVGPathSegList.cpp:
+    (WebCore::SVGPathSegList::clear): SVGPathSegListValues::clearContextAndRoles()
+    will now be called from SVGPathSegListValues::clear() via SVGListProperty::clearValues().
+    (WebCore::SVGPathSegList::replaceItem):
+    (WebCore::SVGPathSegList::removeItem):
+    (WebCore::SVGPathSegList::clearContextAndRoles): Deleted.
+    * svg/SVGPathSegList.h: SVGPathSegListValues::clearContextAndRoles() will
+    now be called from SVGPathSegListValues::clear() via SVGListProperty::initializeValues().
+    * svg/SVGPathSegListValues.cpp:
+    (WebCore::SVGPathSegListValues::clearItemContextAndRole):
+    (WebCore::SVGPathSegListValues::clearContextAndRoles):
+    * svg/SVGPathSegListValues.h:
+    (WebCore::SVGPathSegListValues::operator=):
+    (WebCore::SVGPathSegListValues::clear):
+    
+    LayoutTests:
+    
+    * svg/dom/reuse-pathseg-after-changing-d-expected.txt: Added.
+    * svg/dom/reuse-pathseg-after-changing-d.html: Added.
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@229830 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2018-03-21  Said Abou-Hallawa  <sabouhall...@apple.com>
+
+            Disconnect the SVGPathSegList items from their SVGPathElement before rebuilding a new list
+            https://bugs.webkit.org/show_bug.cgi?id=183723
+            <rdar://problem/38517871>
+
+            Reviewed by Daniel Bates.
+
+            When setting the "d" attribute directly on a path, we rebuild the list
+            of path segments held for creating the property tear off. The old path
+            segments need to get disconnected from the path element. We already do
+            that when a path segment is replaced or removed.
+
+            Test: svg/dom/reuse-pathseg-after-changing-d.html
+
+            * svg/SVGPathElement.cpp:
+            (WebCore::SVGPathElement::svgAttributeChanged):
+            * svg/SVGPathSegList.cpp:
+            (WebCore::SVGPathSegList::clear): SVGPathSegListValues::clearContextAndRoles()
+            will now be called from SVGPathSegListValues::clear() via SVGListProperty::clearValues().
+            (WebCore::SVGPathSegList::replaceItem):
+            (WebCore::SVGPathSegList::removeItem):
+            (WebCore::SVGPathSegList::clearContextAndRoles): Deleted.
+            * svg/SVGPathSegList.h: SVGPathSegListValues::clearContextAndRoles() will
+            now be called from SVGPathSegListValues::clear() via SVGListProperty::initializeValues().
+            * svg/SVGPathSegListValues.cpp:
+            (WebCore::SVGPathSegListValues::clearItemContextAndRole):
+            (WebCore::SVGPathSegListValues::clearContextAndRoles):
+            * svg/SVGPathSegListValues.h:
+            (WebCore::SVGPathSegListValues::operator=):
+            (WebCore::SVGPathSegListValues::clear):
+
 2018-03-21  Jason Marcell  <jmarc...@apple.com>
 
         Cherry-pick r229297. rdar://problem/38682578

Modified: branches/safari-605-branch/Source/WebCore/svg/SVGPathElement.cpp (230232 => 230233)


--- branches/safari-605-branch/Source/WebCore/svg/SVGPathElement.cpp	2018-04-04 03:03:42 UTC (rev 230232)
+++ branches/safari-605-branch/Source/WebCore/svg/SVGPathElement.cpp	2018-04-04 03:27:17 UTC (rev 230233)
@@ -1,6 +1,7 @@
 /*
  * Copyright (C) 2004, 2005, 2006, 2008 Nikolas Zimmermann <zimmerm...@kde.org>
  * Copyright (C) 2004, 2005, 2006, 2007 Rob Buis <b...@kde.org>
+ * Copyright (C) 2018 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -255,7 +256,7 @@
         if (m_pathSegList.shouldSynchronize && !SVGAnimatedProperty::lookupWrapper<SVGPathElement, SVGAnimatedPathSegListPropertyTearOff>(this, dPropertyInfo())->isAnimating()) {
             SVGPathSegListValues newList(PathSegUnalteredRole);
             buildSVGPathSegListValuesFromByteStream(m_pathByteStream, *this, newList, UnalteredParsing);
-            m_pathSegList.value = newList;
+            m_pathSegList.value = WTFMove(newList);
         }
 
         if (renderer)

Modified: branches/safari-605-branch/Source/WebCore/svg/SVGPathSegList.cpp (230232 => 230233)


--- branches/safari-605-branch/Source/WebCore/svg/SVGPathSegList.cpp	2018-04-04 03:03:42 UTC (rev 230232)
+++ branches/safari-605-branch/Source/WebCore/svg/SVGPathSegList.cpp	2018-04-04 03:27:17 UTC (rev 230233)
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) Research In Motion Limited 2010. All rights reserved.
+ * Copyright (C) 2018 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -26,20 +27,11 @@
 
 namespace WebCore {
 
-void SVGPathSegList::clearContextAndRoles()
-{
-    ASSERT(m_values);
-    for (auto& item : *m_values)
-        static_cast<SVGPathSegWithContext*>(item.get())->setContextAndRole(nullptr, PathSegUndefinedRole);
-}
-
 ExceptionOr<void> SVGPathSegList::clear()
 {
     ASSERT(m_values);
     if (m_values->isEmpty())
         return { };
-
-    clearContextAndRoles();
     return Base::clearValues();
 }
 
@@ -50,24 +42,19 @@
 
 ExceptionOr<RefPtr<SVGPathSeg>> SVGPathSegList::replaceItem(Ref<SVGPathSeg>&& newItem, unsigned index)
 {
-    if (index < m_values->size()) {
-        ListItemType replacedItem = m_values->at(index);
-        ASSERT(replacedItem);
-        static_cast<SVGPathSegWithContext*>(replacedItem.get())->setContextAndRole(nullptr, PathSegUndefinedRole);
-    }
-
+    if (index < m_values->size())
+        m_values->clearItemContextAndRole(index);
     return Base::replaceItemValues(WTFMove(newItem), index);
 }
 
 ExceptionOr<RefPtr<SVGPathSeg>> SVGPathSegList::removeItem(unsigned index)
 {
+    if (index < m_values->size())
+        m_values->clearItemContextAndRole(index);
     auto result = Base::removeItemValues(index);
     if (result.hasException())
         return result;
-    auto removedItem = result.releaseReturnValue();
-    if (removedItem)
-        static_cast<SVGPathSegWithContext&>(*removedItem).setContextAndRole(nullptr, PathSegUndefinedRole);
-    return WTFMove(removedItem);
+    return result.releaseReturnValue();
 }
 
 SVGPathElement* SVGPathSegList::contextElement() const

Modified: branches/safari-605-branch/Source/WebCore/svg/SVGPathSegList.h (230232 => 230233)


--- branches/safari-605-branch/Source/WebCore/svg/SVGPathSegList.h	2018-04-04 03:03:42 UTC (rev 230232)
+++ branches/safari-605-branch/Source/WebCore/svg/SVGPathSegList.h	2018-04-04 03:27:17 UTC (rev 230233)
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) Research In Motion Limited 2010. All rights reserved.
+ * Copyright (C) 2018 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -72,7 +73,6 @@
 
     ExceptionOr<RefPtr<SVGPathSeg>> initialize(Ref<SVGPathSeg>&& newItem)
     {
-        clearContextAndRoles();
         return Base::initializeValues(WTFMove(newItem));
     }
 
@@ -101,9 +101,6 @@
     }
 
     SVGPathElement* contextElement() const;
-
-    void clearContextAndRoles();
-
     using Base::m_role;
 
     bool isReadOnly() const final

Modified: branches/safari-605-branch/Source/WebCore/svg/SVGPathSegListValues.cpp (230232 => 230233)


--- branches/safari-605-branch/Source/WebCore/svg/SVGPathSegListValues.cpp	2018-04-04 03:03:42 UTC (rev 230232)
+++ branches/safari-605-branch/Source/WebCore/svg/SVGPathSegListValues.cpp	2018-04-04 03:27:17 UTC (rev 230233)
@@ -3,6 +3,7 @@
  * Copyright (C) 2004, 2005 Rob Buis <b...@kde.org>
  * Copyright (C) 2007 Eric Seidel <e...@webkit.org>
  * Copyright (C) Research In Motion Limited 2010. All rights reserved.
+ * Copyright (C) 2018 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -40,4 +41,17 @@
     downcast<SVGPathElement>(contextElement).pathSegListChanged(m_role, listModification);
 }
 
+void SVGPathSegListValues::clearItemContextAndRole(unsigned index)
+{
+    auto& item = at(index);
+    static_cast<SVGPathSegWithContext&>(*item).setContextAndRole(nullptr, PathSegUndefinedRole);
 }
+    
+void SVGPathSegListValues::clearContextAndRoles()
+{
+    auto count = size();
+    while (count--)
+        clearItemContextAndRole(count);
+}
+    
+}

Modified: branches/safari-605-branch/Source/WebCore/svg/SVGPathSegListValues.h (230232 => 230233)


--- branches/safari-605-branch/Source/WebCore/svg/SVGPathSegListValues.h	2018-04-04 03:03:42 UTC (rev 230232)
+++ branches/safari-605-branch/Source/WebCore/svg/SVGPathSegListValues.h	2018-04-04 03:27:17 UTC (rev 230233)
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2007 Eric Seidel <e...@webkit.org>
+ * Copyright (C) 2018 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -35,16 +36,42 @@
 
 class SVGPathSegListValues : public Vector<RefPtr<SVGPathSeg>> {
 public:
+    using Base = Vector<RefPtr<SVGPathSeg>>;
+    
     explicit SVGPathSegListValues(SVGPathSegRole role)
         : m_role(role)
     {
     }
+    
+    SVGPathSegListValues(const SVGPathSegListValues&) = default;
+    SVGPathSegListValues(SVGPathSegListValues&&) = default;
+    
+    SVGPathSegListValues& operator=(const SVGPathSegListValues& other)
+    {
+        clearContextAndRoles();
+        return static_cast<SVGPathSegListValues&>(Base::operator=(other));
+    }
 
+    SVGPathSegListValues& operator=(SVGPathSegListValues&& other)
+    {
+        clearContextAndRoles();
+        return static_cast<SVGPathSegListValues&>(Base::operator=(WTFMove(other)));
+    }
+    
+    void clear()
+    {
+        clearContextAndRoles();
+        Base::clear();
+    }
+
     String valueAsString() const;
 
     void commitChange(SVGElement& contextElement, ListModification);
+    void clearItemContextAndRole(unsigned index);
 
 private:
+    void clearContextAndRoles();
+
     SVGPathSegRole m_role;
 };
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to