Remove ScopedVector from chrome/browser/ui.

chrome/browser/ui/toolbar/toolbar_actions_bar.cc

Index: chrome/browser/ui/toolbar/toolbar_actions_bar.cc
diff --git a/chrome/browser/ui/toolbar/toolbar_actions_bar.cc b/chrome/browser/ui/toolbar/toolbar_actions_bar.cc
index 72f85cc804889a821b1695b5c2608ed721da1e22..50f1fd01829f1bd6e1ebfb047c8109bc240c7413 100644
--- a/chrome/browser/ui/toolbar/toolbar_actions_bar.cc
+++ b/chrome/browser/ui/toolbar/toolbar_actions_bar.cc
@@ -54,8 +54,8 @@ enum DimensionType { WIDTH, HEIGHT };
 // |FunctionType| should equate to (something similar to)
 // bool Equal(const Type1&, const Type2&), but we can't enforce this
 // because of MSVC compilation limitations.
-template<typename Type1, typename Type2, typename FunctionType>
-void SortContainer(std::vector<Type1>* to_sort,
+template <typename Type1, typename Type2, typename FunctionType>
+void SortContainer(std::vector<std::unique_ptr<Type1>>* to_sort,
                    const std::vector<Type2>& reference,
                    FunctionType equal) {
   CHECK_GE(to_sort->size(), reference.size()) <<
@@ -65,11 +65,11 @@ void SortContainer(std::vector<Type1>* to_sort,
   // Run through the each element and compare it to the reference. If something
   // is out of place, find the correct spot for it.
   for (size_t i = 0; i < reference.size() - 1; ++i) {
-    if (!equal(to_sort->at(i), reference[i])) {
+    if (!equal(to_sort->at(i).get(), reference[i])) {
       // Find the correct index (it's guaranteed to be after our current
       // index, since everything up to this point is correct), and swap.
       size_t j = i + 1;
-      while (!equal(to_sort->at(j), reference[i])) {
+      while (!equal(to_sort->at(j).get(), reference[i])) {
         ++j;
         DCHECK_LT(j, to_sort->size()) <<
             "Item in |reference| not found in |to_sort|.";
@@ -210,10 +210,14 @@ size_t ToolbarActionsBar::GetIconCount() const {
   // If there is a popped out action, it could affect the number of visible
   // icons - but only if it wouldn't otherwise be visible.
   if (popped_out_action_) {
-    size_t popped_out_index =
-        std::find(toolbar_actions_.begin(),
-                  toolbar_actions_.end(),
-                  popped_out_action_) - toolbar_actions_.begin();
+    auto popped_out_action = popped_out_action_;
+    auto popped_out_it = std::find_if(
+        toolbar_actions_.begin(), toolbar_actions_.end(),
+        [popped_out_action](
+            const std::unique_ptr<ToolbarActionViewController>& ptr) {
+          return ptr.get() == popped_out_action;
+        });
+    size_t popped_out_index = popped_out_it - toolbar_actions_.begin();

[Nico, 2017/01/03 18:12:35]

again,

  size_t popped_out_index = 0;
  for (; popped_out_index < toolbar_actions_.size(); ++popped_out_index) {
    if (toolbar_actions_[i] == popped_out_action_)
      break;
  }

is half the code

[Avi (use Gerrit), 2017/01/03 22:54:53]

Done.

     pop_out_modifier = popped_out_index >= model_->visible_icon_count() ? 1 : 0;
   }
 
@@ -294,7 +298,9 @@ gfx::Rect ToolbarActionsBar::GetFrameForIndex(
 
 std::vector<ToolbarActionViewController*>
 ToolbarActionsBar::GetActions() const {
-  std::vector<ToolbarActionViewController*> actions = toolbar_actions_.get();
+  std::vector<ToolbarActionViewController*> actions;
+  for (const auto& action : toolbar_actions_)
+    actions.push_back(action.get());
 
   // If there is an action that should be popped out, and it's not visible by
   // default, make it the final action in the list.
@@ -352,7 +358,7 @@ void ToolbarActionsBar::CreateActions() {
         FROM_HERE_WITH_EXPLICIT_FUNCTION("ToolbarActionsBar::CreateActions4"));
 
     for (size_t i = 0; i < toolbar_actions_.size(); ++i)
-      delegate_->AddViewForAction(toolbar_actions_[i], i);
+      delegate_->AddViewForAction(toolbar_actions_[i].get(), i);
   }
 
   // Once the actions are created, we should animate the changes.
@@ -385,7 +391,7 @@ void ToolbarActionsBar::Update() {
   {
     // Don't layout until the end.
     base::AutoReset<bool> layout_resetter(&suppress_layout_, true);
-    for (ToolbarActionViewController* action : toolbar_actions_)
+    for (const auto& action : toolbar_actions_)
       action->UpdateState();
   }
 
@@ -483,9 +489,12 @@ bool ToolbarActionsBar::IsActionVisibleOnMainBar(
   if (in_overflow_mode())
     return main_bar_->IsActionVisibleOnMainBar(action);
 
-  size_t index = std::find(toolbar_actions_.begin(),
-                           toolbar_actions_.end(),
-                           action) - toolbar_actions_.begin();
+  auto it = std::find_if(
+      toolbar_actions_.begin(), toolbar_actions_.end(),
+      [action](const std::unique_ptr<ToolbarActionViewController>& ptr) {
+        return ptr.get() == action;
+      });
+  size_t index = it - toolbar_actions_.begin();
   return index < GetIconCount() || action == popped_out_action_;

[Nico, 2017/01/03 18:12:35]

nit: I'd compare action to popped_out_action_ first and only walk the whole
array if it isn't found. And again

  for (size_t i = 0; i < std::min(toolbar_actions_.size(), GetIconCount()); ++i)
{
    if (toolbar_actions_[i] == action)
      return true;
  }
  return false;

for the loop seems simpler to me.

[Avi (use Gerrit), 2017/01/03 22:54:53]

Done.

 }
 
@@ -620,7 +629,7 @@ void ToolbarActionsBar::OnToolbarActionAdded(
 
   toolbar_actions_.insert(toolbar_actions_.begin() + index,
                           model_->CreateActionForItem(browser_, this, item));
-  delegate_->AddViewForAction(toolbar_actions_[index], index);
+  delegate_->AddViewForAction(toolbar_actions_[index].get(), index);
 
   // We may need to resize (e.g. to show the new icon, or the chevron). We don't
   // need to check if an extension is upgrading here, because ResizeDelegate()
@@ -644,8 +653,9 @@ void ToolbarActionsBar::OnToolbarActionRemoved(const std::string& action_id) {
   // The action should outlive the UI element (which is owned by the delegate),
   // so we can't delete it just yet. But we should remove it from the list of
   // actions so that any width calculations are correct.
-  std::unique_ptr<ToolbarActionViewController> removed_action(*iter);
-  toolbar_actions_.weak_erase(iter);
+  std::unique_ptr<ToolbarActionViewController> removed_action =
+      std::move(*iter);
+  toolbar_actions_.erase(iter);
   delegate_->RemoveViewForAction(removed_action.get());
   if (popped_out_action_ == removed_action.get())
     UndoPopOut();
@@ -727,26 +737,25 @@ void ToolbarActionsBar::OnToolbarHighlightModeChanged(bool is_highlighting) {
     base::AutoReset<bool> layout_resetter(&suppress_layout_, true);
     base::AutoReset<bool> animation_resetter(&suppress_animation_, true);
     std::set<std::string> seen;
-    for (const ToolbarActionsModel::ToolbarItem item :
-         model_->toolbar_items()) {
-      auto current_pos =
-          std::find_if(toolbar_actions_.begin(), toolbar_actions_.end(),
-                       [&item](const ToolbarActionViewController* action) {
-                         return action->GetId() == item.id;
-                       });
+    for (const auto& item : model_->toolbar_items()) {
+      auto current_pos = std::find_if(
+          toolbar_actions_.begin(), toolbar_actions_.end(),
+          [&item](const std::unique_ptr<ToolbarActionViewController>& action) {
+            return action->GetId() == item.id;
+          });

[Nico, 2017/01/03 18:12:35]

guess :-)

[Avi (use Gerrit), 2017/01/03 22:54:53]

Done.

       if (current_pos == toolbar_actions_.end()) {
         toolbar_actions_.push_back(
-            model_->CreateActionForItem(browser_, this, item).release());
-        delegate_->AddViewForAction(toolbar_actions_.back(),
+            model_->CreateActionForItem(browser_, this, item));
+        delegate_->AddViewForAction(toolbar_actions_.back().get(),
                                     toolbar_actions_.size() - 1);
       }
       seen.insert(item.id);
     }
 
-    for (ToolbarActions::iterator iter = toolbar_actions_.begin();
+    for (auto iter = toolbar_actions_.begin();
          iter != toolbar_actions_.end();) {
       if (seen.count((*iter)->GetId()) == 0) {
-        delegate_->RemoveViewForAction(*iter);
+        delegate_->RemoveViewForAction(iter->get());
         iter = toolbar_actions_.erase(iter);
       } else {
         ++iter;
@@ -787,7 +796,7 @@ void ToolbarActionsBar::ReorderActions() {
                     const ToolbarActionsModel::ToolbarItem& item) {
     return action->GetId() == item.id;
   };
-  SortContainer(&toolbar_actions_.get(), model_->toolbar_items(), compare);
+  SortContainer(&toolbar_actions_, model_->toolbar_items(), compare);
 
   // Our visible browser actions may have changed - re-Layout() and check the
   // size (if we aren't suppressing the layout).
@@ -799,9 +808,9 @@ void ToolbarActionsBar::ReorderActions() {
 
 ToolbarActionViewController* ToolbarActionsBar::GetActionForId(
     const std::string& action_id) {
-  for (ToolbarActionViewController* action : toolbar_actions_) {
+  for (const auto& action : toolbar_actions_) {
     if (action->GetId() == action_id)
-      return action;
+      return action.get();
   }
   return nullptr;
 }