[views] Changes iteration over |children_| to use range-based for loops

ui/views/view.cc

Index: ui/views/view.cc
diff --git a/ui/views/view.cc b/ui/views/view.cc
index 92223157d4c8671aaa2c55aab4c006d1ee25b265..f4413ff4de66666ed07160369973ef0ed92f71d1 100644
--- a/ui/views/view.cc
+++ b/ui/views/view.cc
@@ -11,6 +11,7 @@
 #include <memory>
 #include <utility>
 
+#include "base/containers/adapters.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
@@ -554,8 +555,7 @@ void View::Layout() {
   // weren't changed by the layout manager. If there is no layout manager, we
   // just propagate the Layout() call down the hierarchy, so whoever receives
   // the call can take appropriate action.
-  for (int i = 0, count = child_count(); i < count; ++i) {
-    View* child = child_at(i);
+  for (auto child : children_) {

[sadrul, 2016/12/20 17:35:25]

auto* child

So it's obvious when reading the code that child is a pointer.

[varkha, 2016/12/20 18:48:47]

Done.

     if (child->needs_layout_ || !layout_manager_.get()) {
       TRACE_EVENT1("views", "View::Layout", "class", child->GetClassName());
       child->needs_layout_ = false;
@@ -621,8 +621,8 @@ const View* View::GetViewByID(int id) const {
   if (id == id_)
     return const_cast<View*>(this);
 
-  for (int i = 0, count = child_count(); i < count; ++i) {
-    const View* view = child_at(i)->GetViewByID(id);
+  for (auto child : children_) {

[sadrul, 2016/12/20 17:35:25]

ditto

[varkha, 2016/12/20 18:48:47]

Done.

+    const View* view = child->GetViewByID(id);
     if (view)
       return view;
   }
@@ -651,8 +651,8 @@ void View::GetViewsInGroup(int group, Views* views) {
   if (group_ == group)
     views->push_back(this);
 
-  for (int i = 0, count = child_count(); i < count; ++i)
-    child_at(i)->GetViewsInGroup(group, views);
+  for (auto child : children_)

[sadrul, 2016/12/20 17:35:25]

same

[varkha, 2016/12/20 18:48:47]

Done.

+    child->GetViewsInGroup(group, views);
 }
 
 View* View::GetSelectedViewForGroup(int group) {
@@ -917,8 +917,7 @@ View* View::GetTooltipHandlerForPoint(const gfx::Point& point) {
 
   // Walk the child Views recursively looking for the View that most
   // tightly encloses the specified point.
-  for (int i = child_count() - 1; i >= 0; --i) {
-    View* child = child_at(i);
+  for (auto child : base::Reversed(children_)) {
     if (!child->visible())
       continue;
 
@@ -1437,9 +1436,9 @@ void View::NativeViewHierarchyChanged() {
 
 void View::PaintChildren(const ui::PaintContext& context) {
   TRACE_EVENT1("views", "View::PaintChildren", "class", GetClassName());
-  for (int i = 0, count = child_count(); i < count; ++i)
-    if (!child_at(i)->layer())
-      child_at(i)->Paint(context);
+  for (auto child : children_)

[sadrul, 2016/12/20 17:35:25]

I think we should have {} here

[varkha, 2016/12/20 18:48:47]

Done.

+    if (!child->layer())
+      child->Paint(context);
 }
 
 void View::OnPaint(gfx::Canvas* canvas) {
@@ -1505,8 +1504,8 @@ void View::MoveLayerToParent(ui::Layer* parent_layer,
     SetLayerBounds(gfx::Rect(local_point.x(), local_point.y(),
                              width(), height()));
   } else {
-    for (int i = 0, count = child_count(); i < count; ++i)
-      child_at(i)->MoveLayerToParent(parent_layer, local_point);
+    for (auto child : children_)
+      child->MoveLayerToParent(parent_layer, local_point);
   }
 }
 
@@ -1522,8 +1521,8 @@ void View::UpdateChildLayerVisibility(bool ancestor_visible) {
   if (layer()) {
     layer()->SetVisible(ancestor_visible && visible_);
   } else {
-    for (int i = 0, count = child_count(); i < count; ++i)
-      child_at(i)->UpdateChildLayerVisibility(ancestor_visible && visible_);
+    for (auto child : children_)
+      child->UpdateChildLayerVisibility(ancestor_visible && visible_);
   }
 }
 
@@ -1531,8 +1530,7 @@ void View::UpdateChildLayerBounds(const gfx::Vector2d& offset) {
   if (layer()) {
     SetLayerBounds(GetLocalBounds() + offset);
   } else {
-    for (int i = 0, count = child_count(); i < count; ++i) {
-      View* child = child_at(i);
+    for (auto child : children_) {
       child->UpdateChildLayerBounds(
           offset + gfx::Vector2d(child->GetMirroredX(), child->y()));
     }
@@ -1765,8 +1763,8 @@ std::string View::DoPrintViewGraph(bool first, View* view_with_children) {
   }
 
   // Children.
-  for (int i = 0, count = view_with_children->child_count(); i < count; ++i)
-    result.append(view_with_children->child_at(i)->PrintViewGraph(false));
+  for (auto child : view_with_children->children_)
+    result.append(child->PrintViewGraph(false));
 
   if (first)
     result.append("}\n");
@@ -1875,8 +1873,8 @@ void View::DoRemoveChildView(View* view,
 }
 
 void View::PropagateRemoveNotifications(View* old_parent, View* new_parent) {
-  for (int i = 0, count = child_count(); i < count; ++i)
-    child_at(i)->PropagateRemoveNotifications(old_parent, new_parent);
+  for (auto child : children_)
+    child->PropagateRemoveNotifications(old_parent, new_parent);
 
   ViewHierarchyChangedDetails details(false, old_parent, this, new_parent);
   for (View* v = this; v; v = v->parent_)
@@ -1885,14 +1883,14 @@ void View::PropagateRemoveNotifications(View* old_parent, View* new_parent) {
 
 void View::PropagateAddNotifications(
     const ViewHierarchyChangedDetails& details) {
-  for (int i = 0, count = child_count(); i < count; ++i)
-    child_at(i)->PropagateAddNotifications(details);
+  for (auto child : children_)
+    child->PropagateAddNotifications(details);
   ViewHierarchyChangedImpl(true, details);
 }
 
 void View::PropagateNativeViewHierarchyChanged() {
-  for (int i = 0, count = child_count(); i < count; ++i)
-    child_at(i)->PropagateNativeViewHierarchyChanged();
+  for (auto child : children_)
+    child->PropagateNativeViewHierarchyChanged();
   NativeViewHierarchyChanged();
 }
 
@@ -1916,16 +1914,16 @@ void View::ViewHierarchyChangedImpl(
 }
 
 void View::PropagateNativeThemeChanged(const ui::NativeTheme* theme) {
-  for (int i = 0, count = child_count(); i < count; ++i)
-    child_at(i)->PropagateNativeThemeChanged(theme);
+  for (auto child : children_)
+    child->PropagateNativeThemeChanged(theme);
   OnNativeThemeChanged(theme);
 }
 
 // Size and disposition --------------------------------------------------------
 
 void View::PropagateVisibilityNotifications(View* start, bool is_visible) {
-  for (int i = 0, count = child_count(); i < count; ++i)
-    child_at(i)->PropagateVisibilityNotifications(start, is_visible);
+  for (auto child : children_)
+    child->PropagateVisibilityNotifications(start, is_visible);
   VisibilityChangedImpl(start, is_visible);
 }
 
@@ -2106,8 +2104,8 @@ bool View::ConvertRectFromAncestor(const View* ancestor,
 void View::CreateLayer() {
   // A new layer is being created for the view. So all the layers of the
   // sub-tree can inherit the visibility of the corresponding view.
-  for (int i = 0, count = child_count(); i < count; ++i)
-    child_at(i)->UpdateChildLayerVisibility(true);
+  for (auto child : children_)
+    child->UpdateChildLayerVisibility(true);
 
   SetLayer(base::MakeUnique<ui::Layer>());
   layer()->set_delegate(this);
@@ -2145,8 +2143,8 @@ bool View::UpdateParentLayers() {
     return false;
   }
   bool result = false;
-  for (int i = 0, count = child_count(); i < count; ++i) {
-    if (child_at(i)->UpdateParentLayers())
+  for (auto child : children_) {
+    if (child->UpdateParentLayers())
       result = true;
   }
   return result;
@@ -2161,8 +2159,8 @@ void View::OrphanLayers() {
     // necessary to orphan the child layers.
     return;
   }
-  for (int i = 0, count = child_count(); i < count; ++i)
-    child_at(i)->OrphanLayers();
+  for (auto child : children_)
+    child->OrphanLayers();
 }
 
 void View::ReparentLayer(const gfx::Vector2d& offset, ui::Layer* parent_layer) {
@@ -2391,20 +2389,20 @@ void View::AdvanceFocusIfNecessary() {
 // System events ---------------------------------------------------------------
 
 void View::PropagateThemeChanged() {
-  for (int i = child_count() - 1; i >= 0; --i)
-    child_at(i)->PropagateThemeChanged();
+  for (auto child : base::Reversed(children_))
+    child->PropagateThemeChanged();
   OnThemeChanged();
 }
 
 void View::PropagateLocaleChanged() {
-  for (int i = child_count() - 1; i >= 0; --i)
-    child_at(i)->PropagateLocaleChanged();
+  for (auto child : base::Reversed(children_))
+    child->PropagateLocaleChanged();
   OnLocaleChanged();
 }
 
 void View::PropagateDeviceScaleFactorChanged(float device_scale_factor) {
-  for (int i = child_count() - 1; i >= 0; --i)
-    child_at(i)->PropagateDeviceScaleFactorChanged(device_scale_factor);
+  for (auto child : base::Reversed(children_))
+    child->PropagateDeviceScaleFactorChanged(device_scale_factor);

[sadrul, 2016/12/20 17:35:25]

I wonder why these last three are iterated in reverse order. (I am not
suggesting we should change it, just wondering).

[varkha, 2016/12/20 18:48:47]

Not sure. I guess the desire was to go in visibility order so it is possible
that there is something subtle about the propagation order that favors children
that are above others to receive the notifications first. This goes all the way
to https://codereview.chromium.org/27317 so maybe ben@ would know.

 
   // If the view is drawing to the layer, OnDeviceScaleFactorChanged() is called
   // through LayerDelegate callback.