Use C++11 range-based loop in core/fetch

Use C++11 range-based loop in core/fetch

Improves code readabilty

BUG=none
TEST=no layout test failures

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=183923

[riju_, 2014-10-17 10:21:48]

rijubrata.bhaumik@intel.com changed reviewers:
+ mkwst@chromium.org

[Mike West, 2014-10-17 10:38:49]

A few comments, thanks.

https://codereview.chromium.org/645513003/diff/1/Source/core/fetch/MemoryCach...
File Source/core/fetch/MemoryCache.cpp (right):

https://codereview.chromium.org/645513003/diff/1/Source/core/fetch/MemoryCach...
Source/core/fetch/MemoryCache.cpp:65: for (const auto& resource :
memoryCache()->m_liveResources) {
Nit: Drop the {} for one-liners.

https://codereview.chromium.org/645513003/diff/1/Source/core/fetch/Resource.cpp
File Source/core/fetch/Resource.cpp (right):

https://codereview.chromium.org/645513003/diff/1/Source/core/fetch/Resource.c...
Source/core/fetch/Resource.cpp:705: for (const auto& client : m_clients) {
Nit: How about "clientHash" rather than "client". Otherwise the fact that you're
actually iterating through a HashCountedSet might be surprising.

https://codereview.chromium.org/645513003/diff/1/Source/core/fetch/Resource.c...
Source/core/fetch/Resource.cpp:920: for (const auto resource : resources)
Why did you choose `const auto&` above, and `const auto` here?

https://codereview.chromium.org/645513003/diff/1/Source/core/fetch/ResourceLo...
File Source/core/fetch/ResourceLoadPriorityOptimizer.cpp (right):

https://codereview.chromium.org/645513003/diff/1/Source/core/fetch/ResourceLo...
Source/core/fetch/ResourceLoadPriorityOptimizer.cpp:87: if
(!renderObject->updateImageLoadingPriorities()) {
Nit: Drop the {} for one-liners.

[riju_, 2014-10-17 18:36:48]

https://codereview.chromium.org/645513003/diff/1/Source/core/fetch/MemoryCach...
File Source/core/fetch/MemoryCache.cpp (right):

https://codereview.chromium.org/645513003/diff/1/Source/core/fetch/MemoryCach...
Source/core/fetch/MemoryCache.cpp:65: for (const auto& resource :
memoryCache()->m_liveResources) {
On 2014/10/17 10:38:49, Mike West wrote:
> Nit: Drop the {} for one-liners.

Done.

https://codereview.chromium.org/645513003/diff/1/Source/core/fetch/Resource.cpp
File Source/core/fetch/Resource.cpp (right):

https://codereview.chromium.org/645513003/diff/1/Source/core/fetch/Resource.c...
Source/core/fetch/Resource.cpp:705: for (const auto& client : m_clients) {
On 2014/10/17 10:38:49, Mike West wrote:
> Nit: How about "clientHash" rather than "client". Otherwise the fact that
you're
> actually iterating through a HashCountedSet might be surprising.

Done.

https://codereview.chromium.org/645513003/diff/1/Source/core/fetch/Resource.c...
Source/core/fetch/Resource.cpp:920: for (const auto resource : resources)
On 2014/10/17 10:38:49, Mike West wrote:
> Why did you choose `const auto&` above, and `const auto` here?

my bad. Done.

https://codereview.chromium.org/645513003/diff/1/Source/core/fetch/ResourceLo...
File Source/core/fetch/ResourceLoadPriorityOptimizer.cpp (right):

https://codereview.chromium.org/645513003/diff/1/Source/core/fetch/ResourceLo...
Source/core/fetch/ResourceLoadPriorityOptimizer.cpp:87: if
(!renderObject->updateImageLoadingPriorities()) {
On 2014/10/17 10:38:49, Mike West wrote:
> Nit: Drop the {} for one-liners.

Done.

[Mike West, 2014-10-17 18:53:20]

lgtm

[riju_, 2014-10-17 18:58:25]

The CQ bit was checked by rijubrata.bhaumik@intel.com

[commit-bot: I haz the power, 2014-10-17 18:59:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/645513003/20001

[commit-bot: I haz the power, 2014-10-17 20:45:44]

Committed patchset #2 (id:20001) as 183923

[kenneth.r.christiansen, 2014-10-18 09:55:04]

kenneth.r.christiansen@intel.com changed reviewers:
+ kenneth.r.christiansen@intel.com

[kenneth.r.christiansen, 2014-10-18 09:55:04]

https://codereview.chromium.org/645513003/diff/20001/Source/core/fetch/ImageR...
File Source/core/fetch/ImageResource.cpp (right):

https://codereview.chromium.org/645513003/diff/20001/Source/core/fetch/ImageR...
Source/core/fetch/ImageResource.cpp:390: for (const auto& imageResource :
images) {
I am not sure if calling this like imageResource is that nice as it is a map, I
personally went for entry. This applies to many of the places here.

[kenneth.r.christiansen, 2014-10-18 10:04:50]

On 2014/10/18 at 09:55:04, kenneth.r.christiansen wrote:
>
https://codereview.chromium.org/645513003/diff/20001/Source/core/fetch/ImageR...
> File Source/core/fetch/ImageResource.cpp (right):
> 
>
https://codereview.chromium.org/645513003/diff/20001/Source/core/fetch/ImageR...
> Source/core/fetch/ImageResource.cpp:390: for (const auto& imageResource :
images) {
> I am not sure if calling this like imageResource is that nice as it is a map,
I personally went for entry. This applies to many of the places here.

Maybe calling is hashEntry or mapEntry or so would make the surrounding code
more clear.

[Mike West, 2014-10-18 11:45:36]

On 2014/10/18 10:04:50, kenneth.r.christiansen wrote:
> On 2014/10/18 at 09:55:04, kenneth.r.christiansen wrote:
> >
>
https://codereview.chromium.org/645513003/diff/20001/Source/core/fetch/ImageR...
> > File Source/core/fetch/ImageResource.cpp (right):
> > 
> >
>
https://codereview.chromium.org/645513003/diff/20001/Source/core/fetch/ImageR...
> > Source/core/fetch/ImageResource.cpp:390: for (const auto& imageResource :
> images) {
> > I am not sure if calling this like imageResource is that nice as it is a
map,
> I personally went for entry. This applies to many of the places here.
> 
> Maybe calling is hashEntry or mapEntry or so would make the surrounding code
> more clear.

Happy to review a followup. :)