As r155429 introduced ELEMENT_TYPE_CASTS, CSS_VALUE_TYPE_CASTS can be used by css value type casting. This casting macro will help to detect bad-cast bugs. This patch adds the following methods, - CSSFooValue* toCSSFooValue(CSSValue*) - const CSSFooValue* toCSSFooValue(const CSSValue*) - CSSFooValue& toCSSFooValue(CSSValue&) - const CSSFooValue& toCSSFooValue(const CSSValue&)
Created attachment 211845 [details] WIP
Created attachment 211865 [details] Patch
CC'ing kling, could you take a look this patch for CSSValue type casting ?
Comment on attachment 211865 [details] Patch Attachment 211865 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/1872257
Comment on attachment 211865 [details] Patch Attachment 211865 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/1855248
Comment on attachment 211865 [details] Patch Attachment 211865 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1893171
Comment on attachment 211865 [details] Patch Attachment 211865 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/1859213
Created attachment 211887 [details] Patch
Comment on attachment 211887 [details] Patch Attachment 211887 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/1850365 New failing tests: css3/filters/custom/custom-filter-matN.html css3/filters/custom/custom-filter-u-tile-size.html css3/filters/custom/custom-filter-property-computed-style.html css3/filters/custom/custom-filter-blend-modes.html css3/filters/custom/custom-filter-array.html css3/filters/custom/custom-filter-u-texture-size.html css3/filters/custom/custom-filter-shader-reuse.html css3/filters/custom/custom-filter-property-parsing.html css3/filters/custom/composited/custom-filter-blend-modes.html css3/filters/custom/custom-filter-css-keyword-as-parameter-name.html css3/filters/custom/custom-filter-clamp-css-mix-color.html css3/filters/custom/custom-filter-composite-fractional-source-alpha.html css3/filters/custom/custom-filter-u-mesh-size.html css3/filters/custom/custom-filter-reload.html css3/filters/custom/custom-filter-clamp-css-mix-color-negative.html css3/filters/custom/custom-filter-clamp-css-color-matrix.html css3/filters/custom/custom-filter-unavailable-varying.html css3/filters/custom/custom-filter-change-blend-mode.html css3/filters/custom/custom-filter-color-matrix.html css3/filters/custom/custom-filter-color.html css3/filters/custom/invalid-custom-filter-uniform-types.html css3/filters/custom/custom-filter-mix-bindings.html css3/filters/custom/custom-filter-u-mesh-box.html css3/filters/custom/missing-custom-filter-shader.html css3/filters/custom/invalid-custom-filter-shader.html css3/filters/custom/custom-filter-clamp-css-color-matrix-negative.html
Created attachment 211894 [details] Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-10 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Why do we need both CSS_VALUE_TYPE_CASTS and WEBKIT_CSS_VALUE_TYPE_CASTS? Can't we just have one: CSS_VALUE_TYPE_CASTS(CSSPrimitiveValue) CSS_VALUE_TYPE_CASTS(WebKitCSSTransformValue)
Comment on attachment 211887 [details] Patch Attachment 211887 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/1893278 New failing tests: css3/filters/custom/custom-filter-matN.html css3/filters/custom/custom-filter-u-tile-size.html css3/filters/custom/custom-filter-property-computed-style.html css3/filters/custom/custom-filter-blend-modes.html css3/filters/custom/custom-filter-array.html css3/filters/custom/custom-filter-u-texture-size.html css3/filters/custom/custom-filter-shader-reuse.html css3/filters/custom/custom-filter-property-parsing.html css3/filters/custom/composited/custom-filter-blend-modes.html css3/filters/custom/custom-filter-css-keyword-as-parameter-name.html css3/filters/custom/custom-filter-clamp-css-mix-color.html css3/filters/custom/custom-filter-composite-fractional-source-alpha.html css3/filters/custom/custom-filter-u-mesh-size.html css3/filters/custom/custom-filter-reload.html css3/filters/custom/custom-filter-clamp-css-mix-color-negative.html css3/filters/custom/custom-filter-clamp-css-color-matrix.html css3/filters/custom/custom-filter-unavailable-varying.html css3/filters/custom/custom-filter-change-blend-mode.html css3/filters/custom/custom-filter-color-matrix.html css3/filters/custom/custom-filter-color.html css3/filters/custom/invalid-custom-filter-uniform-types.html css3/filters/custom/custom-filter-mix-bindings.html css3/filters/custom/custom-filter-u-mesh-box.html css3/filters/custom/missing-custom-filter-shader.html css3/filters/custom/invalid-custom-filter-shader.html css3/filters/custom/custom-filter-clamp-css-color-matrix-negative.html
Created attachment 211901 [details] Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-16 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 211887 [details] Patch Attachment 211887 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1850383 New failing tests: css3/filters/custom/custom-filter-matN.html css3/filters/custom/custom-filter-u-tile-size.html css3/filters/custom/custom-filter-property-computed-style.html css3/filters/custom/custom-filter-blend-modes.html css3/filters/custom/custom-filter-array.html css3/filters/custom/custom-filter-u-texture-size.html css3/filters/custom/custom-filter-shader-reuse.html css3/filters/custom/custom-filter-property-parsing.html css3/filters/custom/composited/custom-filter-blend-modes.html css3/filters/custom/custom-filter-css-keyword-as-parameter-name.html css3/filters/custom/custom-filter-clamp-css-mix-color.html css3/filters/custom/custom-filter-composite-fractional-source-alpha.html css3/filters/custom/custom-filter-u-mesh-size.html css3/filters/custom/custom-filter-reload.html css3/filters/custom/custom-filter-clamp-css-mix-color-negative.html css3/filters/custom/custom-filter-clamp-css-color-matrix.html css3/filters/custom/custom-filter-unavailable-varying.html css3/filters/custom/custom-filter-change-blend-mode.html css3/filters/custom/custom-filter-color-matrix.html css3/filters/custom/custom-filter-color.html css3/filters/custom/invalid-custom-filter-uniform-types.html css3/filters/custom/custom-filter-mix-bindings.html css3/filters/custom/custom-filter-u-mesh-box.html css3/filters/custom/missing-custom-filter-shader.html css3/filters/custom/invalid-custom-filter-shader.html css3/filters/custom/custom-filter-clamp-css-color-matrix-negative.html
Created attachment 211905 [details] Archive of layout-test-results from webkit-ews-02 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-02 Port: mac-mountainlion Platform: Mac OS X 10.8.5
(In reply to comment #11) > Why do we need both CSS_VALUE_TYPE_CASTS and WEBKIT_CSS_VALUE_TYPE_CASTS? > Can't we just have one: > > CSS_VALUE_TYPE_CASTS(CSSPrimitiveValue) > CSS_VALUE_TYPE_CASTS(WebKitCSSTransformValue) Because, naming style of is*Value() is different from isWebKitCSS*Value() bool isFilterImageValue() const { return m_classType == FilterImageClass; } bool isWebKitCSSFilterValue() const { return m_classType == WebKitCSSFilterClass; } ASSERT_WITH_SECURITY_IMPLICATION(!value || value->is##ValueTypeName()); \ ASSERT_WITH_SECURITY_IMPLICATION(!value || value->isWebKitCSS##ValueTypeName()); \ I couldn't find a way to support both cases in CSS_VALUE_TYPE_CASTS macro. That's why I added two macros as below,Anyway, I will try to find a way to use one macro further. Any idea ?
Created attachment 211978 [details] Patch
Comment on attachment 211978 [details] Patch Attachment 211978 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/1888184
Comment on attachment 211978 [details] Patch Attachment 211978 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/1928112
Comment on attachment 211978 [details] Patch Attachment 211978 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/1851298
Comment on attachment 211978 [details] Patch Attachment 211978 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1822482
Created attachment 211994 [details] Patch
Comment on attachment 211994 [details] Patch I only add CSS_VALUE_TYPE_CASTS at the moment.
Created attachment 212037 [details] Patch
Comment on attachment 212037 [details] Patch LGTM
Comment on attachment 212037 [details] Patch Clearing flags on attachment: 212037 Committed r156253: <http://trac.webkit.org/changeset/156253>
All reviewed patches have been landed. Closing bug.
Re-opened since this is blocked by bug 121778
I feel this patch is too huge. It looks this patch needs to be separated. Patch will be prepared.
Rolled out in <http://trac.webkit.org/changeset/156260>, because this caused a bunch of assertion failures, and Gyuyoung agreed that this was the right course of action.
Created attachment 212329 [details] Patch
(In reply to comment #31) > Created an attachment (id=212329) [details] > Patch There is no layout test regression on latest patch. I will prepare following patches after landing this master patch.
Comment on attachment 212329 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=212329&action=review > Source/WebCore/ChangeLog:16 > + This patch support CSSImageSetValue and CSSREflectValue first. Other CSS*Values > + will use this macro step by step. Typo: CSSReflectValue > Source/WebCore/css/CSSValue.h:277 > +inline const CSS##ValueTypeName* toCSS##ValueTypeName(const CSSValue* value) \ > +{ \ > + ASSERT_WITH_SECURITY_IMPLICATION(!value || value->is##ValueTypeName()); \ > + return static_cast<const CSS##ValueTypeName*>(value); \ > +} \ > +inline CSS##ValueTypeName* toCSS##ValueTypeName(CSSValue* value) \ > +{ \ > + ASSERT_WITH_SECURITY_IMPLICATION(!value || value->is##ValueTypeName()); \ > + return static_cast<CSS##ValueTypeName*>(value); \ > +} \ Why no versions that take references? Maybe we should have versions that take RefPtr too?
Created attachment 212408 [details] Patch for landing
(In reply to comment #33) > > Source/WebCore/css/CSSValue.h:277 > > +inline const CSS##ValueTypeName* toCSS##ValueTypeName(const CSSValue* value) \ > > +{ \ > > + ASSERT_WITH_SECURITY_IMPLICATION(!value || value->is##ValueTypeName()); \ > > + return static_cast<const CSS##ValueTypeName*>(value); \ > > +} \ > > +inline CSS##ValueTypeName* toCSS##ValueTypeName(CSSValue* value) \ > > +{ \ > > + ASSERT_WITH_SECURITY_IMPLICATION(!value || value->is##ValueTypeName()); \ > > + return static_cast<CSS##ValueTypeName*>(value); \ > > +} \ > > Why no versions that take references? Maybe we should have versions that take RefPtr too? Element.h also has similar inline functions. I think it would be good if we support version both together on new bug. I will file a bug.
Comment on attachment 212408 [details] Patch for landing Rejecting attachment 212408 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 212408, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit /Volumes/Data/EWS/WebKit/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://webkit-queues.appspot.com/results/1822648
Created attachment 212412 [details] Patch for landing
Comment on attachment 212412 [details] Patch for landing Clearing flags on attachment: 212412 Committed r156313: <http://trac.webkit.org/changeset/156313>