Generate toSVGFooElement() for SVGFELight|AnimateMotion|FontFaceSrc|TextPositioningElement

Generate toSVGFooElement() for SVGFELight|AnimateMotion|FontFaceSrc|TextPositioningElement

As a step to use toSVGFooElement(), this cl generate toSVGFooElement() for mentioned SVG Element.
This will help to detect bad type cast.

BUG=309516
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=165019

[gyuyoung-inactive, 2014-01-10 07:50:43]

Add tkent and morrita.

[Stephen Chennney, 2014-01-10 12:32:53]

There is cleanup to be done.

https://codereview.chromium.org/133203003/diff/1/Source/core/svg/SVGFELightEl...
File Source/core/svg/SVGFELightElement.cpp (right):

https://codereview.chromium.org/133203003/diff/1/Source/core/svg/SVGFELightEl...
Source/core/svg/SVGFELightElement.cpp:73: return toSVGFELightElement(node);
You are doing the hasTagName queries twice. I have some issues in this case with
using toBlah at all, as the calling code ensures safety and no other code calls
it. Regardless, this should be re-written to avoid all of the hasTagName queries
here.

https://codereview.chromium.org/133203003/diff/1/Source/core/svg/SVGFontFaceE...
File Source/core/svg/SVGFontFaceElement.cpp (right):

https://codereview.chromium.org/133203003/diff/1/Source/core/svg/SVGFontFaceE...
Source/core/svg/SVGFontFaceElement.cpp:281: srcElement =
toSVGFontFaceSrcElement(child);
Ditto.

https://codereview.chromium.org/133203003/diff/1/Source/core/svg/SVGTextPosit...
File Source/core/svg/SVGTextPositioningElement.cpp (right):

https://codereview.chromium.org/133203003/diff/1/Source/core/svg/SVGTextPosit...
Source/core/svg/SVGTextPositioningElement.cpp:151: return
toSVGTextPositioningElement(node);
This is a change in behavior. The previous code cast anything that was a
textTag, altGlyphTag or tspanTag. Your code also requires those things to have
attr names. Something is wrong.

https://codereview.chromium.org/133203003/diff/1/Source/core/svg/SVGTextPosit...
File Source/core/svg/SVGTextPositioningElement.h (right):

https://codereview.chromium.org/133203003/diff/1/Source/core/svg/SVGTextPosit...
Source/core/svg/SVGTextPositioningElement.h:58: return
node.hasTagName(SVGNames::xAttr) || node.hasTagName(SVGNames::yAttr) ||
node.hasTagName(SVGNames::dxAttr) || node.hasTagName(SVGNames::dyAttr);
I don't think a node will ever have a tag name that is an attribute name. Or am
I misunderstanding something.

[gyuyoung-inactive, 2014-01-12 04:37:30]

https://codereview.chromium.org/133203003/diff/1/Source/core/svg/SVGFELightEl...
File Source/core/svg/SVGFELightElement.cpp (right):

https://codereview.chromium.org/133203003/diff/1/Source/core/svg/SVGFELightEl...
Source/core/svg/SVGFELightElement.cpp:73: return toSVGFELightElement(node);
On 2014/01/10 12:32:53, Stephen Chenney wrote:
> You are doing the hasTagName queries twice. I have some issues in this case
with
> using toBlah at all, as the calling code ensures safety and no other code
calls
> it. Regardless, this should be re-written to avoid all of the hasTagName
queries
> here.

DEFINE_NODE_TYPE_CASTS macro only calls "hasTagName()" on debug build, not
release mode.

Frankly, it looks we don't need to call "hasTagName()" here. However,
toSVGFELightElement() needs to call it when it is used by other places which
don't check tag name in order to guarantee safer type cast. That's why
DEFINE_NODE_TYPE_CASTS checks tag name or child element. If I don't understand
what is your question correctly, please let me know.

#define DEFINE_TYPE_CASTS(thisType, argumentType, argumentName,
pointerPredicate, referencePredicate) \
inline thisType* to##thisType(argumentType* argumentName) \
{ \
    ASSERT_WITH_SECURITY_IMPLICATION(!argumentName || (pointerPredicate)); \
    return static_cast<thisType*>(argumentName); \
} \

BTW, it would be good to use isSVGFELightElement() instead of using
"hasTagName()" conditions.

https://codereview.chromium.org/133203003/diff/1/Source/core/svg/SVGFontFaceE...
File Source/core/svg/SVGFontFaceElement.cpp (right):

https://codereview.chromium.org/133203003/diff/1/Source/core/svg/SVGFontFaceE...
Source/core/svg/SVGFontFaceElement.cpp:281: srcElement =
toSVGFontFaceSrcElement(child);
On 2014/01/10 12:32:53, Stephen Chenney wrote:
> Ditto.

Done.

https://codereview.chromium.org/133203003/diff/1/Source/core/svg/SVGTextPosit...
File Source/core/svg/SVGTextPositioningElement.cpp (right):

https://codereview.chromium.org/133203003/diff/1/Source/core/svg/SVGTextPosit...
Source/core/svg/SVGTextPositioningElement.cpp:151: return
toSVGTextPositioningElement(node);
On 2014/01/10 12:32:53, Stephen Chenney wrote:
> This is a change in behavior. The previous code cast anything that was a
> textTag, altGlyphTag or tspanTag. Your code also requires those things to have
> attr names. Something is wrong.

yes, tosVGTextPositionElement() has wrong behavior.

https://codereview.chromium.org/133203003/diff/1/Source/core/svg/SVGTextPosit...
File Source/core/svg/SVGTextPositioningElement.h (right):

https://codereview.chromium.org/133203003/diff/1/Source/core/svg/SVGTextPosit...
Source/core/svg/SVGTextPositioningElement.h:58: return
node.hasTagName(SVGNames::xAttr) || node.hasTagName(SVGNames::yAttr) ||
node.hasTagName(SVGNames::dxAttr) || node.hasTagName(SVGNames::dyAttr);
On 2014/01/10 12:32:53, Stephen Chenney wrote:
> I don't think a node will ever have a tag name that is an attribute name. Or
am
> I misunderstanding something.

Oops, it is my mistake. It looks SVGTextPositioningElement doesn't have tag
name. So, I add a virtual function to SVGElement to check if a node is
SVGTextPositioningElement lie SVGContentElement.

[tkent, 2014-01-14 00:45:36]

lgtm

https://codereview.chromium.org/133203003/diff/120001/Source/core/svg/SVGElem...
File Source/core/svg/SVGElement.h (right):

https://codereview.chromium.org/133203003/diff/120001/Source/core/svg/SVGElem...
Source/core/svg/SVGElement.h:90: virtual bool isTextPositioning() const { return
false; }
This code is used only for assertion, and we can wrap it with
SECURITY_ASSERT_ENABLED. However it's ok not to do it because the code is very
small.

[commit-bot: I haz the power, 2014-01-14 00:45:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gyuyoung.kim@samsung.com/133203003/120001

[commit-bot: I haz the power, 2014-01-14 03:34:18]

Change committed as 165019