Bug 145912 – Use modern for-loops in WebCore/rendering - 2

• Status: NEW
• Component: WebCore Misc.

[Hunseop Jeong]

Use modern for-loops in WebCore/rendering - 2 of 2.

[Hunseop Jeong]

Created attachment 254788 [details]
Patch

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Hunseop Jeong]

Created attachment 254800 [details]
Patch

[Hunseop Jeong]

Created attachment 254806 [details]
Patch

[Darin Adler]

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

[Hunseop Jeong]

Created attachment 254924 [details]
Patch

[Hunseop Jeong]

(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.

[Hunseop Jeong]

Created attachment 254926 [details]
Patch