- Revision
- 229830
- Author
- [email protected]
- Date
- 2018-03-21 15:22:45 -0700 (Wed, 21 Mar 2018)
Log Message
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 <[email protected]> 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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (229829 => 229830)
--- trunk/LayoutTests/ChangeLog 2018-03-21 21:49:02 UTC (rev 229829)
+++ trunk/LayoutTests/ChangeLog 2018-03-21 22:22:45 UTC (rev 229830)
@@ -1,3 +1,14 @@
+2018-03-21 Said Abou-Hallawa <[email protected]>
+
+ 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 Ryan Haddad <[email protected]>
Skip imported/w3c/web-platform-tests/service-workers/service-worker/appcache-ordering-main.https.html.
Added: trunk/LayoutTests/svg/dom/reuse-pathseg-after-changing-d-expected.txt (0 => 229830)
--- trunk/LayoutTests/svg/dom/reuse-pathseg-after-changing-d-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/dom/reuse-pathseg-after-changing-d-expected.txt 2018-03-21 22:22:45 UTC (rev 229830)
@@ -0,0 +1 @@
+This should not crash.
Added: trunk/LayoutTests/svg/dom/reuse-pathseg-after-changing-d.html (0 => 229830)
--- trunk/LayoutTests/svg/dom/reuse-pathseg-after-changing-d.html (rev 0)
+++ trunk/LayoutTests/svg/dom/reuse-pathseg-after-changing-d.html 2018-03-21 22:22:45 UTC (rev 229830)
@@ -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: trunk/Source/WebCore/ChangeLog (229829 => 229830)
--- trunk/Source/WebCore/ChangeLog 2018-03-21 21:49:02 UTC (rev 229829)
+++ trunk/Source/WebCore/ChangeLog 2018-03-21 22:22:45 UTC (rev 229830)
@@ -1,3 +1,35 @@
+2018-03-21 Said Abou-Hallawa <[email protected]>
+
+ 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 Antoine Quint <[email protected]>
[Web Animations] Ensure animationcancel and transitioncancel events are dispatched
Modified: trunk/Source/WebCore/svg/SVGPathElement.cpp (229829 => 229830)
--- trunk/Source/WebCore/svg/SVGPathElement.cpp 2018-03-21 21:49:02 UTC (rev 229829)
+++ trunk/Source/WebCore/svg/SVGPathElement.cpp 2018-03-21 22:22:45 UTC (rev 229830)
@@ -1,6 +1,7 @@
/*
* Copyright (C) 2004, 2005, 2006, 2008 Nikolas Zimmermann <[email protected]>
* Copyright (C) 2004, 2005, 2006, 2007 Rob Buis <[email protected]>
+ * 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
@@ -258,7 +259,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: trunk/Source/WebCore/svg/SVGPathSegList.cpp (229829 => 229830)
--- trunk/Source/WebCore/svg/SVGPathSegList.cpp 2018-03-21 21:49:02 UTC (rev 229829)
+++ trunk/Source/WebCore/svg/SVGPathSegList.cpp 2018-03-21 22:22:45 UTC (rev 229830)
@@ -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: trunk/Source/WebCore/svg/SVGPathSegList.h (229829 => 229830)
--- trunk/Source/WebCore/svg/SVGPathSegList.h 2018-03-21 21:49:02 UTC (rev 229829)
+++ trunk/Source/WebCore/svg/SVGPathSegList.h 2018-03-21 22:22:45 UTC (rev 229830)
@@ -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: trunk/Source/WebCore/svg/SVGPathSegListValues.cpp (229829 => 229830)
--- trunk/Source/WebCore/svg/SVGPathSegListValues.cpp 2018-03-21 21:49:02 UTC (rev 229829)
+++ trunk/Source/WebCore/svg/SVGPathSegListValues.cpp 2018-03-21 22:22:45 UTC (rev 229830)
@@ -3,6 +3,7 @@
* Copyright (C) 2004, 2005 Rob Buis <[email protected]>
* Copyright (C) 2007 Eric Seidel <[email protected]>
* 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: trunk/Source/WebCore/svg/SVGPathSegListValues.h (229829 => 229830)
--- trunk/Source/WebCore/svg/SVGPathSegListValues.h 2018-03-21 21:49:02 UTC (rev 229829)
+++ trunk/Source/WebCore/svg/SVGPathSegListValues.h 2018-03-21 22:22:45 UTC (rev 229830)
@@ -1,5 +1,6 @@
/*
* Copyright (C) 2007 Eric Seidel <[email protected]>
+ * 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;
};
