Bug 136403 - Introduce CSS_SHAPES_BASIC_TYPE_CASTS, and use it
Summary: Introduce CSS_SHAPES_BASIC_TYPE_CASTS, and use it
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gyuyoung Kim
URL:
Keywords:
Depends on: 136454
Blocks: 136773
  Show dependency treegraph
 
Reported: 2014-08-29 21:38 PDT by Gyuyoung Kim
Modified: 2014-09-11 23:20 PDT (History)
7 users (show)

See Also:


Attachments
Patch (7.49 KB, patch)
2014-08-29 21:41 PDT, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 (639.50 KB, application/zip)
2014-08-30 07:01 PDT, Build Bot
no flags Details
Patch (7.44 KB, patch)
2014-08-30 09:24 PDT, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch (7.53 KB, patch)
2014-09-02 22:02 PDT, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch (7.57 KB, patch)
2014-09-03 00:21 PDT, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch for landing (7.60 KB, patch)
2014-09-04 01:19 PDT, Gyuyoung Kim
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gyuyoung Kim 2014-08-29 21:38:18 PDT
toCSSBasicFoo() will help to detect wrong type casting. So this patch generates it, and use it instead of static_cast<const CSSBasicFoo*>().
Comment 1 Gyuyoung Kim 2014-08-29 21:41:07 PDT
Created attachment 237406 [details]
Patch
Comment 2 Build Bot 2014-08-30 07:01:17 PDT
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
Comment 3 Build Bot 2014-08-30 07:01:20 PDT
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
Comment 4 Gyuyoung Kim 2014-08-30 09:24:30 PDT
Created attachment 237410 [details]
Patch
Comment 5 Gyuyoung Kim 2014-09-01 21:59:55 PDT
Darin, could you take a look ?
Comment 6 WebKit Commit Bot 2014-09-02 09:06:41 PDT
Comment on attachment 237410 [details]
Patch

Clearing flags on attachment: 237410

Committed r173175: <http://trac.webkit.org/changeset/173175>
Comment 7 WebKit Commit Bot 2014-09-02 09:06:48 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Csaba Osztrogonác 2014-09-02 09:27:27 PDT
(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?
Comment 9 WebKit Commit Bot 2014-09-02 09:43:15 PDT
Re-opened since this is blocked by bug 136454
Comment 10 Jessie Berlin 2014-09-02 09:57:25 PDT
(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=]
Comment 11 Daniel Bates 2014-09-02 10:00:43 PDT
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().
Comment 12 Daniel Bates 2014-09-02 10:03:33 PDT
(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
Comment 13 Gyuyoung Kim 2014-09-02 22:02:27 PDT
Created attachment 237546 [details]
Patch
Comment 14 Gyuyoung Kim 2014-09-02 22:03:21 PDT
(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.
Comment 15 Daniel Bates 2014-09-02 22:46:10 PDT
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()?
Comment 16 Gyuyoung Kim 2014-09-02 23:07:33 PDT
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.
Comment 17 Gyuyoung Kim 2014-09-03 00:21:45 PDT
Created attachment 237550 [details]
Patch
Comment 18 Daniel Bates 2014-09-03 06:19:54 PDT
(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).
Comment 19 Daniel Bates 2014-09-03 06:25:22 PDT
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
Comment 20 Gyuyoung Kim 2014-09-04 01:19:56 PDT
Created attachment 237613 [details]
Patch for landing
Comment 21 WebKit Commit Bot 2014-09-04 09:13:45 PDT
Comment on attachment 237613 [details]
Patch for landing

Clearing flags on attachment: 237613

Committed r173259: <http://trac.webkit.org/changeset/173259>
Comment 22 WebKit Commit Bot 2014-09-04 09:13:51 PDT
All reviewed patches have been landed.  Closing bug.