Use modern for-loops in WebCore/rendering - 2 of 2.
Created attachment 254788 [details] Patch
Comment on attachment 254788 [details] Patch Attachment 254788 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5854356633026560 New failing tests: svg/W3C-SVG-1.1/struct-group-03-t.svg svg/transforms/svg-css-transforms.xhtml svg/W3C-SVG-1.1/painting-stroke-04-t.svg transforms/svg-vs-css.xhtml svg/custom/invalid-dasharray.svg
Created attachment 254790 [details] Archive of layout-test-results from ews103 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-mavericks Platform: Mac OS X 10.9.5
Comment on attachment 254788 [details] Patch Attachment 254788 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6255143653736448 New failing tests: svg/W3C-SVG-1.1/struct-group-03-t.svg svg/transforms/svg-css-transforms.xhtml svg/W3C-SVG-1.1/painting-stroke-04-t.svg transforms/svg-vs-css.xhtml svg/custom/invalid-dasharray.svg
Created attachment 254791 [details] Archive of layout-test-results from ews104 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Created attachment 254800 [details] Patch
Created attachment 254806 [details] Patch
Comment on attachment 254806 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=254806&action=review Simon Fraser did ask us to slow down on these. > Source/WebCore/rendering/RenderObject.cpp:1085 > + rects.append(SelectionRect(quad.enclosingBoundingBox(), isHorizontalWritingMode(), view().pageNumberForBlockProgressionOffset(quad.enclosingBoundingBox().x()))); It’s inefficient to call enclosingBoundingBox twice here on each quad. This should be rewritten to use a local variable. > Source/WebCore/rendering/RenderQuote.cpp:70 > + for (auto quoteCharacter : distinctQuoteCharacters) { This is *wrong*. It needs to be auto& or this function won’t work. > Source/WebCore/rendering/style/KeyframeList.cpp:103 > + for (auto& currKeyframe : m_keyframes) { Should just be keyframe, not currKeyframe. > Source/WebCore/rendering/svg/RenderSVGPath.cpp:117 > + FloatPoint radiusVector(point.x() - location.x(), point.y() - location.y()); This should be: FloatPoint radiusVector = point - location; > Source/WebCore/rendering/svg/RenderSVGText.cpp:188 > SVGTextLayoutAttributes* attributes = 0; nullptr
Created attachment 254924 [details] Patch
(In reply to comment #8) > Comment on attachment 254806 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=254806&action=review > > Simon Fraser did ask us to slow down on these. Then, Do I wait to upload these patches? > > > Source/WebCore/rendering/RenderObject.cpp:1085 > > + rects.append(SelectionRect(quad.enclosingBoundingBox(), isHorizontalWritingMode(), view().pageNumberForBlockProgressionOffset(quad.enclosingBoundingBox().x()))); > > It’s inefficient to call enclosingBoundingBox twice here on each quad. This > should be rewritten to use a local variable. > I made the local variable to remove the inefficient call. > > Source/WebCore/rendering/RenderQuote.cpp:70 > > + for (auto quoteCharacter : distinctQuoteCharacters) { > > This is *wrong*. It needs to be auto& or this function won’t work. > I Changed it to use the auto&. > > Source/WebCore/rendering/style/KeyframeList.cpp:103 > > + for (auto& currKeyframe : m_keyframes) { > > Should just be keyframe, not currKeyframe. > This function already use the 'keyframe' for the parameter of function. So I used the 'currKeyframe' to avoid the confusion. > > Source/WebCore/rendering/svg/RenderSVGPath.cpp:117 > > + FloatPoint radiusVector(point.x() - location.x(), point.y() - location.y()); > > This should be: > > FloatPoint radiusVector = point - location; > 'point - location' return the FloatSize. So I changed it to "FloatPoint radiusVector(point - location);" > > Source/WebCore/rendering/svg/RenderSVGText.cpp:188 > > SVGTextLayoutAttributes* attributes = 0; > > nullptr Done.
Created attachment 254926 [details] Patch