Bug 126250 – Convert some of WebCore/dom over to range-for loops

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Sam Weinig]

Convert some of WebCore/dom over to range-for loops

[Sam Weinig]

Created attachment 220042 [details]
Patch

[Andreas Kling]

Comment on attachment 220042 [details]
Patch

r=me

I love auto* instead of just auto for pointers. We should make that a style rule.

[Sam Weinig]

Committed r161096: <http://trac.webkit.org/changeset/161096>

[Csaba Osztrogonác]

(In reply to comment #3)
> Committed r161096: <http://trac.webkit.org/changeset/161096>
FYI: It made all tests crash. Please fix it.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 126256

[Alexey Proskuryakov]

> FYI: It made all tests crash. Please fix it.

I don't think that it's OK to leave tests crashing for any period of time, rolled out in <https://trac.webkit.org/r161097>.

This crashes in both debug and release. Example debug crash (assertion failure): http://build.webkit.org/results/Apple%20Mavericks%20Debug%20WK2%20(Tests)/r161096%20(1233)/canvas/philip/tests/2d.composite.uncovered.fill.copy-crash-log.txt

Full results:

http://build.webkit.org/results/Apple%20Mavericks%20Release%20WK2%20(Tests)/r161096%20(1785)/results.html

http://build.webkit.org/results/Apple%20Mavericks%20Debug%20WK2%20(Tests)/r161096%20(1233)/results.html

[Csaba Osztrogonác]

Are you going to update and relande this patch in the near future?
I checked some random part of the patch, they are still valid.

[Csaba Osztrogonác]

Created attachment 255112 [details]
Patch

rebased to top of trunk, patch for EWS

[Darin Adler]

Comment on attachment 255112 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=255112&action=review

> Source/WebCore/dom/ContainerNode.cpp:458
> +        treeScope().adoptIfNeeded(&child.get());

We have been using child.ptr() instead of &child.get(). Not sure it’s better, but it is one less & character.

> Source/WebCore/dom/ContainerNode.cpp:466
> +                appendChildToContainer(&child.get(), *this);

Ditto.

> Source/WebCore/dom/ContainerNode.cpp:711
> +        treeScope().adoptIfNeeded(&child.get());

Ditto.

> Source/WebCore/dom/ContainerNode.cpp:716
> +            appendChildToContainer(&child.get(), *this);

Ditto.

> Source/WebCore/dom/MutationObserverInterestGroup.cpp:71
> +    for (auto& observerOptionsPair : m_observers) {

Could use an even shorter name here, but I guess this is OK.

> Source/WebCore/dom/Node.cpp:234
> +    for (auto stringSizePair : perTagCount)

This should be auto&; we don’t want to copy all these strings and churn their reference counts for no good reason, even in this logging code.

[Csaba Osztrogonác]

Created attachment 255629 [details]
patch for landing

[WebKit Commit Bot]

Comment on attachment 255629 [details]
patch for landing

Clearing flags on attachment: 255629

Committed r185994: <http://trac.webkit.org/changeset/185994>