83536 2012-04-09 18:52:25 -0700 Don't allow fallthrough for CSSPropertyBaselineShift in CSSComputedStyleDeclaration::getSVGPropertyCSSValue(). 2012-05-28 21:06:50 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 macpherson macpherson dbates jhawkins macpherson menard webkit.review.bot oldest_to_newest 598866 0 macpherson 2012-04-09 18:52:25 -0700 Don't allow fallthrough for CSSPropertyBaselineShift in CSSComputedStyleDeclaration::getSVGPropertyCSSValue(). 598871 1 136370 macpherson 2012-04-09 18:54:30 -0700 Created attachment 136370 Patch 598964 2 136370 dbates 2012-04-09 20:53:43 -0700 Comment on attachment 136370 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136370&action=review > Source/WebCore/ChangeLog:11 > + I don't think this return is reachable, but if it were, falling through to the next case would be > + the wrong thing to do, so this may catch a future bug one day. Although somewhat contrived, the presence of the "return 0;" statement would the catch the case where a person removed the enum values BS_BASELINE, BS_SUB, BS_SUPER, BS_LENGTH and instead declared integer variables of the same name. > Source/WebCore/css/SVGCSSComputedStyleDeclaration.cpp:177 > + return 0; Can we add ASSERT_NOT_REACHED() above this line? 598973 3 136370 dbates 2012-04-09 21:00:56 -0700 Comment on attachment 136370 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136370&action=review > Source/WebCore/ChangeLog:8 > + No new tests / code cleanup from coverity static analysis. Nit: Usually such a sentence comes after the description. > Source/WebCore/ChangeLog:10 > + I don't think this return is reachable, but if it were, falling through to the next case would be In order to understand this description you need to have read the patch. In particular, it is unclear what the phrase "this return" is referring to unless you read the patch. Maybe a sentence like: Add a return statement to the case CSSPropertyBaselineShift so that we don't fall through to the next case statement. 598978 4 136390 macpherson 2012-04-09 21:08:09 -0700 Created attachment 136390 Patch for landing 598995 5 macpherson 2012-04-09 21:50:27 -0700 Comments addressed. Thanks for the review. 599054 6 136390 webkit.review.bot 2012-04-09 23:25:39 -0700 Comment on attachment 136390 Patch for landing Rejecting attachment 136390 from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: ebKit/chromium/gpu --revision 131183 --non-interactive --force --accept theirs-conflict --ignore-externals returned non-zero exit status 1 in /mnt/git/webkit-commit-queue/Source/WebKit/chromium Error: 'depot_tools/gclient sync --force --reset --delete_unversioned_trees' failed 3 tries and returned 256 at Tools/Scripts/update-webkit-chromium line 107. Re-trying 'depot_tools/gclient sync --force --reset --delete_unversioned_trees' No such file or directory at Tools/Scripts/update-webkit line 118. Full output: http://queues.webkit.org/results/12369588 599472 7 136390 webkit.review.bot 2012-04-10 12:11:09 -0700 Comment on attachment 136390 Patch for landing Clearing flags on attachment: 136390 Committed r113748: <http://trac.webkit.org/changeset/113748> 599473 8 webkit.review.bot 2012-04-10 12:11:15 -0700 All reviewed patches have been landed. Closing bug. 635549 9 cmarcelo 2012-05-28 21:06:50 -0700 *** Bug 27902 has been marked as a duplicate of this bug. *** 136370 2012-04-09 18:54:30 -0700 2012-04-09 21:07:59 -0700 Patch bug-83536-20120410115428.patch text/plain 1782 macpherson U3VidmVyc2lvbiBSZXZpc2lvbjogMTEzNjQ3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZDQ3ZWY2MDQ5OWE0NmNi MmE0MGJjNzQ1OTUwMTY4NjcxMWRhMTBkNi4uNDJjMzQ0YWY4YzY5OTI4MDRkM2UyZTRjZTFlMGI2 ZGZhMTg5YmU1OSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDEyLTA0LTA5ICBMdWtl IE1hY3BoZXJzb24gIDxtYWNwaGVyc29uQGNocm9taXVtLm9yZz4KKworICAgICAgICBEb24ndCBh bGxvdyBmYWxsdGhyb3VnaCBmb3IgQ1NTUHJvcGVydHlCYXNlbGluZVNoaWZ0IGluIENTU0NvbXB1 dGVkU3R5bGVEZWNsYXJhdGlvbjo6Z2V0U1ZHUHJvcGVydHlDU1NWYWx1ZSgpLgorICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9ODM1MzYKKworICAgICAgICBS ZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBObyBuZXcgdGVzdHMgLyBjb2Rl IGNsZWFudXAgZnJvbSBjb3Zlcml0eSBzdGF0aWMgYW5hbHlzaXMuCisKKyAgICAgICAgSSBkb24n dCB0aGluayB0aGlzIHJldHVybiBpcyByZWFjaGFibGUsIGJ1dCBpZiBpdCB3ZXJlLCBmYWxsaW5n IHRocm91Z2ggdG8gdGhlIG5leHQgY2FzZSB3b3VsZCBiZQorICAgICAgICB0aGUgd3JvbmcgdGhp bmcgdG8gZG8sIHNvIHRoaXMgbWF5IGNhdGNoIGEgZnV0dXJlIGJ1ZyBvbmUgZGF5LgorCisgICAg ICAgICogY3NzL1NWR0NTU0NvbXB1dGVkU3R5bGVEZWNsYXJhdGlvbi5jcHA6CisgICAgICAgIChX ZWJDb3JlOjpDU1NDb21wdXRlZFN0eWxlRGVjbGFyYXRpb246OmdldFNWR1Byb3BlcnR5Q1NTVmFs dWUpOgorCiAyMDEyLTA0LTA5ICBBbmRyZWFzIEtsaW5nICA8a2xpbmdAd2Via2l0Lm9yZz4KIAog ICAgICAgICBNYWtlIENTU1ZhbHVlUG9vbCBzaGFyZSB2YWx1ZXMgZ2xvYmFsbHkuCmRpZmYgLS1n aXQgYS9Tb3VyY2UvV2ViQ29yZS9jc3MvU1ZHQ1NTQ29tcHV0ZWRTdHlsZURlY2xhcmF0aW9uLmNw cCBiL1NvdXJjZS9XZWJDb3JlL2Nzcy9TVkdDU1NDb21wdXRlZFN0eWxlRGVjbGFyYXRpb24uY3Bw CmluZGV4IDVlM2FhODgwNzcxNTY2ZWIwY2UwMzM4MTg3NmM1YmMwMDQ1Y2Q5NDQuLjQ5ZjA1Y2Nl MDQzM2IyMWVhYjcxM2E3ZTUzMDM5NWY0NWMxYzNlNjQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJD b3JlL2Nzcy9TVkdDU1NDb21wdXRlZFN0eWxlRGVjbGFyYXRpb24uY3BwCisrKyBiL1NvdXJjZS9X ZWJDb3JlL2Nzcy9TVkdDU1NDb21wdXRlZFN0eWxlRGVjbGFyYXRpb24uY3BwCkBAIC0xNzQsNiAr MTc0LDcgQEAgUGFzc1JlZlB0cjxDU1NWYWx1ZT4gQ1NTQ29tcHV0ZWRTdHlsZURlY2xhcmF0aW9u OjpnZXRTVkdQcm9wZXJ0eUNTU1ZhbHVlKENTU1Byb3AKICAgICAgICAgICAgICAgICBjYXNlIEJT X0xFTkdUSDoKICAgICAgICAgICAgICAgICAgICAgcmV0dXJuIFNWR0xlbmd0aDo6dG9DU1NQcmlt aXRpdmVWYWx1ZShzdmdTdHlsZS0+YmFzZWxpbmVTaGlmdFZhbHVlKCkpOwogICAgICAgICAgICAg fQorICAgICAgICAgICAgcmV0dXJuIDA7CiAgICAgICAgIH0KICAgICAgICAgY2FzZSBDU1NQcm9w ZXJ0eUdseXBoT3JpZW50YXRpb25Ib3Jpem9udGFsOgogICAgICAgICAgICAgcmV0dXJuIGdseXBo T3JpZW50YXRpb25Ub0NTU1ByaW1pdGl2ZVZhbHVlKHN2Z1N0eWxlLT5nbHlwaE9yaWVudGF0aW9u SG9yaXpvbnRhbCgpKTsK 136390 2012-04-09 21:08:09 -0700 2012-04-10 12:11:09 -0700 Patch for landing bug-83536-20120410140807.patch text/plain 1764 macpherson U3VidmVyc2lvbiBSZXZpc2lvbjogMTEzNjcyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNmJiYWY4MDk2ZjljOWE4 N2E5NjhkYzRhM2ZlYWQxNDc5OWZhMGJmZi4uY2IwMmIwZjU4MGRjNGVkNzViNzVkYThmNDYxNTEz M2U4MGI2NzJkOSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDEyLTA0LTA5ICBMdWtl IE1hY3BoZXJzb24gIDxtYWNwaGVyc29uQGNocm9taXVtLm9yZz4KKworICAgICAgICBEb24ndCBh bGxvdyBmYWxsdGhyb3VnaCBmb3IgQ1NTUHJvcGVydHlCYXNlbGluZVNoaWZ0IGluIENTU0NvbXB1 dGVkU3R5bGVEZWNsYXJhdGlvbjo6Z2V0U1ZHUHJvcGVydHlDU1NWYWx1ZSgpLgorICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9ODM1MzYKKworICAgICAgICBS ZXZpZXdlZCBieSBEYW5pZWwgQmF0ZXMuCisKKyAgICAgICAgQWRkIGEgcmV0dXJuIHN0YXRlbWVu dCB0byB0aGUgY2FzZSBDU1NQcm9wZXJ0eUJhc2VsaW5lU2hpZnQgc28gdGhhdCB3ZSBkb24ndCBm YWxsIHRocm91Z2ggdG8gdGhlIG5leHQgY2FzZSBzdGF0ZW1lbnQuCisKKyAgICAgICAgTm8gbmV3 IHRlc3RzIC8gY29kZSBjbGVhbnVwIGZyb20gY292ZXJpdHkgc3RhdGljIGFuYWx5c2lzLgorCisg ICAgICAgICogY3NzL1NWR0NTU0NvbXB1dGVkU3R5bGVEZWNsYXJhdGlvbi5jcHA6CisgICAgICAg IChXZWJDb3JlOjpDU1NDb21wdXRlZFN0eWxlRGVjbGFyYXRpb246OmdldFNWR1Byb3BlcnR5Q1NT VmFsdWUpOgorCiAyMDEyLTA0LTA5ICBTaGVyaWZmIEJvdCAgPHdlYmtpdC5yZXZpZXcuYm90QGdt YWlsLmNvbT4KIAogICAgICAgICBVbnJldmlld2VkLCByb2xsaW5nIG91dCByMTEzNjU2LgpkaWZm IC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvY3NzL1NWR0NTU0NvbXB1dGVkU3R5bGVEZWNsYXJhdGlv bi5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9jc3MvU1ZHQ1NTQ29tcHV0ZWRTdHlsZURlY2xhcmF0aW9u LmNwcAppbmRleCA1ZTNhYTg4MDc3MTU2NmViMGNlMDMzODE4NzZjNWJjMDA0NWNkOTQ0Li4xY2Q4 ZmEwYWEwMzM5ZDNlYmJjNjc0MDBiMWQ0ODA0MjU4ZGVhM2ZmIDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViQ29yZS9jc3MvU1ZHQ1NTQ29tcHV0ZWRTdHlsZURlY2xhcmF0aW9uLmNwcAorKysgYi9Tb3Vy Y2UvV2ViQ29yZS9jc3MvU1ZHQ1NTQ29tcHV0ZWRTdHlsZURlY2xhcmF0aW9uLmNwcApAQCAtMTc0 LDYgKzE3NCw4IEBAIFBhc3NSZWZQdHI8Q1NTVmFsdWU+IENTU0NvbXB1dGVkU3R5bGVEZWNsYXJh dGlvbjo6Z2V0U1ZHUHJvcGVydHlDU1NWYWx1ZShDU1NQcm9wCiAgICAgICAgICAgICAgICAgY2Fz ZSBCU19MRU5HVEg6CiAgICAgICAgICAgICAgICAgICAgIHJldHVybiBTVkdMZW5ndGg6OnRvQ1NT UHJpbWl0aXZlVmFsdWUoc3ZnU3R5bGUtPmJhc2VsaW5lU2hpZnRWYWx1ZSgpKTsKICAgICAgICAg ICAgIH0KKyAgICAgICAgICAgIEFTU0VSVF9OT1RfUkVBQ0hFRCgpOworICAgICAgICAgICAgcmV0 dXJuIDA7CiAgICAgICAgIH0KICAgICAgICAgY2FzZSBDU1NQcm9wZXJ0eUdseXBoT3JpZW50YXRp b25Ib3Jpem9udGFsOgogICAgICAgICAgICAgcmV0dXJuIGdseXBoT3JpZW50YXRpb25Ub0NTU1By aW1pdGl2ZVZhbHVlKHN2Z1N0eWxlLT5nbHlwaE9yaWVudGF0aW9uSG9yaXpvbnRhbCgpKTsK