Introduce CSS_VALUE_TYPE_CASTS macro to cast CSSValue

Introduce CSS_VALUE_TYPE_CASTS macro to cast CSSValue

As SVG and HTML have type cast function, CSSValue also can have it. This cl introduces
CSS_VALUE_TYPE_CASTS macro to make type cast function easily. This type cast macros
will help to detect bad-cast bugs as well as improve a code readibility.

This patch adds the following methods,

 - CSSFooValue* toCSSFooValue(CSSValue*)
 - const CSSFooValue* toCSSFooValue(const CSSValue*)

This cl applies the macro to CSSReflectValue and CSSImageSetValue first.

Besides this cl adds these functions to detect unnecessary type casting.
  - void toFooValue(const CSS*Value*)
  - void toFooValue(const CSS*Value&)

This will let us know where does unnecessary type casting happen in compilation time.

BUG=N/A

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

[gyuyoung-inactive, 2013-09-23 06:14:20]

Add tkent to reviewer.

[tkent, 2013-09-24 00:08:34]

We need to update Tools/Scripts/webkitpy/style/checkers/cpp.py so that it shows
an appropriate error message.  See
http://src.chromium.org/viewvc/blink?view=revision&revision=158059

https://codereview.chromium.org/24020008/diff/1/Source/core/css/CSSValue.h
File Source/core/css/CSSValue.h (right):

https://codereview.chromium.org/24020008/diff/1/Source/core/css/CSSValue.h#ne...
Source/core/css/CSSValue.h:240: #define CSS_VALUE_TYPE_CASTS(ValueTypeName) \
I think DEFINE_CSS_VALUE_TYPE_CASTS is better.  It's consistent with
DEFINE_ATTRIBUTE_EVENT_LISTENER.

https://codereview.chromium.org/24020008/diff/1/Source/core/css/CSSValue.h#ne...
Source/core/css/CSSValue.h:252: void toCSS##ValueTypeName(const
CSS##ValueTypeName&);
Please omit the last ':'.  I'd like to use this like
"CSS_VALUE_TYPE_CASTS(Foo);"

[gyuyoung-inactive, 2013-09-24 03:38:53]

On 2013/09/24 00:08:34, tkent wrote:
> We need to update Tools/Scripts/webkitpy/style/checkers/cpp.py so that it
shows
> an appropriate error message.  See
> http://src.chromium.org/viewvc/blink?view=revision&revision=158059

Do you think the style checker should be modified first ? If possible, I would
like to modify it after landing this cl. How do you think ?

> https://codereview.chromium.org/24020008/diff/1/Source/core/css/CSSValue.h
> File Source/core/css/CSSValue.h (right):
> 
>
https://codereview.chromium.org/24020008/diff/1/Source/core/css/CSSValue.h#ne...
> Source/core/css/CSSValue.h:240: #define CSS_VALUE_TYPE_CASTS(ValueTypeName) \
> I think DEFINE_CSS_VALUE_TYPE_CASTS is better.  It's consistent with
> DEFINE_ATTRIBUTE_EVENT_LISTENER.

Done.

>
https://codereview.chromium.org/24020008/diff/1/Source/core/css/CSSValue.h#ne...
> Source/core/css/CSSValue.h:252: void toCSS##ValueTypeName(const
> CSS##ValueTypeName&);
> Please omit the last ':'.  I'd like to use this like
> "CSS_VALUE_TYPE_CASTS(Foo);"

Yes, you can use DEFINE_CSS_VALUE_TYPE_CASTS(Foo); already. I use ';' at the end
of line.

Below functions is to detect unnecessary type casting.

void toCSS##ValueTypeName(const CSS##ValueTypeName*); \
void toCSS##ValueTypeName(const CSS##ValueTypeName&);

For instance, we don't need to do type casting in below case.

const CSSReflectValue* refValue;
CSSReflectValue newValue = ValuetoCSSReflectValue(refValue);

[tkent, 2013-09-24 04:35:26]

Please use 'reply' feature of the code review tool if possible.

On 2013/09/24 03:38:53, gyuyoung wrote:
> On 2013/09/24 00:08:34, tkent wrote:
> > We need to update Tools/Scripts/webkitpy/style/checkers/cpp.py so that it
> shows
> > an appropriate error message.  See
> > http://src.chromium.org/viewvc/blink?view=revision&revision=158059
> 
> Do you think the style checker should be modified first ? If possible, I would
> like to modify it after landing this cl. How do you think ?

It's acceptable to update the style checker in another CL.

> > Source/core/css/CSSValue.h:252: void toCSS##ValueTypeName(const
> > CSS##ValueTypeName&);
> > Please omit the last ':'.  I'd like to use this like
> > "CSS_VALUE_TYPE_CASTS(Foo);"
> 
> Yes, you can use DEFINE_CSS_VALUE_TYPE_CASTS(Foo); already. I use ';' at the
end
> of line.
> 
> Below functions is to detect unnecessary type casting.
> 
> void toCSS##ValueTypeName(const CSS##ValueTypeName*); \
> void toCSS##ValueTypeName(const CSS##ValueTypeName&);

So,

DEFINE_CSS_VALUE_TYPE_CASTS(Foo);

generates two trailing semi-colons.

void toCSSFoo(const CSSFoo&);;

It's not cool though no one don't see it. :-)

[tkent, 2013-09-24 04:36:07]

> It's not cool though no one don't see it. :-)

no one see it :-)

[gyuyoung-inactive, 2013-09-24 05:08:25]

On 2013/09/24 04:35:26, tkent wrote:
> Please use 'reply' feature of the code review tool if possible.

I'm not good at using the codereview yet. So, I often forget to use it. But, now
I'm getting more comfortable to use the codereview system. :)

> It's acceptable to update the style checker in another CL.

I will file a new cl for it.

> > > Source/core/css/CSSValue.h:252: void toCSS##ValueTypeName(const
> > > CSS##ValueTypeName&);
> > > Please omit the last ':'.  I'd like to use this like
> > > "CSS_VALUE_TYPE_CASTS(Foo);"
> > 
> > Yes, you can use DEFINE_CSS_VALUE_TYPE_CASTS(Foo); already. I use ';' at the
> end
> > of line.
> > 
> > Below functions is to detect unnecessary type casting.
> > 
> > void toCSS##ValueTypeName(const CSS##ValueTypeName*); \
> > void toCSS##ValueTypeName(const CSS##ValueTypeName&);
> 
> So,
> 
> DEFINE_CSS_VALUE_TYPE_CASTS(Foo);
> 
> generates two trailing semi-colons.
> 
> void toCSSFoo(const CSSFoo&);;
> 
> It's not cool though no one don't see it. :-)

Ok, let me check it again.

[gyuyoung-inactive, 2013-09-24 05:30:58]

Patch is updated.

[tkent, 2013-09-24 05:41:50]

https://codereview.chromium.org/24020008/diff/11001/Source/core/css/CSSValue.h
File Source/core/css/CSSValue.h (right):

https://codereview.chromium.org/24020008/diff/11001/Source/core/css/CSSValue....
Source/core/css/CSSValue.h:251: void toCSS##ValueTypeName(const
CSS##ValueTypeName*) { } \
Why did you add {} ?
It generates ugly code for DEFINE_CSS_VALUE_TYPE_CASTS(Foo);.
void toCSSFoo(const CSSFoo&) { };

What I requested was to remove the trailing semicolon.

void toCSS##ValueTypeName(const CSS##ValueTypeName*); \
void toCSS##ValueTypeName(const CSS##ValueTypeName&)

[gyuyoung-inactive, 2013-09-24 05:51:53]

On 2013/09/24 05:41:50, tkent wrote:
> https://codereview.chromium.org/24020008/diff/11001/Source/core/css/CSSValue.h
> File Source/core/css/CSSValue.h (right):
> 
>
https://codereview.chromium.org/24020008/diff/11001/Source/core/css/CSSValue....
> Source/core/css/CSSValue.h:251: void toCSS##ValueTypeName(const
> CSS##ValueTypeName*) { } \
> Why did you add {} ?
> It generates ugly code for DEFINE_CSS_VALUE_TYPE_CASTS(Foo);.
> void toCSSFoo(const CSSFoo&) { };
> 
> What I requested was to remove the trailing semicolon.
> 
> void toCSS##ValueTypeName(const CSS##ValueTypeName*); \
> void toCSS##ValueTypeName(const CSS##ValueTypeName&)

Oops, sorry. I misunderstood what you point out. I thought I should remove all
semi-colon from them. Fixed.

[tkent, 2013-09-24 05:53:40]

lgtm

[commit-bot: I haz the power, 2013-09-24 05:53:54]

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

[commit-bot: I haz the power, 2013-09-24 07:56:44]

Change committed as 158254

[gyuyoung-inactive, 2013-09-24 12:05:09]

On 2013/09/24 00:08:34, tkent wrote:
> We need to update Tools/Scripts/webkitpy/style/checkers/cpp.py so that it
shows
> an appropriate error message.  See
> http://src.chromium.org/viewvc/blink?view=revision&revision=158059

Kent, the style check patch was reverted by dpranke. It looks the patch needs to
be reviewed further. I will update it after re-landing it.
https://src.chromium.org/viewvc/blink?revision=158138&view=revision

[tkent, 2013-09-24 22:59:24]

On 2013/09/24 12:05:09, gyuyoung wrote:
> On 2013/09/24 00:08:34, tkent wrote:
> > We need to update Tools/Scripts/webkitpy/style/checkers/cpp.py so that it
> shows
> > an appropriate error message.  See
> > http://src.chromium.org/viewvc/blink?view=revision&revision=158059
> 
> Kent, the style check patch was reverted by dpranke. It looks the patch needs
to
> be reviewed further. I will update it after re-landing it.
> https://src.chromium.org/viewvc/blink?revision=158138&view=revision

Ok, we don't need further work for style checker.