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

ui/views/view.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #define _USE_MATH_DEFINES // For VC++ to get M_PI. This has to be first.
 
 #include "ui/views/view.h"
 
 #include <algorithm>
 #include <cmath>
 #include <memory>
 #include <utility>
 
+#include "base/containers/adapters.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #include "base/stl_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/trace_event/trace_event.h"
 #include "build/build_config.h"
 #include "third_party/skia/include/core/SkRect.h"
@@ skipping 523 matching lines @@
   // If we have a layout manager, let it handle the layout for us.
   if (layout_manager_.get())
     layout_manager_->Layout(this);
 
   // Make sure to propagate the Layout() call to any children that haven't
   // received it yet through the layout manager and need to be laid out. This
   // is needed for the case when the child requires a layout but its bounds
   // 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;
       child->Layout();
     }
   }
 }
 
 void View::InvalidateLayout() {
   // Always invalidate up. This is needed to handle the case of us already being
@@ skipping 45 matching lines @@
 
 View* View::GetAncestorWithClassName(const std::string& name) {
   return const_cast<View*>(const_cast<const View*>(this)->
       GetAncestorWithClassName(name));
 }
 
 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;
   }
   return NULL;
 }
 
 View* View::GetViewByID(int id) {
   return const_cast<View*>(const_cast<const View*>(this)->GetViewByID(id));
 }
 
 void View::SetGroup(int gid) {
   // Don't change the group id once it's set.
   DCHECK(group_ == -1 || group_ == gid);
   group_ = gid;
 }
 
 int View::GetGroup() const {
   return group_;
 }
 
 bool View::IsGroupFocusTraversable() const {
   return true;
 }
 
 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) {
   Views views;
   GetWidget()->GetRootView()->GetViewsInGroup(group, &views);
   return views.empty() ? NULL : views[0];
 }
 
 // Coordinate conversion -------------------------------------------------------
 
@@ skipping 244 matching lines @@
   return true;
 }
 
 View* View::GetTooltipHandlerForPoint(const gfx::Point& point) {
   // TODO(tdanderson): Move this implementation into ViewTargetDelegate.
   if (!HitTestPoint(point) || !CanProcessEventsWithinSubtree())
     return NULL;
 
   // 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;
 
     gfx::Point point_in_child_coords(point);
     ConvertPointToTarget(this, child, &point_in_child_coords);
     View* handler = child->GetTooltipHandlerForPoint(point_in_child_coords);
     if (handler)
       return handler;
   }
   return this;
@@ skipping 498 matching lines @@
 
     if (focus_manager)
       RegisterPendingAccelerators();
   }
 }
 
 // Painting --------------------------------------------------------------------
 
 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) {
   TRACE_EVENT1("views", "View::OnPaint", "class", GetClassName());
   OnPaintBackground(canvas);
   OnPaintBorder(canvas);
 }
 
 void View::OnPaintBackground(gfx::Canvas* canvas) {
   if (background_.get()) {
@@ skipping 45 matching lines @@
 void View::MoveLayerToParent(ui::Layer* parent_layer,
                              const gfx::Point& point) {
   gfx::Point local_point(point);
   if (parent_layer != layer())
     local_point.Offset(GetMirroredX(), y());
   if (layer() && parent_layer != layer()) {
     parent_layer->Add(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);
   }
 }
 
 void View::UpdateLayerVisibility() {
   bool visible = visible_;
   for (const View* v = parent_; visible && v && !v->layer(); v = v->parent_)
     visible = v->visible();
 
   UpdateChildLayerVisibility(visible);
 }
 
 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_);
   }
 }
 
 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()));
     }
   }
 }
 
 void View::OnPaintLayer(const ui::PaintContext& context) {
   Paint(context);
 }
 
@@ skipping 212 matching lines @@
 
     base::snprintf(pp, kMaxPointerStringLength, "%p", parent_);
     result.append("  N");
     result.append(pp + 2);
     result.append(" -> N");
     result.append(p + 2);
     result.append("\n");
   }
 
   // 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");
 
   return result;
 }
 #endif
 
 ////////////////////////////////////////////////////////////////////////////////
 // View, private:
@@ skipping 88 matching lines @@
     UpdateTooltip();
 
   if (layout_manager_)
     layout_manager_->ViewRemoved(this, view);
 
   for (ViewObserver& observer : observers_)
     observer.OnChildViewRemoved(view, this);
 }
 
 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_)
     v->ViewHierarchyChangedImpl(true, details);
 }
 
 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();
 }
 
 void View::ViewHierarchyChangedImpl(
     bool register_accelerators,
     const ViewHierarchyChangedDetails& details) {
   if (register_accelerators) {
     if (details.is_add) {
       // If you get this registration, you are part of a subtree that has been
       // added to the view hierarchy.
       if (GetFocusManager())
         RegisterPendingAccelerators();
     } else {
       if (details.child == this)
         UnregisterAccelerators(true);
     }
   }
 
   ViewHierarchyChanged(details);
   details.parent->needs_layout_ = true;
 }
 
 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);
 }
 
 void View::VisibilityChangedImpl(View* starting_from, bool is_visible) {
   VisibilityChanged(starting_from, is_visible);
 }
 
 void View::BoundsChanged(const gfx::Rect& previous_bounds) {
   if (visible_) {
     // Paint the new bounds.
@@ skipping 160 matching lines @@
   bool result = GetTransformRelativeTo(ancestor, &trans);
   trans.TransformRectReverse(rect);
   return result;
 }
 
 // Accelerated painting --------------------------------------------------------
 
 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);
   layer()->set_name(GetClassName());
 
   UpdateParentLayers();
   UpdateLayerVisibility();
 
   // The new layer needs to be ordered in the layer tree according
   // to the view tree. Children of this layer were added in order
@@ skipping 17 matching lines @@
   if (layer()) {
     if (!layer()->parent()) {
       UpdateParentLayer();
       return true;
     }
     // The layers of any child views are already in place, so we can stop
     // iterating here.
     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;
 }
 
 void View::OrphanLayers() {
   if (layer()) {
     if (layer()->parent())
       layer()->parent()->Remove(layer());
 
     // The layer belonging to this View has already been orphaned. It is not
     // 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) {
   layer()->SetBounds(GetLocalBounds() + offset);
   DCHECK_NE(layer(), parent_layer);
   if (parent_layer)
     parent_layer->Add(layer());
   layer()->SchedulePaint(GetLocalBounds());
   MoveLayerToParent(layer(), gfx::Point());
 }
@@ skipping 208 matching lines @@
     return;
 
   FocusManager* focus_manager = GetFocusManager();
   if (focus_manager)
     focus_manager->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.
   if (!layer())
     OnDeviceScaleFactorChanged(device_scale_factor);
 }
 
 // Tooltips --------------------------------------------------------------------
 
 void View::UpdateTooltip() {
@@ skipping 30 matching lines @@
   // Message the RootView to do the drag and drop. That way if we're removed
   // the RootView can detect it and avoid calling us back.
   gfx::Point widget_location(event.location());
   ConvertPointToWidget(this, &widget_location);
   widget->RunShellDrag(this, data, widget_location, drag_operations, source);
   // WARNING: we may have been deleted.
   return true;
 }
 
 }  // namespace views