Bug 150664 – Use modern for-loops in WebCore/dom.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Hunseop Jeong]

Refactor the for-loops using the ranged-for loops and auto keyword in WebCore/dom.

[Hunseop Jeong]

Created attachment 264306 [details]
Patch

[Hunseop Jeong]

Created attachment 264307 [details]
Patch

[Darin Adler]

Comment on attachment 264307 [details]
Patch

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

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

I believe the preferred style is child.ptr() rather than &child.get().

> Source/WebCore/dom/DocumentMarkerController.cpp:619
> +    for (auto& marker : m_markers) {

Not good to use "marker" as the name both for this outer loop and the inner loop.

> Source/WebCore/dom/DocumentMarkerController.cpp:623
> -        MarkerList* list = i->value.get();
> +        MarkerList* list = marker.value.get();

Not sure why we need this local variable.

> Source/WebCore/dom/DocumentMarkerController.cpp:800
> +        for (auto& documentMarker : *marker.value.get())

We don’t need .get() here.

> Source/WebCore/dom/EventListenerMap.cpp:71
> +            for (auto& eventListener : *entry.second.get()) {

No need for .get() here.

> Source/WebCore/dom/SelectorQuery.cpp:448
> +                SelectorCompiler::QuerySelectorSimpleSelectorChecker selectorChecker = SelectorCompiler::querySelectorSimpleSelectorCheckerFunction(compiledSelectorChecker, selector.compilationStatus);

We should use auto here; this line is so long.

> Source/WebCore/dom/SelectorQuery.cpp:452
> +                SelectorCompiler::QuerySelectorSelectorCheckerWithCheckingContext selectorChecker = SelectorCompiler::querySelectorSelectorCheckerFunctionWithCheckingContext(compiledSelectorChecker, selector.compilationStatus);

We should use auto here; this line is so long.

[Hunseop Jeong]

Created attachment 264378 [details]
Patch

[Hunseop Jeong]

(In reply to comment #3)
> Comment on attachment 264307 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=264307&action=review
> 
> > Source/WebCore/dom/ContainerNode.cpp:285
> > +        treeScope().adoptIfNeeded(&child.get());
> 
> I believe the preferred style is child.ptr() rather than &child.get().
Done.
> 
> > Source/WebCore/dom/DocumentMarkerController.cpp:619
> > +    for (auto& marker : m_markers) {
> 
> Not good to use "marker" as the name both for this outer loop and the inner
> loop.
Oops,, I changed the "marker" to documentMarker in inner loop.
> 
> > Source/WebCore/dom/DocumentMarkerController.cpp:623
> > -        MarkerList* list = i->value.get();
> > +        MarkerList* list = marker.value.get();
> 
> Not sure why we need this local variable.
> 
> > Source/WebCore/dom/DocumentMarkerController.cpp:800
> > +        for (auto& documentMarker : *marker.value.get())
> 
> We don’t need .get() here.
Removed the .get()
> 
> > Source/WebCore/dom/EventListenerMap.cpp:71
> > +            for (auto& eventListener : *entry.second.get()) {
> 
> No need for .get() here.
Ditto.
> 
> > Source/WebCore/dom/SelectorQuery.cpp:448
> > +                SelectorCompiler::QuerySelectorSimpleSelectorChecker selectorChecker = SelectorCompiler::querySelectorSimpleSelectorCheckerFunction(compiledSelectorChecker, selector.compilationStatus);
> 
> We should use auto here; this line is so long.
Done.
> 
> > Source/WebCore/dom/SelectorQuery.cpp:452
> > +                SelectorCompiler::QuerySelectorSelectorCheckerWithCheckingContext selectorChecker = SelectorCompiler::querySelectorSelectorCheckerFunctionWithCheckingContext(compiledSelectorChecker, selector.compilationStatus);
> 
> We should use auto here; this line is so long.
Ditto.

Thanks for the review.

[WebKit Commit Bot]

Comment on attachment 264378 [details]
Patch

Clearing flags on attachment: 264378

Committed r191792: <http://trac.webkit.org/changeset/191792>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.