Summary: | Introduce CSS_SHAPES_BASIC_TYPE_CASTS, and use it | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gyuyoung Kim <gyuyoung.kim> | ||||||||||||||
Component: | WebCore Misc. | Assignee: | Gyuyoung Kim <gyuyoung.kim> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | buildbot, commit-queue, darin, dbates, jberlin, ossy, rniwa | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Bug Depends on: | 136454 | ||||||||||||||||
Bug Blocks: | 136773 | ||||||||||||||||
Attachments: |
|
Description
Gyuyoung Kim
2014-08-29 21:38:18 PDT
Created attachment 237406 [details]
Patch
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 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
Created attachment 237410 [details]
Patch
Darin, could you take a look ? Comment on attachment 237410 [details] Patch Clearing flags on attachment: 237410 Committed r173175: <http://trac.webkit.org/changeset/173175> All reviewed patches have been landed. Closing bug. (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? Re-opened since this is blocked by bug 136454 (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 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(). (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 Created attachment 237546 [details]
Patch
(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 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 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. Created attachment 237550 [details]
Patch
(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 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 Created attachment 237613 [details]
Patch for landing
Comment on attachment 237613 [details] Patch for landing Clearing flags on attachment: 237613 Committed r173259: <http://trac.webkit.org/changeset/173259> All reviewed patches have been landed. Closing bug. |