Use DEFINE_NODE_TYPE_CASTS_WITH_FUNCTION to generate toSVGSMILElement()

Use DEFINE_NODE_TYPE_CASTS_WITH_FUNCTION to generate toSVGSMILElement()

DEFINE_NODE_TYPE_CASTS_WITH_FUNCTION supports more plenty toFoo helper functions than
manual toFoo(). This cl changes isSMILElement from static member to inline function to generate
toSVGSMILElement().

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

[gyuyoung-inactive, 2013-12-04 01:42:47]

Add tkent.

[tkent, 2013-12-04 01:55:15]

https://codereview.chromium.org/103393002/diff/20001/Source/core/svg/animatio...
File Source/core/svg/animation/SVGSMILElement.h (right):

https://codereview.chromium.org/103393002/diff/20001/Source/core/svg/animatio...
Source/core/svg/animation/SVGSMILElement.h:241: inline bool
isSVGSMILElement(Node* node)
Having both of a pointer version and a reference version of isSVGSMILElement()
is not nice.

I recommend to remove isSVGSMILElement(Node*), and replace existing call sites
of |if (SVGSMILElement::isSMILElement(node))| with |if (node &&
isSVGSMILElement(*node))|.

[gyuyoung-inactive, 2013-12-04 02:07:27]

On 2013/12/04 01:55:15, tkent wrote:
>
https://codereview.chromium.org/103393002/diff/20001/Source/core/svg/animatio...
> File Source/core/svg/animation/SVGSMILElement.h (right):
> 
>
https://codereview.chromium.org/103393002/diff/20001/Source/core/svg/animatio...
> Source/core/svg/animation/SVGSMILElement.h:241: inline bool
> isSVGSMILElement(Node* node)
> Having both of a pointer version and a reference version of isSVGSMILElement()
> is not nice.
> 
> I recommend to remove isSVGSMILElement(Node*), and replace existing call sites
> of |if (SVGSMILElement::isSMILElement(node))| with |if (node &&
> isSVGSMILElement(*node))|.

Though I also considered it, I didn't want to modify existing code. However, if
you think it is better, I don't mind. Patch is coming.

[gyuyoung-inactive, 2013-12-04 03:45:10]

On 2013/12/04 02:07:27, gyuyoung wrote:
> On 2013/12/04 01:55:15, tkent wrote:
> >
>
https://codereview.chromium.org/103393002/diff/20001/Source/core/svg/animatio...
> > File Source/core/svg/animation/SVGSMILElement.h (right):
> > 
> >
>
https://codereview.chromium.org/103393002/diff/20001/Source/core/svg/animatio...
> > Source/core/svg/animation/SVGSMILElement.h:241: inline bool
> > isSVGSMILElement(Node* node)
> > Having both of a pointer version and a reference version of
isSVGSMILElement()
> > is not nice.
> > 
> > I recommend to remove isSVGSMILElement(Node*), and replace existing call
sites
> > of |if (SVGSMILElement::isSMILElement(node))| with |if (node &&
> > isSVGSMILElement(*node))|.
> 
> Though I also considered it, I didn't want to modify existing code. However,
if
> you think it is better, I don't mind. Patch is coming.

Updated.

[tkent, 2013-12-04 03:50:01]

https://codereview.chromium.org/103393002/diff/40001/Source/core/svg/animatio...
File Source/core/svg/animation/SMILTimeContainer.cpp (right):

https://codereview.chromium.org/103393002/diff/40001/Source/core/svg/animatio...
Source/core/svg/animation/SMILTimeContainer.cpp:232: if (element &&
isSVGSMILElement(*element))
|element &&| is redundant because the |for| condition ensures element is not
NULL.

https://codereview.chromium.org/103393002/diff/40001/Source/core/svg/animatio...
File Source/core/svg/animation/SVGSMILElement.cpp (right):

https://codereview.chromium.org/103393002/diff/40001/Source/core/svg/animatio...
Source/core/svg/animation/SVGSMILElement.cpp:581: if (condition.m_syncbase &&
!isSVGSMILElement(*condition.m_syncbase)) {
This is wrong.
!isSMILElement(foo) is equivalent to !foo || !isSVGSMILElement(*foo).

[gyuyoung-inactive, 2013-12-04 03:53:34]

On 2013/12/04 03:50:01, tkent wrote:
>
https://codereview.chromium.org/103393002/diff/40001/Source/core/svg/animatio...
> File Source/core/svg/animation/SMILTimeContainer.cpp (right):
> 
>
https://codereview.chromium.org/103393002/diff/40001/Source/core/svg/animatio...
> Source/core/svg/animation/SMILTimeContainer.cpp:232: if (element &&
> isSVGSMILElement(*element))
> |element &&| is redundant because the |for| condition ensures element is not
> NULL.
> 
>
https://codereview.chromium.org/103393002/diff/40001/Source/core/svg/animatio...
> File Source/core/svg/animation/SVGSMILElement.cpp (right):
> 
>
https://codereview.chromium.org/103393002/diff/40001/Source/core/svg/animatio...
> Source/core/svg/animation/SVGSMILElement.cpp:581: if (condition.m_syncbase &&
> !isSVGSMILElement(*condition.m_syncbase)) {
> This is wrong.
> !isSMILElement(foo) is equivalent to !foo || !isSVGSMILElement(*foo).

Ah, yes, right. Sorry about mistakes. Thank you for pointing them out.

[tkent, 2013-12-04 05:33:15]

https://codereview.chromium.org/103393002/diff/60001/Source/core/svg/animatio...
File Source/core/svg/animation/SVGSMILElement.cpp (right):

https://codereview.chromium.org/103393002/diff/60001/Source/core/svg/animatio...
Source/core/svg/animation/SVGSMILElement.cpp:581: if (!condition.m_syncbase &&
!isSVGSMILElement(*condition.m_syncbase)) {
This is still incorrect.  Imagine that m_syncbase is NULL.

[gyuyoung-inactive, 2013-12-04 05:54:44]

https://codereview.chromium.org/103393002/diff/60001/Source/core/svg/animatio...
File Source/core/svg/animation/SVGSMILElement.cpp (right):

https://codereview.chromium.org/103393002/diff/60001/Source/core/svg/animatio...
Source/core/svg/animation/SVGSMILElement.cpp:581: if (!condition.m_syncbase &&
!isSVGSMILElement(*condition.m_syncbase)) {
On 2013/12/04 05:33:16, tkent wrote:
> This is still incorrect.  Imagine that m_syncbase is NULL.

I fixed this as below,

if (!condition.m_syncbase || !isSVGSMILElement(*condition.m_syncbase)) {

[tkent, 2013-12-04 05:56:03]

lgtm

[commit-bot: I haz the power, 2013-12-04 05:56:16]

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

[commit-bot: I haz the power, 2013-12-04 10:16:07]

Change committed as 163161