Merged r236991 - REGRESSION(r234620): SVGLangSpace::svgAttributeChang…

…ed() should invalidate the renderer of the SVGGeometryElement descendant only https://bugs.webkit.org/show_bug.cgi?id=190411 Reviewed by Simon Fraser. Source/WebCore: Test: svg/dynamic-updates/SVGStopElement-dom-xml-lang-attrr.html When changing the attributes of the SVGLangSpace, we should invalidate the renderer of the SVGGeometryElement descendant only. Renderer of other elements, like SVGStopElement, should not be invalidated because they do not have geometry and they can be used as resources for drawing another SVGGeometryElement. * svg/SVGElement.h: (WebCore::SVGElement::isSVGGeometryElement const): * svg/SVGGeometryElement.h: (isType): * svg/SVGLangSpace.cpp: (WebCore::SVGLangSpace::svgAttributeChanged): LayoutTests: * svg/dynamic-updates/SVGStopElement-dom-xml-lang-attrr-expected.txt: Added. * svg/dynamic-updates/SVGStopElement-dom-xml-lang-attrr.html: Added.
WebKit · Oct 28, 2018 · 8c49731 · 8c49731
1 parent 3d69c41
commit 8c49731
Show file tree

Hide file tree

Showing 7 changed files with 71 additions and 5 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2018-10-09  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        REGRESSION(r234620): SVGLangSpace::svgAttributeChanged() should invalidate the renderer of the SVGGeometryElement descendant only
+        https://bugs.webkit.org/show_bug.cgi?id=190411
+
+        Reviewed by Simon Fraser.
+
+        * svg/dynamic-updates/SVGStopElement-dom-xml-lang-attrr-expected.txt: Added.
+        * svg/dynamic-updates/SVGStopElement-dom-xml-lang-attrr.html: Added.
+
 2018-10-09  Philippe Normand  <pnormand@igalia.com>
 
         [GStreamer] Stealing cross-origin video pixel with HLS

diff --git a/LayoutTests/svg/dynamic-updates/SVGStopElement-dom-xml-lang-attrr-expected.txt b/LayoutTests/svg/dynamic-updates/SVGStopElement-dom-xml-lang-attrr-expected.txt
@@ -0,0 +1,3 @@
+Passes if no crash happens.
+
+
diff --git a/LayoutTests/svg/dynamic-updates/SVGStopElement-dom-xml-lang-attrr.html b/LayoutTests/svg/dynamic-updates/SVGStopElement-dom-xml-lang-attrr.html
@@ -0,0 +1,21 @@
+<body>
+    <p>Passes if no crash happens.</p>
+    <svg>
+        <linearGradient id="gradient">
+            <stop id="stop1" offset="0%" stop-color="green" />
+            <stop id="stop2" offset="50%" stop-color="green" />
+        </linearGradient>
+        <rect fill="url(#gradient)" x="10" y="10" width="200" height="100"/>
+    </svg>
+    <script>
+        if (window.testRunner) {
+            testRunner.dumpAsText();
+            testRunner.waitUntilDone();
+        }
+        setTimeout(function(){ 
+            stop1.setAttributeNS("http://www.w3.org/XML/1998/namespace", "xml:lang", "jw");
+            if (window.testRunner)
+                testRunner.notifyDone();
+        }, 0);
+    </script>
+</body>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,25 @@
+2018-10-09  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        REGRESSION(r234620): SVGLangSpace::svgAttributeChanged() should invalidate the renderer of the SVGGeometryElement descendant only
+        https://bugs.webkit.org/show_bug.cgi?id=190411
+
+        Reviewed by Simon Fraser.
+
+        Test: svg/dynamic-updates/SVGStopElement-dom-xml-lang-attrr.html
+
+        When changing the attributes of the SVGLangSpace, we should invalidate
+        the renderer of the SVGGeometryElement descendant only. Renderer of other
+        elements, like SVGStopElement, should not be invalidated because they do
+        not have geometry and they can be used as resources for drawing another
+        SVGGeometryElement.
+
+        * svg/SVGElement.h:
+        (WebCore::SVGElement::isSVGGeometryElement const):
+        * svg/SVGGeometryElement.h:
+        (isType):
+        * svg/SVGLangSpace.cpp:
+        (WebCore::SVGLangSpace::svgAttributeChanged):
+
 2018-10-09  Michael Catanzaro  <mcatanzaro@igalia.com>
 
         [WPE][GTK] Complex text crashes with harfbuzz 1.8.8

diff --git a/Source/WebCore/svg/SVGElement.h b/Source/WebCore/svg/SVGElement.h
@@ -65,6 +65,7 @@ class SVGElement : public StyledElement, public SVGLangSpace {
     virtual AffineTransform localCoordinateSpaceTransform(SVGLocatable::CTMScope) const;
 
     virtual bool isSVGGraphicsElement() const { return false; }
+    virtual bool isSVGGeometryElement() const { return false; }
     virtual bool isFilterEffect() const { return false; }
     virtual bool isGradientStop() const { return false; }
     virtual bool isTextContent() const { return false; }

diff --git a/Source/WebCore/svg/SVGGeometryElement.h b/Source/WebCore/svg/SVGGeometryElement.h
@@ -54,6 +54,7 @@ class SVGGeometryElement : public SVGGraphicsElement {
     void svgAttributeChanged(const QualifiedName&) override;
 
 private:
+    bool isSVGGeometryElement() const override { return true; }
     const SVGAttributeOwnerProxy& attributeOwnerProxy() const override { return m_attributeOwnerProxy; }
 
     static void registerAttributes();
@@ -64,3 +65,8 @@ class SVGGeometryElement : public SVGGraphicsElement {
 };
 
 } // namespace WebCore
+
+SPECIALIZE_TYPE_TRAITS_BEGIN(WebCore::SVGGeometryElement)
+    static bool isType(const WebCore::SVGElement& element) { return element.isSVGGeometryElement(); }
+    static bool isType(const WebCore::Node& node) { return is<WebCore::SVGElement>(node) && isType(downcast<WebCore::SVGElement>(node)); }
+SPECIALIZE_TYPE_TRAITS_END()
diff --git a/Source/WebCore/svg/SVGLangSpace.cpp b/Source/WebCore/svg/SVGLangSpace.cpp
@@ -24,7 +24,7 @@
 
 #include "RenderSVGResource.h"
 #include "RenderSVGShape.h"
-#include "SVGElement.h"
+#include "SVGGeometryElement.h"
 #include "XMLNames.h"
 #include <wtf/NeverDestroyed.h>
 
@@ -67,10 +67,13 @@ void SVGLangSpace::svgAttributeChanged(const QualifiedName& attrName)
     if (!isKnownAttribute(attrName))
         return;
 
-    if (auto* renderer = downcast<RenderSVGShape>(m_contextElement.renderer())) {
-        SVGElement::InstanceInvalidationGuard guard(m_contextElement);
-        RenderSVGResource::markForLayoutAndParentResourceInvalidation(*renderer);
-    }
+    auto* renderer = m_contextElement.renderer();
+    if (!is<RenderSVGShape>(renderer))
+        return;
+
+    ASSERT(is<SVGGeometryElement>(m_contextElement));
+    SVGElement::InstanceInvalidationGuard guard(m_contextElement);
+    RenderSVGResource::markForLayoutAndParentResourceInvalidation(*renderer);
 }
 
 }