Implement child reordering via std::rotate() instead of erase() + ins…

…ert().

This is more performant and avoids invalidating outstanding iterators.

Bug: none
Change-Id: I73a340c6c16ce8f5c9dd0d28e7cd66a9e5f16aad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1526299
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#646083}

components/ui_devtools/ui_element.cc

@@ -76,19 +76,23 @@ void UIElement::RemoveChild(UIElement* child, bool notify_delegate) {
   children_.erase(iter);
 }
 
-void UIElement::ReorderChild(UIElement* child, int new_index) {
-  auto iter = std::find(children_.begin(), children_.end(), child);
-  DCHECK(iter != children_.end());
-
-  // Don't re-order if the new position is the same as the old position.
-  if (std::distance(children_.begin(), iter) == new_index)
+void UIElement::ReorderChild(UIElement* child, int index) {
+  auto i = std::find(children_.begin(), children_.end(), child);
+  DCHECK(i != children_.end());
+  DCHECK_GE(index, 0);
+  DCHECK_LT(static_cast<size_t>(index), children_.size());
+
+  // If |child| is already at the desired position, there's nothing to do.
+  const auto pos = std::next(children_.begin(), index);
+  if (i == pos)
     return;
-  children_.erase(iter);
 
-  // Move child to new position |new_index| in vector |children_|.
-  new_index = std::min(static_cast<int>(children_.size()) - 1, new_index);
-  iter = children_.begin() + new_index;
-  children_.insert(iter, child);
+  // Rotate |child| to be at the desired position.
+  if (pos < i)
+    std::rotate(pos, i, std::next(i));
+  else
+    std::rotate(i, std::next(i), std::next(pos));
+
   delegate()->OnUIElementReordered(child->parent(), child);
 }
 

components/ui_devtools/ui_element.h

@@ -65,8 +65,8 @@ class UI_DEVTOOLS_EXPORT UIElement {
   // OnUIElementRemoved(), which destroys the DOM node for |child|.
   void RemoveChild(UIElement* child, bool notify_delegate = true);
 
-  // Move |child| to position new_index in |children_|.
-  void ReorderChild(UIElement* child, int new_index);
+  // Moves |child| to position |index| in |children_|.
+  void ReorderChild(UIElement* child, int index);
 
   template <class T>
   int FindUIElementIdForBackendElement(T* element) const;

ui/views/view.cc

@@ -156,19 +156,24 @@ Widget* View::GetWidget() {
 
 void View::ReorderChildView(View* view, int index) {
   DCHECK_EQ(view->parent_, this);
+  const auto i = std::find(children_.begin(), children_.end(), view);
+  DCHECK(i != children_.end());
+
+  // If |view| is already at the desired position, there's nothing to do.
   const bool move_to_end = (index < 0) || (size_t{index} >= children_.size());
-  if (move_to_end)
-    index = child_count() - 1;
-  if (children_[index] == view)
+  const auto pos = move_to_end ? std::prev(children_.end())
+                               : std::next(children_.begin(), index);
+  if (i == pos)
     return;
 
-  const Views::iterator i(std::find(children_.begin(), children_.end(), view));
-  DCHECK(i != children_.end());
+    // Rotate |view| to be at the desired position.
 #if DCHECK_IS_ON()
   DCHECK(!iterating_);
 #endif
-  children_.erase(i);
-  const auto pos = children_.insert(std::next(children_.begin(), index), view);
+  if (pos < i)
+    std::rotate(pos, i, std::next(i));
+  else
+    std::rotate(i, std::next(i), std::next(pos));
 
   // Update focus siblings.  Unhook |view| from the focus cycle first so
   // SetFocusSiblings() won't traverse through it.