Bug 136403 – Introduce CSS_SHAPES_BASIC_TYPE_CASTS, and use it

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Gyuyoung Kim]

toCSSBasicFoo() will help to detect wrong type casting. So this patch generates it, and use it instead of static_cast<const CSSBasicFoo*>().

[Gyuyoung Kim]

Created attachment 237406 [details]
Patch

[Build Bot]

Comment on attachment 237406 [details]
Patch

Attachment 237406 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5585054516379648

New failing tests:
compositing/checkerboard.html
compositing/bounds-in-flipped-writing-mode.html
animations/3d/transform-origin-vs-functions.html
http/tests/appcache/abort-cache-onchecking.html
http/tests/appcache/abort-cache-onchecking-resource-404.html
animations/animation-controller-drt-api.html
canvas/philip/tests/2d.clearRect+fillRect.basic.html
accessibility/alt-tag-on-image-with-nonimage-role.html

[Build Bot]

Created attachment 237408 [details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-15  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Gyuyoung Kim]

Created attachment 237410 [details]
Patch

[Gyuyoung Kim]

Darin, could you take a look ?

[WebKit Commit Bot]

Comment on attachment 237410 [details]
Patch

Clearing flags on attachment: 237410

Committed r173175: <http://trac.webkit.org/changeset/173175>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

(In reply to comment #6)
> (From update of attachment 237410 [details])
> Clearing flags on attachment: 237410
> 
> Committed r173175: <http://trac.webkit.org/changeset/173175>

It broke the debug builds. Could you check it?

[WebKit Commit Bot]

Re-opened since this is blocked by bug 136454

[Jessie Berlin]

(In reply to comment #9)
> Re-opened since this is blocked by bug 136454

Here is what the errors looked like:

http://build.webkit.org/builders/Apple%20Mavericks%20Debug%20%28Build%29/builds/10344/steps/compile-webkit/logs/errors

(view as text)
/Volumes/Data/slave/mavericks-debug/build/Source/WebCore/css/CSSBasicShapes.h:133:42: error: use of undeclared identifier 'CSSBasicShapeInsetType'; did you mean 'toCSSBasicShapeInset'?
/Volumes/Data/slave/mavericks-debug/build/Source/WebCore/css/CSSBasicShapes.h:133:1: error: comparison between pointer and integer ('int' and 'WebCore::CSSBasicShapeInset *(*)(WebCore::CSSBasicShape *)')
/Volumes/Data/slave/mavericks-debug/build/Source/WebCore/css/CSSBasicShapes.h:133:42: error: use of undeclared identifier 'CSSBasicShapeInsetType'; did you mean 'toCSSBasicShapeInset'?
/Volumes/Data/slave/mavericks-debug/build/Source/WebCore/css/CSSBasicShapes.h:133:1: error: comparison between pointer and integer ('int' and 'WebCore::CSSBasicShapeInset *(*)(WebCore::CSSBasicShape *)')
/Volumes/Data/slave/mavericks-debug/build/Source/WebCore/css/CSSBasicShapes.h:133:42: error: use of undeclared identifier 'CSSBasicShapeInsetType'; did you mean 'toCSSBasicShapeInset'?
/Volumes/Data/slave/mavericks-debug/build/Source/WebCore/css/CSSBasicShapes.h:133:1: error: comparison between pointer and integer ('int' and 'WebCore::CSSBasicShapeInset *(*)(WebCore::CSSBasicShape *)')
/Volumes/Data/slave/mavericks-debug/build/Source/WebCore/css/CSSBasicShapes.h:133:42: error: use of undeclared identifier 'CSSBasicShapeInsetType'; did you mean 'toCSSBasicShapeInset'?
/Volumes/Data/slave/mavericks-debug/build/Source/WebCore/css/CSSBasicShapes.h:133:1: error: comparison between pointer and integer ('int' and 'WebCore::CSSBasicShapeInset *(*)(WebCore::CSSBasicShape *)')
/Volumes/Data/slave/mavericks-debug/build/Source/WebCore/css/CSSBasicShapes.h:159:43: error: use of undeclared identifier 'CSSBasicShapeCircleType'; did you mean 'toCSSBasicShapeCircle'?
/Volumes/Data/slave/mavericks-debug/build/Source/WebCore/css/CSSBasicShapes.h:159:1: error: comparison between pointer and integer ('int' and 'WebCore::CSSBasicShapeCircle *(*)(WebCore::CSSBasicShape *)')
/Volumes/Data/slave/mavericks-debug/build/Source/WebCore/css/CSSBasicShapes.h:159:43: error: use of undeclared identifier 'CSSBasicShapeCircleType'; did you mean 'toCSSBasicShapeCircle'?
/Volumes/Data/slave/mavericks-debug/build/Source/WebCore/css/CSSBasicShapes.h:159:1: error: comparison between pointer and integer ('int' and 'WebCore::CSSBasicShapeCircle *(*)(WebCore::CSSBasicShape *)')
/Volumes/Data/slave/mavericks-debug/build/Source/WebCore/css/CSSBasicShapes.h:159:43: error: use of undeclared identifier 'CSSBasicShapeCircleType'; did you mean 'toCSSBasicShapeCircle'?
/Volumes/Data/slave/mavericks-debug/build/Source/WebCore/css/CSSBasicShapes.h:159:1: error: comparison between pointer and integer ('int' and 'WebCore::CSSBasicShapeCircle *(*)(WebCore::CSSBasicShape *)')
/Volumes/Data/slave/mavericks-debug/build/Source/WebCore/css/CSSBasicShapes.h:159:43: error: use of undeclared identifier 'CSSBasicShapeCircleType'; did you mean 'toCSSBasicShapeCircle'?
/Volumes/Data/slave/mavericks-debug/build/Source/WebCore/css/CSSBasicShapes.h:159:1: error: comparison between pointer and integer ('int' and 'WebCore::CSSBasicShapeCircle *(*)(WebCore::CSSBasicShape *)')
/Volumes/Data/slave/mavericks-debug/build/Source/WebCore/css/CSSBasicShapes.h:188:44: error: use of undeclared identifier 'CSSBasicShapeEllipseType'; did you mean 'toCSSBasicShapeEllipse'?
/Volumes/Data/slave/mavericks-debug/build/Source/WebCore/css/CSSBasicShapes.h:188:1: error: comparison between pointer and integer ('int' and 'WebCore::CSSBasicShapeEllipse *(*)(WebCore::CSSBasicShape *)')
/Volumes/Data/slave/mavericks-debug/build/Source/WebCore/css/CSSBasicShapes.h:188:44: error: use of undeclared identifier 'CSSBasicShapeEllipseType'; did you mean 'toCSSBasicShapeEllipse'?
fatal error: too many errors emitted, stopping now [-ferror-limit=]

[Daniel Bates]

Comment on attachment 237410 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=237410&action=review

> Source/WebCore/css/CSSBasicShapes.h:65
> +#define CSS_BASIC_TYPE_CASTS(ToValueTypeName, predicate) \

Did you mean to call this function CSS_BASIC_SHAPES_TYPE_CASTS? Otherwise, what do you mean by "CSS_BASIC_TYPE"? I mean, this cast seems specific to the CSSBascShapes class hierarchy.

> Source/WebCore/css/CSSBasicShapes.h:133
> +CSS_BASIC_TYPE_CASTS(CSSBasicShapeInset, CSSBasicShapeInsetType)

This led to compile errors as seen in comment #10. Either the second argument needs to be CSSBasicShape::CSSBasicShapeInsetType or, even better, we should namespace qualify the predicate in the definition of CSS_BASIC_TYPE_CASTS().

[Daniel Bates]

(In reply to comment #11)
> This led to compile errors as seen in comment #10. Either the second argument needs to be CSSBasicShape::CSSBasicShapeInsetType or, even better, we should namespace qualify the predicate in the definition of CSS_BASIC_TYPE_CASTS().

*CSSBasicShape-qualify

[Gyuyoung Kim]

Created attachment 237546 [details]
Patch

[Gyuyoung Kim]

(In reply to comment #11)
> (From update of attachment 237410 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=237410&action=review
> 
> > Source/WebCore/css/CSSBasicShapes.h:65
> > +#define CSS_BASIC_TYPE_CASTS(ToValueTypeName, predicate) \
> 
> Did you mean to call this function CSS_BASIC_SHAPES_TYPE_CASTS? Otherwise, what do you mean by "CSS_BASIC_TYPE"? I mean, this cast seems specific to the CSSBascShapes class hierarchy.
> 
> > Source/WebCore/css/CSSBasicShapes.h:133
> > +CSS_BASIC_TYPE_CASTS(CSSBasicShapeInset, CSSBasicShapeInsetType)
> 
> This led to compile errors as seen in comment #10. Either the second argument needs to be CSSBasicShape::CSSBasicShapeInsetType or, even better, we should namespace qualify the predicate in the definition of CSS_BASIC_TYPE_CASTS().

I'm so sorry about my stupid mistake. I fix it in latest patch. Thanks.

[Daniel Bates]

Comment on attachment 237546 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=237546&action=review

> Source/WebCore/css/CSSBasicShapes.h:65
> +#define CSS_BASIC_TYPE_CASTS(ToValueTypeName, predicate) \

Would it make sense it call this macro function CSS_SHAPES_BASIC_TYPE_CASTS()?

[Gyuyoung Kim]

Comment on attachment 237546 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=237546&action=review

>> Source/WebCore/css/CSSBasicShapes.h:65
>> +#define CSS_BASIC_TYPE_CASTS(ToValueTypeName, predicate) \
> 
> Would it make sense it call this macro function CSS_SHAPES_BASIC_TYPE_CASTS()?

Sure, it looks more make sense. Patch is going to be updated.

[Gyuyoung Kim]

Created attachment 237550 [details]
Patch

[Daniel Bates]

(In reply to comment #15)
> (From update of attachment 237546 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=237546&action=review
> 
> > Source/WebCore/css/CSSBasicShapes.h:65
> > +#define CSS_BASIC_TYPE_CASTS(ToValueTypeName, predicate) \
> 
> Would it make sense it call this macro function CSS_SHAPES_BASIC_TYPE_CASTS()?

err, I meant to write:

Would it make sense to call this macro function CSS_BASIC_SHAPE_TYPE_CASTS()?

(So that the name of the macro corresponds to the name of the class CSSBasicShape).

[Daniel Bates]

Comment on attachment 237550 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=237550&action=review

> Source/WebCore/ChangeLog:8
> +        toCSSBasicFoo() will help to detect wrong type casting. So this patch generates it, and use it

toCSSBasicFoo => toCSSBasicShapeFoo

> Source/WebCore/ChangeLog:9
> +        instead of static_cast<const CSSBasicFoo*>().

CSSBasicFoo => CSSBasicShapeFoo

[Gyuyoung Kim]

Created attachment 237613 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 237613 [details]
Patch for landing

Clearing flags on attachment: 237613

Committed r173259: <http://trac.webkit.org/changeset/173259>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.