Bug 150664 - Use modern for-loops in WebCore/dom.
Summary: Use modern for-loops in WebCore/dom.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hunseop Jeong
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-10-28 23:56 PDT by Hunseop Jeong
Modified: 2015-10-30 10:05 PDT (History)
6 users (show)

See Also:


Attachments
Patch (43.61 KB, patch)
2015-10-29 01:28 PDT, Hunseop Jeong
no flags Details | Formatted Diff | Diff
Patch (43.17 KB, patch)
2015-10-29 02:39 PDT, Hunseop Jeong
no flags Details | Formatted Diff | Diff
Patch (43.14 KB, patch)
2015-10-29 21:35 PDT, Hunseop Jeong
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hunseop Jeong 2015-10-28 23:56:08 PDT
Refactor the for-loops using the ranged-for loops and auto keyword in WebCore/dom.
Comment 1 Hunseop Jeong 2015-10-29 01:28:30 PDT
Created attachment 264306 [details]
Patch
Comment 2 Hunseop Jeong 2015-10-29 02:39:25 PDT
Created attachment 264307 [details]
Patch
Comment 3 Darin Adler 2015-10-29 11:07:14 PDT
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.
Comment 4 Hunseop Jeong 2015-10-29 21:35:33 PDT
Created attachment 264378 [details]
Patch
Comment 5 Hunseop Jeong 2015-10-29 21:42:09 PDT
(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.
Comment 6 WebKit Commit Bot 2015-10-30 10:04:57 PDT
Comment on attachment 264378 [details]
Patch

Clearing flags on attachment: 264378

Committed r191792: <http://trac.webkit.org/changeset/191792>
Comment 7 WebKit Commit Bot 2015-10-30 10:05:01 PDT
All reviewed patches have been landed.  Closing bug.