Further tweaks to TabStripModel::MoveSelectedTabsTo.

BUG=30572
TEST=none, just make sure tab selection isn't broke.

Review URL: http://codereview.chromium.org/6646050

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@77669 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 38 insertions, 19 deletions

diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc
index c855e10..41232ac 100644
--- a/chrome/browser/tabs/tab_strip_model.cc
+++ b/chrome/browser/tabs/tab_strip_model.cc
@@ -268,31 +268,36 @@ void TabStripModel::MoveTabContentsAt(int index,
 }
 
 void TabStripModel::MoveSelectedTabsTo(int index) {
-  size_t selected_pinned_count = 0;
-  size_t selected_count = selection_model_.selected_indices().size();
-  for (size_t i = 0; i < selected_count &&
-           IsTabPinned(selection_model_.selected_indices()[i]); ++i) {
-    selected_pinned_count++;
+  int total_mini_count = IndexOfFirstNonMiniTab();
+  int selected_mini_count = 0;
+  int selected_count =
+      static_cast<int>(selection_model_.selected_indices().size());
+  for (int i = 0; i < selected_count &&
+           IsMiniTab(selection_model_.selected_indices()[i]); ++i) {
+    selected_mini_count++;
   }
 
-  size_t total_pinned_count = 0;
-  for (int i = 0; i < count() && IsTabPinned(i); ++i)
-    total_pinned_count++;
-
-  // To maintain that all pinned tabs occur before non-pinned tabs we move them
+  // To maintain that all mini-tabs occur before non-mini-tabs we move them
   // first.
-  if (selected_pinned_count > 0) {
+  if (selected_mini_count > 0) {
     MoveSelectedTabsToImpl(
-        std::min(static_cast<int>(total_pinned_count - selected_pinned_count),
-                 index), 0u, selected_pinned_count);
+        std::min(total_mini_count - selected_mini_count, index), 0u,
+        selected_mini_count);
+    if (index > total_mini_count - selected_mini_count) {
+      // We're being told to drag mini-tabs to an invalid location. Adjust the
+      // index such that non-mini-tabs end up at a location as though we could
+      // move the mini-tabs to index. See description in header for more
+      // details.
+      index += selected_mini_count;
+    }
   }
-  if (selected_pinned_count == selected_count)
+  if (selected_mini_count == selected_count)
     return;
 
   // Then move the non-pinned tabs.
-  MoveSelectedTabsToImpl(std::max(index, static_cast<int>(total_pinned_count)),
-                         selected_pinned_count,
-                         selected_count - selected_pinned_count);
+  MoveSelectedTabsToImpl(std::max(index, total_mini_count),
+                         selected_mini_count,
+                         selected_count - selected_mini_count);
 }
 
 TabContentsWrapper* TabStripModel::GetSelectedTabContents() const {
diff --git a/chrome/browser/tabs/tab_strip_model.h b/chrome/browser/tabs/tab_strip_model.h
index d825d17..04c3743 100644
--- a/chrome/browser/tabs/tab_strip_model.h
+++ b/chrome/browser/tabs/tab_strip_model.h
@@ -230,6 +230,18 @@ class TabStripModel : public NotificationObserver {
   // did not contain any of the selected tabs. For example, if the tabstrip
   // contains [A b c D E f] (upper case selected) and this is invoked with 1 the
   // result is [b A D E c f].
+  // This method maintains that all mini-tabs occur before non-mini-tabs.  When
+  // mini-tabs are selected the move is processed in two chunks: first mini-tabs
+  // are moved, then non-mini-tabs are moved. If the index is after
+  // (mini-tab-count - selected-mini-tab-count), then the index the non-mini
+  // selected tabs are moved to is (index + selected-mini-tab-count). For
+  // example, if the model consists of [A b c D E f] (A b c are mini) and this
+  // is inokved with 2, the result is [b c A D E f]. In this example nothing
+  // special happened because the target index was <= (mini-tab-count -
+  // selected-mini-tab-count). If the target index were 3, then the result would
+  // be [b c A f D F]. A, being mini, can move no further than index 2. The
+  // non-mini-tabs are moved to the target index + selected-mini-tab-count (3 +
+  // 1)
   void MoveSelectedTabsTo(int index);
 
   // Returns the currently selected TabContents, or NULL if there is none.
diff --git a/chrome/browser/tabs/tab_strip_model_unittest.cc b/chrome/browser/tabs/tab_strip_model_unittest.cc
index d6442e2..b9db965 100644
--- a/chrome/browser/tabs/tab_strip_model_unittest.cc
+++ b/chrome/browser/tabs/tab_strip_model_unittest.cc
@@ -1967,9 +1967,11 @@ TEST_F(TabStripModelTest, MoveSelectedTabsTo) {
 
     // With pinned tabs.
     { 6, 2, "2 3", 2, "0p 1p 2 3 4 5" },
-    { 6, 2, "0 4", 3, "1p 0p 2 4 3 5" },
+    { 6, 2, "0 4", 3, "1p 0p 2 3 4 5" },
     { 6, 3, "1 2 4", 0, "1p 2p 0p 4 3 5" },
-    { 8, 3, "1 3 4", 4, "0p 2p 1p 5 3 4 6 7" },
+    { 8, 3, "1 3 4", 4, "0p 2p 1p 5 6 3 4 7" },
+
+    { 7, 4, "2 3 4", 3, "0p 1p 2p 3p 5 4 6" },
   };
 
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_data); ++i) {