RESOLVED FIXED 164696
[SVG] Moving more special casing of SVG out of the bindings - SVGNumber/SVGPoint/SVGRect/SVGLength/SVGTransform/SVGMatrix
https://bugs.webkit.org/show_bug.cgi?id=164696
Summary [SVG] Moving more special casing of SVG out of the bindings - SVGNumber/SVGPo...
Sam Weinig
Reported 2016-11-12 21:32:04 PST
[SVG] Moving more special casing of SVG out of the bindings - SVGPoint/SVGRect/SVGLength/SVGTransform/SVGMatrix
Attachments
Patch (356.16 KB, patch)
2016-11-12 21:37 PST, Sam Weinig
no flags
Patch (354.26 KB, patch)
2016-11-12 21:59 PST, Sam Weinig
no flags
Patch (375.38 KB, patch)
2016-11-13 10:28 PST, Sam Weinig
no flags
Patch (375.40 KB, patch)
2016-11-13 10:36 PST, Sam Weinig
no flags
Patch (329.66 KB, patch)
2016-11-13 10:39 PST, Sam Weinig
no flags
Patch (335.04 KB, patch)
2016-11-13 10:41 PST, Sam Weinig
no flags
Patch (352.55 KB, patch)
2016-11-13 10:51 PST, Sam Weinig
no flags
Patch (357.14 KB, patch)
2016-11-13 11:21 PST, Sam Weinig
no flags
Patch (358.42 KB, patch)
2016-11-13 12:06 PST, Sam Weinig
no flags
Patch (357.56 KB, patch)
2016-11-13 13:04 PST, Sam Weinig
no flags
Patch (357.79 KB, patch)
2016-11-13 13:14 PST, Sam Weinig
no flags
Archive of layout-test-results from ews103 for mac-yosemite (1.28 MB, application/zip)
2016-11-13 14:14 PST, Build Bot
no flags
Archive of layout-test-results from ews107 for mac-yosemite-wk2 (1.20 MB, application/zip)
2016-11-13 14:19 PST, Build Bot
no flags
Archive of layout-test-results from ews117 for mac-yosemite (1.63 MB, application/zip)
2016-11-13 14:29 PST, Build Bot
no flags
Patch (358.37 KB, patch)
2016-11-13 14:50 PST, Sam Weinig
no flags
Patch (358.83 KB, patch)
2016-11-13 16:51 PST, Sam Weinig
darin: review+
Sam Weinig
Comment 1 2016-11-12 21:37:38 PST
Sam Weinig
Comment 2 2016-11-12 21:59:18 PST
WebKit Commit Bot
Comment 3 2016-11-12 22:02:25 PST
Attachment 294651 [details] did not pass style-queue: ERROR: Source/WebCore/svg/SVGPolyElement.cpp:25: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/SVGRect.h:22: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:156: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:257: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:325: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:432: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:33: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:34: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:35: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:36: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:37: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:38: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:39: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:77: The parameter name "a" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:39: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:44: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:46: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:50: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:52: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:53: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 21 in 117 files If any of these errors are false positives, please file a bug against check-webkit-style.
Sam Weinig
Comment 4 2016-11-13 10:28:52 PST
Sam Weinig
Comment 5 2016-11-13 10:36:11 PST
Sam Weinig
Comment 6 2016-11-13 10:39:33 PST
Sam Weinig
Comment 7 2016-11-13 10:41:01 PST
Sam Weinig
Comment 8 2016-11-13 10:51:04 PST
WebKit Commit Bot
Comment 9 2016-11-13 10:53:20 PST
Attachment 294669 [details] did not pass style-queue: ERROR: Source/WebCore/svg/SVGPolyElement.cpp:25: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/SVGRect.h:22: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:156: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:257: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:325: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:432: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:33: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:34: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:35: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:36: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:37: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:38: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:39: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:77: The parameter name "a" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:39: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:44: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:46: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:50: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:52: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:53: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGMatrix.h:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 22 in 118 files If any of these errors are false positives, please file a bug against check-webkit-style.
Sam Weinig
Comment 10 2016-11-13 11:21:12 PST
WebKit Commit Bot
Comment 11 2016-11-13 11:50:01 PST
Attachment 294671 [details] did not pass style-queue: ERROR: Source/WebCore/svg/SVGPolyElement.cpp:25: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/SVGRect.h:22: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:156: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:257: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:325: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:432: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:33: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:34: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:35: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:36: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:37: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:38: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:39: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:77: The parameter name "a" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:39: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:44: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:46: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:50: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:52: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:53: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGMatrix.h:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 22 in 118 files If any of these errors are false positives, please file a bug against check-webkit-style.
Sam Weinig
Comment 12 2016-11-13 12:06:06 PST
Sam Weinig
Comment 13 2016-11-13 13:04:41 PST
WebKit Commit Bot
Comment 14 2016-11-13 13:07:17 PST
Attachment 294674 [details] did not pass style-queue: ERROR: Source/WebCore/svg/SVGPolyElement.cpp:25: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/SVGRect.h:22: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:156: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:257: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:325: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:432: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:33: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:34: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:35: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:36: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:37: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:38: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:39: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:77: The parameter name "a" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:39: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:44: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:46: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:50: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:52: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:53: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGMatrix.h:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/ChangeLog:367: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] ERROR: Source/WebCore/ChangeLog:369: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] Total errors found: 23 in 118 files If any of these errors are false positives, please file a bug against check-webkit-style.
Sam Weinig
Comment 15 2016-11-13 13:14:22 PST
WebKit Commit Bot
Comment 16 2016-11-13 13:16:32 PST
Attachment 294675 [details] did not pass style-queue: ERROR: Source/WebCore/svg/SVGPolyElement.cpp:25: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/SVGRect.h:22: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:156: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:257: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:325: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:432: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:33: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:34: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:35: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:36: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:37: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:38: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:39: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:77: The parameter name "a" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:39: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:44: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:46: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:50: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:52: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:53: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGMatrix.h:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/ChangeLog:367: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] ERROR: Source/WebCore/ChangeLog:369: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] Total errors found: 23 in 118 files If any of these errors are false positives, please file a bug against check-webkit-style.
Build Bot
Comment 17 2016-11-13 14:14:35 PST
Comment on attachment 294675 [details] Patch Attachment 294675 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2510482 Number of test failures exceeded the failure limit.
Build Bot
Comment 18 2016-11-13 14:14:38 PST
Created attachment 294676 [details] Archive of layout-test-results from ews103 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-yosemite Platform: Mac OS X 10.10.5
Build Bot
Comment 19 2016-11-13 14:19:32 PST
Comment on attachment 294675 [details] Patch Attachment 294675 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/2510491 Number of test failures exceeded the failure limit.
Build Bot
Comment 20 2016-11-13 14:19:35 PST
Created attachment 294677 [details] Archive of layout-test-results from ews107 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Build Bot
Comment 21 2016-11-13 14:29:16 PST
Comment on attachment 294675 [details] Patch Attachment 294675 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2510498 Number of test failures exceeded the failure limit.
Build Bot
Comment 22 2016-11-13 14:29:19 PST
Created attachment 294678 [details] Archive of layout-test-results from ews117 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-yosemite Platform: Mac OS X 10.10.5
Sam Weinig
Comment 23 2016-11-13 14:50:35 PST
WebKit Commit Bot
Comment 24 2016-11-13 14:52:50 PST
Attachment 294679 [details] did not pass style-queue: ERROR: Source/WebCore/svg/SVGPolyElement.cpp:25: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/SVGRect.h:22: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:156: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:257: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:325: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:432: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:33: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:34: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:35: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:36: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:37: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:38: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:39: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:77: The parameter name "a" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:39: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:44: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:46: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:50: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:52: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:53: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGMatrix.h:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/ChangeLog:367: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] ERROR: Source/WebCore/ChangeLog:369: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] Total errors found: 23 in 119 files If any of these errors are false positives, please file a bug against check-webkit-style.
Sam Weinig
Comment 25 2016-11-13 16:51:38 PST
WebKit Commit Bot
Comment 26 2016-11-13 16:53:56 PST
Attachment 294682 [details] did not pass style-queue: ERROR: Source/WebCore/svg/SVGPolyElement.cpp:25: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/SVGRect.h:22: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:156: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:257: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:325: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:432: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:33: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:34: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:35: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:36: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:37: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:38: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:39: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:77: The parameter name "a" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:39: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:44: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:46: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:50: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:52: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:53: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGMatrix.h:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/ChangeLog:367: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] ERROR: Source/WebCore/ChangeLog:369: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] Total errors found: 23 in 120 files If any of these errors are false positives, please file a bug against check-webkit-style.
Sam Weinig
Comment 27 2016-11-13 18:45:20 PST
Ready for review.
Darin Adler
Comment 28 2016-11-13 23:12:15 PST
Comment on attachment 294682 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=294682&action=review > Source/WebCore/css/SVGCSSComputedStyleDeclaration.cpp:89 > RefPtr<CSSValueList> list = CSSValueList::createCommaSeparated(); I prefer auto here. That will give us Ref rather than RefPtr. > Source/WebCore/css/SVGCSSComputedStyleDeclaration.cpp:90 > + for (const auto& length : dashes) I suggest just auto&; the const will happen without us explicitly asking for it. > Source/WebCore/html/canvas/DOMPath.h:55 > + void addPath(const DOMPath* path, SVGMatrix& matrix) { addPath(path, matrix.propertyReference()); } Should this definitely be SVGMatrix& and not const SVGMatrix&? Also strange that the first argument of these three functions is a pointer, not a reference. Finally, does this really need to be inside #if ENABLE(CANVAS_PATH)? > Source/WebCore/rendering/style/RenderStyle.h:1755 > + void setStrokeDashArray(Vector<SVGLengthValue> array) { accessSVGStyle().setStrokeDashArray(array); } Should take const Vector& or Vector&&, definitely not Vector, since that means it will get copied twice! > Source/WebCore/rendering/style/SVGRenderStyle.h:81 > + static Vector<SVGLengthValue> initialStrokeDashArray() { return { }; } Extra stray space here. > Source/WebCore/rendering/style/SVGRenderStyle.h:218 > + void setStrokeDashArray(const Vector<SVGLengthValue>& obj) "obj"; hmmpf, not really better than "x" or "a". Especially since this is not an "object" in any interesting sense. Used over and over again below. > Source/WebCore/rendering/svg/SVGRenderTreeAsText.cpp:194 > + const Vector<SVGLengthValue>& dashes = svgStyle.strokeDashArray(); auto& or const auto&? > Source/WebCore/rendering/svg/SVGRenderTreeAsText.cpp:197 > + for (const auto& length : dashes) auto& should do, no need to emphasize const > Source/WebCore/rendering/svg/SVGRenderTreeAsText.cpp:198 > + dashArray.append((length).value(lengthContext)); no need for parentheses around length > Source/WebCore/svg/LinearGradientAttributes.h:46 > + SVGLengthValue x1() const { return m_x1; } > + SVGLengthValue y1() const { return m_y1; } > + SVGLengthValue x2() const { return m_x2; } > + SVGLengthValue y2() const { return m_y2; } > + > + void setX1(const SVGLengthValue& value) { m_x1 = value; m_x1Set = true; } > + void setY1(const SVGLengthValue& value) { m_y1 = value; m_y1Set = true; } > + void setX2(const SVGLengthValue& value) { m_x2 = value; m_x2Set = true; } > + void setY2(const SVGLengthValue& value) { m_y2 = value; m_y2Set = true; } If an SVGLengthValue is efficient enough to be used as a return value then it’s efficient enough to be used as an argument. Either both should be by value or both should be const&. > Source/WebCore/svg/PatternAttributes.h:56 > + SVGLengthValue x() const { return m_x; } > + SVGLengthValue y() const { return m_y; } > + SVGLengthValue width() const { return m_width; } > + SVGLengthValue height() const { return m_height; } Ditto, and same below. I won’t keep commenting on it. > Source/WebCore/svg/SVGLengthValue.cpp:39 > +static inline unsigned int storeUnit(SVGLengthMode mode, SVGLengthType type) File uses a lot of "unsigned int" instead of "unsigned". > Source/WebCore/svg/SVGLengthValue.cpp:47 > + unsigned int mode = unit >> 4; > + return static_cast<SVGLengthMode>(mode); No need for a local variable for this. > Source/WebCore/svg/SVGLengthValue.cpp:54 > + unsigned int mode = unit >> 4; > + unsigned int type = unit ^ (mode << 4); > + return static_cast<SVGLengthType>(type); This is a strange way to mask off high bits, shifting twice and using exclusive or. I would instead write: return static_cast<SVGLengthType>(unit & ((1 << 4) - 1)); > Source/WebCore/svg/SVGLengthValue.cpp:123 > + : m_valueInSpecifiedUnits(0) Seems like this could be initialized in the class definition instead. > Source/WebCore/svg/SVGLengthValue.cpp:276 > + SVGLengthType svgType; I would call this just "type". > Source/WebCore/svg/SVGLengthValue.cpp:315 > + case CSSPrimitiveValue::CSS_UNKNOWN: > + default: > + svgType = LengthTypeUnknown; > + break; > + }; > + > + if (svgType == LengthTypeUnknown) > + return { }; Seems like we should just return from inside the switch instead. > Source/WebCore/svg/SVGMatrixValue.h:2 > + * Copyright (C) Research In Motion Limited 2010. All rights reserved. Looks like you rewrote almost this entire file, but left a different copyright and license on it. > Source/WebCore/svg/SVGMatrixValue.h:34 > + SVGMatrixValue() { } > + SVGMatrixValue(const AffineTransform& other) > + : AffineTransform(other) > + { > + } = default for these two? > Source/WebCore/svg/SVGSVGElement.cpp:255 > + SVGLengthValue length = SVGLengthValue::construct(LengthModeWidth, value, parseError, ForbidNegativeLengths); auto would be nice here > Source/WebCore/svg/SVGSVGElement.cpp:259 > + length = SVGLengthValue(LengthModeWidth, ASCIILiteral("100%")); maybe use initializer list syntax? length = { LengthModeWidth, String { ASCIILiteral { "100%" } } }; Or maybe not. > Source/WebCore/svg/SVGSVGElement.cpp:263 > + SVGLengthValue length = SVGLengthValue::construct(LengthModeHeight, value, parseError, ForbidNegativeLengths); Ditto. > Source/WebCore/svg/SVGSVGElement.cpp:267 > + length = SVGLengthValue(LengthModeHeight, ASCIILiteral("100%")); Ditto. > Source/WebCore/svg/SVGSVGElement.h:95 > + Ref<NodeList> getIntersectionList(SVGRect&, SVGElement* referenceElement); > + Ref<NodeList> getEnclosureList(SVGRect&, SVGElement* referenceElement); > + static bool checkIntersection(const SVGElement*, SVGRect&); > + static bool checkEnclosure(const SVGElement*, SVGRect&); Can some of these be const SVGRect& instead? > Source/WebCore/svg/SVGSVGElement.h:105 > + static Ref<SVGTransform> createSVGTransformFromMatrix(SVGMatrix&); Can this be const SVGMatrix& like before? > Source/WebCore/svg/SVGTransformValue.cpp:36 > + : m_type(SVG_TRANSFORM_UNKNOWN) > + , m_angle(0) Could we initialize these in the class definition? > Source/WebCore/svg/SVGTransformValue.cpp:45 > + m_matrix = AffineTransform(0, 0, 0, 0, 0, 0); Is there a nicer way to write this? > Source/WebCore/svg/SVGTransformValue.cpp:210 > + double angleInRad = deg2rad(m_angle); > + double cosAngle = cos(angleInRad); > + double sinAngle = sin(angleInRad); Should use std:: versions because they are properly overloaded, although I suppose here we want to use double. > Source/WebCore/svg/SVGTransformValue.cpp:212 > + float cx = narrowPrecisionToFloat(cosAngle != 1 ? (m_matrix.e() * (1 - cosAngle) - m_matrix.f() * sinAngle) / (1 - cosAngle) / 2 : 0); > + float cy = narrowPrecisionToFloat(cosAngle != 1 ? (m_matrix.e() * sinAngle / (1 - cosAngle) + m_matrix.f()) / 2 : 0); Why narrow to float? > Source/WebCore/svg/SVGTransformValue.h:67 > + // Internal use only (animation system) Not sure this comment is all that great. > Source/WebCore/svg/SVGTransformValue.h:77 > + friend bool operator==(const SVGTransformValue& a, const SVGTransformValue& b); No need for argument names. > Source/WebCore/svg/properties/SVGTransformListPropertyTearOff.h:62 > + RefPtr<SVGTransform> wrapper = m_values->consolidate(); auto?
Sam Weinig
Comment 29 2016-11-14 13:21:50 PST
Note You need to log in before you can comment on or make changes to this bug.