[CTTE] Tighter element types for RenderSVGShape and subclasses.

<https://webkit.org/b/121302> Reviewed by Antti Koivisto. Codify the following: - RenderSVGPath always has an SVGGraphicsElement. - RenderSVGEllipse always has an SVGGraphicsElement. - RenderSVGRect always has an SVGRectElement. - RenderSVGShape always has an SVGGraphicsElement. None of these renderers are ever anonymous, so delete element() and provide strongly typed reference getters instead. Canonical link: https://commits.webkit.org/139261@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@155704 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Sep 13, 2013 · 791a94b · 791a94b
1 parent 7b5121a
commit 791a94b
Show file tree

Hide file tree

Showing 16 changed files with 101 additions and 70 deletions.
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,20 @@
+2013-09-13  Andreas Kling  <akling@apple.com>
+
+        [CTTE] Tighter element types for RenderSVGShape and subclasses.
+        <https://webkit.org/b/121302>
+
+        Reviewed by Antti Koivisto.
+
+        Codify the following:
+
+        - RenderSVGPath always has an SVGGraphicsElement.
+        - RenderSVGEllipse always has an SVGGraphicsElement.
+        - RenderSVGRect always has an SVGRectElement.
+        - RenderSVGShape always has an SVGGraphicsElement.
+
+        None of these renderers are ever anonymous, so delete element() and provide
+        strongly typed reference getters instead.
+
 2013-09-13  Andreas Kling  <akling@apple.com>
 
         [CTTE] RenderSVGResourceFilterPrimitive always has an SVGFilterPrimitiveStandardAttributes.

diff --git a/Source/WebCore/rendering/svg/RenderSVGEllipse.cpp b/Source/WebCore/rendering/svg/RenderSVGEllipse.cpp
@@ -36,8 +36,8 @@
 
 namespace WebCore {
 
-RenderSVGEllipse::RenderSVGEllipse(SVGGraphicsElement* node)
-    : RenderSVGShape(node)
+RenderSVGEllipse::RenderSVGEllipse(SVGGraphicsElement& element)
+    : RenderSVGShape(element)
     , m_usePathFallback(false)
 {
 }
@@ -77,24 +77,21 @@ void RenderSVGEllipse::updateShapeFromElement()
 
 void RenderSVGEllipse::calculateRadiiAndCenter()
 {
-    ASSERT(element());
-    if (isSVGCircleElement(element())) {
-
-        SVGCircleElement* circle = toSVGCircleElement(element());
-
-        SVGLengthContext lengthContext(circle);
-        float radius = circle->r().value(lengthContext);
+    if (isSVGCircleElement(graphicsElement())) {
+        SVGCircleElement& circle = toSVGCircleElement(graphicsElement());
+        SVGLengthContext lengthContext(&circle);
+        float radius = circle.r().value(lengthContext);
         m_radii = FloatSize(radius, radius);
-        m_center = FloatPoint(circle->cx().value(lengthContext), circle->cy().value(lengthContext));
+        m_center = FloatPoint(circle.cx().value(lengthContext), circle.cy().value(lengthContext));
         return;
     }
 
-    ASSERT(isSVGEllipseElement(element()));
-    SVGEllipseElement* ellipse = toSVGEllipseElement(element());
+    ASSERT(isSVGEllipseElement(graphicsElement()));
+    SVGEllipseElement& ellipse = toSVGEllipseElement(graphicsElement());
 
-    SVGLengthContext lengthContext(ellipse);
-    m_radii = FloatSize(ellipse->rx().value(lengthContext), ellipse->ry().value(lengthContext));
-    m_center = FloatPoint(ellipse->cx().value(lengthContext), ellipse->cy().value(lengthContext));
+    SVGLengthContext lengthContext(&ellipse);
+    m_radii = FloatSize(ellipse.rx().value(lengthContext), ellipse.ry().value(lengthContext));
+    m_center = FloatPoint(ellipse.cx().value(lengthContext), ellipse.cy().value(lengthContext));
 }
 
 void RenderSVGEllipse::fillShape(GraphicsContext* context) const

diff --git a/Source/WebCore/rendering/svg/RenderSVGEllipse.h b/Source/WebCore/rendering/svg/RenderSVGEllipse.h
@@ -35,7 +35,7 @@ namespace WebCore {
 
 class RenderSVGEllipse FINAL : public RenderSVGShape {
 public:
-    explicit RenderSVGEllipse(SVGGraphicsElement*);
+    explicit RenderSVGEllipse(SVGGraphicsElement&);
     virtual ~RenderSVGEllipse();
 
 private:

diff --git a/Source/WebCore/rendering/svg/RenderSVGPath.cpp b/Source/WebCore/rendering/svg/RenderSVGPath.cpp
@@ -36,8 +36,8 @@
 
 namespace WebCore {
 
-RenderSVGPath::RenderSVGPath(SVGGraphicsElement* node)
-    : RenderSVGShape(node)
+RenderSVGPath::RenderSVGPath(SVGGraphicsElement& element)
+    : RenderSVGShape(element)
 {
 }
 

diff --git a/Source/WebCore/rendering/svg/RenderSVGPath.h b/Source/WebCore/rendering/svg/RenderSVGPath.h
@@ -33,7 +33,7 @@ namespace WebCore {
 
 class RenderSVGPath FINAL : public RenderSVGShape {
 public:
-    explicit RenderSVGPath(SVGGraphicsElement*);
+    explicit RenderSVGPath(SVGGraphicsElement&);
     virtual ~RenderSVGPath();
 
 private:

diff --git a/Source/WebCore/rendering/svg/RenderSVGRect.cpp b/Source/WebCore/rendering/svg/RenderSVGRect.cpp
@@ -35,8 +35,8 @@
 
 namespace WebCore {
 
-RenderSVGRect::RenderSVGRect(SVGRectElement* node)
-    : RenderSVGShape(node)
+RenderSVGRect::RenderSVGRect(SVGRectElement& element)
+    : RenderSVGShape(element)
     , m_usePathFallback(false)
 {
 }
@@ -45,30 +45,33 @@ RenderSVGRect::~RenderSVGRect()
 {
 }
 
+SVGRectElement& RenderSVGRect::rectElement() const
+{
+    return toSVGRectElement(RenderSVGShape::graphicsElement());
+}
+
 void RenderSVGRect::updateShapeFromElement()
 {
     // Before creating a new object we need to clear the cached bounding box
     // to avoid using garbage.
     m_fillBoundingBox = FloatRect();
     m_innerStrokeRect = FloatRect();
     m_outerStrokeRect = FloatRect();
-    SVGRectElement* rect = toSVGRectElement(element());
-    ASSERT(rect);
 
     // Fallback to RenderSVGShape if rect has rounded corners or a non-scaling stroke.
-    if (rect->hasAttribute(SVGNames::rxAttr) || rect->hasAttribute(SVGNames::ryAttr) || hasNonScalingStroke()) {
+    if (rectElement().hasAttribute(SVGNames::rxAttr) || rectElement().hasAttribute(SVGNames::ryAttr) || hasNonScalingStroke()) {
         RenderSVGShape::updateShapeFromElement();
         m_usePathFallback = true;
         return;
     } else
         m_usePathFallback = false;
 
-    SVGLengthContext lengthContext(rect);
-    FloatSize boundingBoxSize(rect->width().value(lengthContext), rect->height().value(lengthContext));
+    SVGLengthContext lengthContext(&rectElement());
+    FloatSize boundingBoxSize(rectElement().width().value(lengthContext), rectElement().height().value(lengthContext));
     if (boundingBoxSize.isEmpty())
         return;
 
-    m_fillBoundingBox = FloatRect(FloatPoint(rect->x().value(lengthContext), rect->y().value(lengthContext)), boundingBoxSize);
+    m_fillBoundingBox = FloatRect(FloatPoint(rectElement().x().value(lengthContext), rectElement().y().value(lengthContext)), boundingBoxSize);
 
     // To decide if the stroke contains a point we create two rects which represent the inner and
     // the outer stroke borders. A stroke contains the point, if the point is between them.

diff --git a/Source/WebCore/rendering/svg/RenderSVGRect.h b/Source/WebCore/rendering/svg/RenderSVGRect.h
@@ -36,10 +36,14 @@ namespace WebCore {
 
 class RenderSVGRect FINAL : public RenderSVGShape {
 public:
-    explicit RenderSVGRect(SVGRectElement*);
+    explicit RenderSVGRect(SVGRectElement&);
     virtual ~RenderSVGRect();
 
+    SVGRectElement& rectElement() const;
+
 private:
+    void graphicsElement() const WTF_DELETED_FUNCTION;
+
     virtual const char* renderName() const { return "RenderSVGRect"; }
 
     virtual void updateShapeFromElement();

diff --git a/Source/WebCore/rendering/svg/RenderSVGShape.cpp b/Source/WebCore/rendering/svg/RenderSVGShape.cpp
@@ -52,8 +52,8 @@
 
 namespace WebCore {
 
-RenderSVGShape::RenderSVGShape(SVGGraphicsElement* node)
-    : RenderSVGModelObject(node)
+RenderSVGShape::RenderSVGShape(SVGGraphicsElement& element)
+    : RenderSVGModelObject(&element)
     , m_needsBoundariesUpdate(false) // Default is false, the cached rects are empty from the beginning.
     , m_needsShapeUpdate(true) // Default is true, so we grab a Path object once from SVGGraphicsElement.
     , m_needsTransformUpdate(true) // Default is true, so we grab a AffineTransform object once from SVGGraphicsElement.
@@ -70,7 +70,7 @@ void RenderSVGShape::updateShapeFromElement()
     m_path = adoptPtr(new Path);
     ASSERT(RenderSVGShape::isEmpty());
 
-    updatePathFromGraphicsElement(toSVGGraphicsElement(element()), path());
+    updatePathFromGraphicsElement(&graphicsElement(), path());
     processMarkerPositions();
 
     m_fillBoundingBox = calculateObjectBoundingBox();
@@ -158,7 +158,7 @@ void RenderSVGShape::layout()
     }
 
     if (m_needsTransformUpdate) {
-        m_localTransform = toSVGGraphicsElement(element())->animatedLocalTransform();
+        m_localTransform = graphicsElement().animatedLocalTransform();
         m_needsTransformUpdate = false;
         updateCachedBoundariesInParents = true;
     }
@@ -197,15 +197,15 @@ bool RenderSVGShape::setupNonScalingStrokeContext(AffineTransform& strokeTransfo
 
 AffineTransform RenderSVGShape::nonScalingStrokeTransform() const
 {
-    return toSVGGraphicsElement(element())->getScreenCTM(SVGLocatable::DisallowStyleUpdate);
+    return graphicsElement().getScreenCTM(SVGLocatable::DisallowStyleUpdate);
 }
 
 bool RenderSVGShape::shouldGenerateMarkerPositions() const
 {
     if (!style()->svgStyle()->hasMarkers())
         return false;
 
-    if (!toSVGGraphicsElement(element())->supportsMarkers())
+    if (!graphicsElement().supportsMarkers())
         return false;
 
     SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(this);
@@ -412,7 +412,7 @@ void RenderSVGShape::updateRepaintBoundingBox()
 
 float RenderSVGShape::strokeWidth() const
 {
-    SVGLengthContext lengthContext(element());
+    SVGLengthContext lengthContext(&graphicsElement());
     return style()->svgStyle()->strokeWidth().value(lengthContext);
 }
 

diff --git a/Source/WebCore/rendering/svg/RenderSVGShape.h b/Source/WebCore/rendering/svg/RenderSVGShape.h
@@ -30,6 +30,7 @@
 #include "AffineTransform.h"
 #include "FloatRect.h"
 #include "RenderSVGModelObject.h"
+#include "SVGGraphicsElement.h"
 #include "SVGMarkerData.h"
 #include "StrokeStyleApplier.h"
 #include <wtf/OwnPtr.h>
@@ -66,10 +67,12 @@ class BoundingRectStrokeStyleApplier : public StrokeStyleApplier {
 
 class RenderSVGShape : public RenderSVGModelObject {
 public:
-    explicit RenderSVGShape(SVGGraphicsElement*);
-    RenderSVGShape(SVGGraphicsElement*, Path*, bool);
+    explicit RenderSVGShape(SVGGraphicsElement&);
+    RenderSVGShape(SVGGraphicsElement&, Path*, bool);
     virtual ~RenderSVGShape();
 
+    SVGGraphicsElement& graphicsElement() const { return *toSVGGraphicsElement(RenderSVGModelObject::element()); }
+
     void setNeedsShapeUpdate() { m_needsShapeUpdate = true; }
     virtual void setNeedsBoundariesUpdate() OVERRIDE FINAL { m_needsBoundariesUpdate = true; }
     virtual bool needsBoundariesUpdate() OVERRIDE FINAL { return m_needsBoundariesUpdate; }
@@ -85,6 +88,8 @@ class RenderSVGShape : public RenderSVGModelObject {
     }
 
 protected:
+    void element() const WTF_DELETED_FUNCTION;
+
     virtual void updateShapeFromElement();
     virtual bool isEmpty() const;
     virtual bool shapeDependentStrokeContains(const FloatPoint&);

diff --git a/Source/WebCore/rendering/svg/SVGRenderTreeAsText.cpp b/Source/WebCore/rendering/svg/SVGRenderTreeAsText.cpp
@@ -275,15 +275,14 @@ static void writeStyle(TextStream& ts, const RenderObject& object)
     writeIfNotDefault(ts, "opacity", style->opacity(), RenderStyle::initialOpacity());
     if (object.isSVGShape()) {
         const RenderSVGShape& shape = static_cast<const RenderSVGShape&>(object);
-        ASSERT(shape.element());
 
         Color fallbackColor;
         if (RenderSVGResource* strokePaintingResource = RenderSVGResource::strokePaintingResource(const_cast<RenderSVGShape*>(&shape), shape.style(), fallbackColor)) {
             TextStreamSeparator s(" ");
             ts << " [stroke={" << s;
             writeSVGPaintingResource(ts, strokePaintingResource);
 
-            SVGLengthContext lengthContext(shape.element());
+            SVGLengthContext lengthContext(&shape.graphicsElement());
             double dashOffset = svgStyle->strokeDashOffset().value(lengthContext);
             double strokeWidth = svgStyle->strokeWidth().value(lengthContext);
             const Vector<SVGLength>& dashes = svgStyle->strokeDashArray();
@@ -333,40 +332,40 @@ static TextStream& operator<<(TextStream& ts, const RenderSVGShape& shape)
 {
     writePositionAndStyle(ts, shape);
 
-    SVGElement* svgElement = shape.element();
-    SVGLengthContext lengthContext(svgElement);
+    SVGGraphicsElement& svgElement = shape.graphicsElement();
+    SVGLengthContext lengthContext(&svgElement);
 
     if (isSVGRectElement(svgElement)) {
-        SVGRectElement* element = toSVGRectElement(svgElement);
-        writeNameValuePair(ts, "x", element->x().value(lengthContext));
-        writeNameValuePair(ts, "y", element->y().value(lengthContext));
-        writeNameValuePair(ts, "width", element->width().value(lengthContext));
-        writeNameValuePair(ts, "height", element->height().value(lengthContext));
+        const SVGRectElement& element = toSVGRectElement(svgElement);
+        writeNameValuePair(ts, "x", element.x().value(lengthContext));
+        writeNameValuePair(ts, "y", element.y().value(lengthContext));
+        writeNameValuePair(ts, "width", element.width().value(lengthContext));
+        writeNameValuePair(ts, "height", element.height().value(lengthContext));
     } else if (isSVGLineElement(svgElement)) {
-        SVGLineElement* element = toSVGLineElement(svgElement);
-        writeNameValuePair(ts, "x1", element->x1().value(lengthContext));
-        writeNameValuePair(ts, "y1", element->y1().value(lengthContext));
-        writeNameValuePair(ts, "x2", element->x2().value(lengthContext));
-        writeNameValuePair(ts, "y2", element->y2().value(lengthContext));
+        const SVGLineElement& element = toSVGLineElement(svgElement);
+        writeNameValuePair(ts, "x1", element.x1().value(lengthContext));
+        writeNameValuePair(ts, "y1", element.y1().value(lengthContext));
+        writeNameValuePair(ts, "x2", element.x2().value(lengthContext));
+        writeNameValuePair(ts, "y2", element.y2().value(lengthContext));
     } else if (isSVGEllipseElement(svgElement)) {
-        SVGEllipseElement* element = toSVGEllipseElement(svgElement);
-        writeNameValuePair(ts, "cx", element->cx().value(lengthContext));
-        writeNameValuePair(ts, "cy", element->cy().value(lengthContext));
-        writeNameValuePair(ts, "rx", element->rx().value(lengthContext));
-        writeNameValuePair(ts, "ry", element->ry().value(lengthContext));
+        const SVGEllipseElement& element = toSVGEllipseElement(svgElement);
+        writeNameValuePair(ts, "cx", element.cx().value(lengthContext));
+        writeNameValuePair(ts, "cy", element.cy().value(lengthContext));
+        writeNameValuePair(ts, "rx", element.rx().value(lengthContext));
+        writeNameValuePair(ts, "ry", element.ry().value(lengthContext));
     } else if (isSVGCircleElement(svgElement)) {
-        SVGCircleElement* element = toSVGCircleElement(svgElement);
-        writeNameValuePair(ts, "cx", element->cx().value(lengthContext));
-        writeNameValuePair(ts, "cy", element->cy().value(lengthContext));
-        writeNameValuePair(ts, "r", element->r().value(lengthContext));
-    } else if (svgElement->hasTagName(SVGNames::polygonTag) || svgElement->hasTagName(SVGNames::polylineTag)) {
-        SVGPolyElement* element = toSVGPolyElement(svgElement);
-        writeNameAndQuotedValue(ts, "points", element->pointList().valueAsString());
+        const SVGCircleElement& element = toSVGCircleElement(svgElement);
+        writeNameValuePair(ts, "cx", element.cx().value(lengthContext));
+        writeNameValuePair(ts, "cy", element.cy().value(lengthContext));
+        writeNameValuePair(ts, "r", element.r().value(lengthContext));
+    } else if (svgElement.hasTagName(SVGNames::polygonTag) || svgElement.hasTagName(SVGNames::polylineTag)) {
+        const SVGPolyElement& element = toSVGPolyElement(svgElement);
+        writeNameAndQuotedValue(ts, "points", element.pointList().valueAsString());
     } else if (isSVGPathElement(svgElement)) {
-        SVGPathElement* element = toSVGPathElement(svgElement);
+        const SVGPathElement& element = toSVGPathElement(svgElement);
         String pathString;
         // FIXME: We should switch to UnalteredParsing here - this will affect the path dumping output of dozens of tests.
-        buildStringFromByteStream(element->pathByteStream(), pathString, NormalizedParsing);
+        buildStringFromByteStream(element.pathByteStream(), pathString, NormalizedParsing);
         writeNameAndQuotedValue(ts, "data", pathString);
     } else
         ASSERT_NOT_REACHED();

diff --git a/Source/WebCore/svg/SVGCircleElement.cpp b/Source/WebCore/svg/SVGCircleElement.cpp
@@ -141,7 +141,7 @@ bool SVGCircleElement::selfHasRelativeLengths() const
 
 RenderObject* SVGCircleElement::createRenderer(RenderArena* arena, RenderStyle*)
 {
-    return new (arena) RenderSVGEllipse(this);
+    return new (arena) RenderSVGEllipse(*this);
 }
 
 }

diff --git a/Source/WebCore/svg/SVGEllipseElement.cpp b/Source/WebCore/svg/SVGEllipseElement.cpp
@@ -147,7 +147,7 @@ bool SVGEllipseElement::selfHasRelativeLengths() const
 
 RenderObject* SVGEllipseElement::createRenderer(RenderArena* arena, RenderStyle*)
 {
-    return new (arena) RenderSVGEllipse(this);
+    return new (arena) RenderSVGEllipse(*this);
 }
 
 }

diff --git a/Source/WebCore/svg/SVGGraphicsElement.cpp b/Source/WebCore/svg/SVGGraphicsElement.cpp
@@ -165,7 +165,7 @@ FloatRect SVGGraphicsElement::getBBox(StyleUpdateStrategy styleUpdateStrategy)
 RenderObject* SVGGraphicsElement::createRenderer(RenderArena* arena, RenderStyle*)
 {
     // By default, any subclass is expected to do path-based drawing
-    return new (arena) RenderSVGPath(this);
+    return new (arena) RenderSVGPath(*this);
 }
 
 void SVGGraphicsElement::toClipPath(Path& path)

diff --git a/Source/WebCore/svg/SVGPathElement.cpp b/Source/WebCore/svg/SVGPathElement.cpp
@@ -408,7 +408,7 @@ FloatRect SVGPathElement::getBBox(StyleUpdateStrategy styleUpdateStrategy)
 RenderObject* SVGPathElement::createRenderer(RenderArena* arena, RenderStyle*)
 {
     // By default, any subclass is expected to do path-based drawing
-    return new (arena) RenderSVGPath(this);
+    return new (arena) RenderSVGPath(*this);
 }
 
 }

diff --git a/Source/WebCore/svg/SVGPolyElement.h b/Source/WebCore/svg/SVGPolyElement.h
@@ -65,6 +65,12 @@ class SVGPolyElement : public SVGGraphicsElement
     mutable SVGSynchronizableAnimatedProperty<SVGPointList> m_points;
 };
 
+inline SVGPolyElement& toSVGPolyElement(SVGElement& element)
+{
+    ASSERT_WITH_SECURITY_IMPLICATION(element.hasTagName(SVGNames::polygonTag) || element.hasTagName(SVGNames::polylineTag));
+    return static_cast<SVGPolyElement&>(element);
+}
+
 inline SVGPolyElement* toSVGPolyElement(SVGElement* element)
 {
     ASSERT_WITH_SECURITY_IMPLICATION(!element || element->hasTagName(SVGNames::polygonTag) || element->hasTagName(SVGNames::polylineTag));

diff --git a/Source/WebCore/svg/SVGRectElement.cpp b/Source/WebCore/svg/SVGRectElement.cpp
@@ -162,7 +162,7 @@ bool SVGRectElement::selfHasRelativeLengths() const
 
 RenderObject* SVGRectElement::createRenderer(RenderArena* arena, RenderStyle*)
 {
-    return new (arena) RenderSVGRect(this);
+    return new (arena) RenderSVGRect(*this);
 }
 
 }