[Extension Toolbar] Refactor and finish pop out logic for actions

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 7bf048a702d6ba30f66a2cc2e12d0d411dd11d81..fd57dce1d03d46a08af4729f32ee6877cf4b0ccd 100644
--- a/chrome/browser/ui/toolbar/toolbar_actions_bar.cc
+++ b/chrome/browser/ui/toolbar/toolbar_actions_bar.cc
@@ -12,6 +12,7 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/extensions/extension_action_view_controller.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
 #include "chrome/browser/ui/toolbar/component_toolbar_actions_factory.h"
 #include "chrome/browser/ui/toolbar/toolbar_action_view_controller.h"
 #include "chrome/browser/ui/toolbar/toolbar_actions_bar_delegate.h"
@@ -26,6 +27,8 @@
 
 namespace {
 
+using WeakToolbarActions = std::vector<ToolbarActionViewController*>;
+
 #if defined(OS_MACOSX)
 const int kItemSpacing = 2;
 const int kLeftPadding = kItemSpacing + 1;
@@ -59,8 +62,289 @@ int GetIconDimension(DimensionType type) {
   return type == WIDTH ? icon_width : icon_height;
 }
 
+// Sorts |to_sort| to match |reference|, using |equal| as a comparison
+// function. This allows comparing two different types without needing to
+// construct a more cumbersome comparator class.
+template<typename Type1, typename Type2>
+void SortContainer(std::vector<Type1>* to_sort,
+                   const std::vector<Type2>& reference,
+                   bool (*equal)(const Type1&, const Type2&)) {
+  DCHECK_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])) {
+      // Find where the correct index (it's guaranteed to be after our current

[Finnur, 2014/12/12 15:06:07]

nit: Missing the word 'is'?

[Devlin, 2014/12/12 18:27:06]

Done.

+      // index, since everything up to this point is correct), and swap.
+      size_t j = i + 1;
+      while (!(*equal)(to_sort->at(j), reference[i])) {
+        ++j;
+        DCHECK_LE(j, to_sort->size()) <<
+            "Item in |reference| not found in |to_sort|.";
+      }
+      std::swap(to_sort->at(i), to_sort->at(j));
+    }
+  }
+}
+
 }  // namespace
 
+bool ToolbarActionsBar::pop_out_actions_to_run = false;
+
+// A class to implement an optional tab ordering that "pops out" actions that
+// want to run on a particular page, as part of our experimentation with how to
+// best signal that actions like extension page actions want to run.
+// TODO(devlin): Once we finally settle on the right behavior, determine if
+// we need this.
+class ToolbarActionsBar::TabOrderHelper
+    : public TabStripModelObserver {
+ public:
+  TabOrderHelper(ToolbarActionsBar* toolbar,
+                 Browser* browser,
+                 extensions::ExtensionToolbarModel* model);
+  ~TabOrderHelper() override;
+
+  // Returns the number of extra icons that should appear on the given |tab_id|
+  // because of actions that are going to pop out.
+  size_t GetExtraIconCount(int tab_id);
+
+  // Returns the item order of actions for the tab with the given
+  // |web_contents|.
+  WeakToolbarActions GetActionOrder(content::WebContents* web_contents);
+
+  // Notifies the TabOrderHelper that a given |action| does/doesn't want to run
+  // on the tab indicated by |tab_id|.
+  void SetActionWantsToRun(ToolbarActionViewController* action,
+                           int tab_id,
+                           bool wants_to_run);
+
+  // Notifies the TabOrderHelper that a given |action| has been removed.
+  void ActionRemoved(ToolbarActionViewController* action);
+
+  // Handles a resize, including updating any actions that want to act and
+  // updating the model.
+  void HandleResize(size_t resized_count, int tab_id);
+
+  // Handles a drag and drop, including updating any actions that want to act
+  // and updating the model.
+  void HandleDragDrop(int dragged,
+                      int dropped,
+                      ToolbarActionsBar::DragType drag_type,
+                      int tab_id);
+
+ private:
+  // TabStripModelObserver:
+  void TabInsertedAt(content::WebContents* web_contents,
+                     int index,
+                     bool foreground) override;
+  void TabDetachedAt(content::WebContents* web_contents, int index) override;
+  void TabStripModelDeleted() override;
+
+  // The set of tabs for the given action (the key) is currently "popped out".
+  // "Popped out" actions are those that were in the overflow menu normally, but
+  // want to run and are moved to the main bar so the user can see them.
+  std::map<ToolbarActionViewController*, std::set<int>> popped_out_in_tabs_;
+
+  // The set of tab ids that have been checked for whether actions need to be
+  // popped out or not.
+  std::set<int> tabs_checked_for_pop_out_;
+
+  // The owning ToolbarActionsBar.
+  ToolbarActionsBar* toolbar_;
+
+  // The associated toolbar model.
+  extensions::ExtensionToolbarModel* model_;
+
+  // A scoped tab strip observer so we can clean up |tabs_checked_for_popout_|.
+  ScopedObserver<TabStripModel, TabStripModelObserver> tab_strip_observer_;
+
+  DISALLOW_COPY_AND_ASSIGN(TabOrderHelper);
+};
+
+ToolbarActionsBar::TabOrderHelper::TabOrderHelper(
+    ToolbarActionsBar* toolbar,
+    Browser* browser,
+    extensions::ExtensionToolbarModel* model)
+    : toolbar_(toolbar),
+      model_(model),
+      tab_strip_observer_(this) {
+  tab_strip_observer_.Add(browser->tab_strip_model());
+}
+
+ToolbarActionsBar::TabOrderHelper::~TabOrderHelper() {
+}
+
+size_t ToolbarActionsBar::TabOrderHelper::GetExtraIconCount(int tab_id) {
+  size_t extra_icons = 0;
+  const WeakToolbarActions& toolbar_actions = toolbar_->toolbar_actions();
+  for (ToolbarActionViewController* action : toolbar_actions) {
+    auto actions_tabs = popped_out_in_tabs_.find(action);
+    if (actions_tabs != popped_out_in_tabs_.end() &&
+        actions_tabs->second.count(tab_id))
+      ++extra_icons;
+  }
+  return extra_icons;
+}
+
+void ToolbarActionsBar::TabOrderHelper::HandleResize(size_t resized_count,
+                                                     int tab_id) {
+  int extra = GetExtraIconCount(tab_id);
+  size_t tab_icon_count = model_->visible_icon_count() + extra;
+  bool reorder_necessary = false;
+  const WeakToolbarActions& toolbar_actions = toolbar_->toolbar_actions();
+  if (resized_count < tab_icon_count) {
+    for (int i = resized_count; i < extra; ++i) {
+      // If an extension that was popped out to act is overflowed, then it
+      // should no longer be popped out, and it also doesn't count for adjusting
+      // the visible count (since it wasn't really out to begin with).
+      if (popped_out_in_tabs_[toolbar_actions[i]].count(tab_id)) {
+        reorder_necessary = true;
+        popped_out_in_tabs_[toolbar_actions[i]].erase(tab_id);
+        ++(resized_count);
+      }
+    }
+  } else {
+    // If the user increases the toolbar size while actions that popped out are
+    // visible, we need to re-arrange the icons in other windows to be
+    // consistent with what the user sees.
+    // That is, if the normal order is A, B, [C, D] (with C and D hidden), C
+    // pops out to act, and then the user increases the size of the toolbar,
+    // the user sees uncovering D (since C is already out). This is what should
+    // happen in all windows.
+    for (size_t i = tab_icon_count; i < resized_count; ++i) {
+      if (toolbar_actions[i]->GetId() !=
+              model_->toolbar_items()[i - extra]->id())
+        model_->MoveExtensionIcon(toolbar_actions[i]->GetId(), i - extra);
+    }
+  }
+
+  resized_count -= extra;
+  model_->SetVisibleIconCount(resized_count);
+  if (reorder_necessary)
+    toolbar_->ReorderActions();
+}
+
+void ToolbarActionsBar::TabOrderHelper::HandleDragDrop(
+    int dragged_index,
+    int dropped_index,
+    ToolbarActionsBar::DragType drag_type,
+    int tab_id) {
+  const WeakToolbarActions& toolbar_actions = toolbar_->toolbar_actions();
+  ToolbarActionViewController* action = toolbar_actions[dragged_index];
+  int delta = 0;
+  switch (drag_type) {
+    case ToolbarActionsBar::DRAG_TO_OVERFLOW:
+      // If the user moves an action back into overflow, then we don't adjust
+      // the base visible count, but do stop popping that action out.
+      if (popped_out_in_tabs_[action].count(tab_id))
+        popped_out_in_tabs_[action].erase(tab_id);
+      else
+        delta = -1;
+      break;
+    case ToolbarActionsBar::DRAG_TO_MAIN:
+      delta = 1;
+      break;
+    case ToolbarActionsBar::DRAG_TO_SAME:
+      // If the user moves an action that had popped out to be on the toolbar,
+      // then we treat it as "pinning" the action, and adjust the base visible
+      // count to accommodate.
+      if (popped_out_in_tabs_[action].count(tab_id)) {
+        delta = 1;
+        popped_out_in_tabs_[action].erase(tab_id);
+      }
+      break;
+  }
+
+  // If there are any actions that are in front of the dropped index only
+  // because they were popped out, decrement the dropped index.
+  for (int i = 0; i < dropped_index; ++i) {
+    if (i != dragged_index &&
+        model_->GetIndexForId(toolbar_actions[i]->GetId()) >= dropped_index)
+      --dropped_index;
+  }
+
+  model_->MoveExtensionIcon(action->GetId(), dropped_index);
+
+  if (delta)
+    model_->SetVisibleIconCount(model_->visible_icon_count() + delta);
+}
+
+void ToolbarActionsBar::TabOrderHelper::SetActionWantsToRun(
+    ToolbarActionViewController* action,
+    int tab_id,
+    bool wants_to_run) {
+  bool is_overflowed = model_->GetIndexForId(action->GetId()) >=
+      static_cast<int>(model_->visible_icon_count());
+  bool reorder_necessary = false;
+  if (wants_to_run && is_overflowed) {
+    popped_out_in_tabs_[action].insert(tab_id);
+    reorder_necessary = true;
+  } else if (!wants_to_run && popped_out_in_tabs_[action].count(tab_id)) {
+    popped_out_in_tabs_[action].erase(tab_id);
+    reorder_necessary = true;
+  }
+  if (reorder_necessary)
+    toolbar_->ReorderActions();
+}
+
+void ToolbarActionsBar::TabOrderHelper::ActionRemoved(
+    ToolbarActionViewController* action) {
+  popped_out_in_tabs_.erase(action);
+}
+
+WeakToolbarActions ToolbarActionsBar::TabOrderHelper::GetActionOrder(
+    content::WebContents* web_contents) {
+  WeakToolbarActions toolbar_actions = toolbar_->toolbar_actions();
+  // First, make sure that we've checked any actions that want to run.
+  int tab_id = SessionTabHelper::IdForTab(web_contents);
+  if (!tabs_checked_for_pop_out_.count(tab_id)) {
+    tabs_checked_for_pop_out_.insert(tab_id);
+    for (ToolbarActionViewController* toolbar_action : toolbar_actions) {
+      if (toolbar_action->WantsToRun(web_contents))
+        popped_out_in_tabs_[toolbar_action].insert(tab_id);
+    }
+  }
+
+  // Then, shift any actions that want to run to the front.
+  size_t insert_at = 0;
+  // Rotate any actions that want to run to the boundary between visible and
+  // overflowed actions.
+  for (WeakToolbarActions::iterator iter =
+           toolbar_actions.begin() + model_->visible_icon_count();
+       iter != toolbar_actions.end(); ++iter) {
+    if (popped_out_in_tabs_[(*iter)].count(tab_id)) {
+      std::rotate(toolbar_actions.begin() + insert_at, iter, iter + 1);
+      ++insert_at;
+    }
+  }
+
+  return toolbar_actions;
+}
+
+void ToolbarActionsBar::TabOrderHelper::TabInsertedAt(
+    content::WebContents* web_contents,
+    int index,
+    bool foreground) {
+  if (foreground)
+    toolbar_->ReorderActions();
+}
+
+void ToolbarActionsBar::TabOrderHelper::TabDetachedAt(
+    content::WebContents* web_contents,
+    int index) {
+  int tab_id = SessionTabHelper::IdForTab(web_contents);
+  for (auto& tabs : popped_out_in_tabs_)
+    tabs.second.erase(tab_id);
+  tabs_checked_for_pop_out_.erase(tab_id);
+}
+
+void ToolbarActionsBar::TabOrderHelper::TabStripModelDeleted() {
+  tab_strip_observer_.RemoveAll();
+}
+
 ToolbarActionsBar::PlatformSettings::PlatformSettings(bool in_overflow_mode)
     : left_padding(in_overflow_mode ? kOverflowLeftPadding : kLeftPadding),
       right_padding(in_overflow_mode ? kOverflowRightPadding : kRightPadding),
@@ -72,19 +356,22 @@ ToolbarActionsBar::PlatformSettings::PlatformSettings(bool in_overflow_mode)
 
 ToolbarActionsBar::ToolbarActionsBar(ToolbarActionsBarDelegate* delegate,
                                      Browser* browser,
-                                     bool in_overflow_mode)
+                                     ToolbarActionsBar* main_bar)
     : delegate_(delegate),
       browser_(browser),
       model_(extensions::ExtensionToolbarModel::Get(browser_->profile())),
-      in_overflow_mode_(in_overflow_mode),
-      platform_settings_(in_overflow_mode),
+      main_bar_(main_bar),
+      overflow_bar_(nullptr),
+      platform_settings_(main_bar != nullptr),
       model_observer_(this),
-      tab_strip_observer_(this),
       suppress_layout_(false),
       suppress_animation_(true) {
   if (model_)  // |model_| can be null in unittests.
     model_observer_.Add(model_);
-  tab_strip_observer_.Add(browser_->tab_strip_model());
+  if (in_overflow_mode())
+    main_bar_->overflow_bar_ = this;
+  else if (pop_out_actions_to_run)
+    tab_order_helper_.reset(new TabOrderHelper(this, browser_, model_));
 }
 
 ToolbarActionsBar::~ToolbarActionsBar() {
@@ -92,6 +379,9 @@ ToolbarActionsBar::~ToolbarActionsBar() {
   // the order of deletion between the views and the ToolbarActionsBar.
   DCHECK(toolbar_actions_.empty()) <<
       "Must call DeleteActions() before destruction.";
+  if (in_overflow_mode())
+    main_bar_->overflow_bar_ = nullptr;
+  DCHECK(overflow_bar_ == nullptr) << "Overflow bar cannot outlive main bar";
 }
 
 // static
@@ -106,7 +396,7 @@ int ToolbarActionsBar::IconHeight() {
 
 gfx::Size ToolbarActionsBar::GetPreferredSize() const {
   int icon_count = GetIconCount();
-  if (in_overflow_mode_) {
+  if (in_overflow_mode()) {
     // In overflow, we always have a preferred size of a full row (even if we
     // don't use it), and always of at least one row. The parent may decide to
     // show us even when empty, e.g. as a drag target for dragging in icons from
@@ -174,7 +464,17 @@ size_t ToolbarActionsBar::GetIconCount() const {
   if (!model_)
     return 0u;
 
-  size_t visible_icons = model_->visible_icon_count();
+  size_t extra_icons = 0;
+  if (tab_order_helper_) {
+    extra_icons = tab_order_helper_->GetExtraIconCount(
+        SessionTabHelper::IdForTab(
+            browser_->tab_strip_model()->GetActiveWebContents()));
+  }
+
+  size_t visible_icons = in_overflow_mode() ?
+      toolbar_actions_.size() - main_bar_->GetIconCount() :
+      model_->visible_icon_count() + extra_icons;
+
 #if DCHECK_IS_ON
   // Good time for some sanity checks: We should never try to display more
   // icons than we have, and we should always have a view per item in the model.
@@ -195,18 +495,7 @@ size_t ToolbarActionsBar::GetIconCount() const {
   }
 #endif
 
-  int tab_id = SessionTabHelper::IdForTab(
-      browser_->tab_strip_model()->GetActiveWebContents());
-  for (ToolbarActionViewController* action : toolbar_actions_) {
-    auto actions_tabs = popped_out_in_tabs_.find(action);
-    if (actions_tabs != popped_out_in_tabs_.end() &&
-        actions_tabs->second.count(tab_id))
-      ++visible_icons;
-  }
-
-  // The overflow displays any icons not shown by the main bar.
-  return in_overflow_mode_ ?
-      model_->toolbar_items().size() - visible_icons : visible_icons;
+  return visible_icons;
 }
 
 void ToolbarActionsBar::CreateActions() {
@@ -246,8 +535,12 @@ void ToolbarActionsBar::CreateActions() {
       component_actions.weak_clear();
     }
 
-    if (!toolbar_actions_.empty())
-      ReorderActions();
+    if (!toolbar_actions_.empty()) {
+      if (in_overflow_mode())
+        CopyActionOrder();
+      else
+        ReorderActions();
+    }
 
     for (size_t i = 0; i < toolbar_actions_.size(); ++i)
       delegate_->AddViewForAction(toolbar_actions_[i], i);
@@ -282,93 +575,49 @@ void ToolbarActionsBar::Update() {
 }
 
 void ToolbarActionsBar::SetOverflowRowWidth(int width) {
-  DCHECK(in_overflow_mode_);
+  DCHECK(in_overflow_mode());
   platform_settings_.icons_per_overflow_menu_row =
       std::max((width - kItemSpacing) / IconWidth(true), 1);
 }
 
 void ToolbarActionsBar::OnResizeComplete(int width) {
-  DCHECK(!in_overflow_mode_);  // The user can't resize the overflow container.
+  DCHECK(!in_overflow_mode());  // The user can't resize the overflow container.
   size_t resized_count = WidthToIconCount(width);
-  size_t tab_icon_count = GetIconCount();
-  int tab_id = SessionTabHelper::IdForTab(GetCurrentWebContents());
-  int delta = tab_icon_count - model_->visible_icon_count();
-  bool reorder_necessary = false;
-  if (resized_count < tab_icon_count) {
-    for (int i = resized_count; i < delta; ++i) {
-      // If an extension that was popped out to act is overflowed, then it
-      // should no longer be popped out, and it also doesn't count for adjusting
-      // the visible count (since it wasn't really out to begin with).
-      if (popped_out_in_tabs_[toolbar_actions_[i]].count(tab_id)) {
-        reorder_necessary = true;
-        popped_out_in_tabs_[toolbar_actions_[i]].erase(tab_id);
-        ++resized_count;
-      }
-    }
+  // Save off the desired number of visible icons. We do this now instead of
+  // at the end of the animation so that even if the browser is shut down
+  // while animating, the right value will be restored on next run.
+  if (tab_order_helper_) {
+    tab_order_helper_->HandleResize(
+        resized_count,
+        SessionTabHelper::IdForTab(GetCurrentWebContents()));
   } else {
-    // If the user increases the toolbar size while actions that popped out are
-    // visible, we need to re-arrange the icons in other windows to be
-    // consistent with what the user sees.
-    // That is, if the normal order is A, B, [C, D] (with C and D hidden), C
-    // pops out to act, and then the user increases the size of the toolbar,
-    // the user sees uncovering D (since C is already out). This is what should
-    // happen in all windows.
-    for (size_t i = tab_icon_count; i < resized_count; ++i) {
-      if (toolbar_actions_[i]->GetId() !=
-              model_->toolbar_items()[i - delta]->id())
-        model_->MoveExtensionIcon(toolbar_actions_[i]->GetId(), i - delta);
-    }
+    model_->SetVisibleIconCount(resized_count);
   }
-  resized_count -= delta;
-  // Save off the desired number of visible icons.  We do this now instead of at
-  // the end of the animation so that even if the browser is shut down while
-  // animating, the right value will be restored on next run.
-  model_->SetVisibleIconCount(resized_count);
-  if (reorder_necessary)
-    ReorderActions();
 }
 
 void ToolbarActionsBar::OnDragDrop(int dragged_index,
                                    int dropped_index,
                                    DragType drag_type) {
-  ToolbarActionViewController* action = toolbar_actions_[dragged_index];
-  int tab_id = SessionTabHelper::IdForTab(GetCurrentWebContents());
-  int delta = 0;
-  switch (drag_type) {
-    case DRAG_TO_OVERFLOW:
-      // If the user moves an action back into overflow, then we don't adjust
-      // the base visible count, but do stop popping that action out.
-      if (popped_out_in_tabs_[action].count(tab_id))
-        popped_out_in_tabs_[action].erase(tab_id);
-      else
-        delta = -1;
-      break;
-    case DRAG_TO_MAIN:
+  // All drag-and-drop commands should go to the main bar.
+  DCHECK(!in_overflow_mode());
+
+  if (tab_order_helper_) {
+    tab_order_helper_->HandleDragDrop(
+        dragged_index,
+        dropped_index,
+        drag_type,
+        SessionTabHelper::IdForTab(GetCurrentWebContents()));
+  } else {
+    int delta = 0;
+    if (drag_type == DRAG_TO_OVERFLOW)
+      delta = -1;
+    else if (drag_type == DRAG_TO_MAIN)
       delta = 1;
-      break;
-    case DRAG_TO_SAME:
-      // If the user moves an action that had popped out to be on the toolbar,
-      // then we treat it as "pinning" the action, and adjust the base visible
-      // count to accommodate.
-      if (popped_out_in_tabs_[action].count(tab_id)) {
-        delta = 1;
-        popped_out_in_tabs_[action].erase(tab_id);
-      }
-      break;
-  }
-
-  // If there are any actions that are in front of the dropped index only
-  // because they were popped out, decrement the dropped index.
-  for (int i = 0; i < dropped_index; ++i) {
-    if (i != dragged_index &&
-        model_->GetIndexForId(toolbar_actions_[i]->GetId()) >= dropped_index)
-      --dropped_index;
+    model_->MoveExtensionIcon(toolbar_actions_[dragged_index]->GetId(),
+                              dropped_index);
+    if (delta)
+      model_->SetVisibleIconCount(model_->visible_icon_count() + delta);
   }
-
-  model_->MoveExtensionIcon(action->GetId(), dropped_index);
-
-  if (delta)
-    model_->SetVisibleIconCount(model_->visible_icon_count() + delta);
 }
 
 void ToolbarActionsBar::ToolbarExtensionAdded(
@@ -414,7 +663,8 @@ void ToolbarActionsBar::ToolbarExtensionRemoved(
     return;
 
   delegate_->RemoveViewForAction(*iter);
-  popped_out_in_tabs_.erase(*iter);
+  if (tab_order_helper_)
+    tab_order_helper_->ActionRemoved(*iter);
   toolbar_actions_.erase(iter);
 
   // If the extension is being upgraded we don't want the bar to shrink
@@ -454,23 +704,13 @@ void ToolbarActionsBar::ToolbarExtensionUpdated(
   if (action) {
     content::WebContents* web_contents = GetCurrentWebContents();
     action->UpdateState();
-    bool wants_to_run = action->WantsToRun(web_contents);
-
-    // The action may need to be popped in or out of overflow.
-    int index = std::find(toolbar_actions_.begin(),
-                          toolbar_actions_.end(),
-                          action) - toolbar_actions_.begin();
-    bool reorder_necessary = false;
-    int tab_id = SessionTabHelper::IdForTab(web_contents);
-    if (wants_to_run && static_cast<size_t>(index) >= GetIconCount()) {
-      popped_out_in_tabs_[action].insert(tab_id);
-      reorder_necessary = true;
-    } else if (!wants_to_run && popped_out_in_tabs_[action].count(tab_id)) {
-      popped_out_in_tabs_[action].erase(tab_id);
-      reorder_necessary = true;
+
+    if (tab_order_helper_) {
+      tab_order_helper_->SetActionWantsToRun(
+          action,
+          SessionTabHelper::IdForTab(web_contents),
+          action->WantsToRun(web_contents));
     }
-    if (reorder_necessary)
-      ReorderActions();
   }
 }
 
@@ -531,69 +771,28 @@ Browser* ToolbarActionsBar::GetBrowser() {
   return browser_;
 }
 
-void ToolbarActionsBar::TabInsertedAt(content::WebContents* web_contents,
-                                      int index,
-                                      bool foreground) {
-  if (foreground)
-    ReorderActions();
-}
-
-void ToolbarActionsBar::TabDetachedAt(content::WebContents* web_contents,
-                                      int index) {
-  int tab_id = SessionTabHelper::IdForTab(web_contents);
-  for (auto& tabs : popped_out_in_tabs_)
-    tabs.second.erase(tab_id);
-  tabs_checked_for_pop_out_.erase(tab_id);
-}
-
-void ToolbarActionsBar::TabStripModelDeleted() {
-  tab_strip_observer_.RemoveAll();
-}
-
 void ToolbarActionsBar::ReorderActions() {
-  if (toolbar_actions_.empty())
+  if (toolbar_actions_.empty() || in_overflow_mode())
     return;
 
-  // First, reset the order to that of the model. Run through the views and
-  // compare them to the model; if something is out of place, find the correct
-  // spot for it.
-  const extensions::ExtensionList& model_order = model_->toolbar_items();
-  for (int i = 0; i < static_cast<int>(model_order.size() - 1); ++i) {
-    if (model_order[i]->id() != toolbar_actions_[i]->GetId()) {
-      // Find where the correct view is (it's guaranteed to be after our current
-      // index, since everything up to this point is correct).
-      size_t j = i + 1;
-      while (model_order[i]->id() != toolbar_actions_[j]->GetId())
-        ++j;
-      std::swap(toolbar_actions_[i], toolbar_actions_[j]);
-    }
-  }
-
-  // Only adjust the order if the model isn't highlighting a particular subset.
-  if (!model_->is_highlighting()) {
-    // First, make sure that we've checked any actions that want to run.
-    content::WebContents* web_contents = GetCurrentWebContents();
-    int tab_id = SessionTabHelper::IdForTab(web_contents);
-    if (!tabs_checked_for_pop_out_.count(tab_id)) {
-      tabs_checked_for_pop_out_.insert(tab_id);
-      for (ToolbarActionViewController* toolbar_action : toolbar_actions_) {
-        if (toolbar_action->WantsToRun(web_contents))
-          popped_out_in_tabs_[toolbar_action].insert(tab_id);
-      }
-    }
-
-    // Then, shift any actions that want to run to the front.
-    size_t insert_at = 0;
-    // Rotate any actions that want to run to the boundary between visible and
-    // overflowed actions.
-    for (ToolbarActions::iterator iter =
-             toolbar_actions_.begin() + model_->visible_icon_count();
-         iter != toolbar_actions_.end(); ++iter) {
-      if (popped_out_in_tabs_[(*iter)].count(tab_id)) {
-        std::rotate(toolbar_actions_.begin() + insert_at, iter, iter + 1);
-        ++insert_at;
-      }
-    }
+  // First, reset the order to that of the model.
+  auto compare = [](ToolbarActionViewController* const& action,
+                    const scoped_refptr<const extensions::Extension>& ext) {
+    return action->GetId() == ext->id();
+  };
+  SortContainer(&toolbar_actions_.get(), model_->toolbar_items(), +compare);

[Finnur, 2014/12/12 15:06:06]

The + there is causing a compile failure.

[Devlin, 2014/12/12 18:27:06]

Ah, C++11 lambdas, aren't you lovely.

Fixed.

+
+  // Only adjust the order if the model isn't highlighting a particular
+  // subset (and the specialized tab order is enabled).
+  if (!model_->is_highlighting() && tab_order_helper_) {
+    WeakToolbarActions new_order =
+        tab_order_helper_->GetActionOrder(GetCurrentWebContents());
+    auto compare = [](ToolbarActionViewController* const& first,
+                      ToolbarActionViewController* const& second) {
+      return first == second;
+    };
+    SortContainer(
+        &toolbar_actions_.get(), new_order, +compare);
   }
 
   // Our visible browser actions may have changed - re-Layout() and check the
@@ -602,6 +801,25 @@ void ToolbarActionsBar::ReorderActions() {
     ResizeDelegate(gfx::Tween::EASE_OUT, false);
     delegate_->Redraw(true);
   }
+
+  if (overflow_bar_)
+    overflow_bar_->CopyActionOrder();
+}
+
+void ToolbarActionsBar::CopyActionOrder() {
+  DCHECK(in_overflow_mode());
+  if (!main_bar_->toolbar_actions().empty()) {
+    auto compare = [](ToolbarActionViewController* const& first,
+                      ToolbarActionViewController* const& second) {
+      return first->GetId() == second->GetId();
+    };
+    SortContainer(
+        &toolbar_actions_.get(), main_bar_->toolbar_actions(), +compare);
+    if (!suppress_layout_) {
+      ResizeDelegate(gfx::Tween::EASE_OUT, false);
+      delegate_->Redraw(true);
+    }
+  }
 }
 
 ToolbarActionViewController* ToolbarActionsBar::GetActionForId(