Remove ScopedVector from chrome/browser/ui.

chrome/browser/ui/toolbar/toolbar_actions_bar.cc

 // Copyright 2014 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.
 
 #include "chrome/browser/ui/toolbar/toolbar_actions_bar.h"
 
 #include <utility>
 
 #include "base/auto_reset.h"
 #include "base/location.h"
@@ skipping 36 matching lines @@
 // Takes a reference vector |reference| of length n, where n is less than or
 // equal to the length of |to_sort|, and rearranges |to_sort| so that
 // |to_sort|'s first n elements match the n elements of |reference| (the order
 // of any remaining elements in |to_sort| is unspecified).
 // |equal| is used to compare the elements of |to_sort| and |reference|.
 // This allows us to sort a vector to match another vector of two different
 // types without needing to construct a more cumbersome comparator class.
 // |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()) <<
       "|to_sort| must contain all elements in |reference|.";
   if (reference.empty())
     return;
   // 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|.";
       }
       std::swap(to_sort->at(i), to_sort->at(j));
     }
   }
 }
 
 // How long to wait until showing an extension message bubble.
@@ skipping 120 matching lines @@
 }
 
 size_t ToolbarActionsBar::GetIconCount() const {
   if (!model_)
     return 0;
 
   int pop_out_modifier = 0;
   // 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;
   }
 
   // We purposefully do not account for any "popped out" actions in overflow
   // mode. This is because the popup cannot be showing while the overflow menu
   // is open, so there's no concern there. Also, if the user has a popped out
   // action, and immediately opens the overflow menu, we *want* the action there
   // (since it will close the popup, but do so asynchronously, and we don't
   // want to "slide" the action back in.
   size_t visible_icons = in_overflow_mode() ?
@@ skipping 60 matching lines @@
 
   return gfx::Rect(platform_settings().item_spacing +
                        index_in_row * IconWidth(true),
                    row_index * IconHeight(),
                    IconWidth(false),
                    IconHeight());
 }
 
 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.
   if (popped_out_action_) {
     size_t index =
         std::find(actions.begin(), actions.end(), popped_out_action_) -
         actions.begin();
     DCHECK_NE(actions.size(), index);
     size_t visible = GetIconCount();
     if (index >= visible) {
@@ skipping 37 matching lines @@
       tracked_objects::ScopedTracker tracking_profile3(
           FROM_HERE_WITH_EXPLICIT_FUNCTION(
               "ToolbarActionsBar::CreateActions3"));
       ReorderActions();
     }
 
     tracked_objects::ScopedTracker tracking_profile4(
         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.
   suppress_animation_ = false;
 
   // CreateActions() can be called multiple times, so we need to make sure we
   // haven't already shown the bubble.
   // Extension bubbles can also highlight a subset of actions, so don't show the
   // bubble if the toolbar is already highlighting a different set.
   if (should_check_extension_bubble_ && !is_highlighting()) {
@@ skipping 12 matching lines @@
   toolbar_actions_.clear();
 }
 
 void ToolbarActionsBar::Update() {
   if (toolbar_actions_.empty())
     return;  // Nothing to do.
 
   {
     // 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();
   }
 
   ReorderActions();  // Also triggers a draw.
 }
 
 bool ToolbarActionsBar::ShowToolbarActionPopup(const std::string& action_id,
                                                bool grant_active_tab) {
   // Don't override another popup, and only show in the active window.
   if (popup_owner() || !browser_->window()->IsActive())
@@ skipping 77 matching lines @@
 
 void ToolbarActionsBar::OnBubbleClosed() {
   is_showing_bubble_ = false;
 }
 
 bool ToolbarActionsBar::IsActionVisibleOnMainBar(
     const ToolbarActionViewController* action) const {
   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.

 }
 
 void ToolbarActionsBar::PopOutAction(ToolbarActionViewController* controller,
                                      bool is_sticky,
                                      const base::Closure& closure) {
   DCHECK(!in_overflow_mode()) << "Only the main bar can pop out actions.";
   DCHECK(!popped_out_action_) << "Only one action can be popped out at a time!";
   bool needs_redraw = !IsActionVisibleOnMainBar(controller);
   popped_out_action_ = controller;
   is_popped_out_sticky_ = is_sticky;
@@ skipping 113 matching lines @@
 void ToolbarActionsBar::OnToolbarActionAdded(
     const ToolbarActionsModel::ToolbarItem& item,
     int index) {
   CHECK(model_->actions_initialized());
   CHECK(GetActionForId(item.id) == nullptr)
       << "Asked to add a toolbar action view for an action that already "
          "exists";
 
   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()
   // checks to see if the container is already the proper size, and because
   // if the action is newly incognito enabled, even though it's a reload, it's
   // a new extension to this toolbar.
   // We suppress the chevron during animation because, if we're expanding to
   // show a new icon, we don't want to have the chevron visible only for the
   // duration of the animation.
   ResizeDelegate(gfx::Tween::LINEAR, true);
 }
 
 void ToolbarActionsBar::OnToolbarActionRemoved(const std::string& action_id) {
   ToolbarActions::iterator iter = toolbar_actions_.begin();
   while (iter != toolbar_actions_.end() && (*iter)->GetId() != action_id)
     ++iter;
 
   if (iter == toolbar_actions_.end())
     return;
 
   // 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();
   removed_action.reset();
 
   // If the extension is being upgraded we don't want the bar to shrink
   // because the icon is just going to get re-added to the same location.
   // There is an exception if this is an off-the-record profile, and the
   // extension is no longer incognito-enabled.
   if (!extensions::ExtensionSystem::Get(browser_->profile())
@@ skipping 61 matching lines @@
 }
 
 void ToolbarActionsBar::OnToolbarHighlightModeChanged(bool is_highlighting) {
   if (!model_->actions_initialized())
     return;
 
   {
     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;
       }
     }
   }
 
   ReorderActions();
 }
 
@@ skipping 20 matching lines @@
 
 void ToolbarActionsBar::ReorderActions() {
   if (toolbar_actions_.empty())
     return;
 
   // First, reset the order to that of the model.
   auto compare = [](ToolbarActionViewController* const& action,
                     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).
   if (!suppress_layout_) {
     ResizeDelegate(gfx::Tween::EASE_OUT, false);
     delegate_->Redraw(true);
   }
 }
 
 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;
 }
 
 content::WebContents* ToolbarActionsBar::GetCurrentWebContents() {
   return browser_->tab_strip_model()->GetActiveWebContents();
 }