SVGElement should detach itself from all its properties before it is deleted
authorsaid@apple.com <said@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 17 May 2019 17:26:05 +0000 (17:26 +0000)
committersaid@apple.com <said@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 17 May 2019 17:26:05 +0000 (17:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=197954

Reviewed by Simon Fraser.

Source/WebCore:

Before deleting the SVGElement node, SVGElement::detachAllProperties()
needs to be called. This will make the properties be detached objects
which means no change will be committed unless these properties are
attached to another owner.

Test: svg/dom/svg-properties-detach-change.html

* dom/Node.cpp:
(WebCore::Node::removedLastRef):
* svg/SVGElement.h:
(WebCore::SVGElement::detachAllProperties):

LayoutTests:

* svg/dom/svg-properties-detach-change-expected.txt: Added.
* svg/dom/svg-properties-detach-change.html : Added.

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

LayoutTests/ChangeLog
LayoutTests/svg/dom/svg-properties-detach-change-expected.txt [new file with mode: 0644]
LayoutTests/svg/dom/svg-properties-detach-change.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Node.cpp
Source/WebCore/svg/SVGElement.h

index bc65398..26a7c71 100644 (file)
@@ -1,3 +1,13 @@
+2019-05-16  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        SVGElement should detach itself from all its properties before it is deleted
+        https://bugs.webkit.org/show_bug.cgi?id=197954
+
+        Reviewed by Simon Fraser.
+
+        * svg/dom/svg-properties-detach-change-expected.txt: Added.
+        * svg/dom/svg-properties-detach-change.html : Added.
+
 2019-05-17  Eric Carlson  <eric.carlson@apple.com>
 
         Allow sequential playback of media files when initial playback started with a user gesture
diff --git a/LayoutTests/svg/dom/svg-properties-detach-change-expected.txt b/LayoutTests/svg/dom/svg-properties-detach-change-expected.txt
new file mode 100644 (file)
index 0000000..ae626f2
--- /dev/null
@@ -0,0 +1,38 @@
+This test verifying changing the values of detached properties - utilizing the properties of SVGRectElement and SVGTextElement
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Check the SVGLength properties
+PASS x.value is 10
+PASS y.value is 20
+PASS width.value is 100
+PASS height.value is 200
+
+Check the items of the SVGLengthList properties
+PASS xList.getItem(0).value is 10
+PASS yList.getItem(0).value is 20
+PASS dxList.getItem(0).value is 100
+PASS dyList.getItem(0).value is 200
+
+Check the items of the SVGLengthList properties of the SVGTextElement
+PASS text.x.baseVal.getItem(0).value is 100
+PASS text.y.baseVal.getItem(0).value is 200
+PASS text.dx.baseVal.getItem(0).value is 1000
+PASS text.dy.baseVal.getItem(0).value is 2000
+
+Check the SVGLength properties after removing the SVGTextElement
+PASS x.value is 100
+PASS y.value is 200
+PASS width.value is 1000
+PASS height.value is 2000
+
+Check the SVGLength properties after resetting their values
+PASS x.value is 10
+PASS y.value is 20
+PASS width.value is 100
+PASS height.value is 200
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/svg/dom/svg-properties-detach-change.html b/LayoutTests/svg/dom/svg-properties-detach-change.html
new file mode 100644 (file)
index 0000000..efaaeba
--- /dev/null
@@ -0,0 +1,94 @@
+<html>
+<head>
+<script src="../../resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script>
+description("This test verifying changing the values of detached properties - utilizing the properties of SVGRectElement and SVGTextElement");
+
+var x = document.createElementNS("http://www.w3.org/2000/svg", "rect").x.baseVal;
+var y = document.createElementNS("http://www.w3.org/2000/svg", "rect").y.baseVal;
+var width = document.createElementNS("http://www.w3.org/2000/svg", "rect").width.baseVal;
+var height = document.createElementNS("http://www.w3.org/2000/svg", "rect").height.baseVal;
+
+var xList = document.createElementNS("http://www.w3.org/2000/svg", "text").x.baseVal;
+var yList = document.createElementNS("http://www.w3.org/2000/svg", "text").y.baseVal;
+var dxList = document.createElementNS("http://www.w3.org/2000/svg", "text").dx.baseVal;
+var dyList = document.createElementNS("http://www.w3.org/2000/svg", "text").dy.baseVal;
+
+var text = document.createElementNS("http://www.w3.org/2000/svg", "text");
+
+x.newValueSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER, 10);
+y.newValueSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER, 20);
+width.newValueSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER, 100);
+height.newValueSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER, 200);
+
+xList.initialize(x);
+yList.initialize(y);
+dxList.initialize(width);
+dyList.initialize(height);
+
+text.x.baseVal.initialize(x);
+text.y.baseVal.initialize(y);
+text.dx.baseVal.initialize(width);
+text.dy.baseVal.initialize(height);
+
+text.x.baseVal.getItem(0).newValueSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER, 100);
+text.y.baseVal.getItem(0).newValueSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER, 200);
+text.dx.baseVal.getItem(0).newValueSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER, 1000);
+text.dy.baseVal.getItem(0).newValueSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER, 2000);
+
+debug("Check the SVGLength properties");
+shouldBe("x.value", "10");
+shouldBe("y.value", "20");
+shouldBe("width.value", "100");
+shouldBe("height.value", "200");
+
+debug("");
+debug("Check the items of the SVGLengthList properties");
+shouldBe("xList.getItem(0).value", "10");
+shouldBe("yList.getItem(0).value", "20");
+shouldBe("dxList.getItem(0).value", "100");
+shouldBe("dyList.getItem(0).value", "200");
+
+debug("");
+debug("Check the items of the SVGLengthList properties of the SVGTextElement");
+shouldBe("text.x.baseVal.getItem(0).value", "100");
+shouldBe("text.y.baseVal.getItem(0).value", "200");
+shouldBe("text.dx.baseVal.getItem(0).value", "1000");
+shouldBe("text.dy.baseVal.getItem(0).value", "2000");
+
+x = text.x.baseVal.getItem(0);
+y = text.y.baseVal.getItem(0);
+width = text.dx.baseVal.getItem(0);
+height = text.dy.baseVal.getItem(0);
+
+text.remove();
+text = null;
+
+debug("");
+debug("Check the SVGLength properties after removing the SVGTextElement");
+shouldBe("x.value", "100");
+shouldBe("y.value", "200");
+shouldBe("width.value", "1000");
+shouldBe("height.value", "2000");
+
+x.newValueSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER, 10);
+y.newValueSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER, 20);
+width.newValueSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER, 100);
+height.newValueSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER, 200);
+
+debug("");
+debug("Check the SVGLength properties after resetting their values");
+shouldBe("x.value", "10");
+shouldBe("y.value", "20");
+shouldBe("width.value", "100");
+shouldBe("height.value", "200");
+
+successfullyParsed = true;
+</script>
+<script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
index dbdffce..fef9a73 100644 (file)
@@ -1,3 +1,22 @@
+2019-05-16  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        SVGElement should detach itself from all its properties before it is deleted
+        https://bugs.webkit.org/show_bug.cgi?id=197954
+
+        Reviewed by Simon Fraser.
+
+        Before deleting the SVGElement node, SVGElement::detachAllProperties()
+        needs to be called. This will make the properties be detached objects
+        which means no change will be committed unless these properties are
+        attached to another owner.
+
+        Test: svg/dom/svg-properties-detach-change.html
+
+        * dom/Node.cpp:
+        (WebCore::Node::removedLastRef):
+        * svg/SVGElement.h:
+        (WebCore::SVGElement::detachAllProperties):
+
 2019-05-17  Eric Carlson  <eric.carlson@apple.com>
 
         Allow sequential playback of media files when initial playback started with a user gesture
index d6f434b..15041e9 100644 (file)
@@ -62,6 +62,7 @@
 #include "RenderBox.h"
 #include "RenderTextControl.h"
 #include "RenderView.h"
+#include "SVGElement.h"
 #include "ScopedEventQueue.h"
 #include "ScriptDisallowedScope.h"
 #include "StorageEvent.h"
@@ -2509,6 +2510,12 @@ void Node::removedLastRef()
         return;
     }
 
+    // Now it is time to detach the SVGElement from all its properties. These properties
+    // may outlive the SVGElement. The only difference after the detach is no commit will
+    // be carried out unless these properties are attached to another owner.
+    if (is<SVGElement>(*this))
+        downcast<SVGElement>(*this).detachAllProperties();
+
 #ifndef NDEBUG
     m_deletionHasBegun = true;
 #endif
index a4beeea..b4a701d 100644 (file)
@@ -128,6 +128,7 @@ public:
 
     using PropertyRegistry = SVGPropertyOwnerRegistry<SVGElement>;
     virtual const SVGPropertyRegistry& propertyRegistry() const { return m_propertyRegistry; }
+    void detachAllProperties() { propertyRegistry().detachAllProperties(); }
 
     bool isAnimatedPropertyAttribute(const QualifiedName&) const;
     bool isAnimatedAttribute(const QualifiedName&) const;