honor index in createTab()

BUG=11200
R=brettw,beng

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

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

3 files changed, 65 insertions, 48 deletions

diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc
index dd06cc1..e1a93f3 100644
--- a/chrome/browser/tabs/tab_strip_model.cc
+++ b/chrome/browser/tabs/tab_strip_model.cc
@@ -102,8 +102,13 @@ void TabStripModel::InsertTabContentsAt(int index,
   FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
       TabInsertedAt(contents, index, foreground));
 
-  if (foreground)
+  if (foreground) {
     ChangeSelectedContentsFrom(selected_contents, index, false);
+  } else if (index <= selected_index_) {
+    // If a tab is inserted before the current selected index that is not
+    // foreground, |selected_index| needs to be incremented.
+    ++selected_index_;
+  }
 }
 
 void TabStripModel::ReplaceNavigationControllerAt(
@@ -327,9 +332,10 @@ bool TabStripModel::ShouldResetGroupOnSelect(TabContents* contents) const {
 
 void TabStripModel::AddTabContents(TabContents* contents,
                                    int index,
+                                   bool force_index,
                                    PageTransition::Type transition,
                                    bool foreground) {
-  if (transition == PageTransition::LINK) {
+  if (transition == PageTransition::LINK && !force_index) {
     // Only try to be clever if we're opening a LINK.
     index = order_controller_->DetermineInsertionIndex(
         contents, transition, foreground);
diff --git a/chrome/browser/tabs/tab_strip_model.h b/chrome/browser/tabs/tab_strip_model.h
index 66c6b395..7f496c8 100644
--- a/chrome/browser/tabs/tab_strip_model.h
+++ b/chrome/browser/tabs/tab_strip_model.h
@@ -372,12 +372,13 @@ class TabStripModel : public NotificationObserver {
   // Command level API /////////////////////////////////////////////////////////
 
   // Adds a TabContents at the best position in the TabStripModel given the
-  // specified insertion index, transition, etc. Ultimately, the insertion
-  // index of the TabContents is left up to the Order Controller associated
-  // with this TabStripModel, so the final insertion index may differ from
-  // |index|.
+  // specified insertion index, transition, etc. If |force_index|
+  // is false, the insertion index of the TabContents is left up to the Order
+  // Controller associated with this TabStripModel, so the final insertion index
+  // may differ from |index|.
   void AddTabContents(TabContents* contents,
                       int index,
+                      bool force_index,
                       PageTransition::Type transition,
                       bool foreground);
 
diff --git a/chrome/browser/tabs/tab_strip_model_unittest.cc b/chrome/browser/tabs/tab_strip_model_unittest.cc
index 0c77cba..a5b63a7 100644
--- a/chrome/browser/tabs/tab_strip_model_unittest.cc
+++ b/chrome/browser/tabs/tab_strip_model_unittest.cc
@@ -743,12 +743,12 @@ TEST_F(TabStripModelTest, AddTabContents_MiddleClickLinksAndClose) {
   // Open the Home Page
   TabContents* homepage_contents = CreateTabContents();
   tabstrip.AddTabContents(
-      homepage_contents, -1, PageTransition::AUTO_BOOKMARK, true);
+      homepage_contents, -1, false, PageTransition::AUTO_BOOKMARK, true);
 
   // Open some other tab, by user typing.
   TabContents* typed_page_contents = CreateTabContents();
   tabstrip.AddTabContents(
-      typed_page_contents, -1, PageTransition::TYPED, true);
+      typed_page_contents, -1, false, PageTransition::TYPED, true);
 
   EXPECT_EQ(2, tabstrip.count());
 
@@ -759,13 +759,13 @@ TEST_F(TabStripModelTest, AddTabContents_MiddleClickLinksAndClose) {
   // page.
   TabContents* middle_click_contents1 = CreateTabContents();
   tabstrip.AddTabContents(
-    middle_click_contents1, -1, PageTransition::LINK, false);
+    middle_click_contents1, -1, false, PageTransition::LINK, false);
   TabContents* middle_click_contents2 = CreateTabContents();
   tabstrip.AddTabContents(
-    middle_click_contents2, -1, PageTransition::LINK, false);
+    middle_click_contents2, -1, false, PageTransition::LINK, false);
   TabContents* middle_click_contents3 = CreateTabContents();
   tabstrip.AddTabContents(
-    middle_click_contents3, -1, PageTransition::LINK, false);
+    middle_click_contents3, -1, false, PageTransition::LINK, false);
 
   EXPECT_EQ(5, tabstrip.count());
 
@@ -807,12 +807,12 @@ TEST_F(TabStripModelTest, AddTabContents_LeftClickPopup) {
   // Open the Home Page
   TabContents* homepage_contents = CreateTabContents();
   tabstrip.AddTabContents(
-    homepage_contents, -1, PageTransition::AUTO_BOOKMARK, true);
+    homepage_contents, -1, false, PageTransition::AUTO_BOOKMARK, true);
 
   // Open some other tab, by user typing.
   TabContents* typed_page_contents = CreateTabContents();
   tabstrip.AddTabContents(
-    typed_page_contents, -1, PageTransition::TYPED, true);
+    typed_page_contents, -1, false, PageTransition::TYPED, true);
 
   EXPECT_EQ(2, tabstrip.count());
 
@@ -821,7 +821,8 @@ TEST_F(TabStripModelTest, AddTabContents_LeftClickPopup) {
 
   // Open a tab by simulating a left click on a link that opens in a new tab.
   TabContents* left_click_contents = CreateTabContents();
-  tabstrip.AddTabContents(left_click_contents, -1, PageTransition::LINK, true);
+  tabstrip.AddTabContents(left_click_contents, -1, false, PageTransition::LINK,
+      true);
 
   // Verify the state meets our expectations.
   EXPECT_EQ(3, tabstrip.count());
@@ -854,12 +855,12 @@ TEST_F(TabStripModelTest, AddTabContents_CreateNewBlankTab) {
   // Open the Home Page
   TabContents* homepage_contents = CreateTabContents();
   tabstrip.AddTabContents(
-    homepage_contents, -1, PageTransition::AUTO_BOOKMARK, true);
+    homepage_contents, -1, false, PageTransition::AUTO_BOOKMARK, true);
 
   // Open some other tab, by user typing.
   TabContents* typed_page_contents = CreateTabContents();
   tabstrip.AddTabContents(
-    typed_page_contents, -1, PageTransition::TYPED, true);
+    typed_page_contents, -1, false, PageTransition::TYPED, true);
 
   EXPECT_EQ(2, tabstrip.count());
 
@@ -868,7 +869,8 @@ TEST_F(TabStripModelTest, AddTabContents_CreateNewBlankTab) {
 
   // Open a new blank tab in the foreground.
   TabContents* new_blank_contents = CreateTabContents();
-  tabstrip.AddTabContents(new_blank_contents, -1, PageTransition::TYPED, true);
+  tabstrip.AddTabContents(new_blank_contents, -1, false, PageTransition::TYPED,
+      true);
 
   // Verify the state of the tabstrip.
   EXPECT_EQ(3, tabstrip.count());
@@ -879,10 +881,10 @@ TEST_F(TabStripModelTest, AddTabContents_CreateNewBlankTab) {
   // Now open a couple more blank tabs in the background.
   TabContents* background_blank_contents1 = CreateTabContents();
   tabstrip.AddTabContents(
-      background_blank_contents1, -1, PageTransition::TYPED, false);
+      background_blank_contents1, -1, false, PageTransition::TYPED, false);
   TabContents* background_blank_contents2 = CreateTabContents();
   tabstrip.AddTabContents(
-      background_blank_contents2, -1, PageTransition::GENERATED, false);
+      background_blank_contents2, -1, false, PageTransition::GENERATED, false);
   EXPECT_EQ(5, tabstrip.count());
   EXPECT_EQ(homepage_contents, tabstrip.GetTabContentsAt(0));
   EXPECT_EQ(typed_page_contents, tabstrip.GetTabContentsAt(1));
@@ -904,12 +906,12 @@ TEST_F(TabStripModelTest, AddTabContents_ForgetOpeners) {
   // Open the Home Page
   TabContents* homepage_contents = CreateTabContents();
   tabstrip.AddTabContents(
-    homepage_contents, -1, PageTransition::AUTO_BOOKMARK, true);
+    homepage_contents, -1, false, PageTransition::AUTO_BOOKMARK, true);
 
   // Open some other tab, by user typing.
   TabContents* typed_page_contents = CreateTabContents();
   tabstrip.AddTabContents(
-    typed_page_contents, -1, PageTransition::TYPED, true);
+    typed_page_contents, -1, false, PageTransition::TYPED, true);
 
   EXPECT_EQ(2, tabstrip.count());
 
@@ -920,13 +922,13 @@ TEST_F(TabStripModelTest, AddTabContents_ForgetOpeners) {
   // page.
   TabContents* middle_click_contents1 = CreateTabContents();
   tabstrip.AddTabContents(
-    middle_click_contents1, -1, PageTransition::LINK, false);
+    middle_click_contents1, -1, false, PageTransition::LINK, false);
   TabContents* middle_click_contents2 = CreateTabContents();
   tabstrip.AddTabContents(
-    middle_click_contents2, -1, PageTransition::LINK, false);
+    middle_click_contents2, -1, false, PageTransition::LINK, false);
   TabContents* middle_click_contents3 = CreateTabContents();
   tabstrip.AddTabContents(
-    middle_click_contents3, -1, PageTransition::LINK, false);
+    middle_click_contents3, -1, false, PageTransition::LINK, false);
 
   // Break out of the context by selecting a tab in a different context.
   EXPECT_EQ(typed_page_contents, tabstrip.GetTabContentsAt(4));
@@ -965,12 +967,12 @@ TEST_F(TabStripModelTest, AppendContentsReselectionTest) {
   // Open the Home Page
   TabContents* homepage_contents = CreateTabContents();
   tabstrip.AddTabContents(
-    homepage_contents, -1, PageTransition::AUTO_BOOKMARK, true);
+    homepage_contents, -1, false, PageTransition::AUTO_BOOKMARK, true);
 
   // Open some other tab, by user typing.
   TabContents* typed_page_contents = CreateTabContents();
   tabstrip.AddTabContents(
-    typed_page_contents, -1, PageTransition::TYPED, false);
+    typed_page_contents, -1, false, PageTransition::TYPED, false);
 
   // The selected tab should still be the first.
   EXPECT_EQ(0, tabstrip.selected_index());
@@ -995,13 +997,15 @@ TEST_F(TabStripModelTest, ReselectionConsidersChildrenTest) {
   // Open page A
   TabContents* page_a_contents = CreateTabContents();
   strip.AddTabContents(
-      page_a_contents, -1, PageTransition::AUTO_BOOKMARK, true);
+      page_a_contents, -1, false, PageTransition::AUTO_BOOKMARK, true);
 
   // Simulate middle click to open page A.A and A.B
   TabContents* page_a_a_contents = CreateTabContents();
-  strip.AddTabContents(page_a_a_contents, -1, PageTransition::LINK, false);
+  strip.AddTabContents(page_a_a_contents, -1, false, PageTransition::LINK,
+      false);
   TabContents* page_a_b_contents = CreateTabContents();
-  strip.AddTabContents(page_a_b_contents, -1, PageTransition::LINK, false);
+  strip.AddTabContents(page_a_b_contents, -1, false, PageTransition::LINK,
+      false);
 
   // Select page A.A
   strip.SelectTabContentsAt(1, true);
@@ -1009,7 +1013,8 @@ TEST_F(TabStripModelTest, ReselectionConsidersChildrenTest) {
 
   // Simulate a middle click to open page A.A.A
   TabContents* page_a_a_a_contents = CreateTabContents();
-  strip.AddTabContents(page_a_a_a_contents, -1, PageTransition::LINK, false);
+  strip.AddTabContents(page_a_a_a_contents, -1, false, PageTransition::LINK,
+      false);
 
   EXPECT_EQ(page_a_a_a_contents, strip.GetTabContentsAt(2));
 
@@ -1041,22 +1046,23 @@ TEST_F(TabStripModelTest, AddTabContents_NewTabAtEndOfStripInheritsGroup) {
 
   // Open page A
   TabContents* page_a_contents = CreateTabContents();
-  strip.AddTabContents(page_a_contents, -1, PageTransition::START_PAGE, true);
+  strip.AddTabContents(page_a_contents, -1, false, PageTransition::START_PAGE,
+      true);
 
   // Open pages B, C and D in the background from links on page A...
   TabContents* page_b_contents = CreateTabContents();
   TabContents* page_c_contents = CreateTabContents();
   TabContents* page_d_contents = CreateTabContents();
-  strip.AddTabContents(page_b_contents, -1, PageTransition::LINK, false);
-  strip.AddTabContents(page_c_contents, -1, PageTransition::LINK, false);
-  strip.AddTabContents(page_d_contents, -1, PageTransition::LINK, false);
+  strip.AddTabContents(page_b_contents, -1, false, PageTransition::LINK, false);
+  strip.AddTabContents(page_c_contents, -1, false, PageTransition::LINK, false);
+  strip.AddTabContents(page_d_contents, -1, false, PageTransition::LINK, false);
 
   // Switch to page B's tab.
   strip.SelectTabContentsAt(1, true);
 
   // Open a New Tab at the end of the strip (simulate Ctrl+T)
   TabContents* new_tab_contents = CreateTabContents();
-  strip.AddTabContents(new_tab_contents, -1, PageTransition::TYPED, true);
+  strip.AddTabContents(new_tab_contents, -1, false, PageTransition::TYPED, true);
 
   EXPECT_EQ(4, strip.GetIndexOfTabContents(new_tab_contents));
   EXPECT_EQ(4, strip.selected_index());
@@ -1071,7 +1077,7 @@ TEST_F(TabStripModelTest, AddTabContents_NewTabAtEndOfStripInheritsGroup) {
   // This is like typing a URL in the address bar and pressing Alt+Enter. The
   // behavior should be the same as above.
   TabContents* page_e_contents = CreateTabContents();
-  strip.AddTabContents(page_e_contents, -1, PageTransition::TYPED, true);
+  strip.AddTabContents(page_e_contents, -1, false, PageTransition::TYPED, true);
 
   EXPECT_EQ(4, strip.GetIndexOfTabContents(page_e_contents));
   EXPECT_EQ(4, strip.selected_index());
@@ -1086,7 +1092,7 @@ TEST_F(TabStripModelTest, AddTabContents_NewTabAtEndOfStripInheritsGroup) {
   // in New Tab". No opener relationship should be preserved between this Tab
   // and the one that was active when the gesture was performed.
   TabContents* page_f_contents = CreateTabContents();
-  strip.AddTabContents(page_f_contents, -1, PageTransition::AUTO_BOOKMARK,
+  strip.AddTabContents(page_f_contents, -1, false, PageTransition::AUTO_BOOKMARK,
                        true);
 
   EXPECT_EQ(4, strip.GetIndexOfTabContents(page_f_contents));
@@ -1111,19 +1117,20 @@ TEST_F(TabStripModelTest, NavigationForgetsOpeners) {
 
   // Open page A
   TabContents* page_a_contents = CreateTabContents();
-  strip.AddTabContents(page_a_contents, -1, PageTransition::START_PAGE, true);
+  strip.AddTabContents(page_a_contents, -1, false, PageTransition::START_PAGE,
+      true);
 
   // Open pages B, C and D in the background from links on page A...
   TabContents* page_b_contents = CreateTabContents();
   TabContents* page_c_contents = CreateTabContents();
   TabContents* page_d_contents = CreateTabContents();
-  strip.AddTabContents(page_b_contents, -1, PageTransition::LINK, false);
-  strip.AddTabContents(page_c_contents, -1, PageTransition::LINK, false);
-  strip.AddTabContents(page_d_contents, -1, PageTransition::LINK, false);
+  strip.AddTabContents(page_b_contents, -1, false, PageTransition::LINK, false);
+  strip.AddTabContents(page_c_contents, -1, false, PageTransition::LINK, false);
+  strip.AddTabContents(page_d_contents, -1, false, PageTransition::LINK, false);
 
   // Open page E in a different opener group from page A.
   TabContents* page_e_contents = CreateTabContents();
-  strip.AddTabContents(page_e_contents, -1, PageTransition::START_PAGE, false);
+  strip.AddTabContents(page_e_contents, -1, false, PageTransition::START_PAGE, false);
 
   // Tell the TabStripModel that we are navigating page D via a link click.
   strip.SelectTabContentsAt(3, true);
@@ -1155,14 +1162,15 @@ TEST_F(TabStripModelTest, NavigationForgettingDoesntAffectNewTab) {
   // Open a tab and several tabs from it, then select one of the tabs that was
   // opened.
   TabContents* page_a_contents = CreateTabContents();
-  strip.AddTabContents(page_a_contents, -1, PageTransition::START_PAGE, true);
+  strip.AddTabContents(page_a_contents, -1, false, PageTransition::START_PAGE,
+      true);
 
   TabContents* page_b_contents = CreateTabContents();
   TabContents* page_c_contents = CreateTabContents();
   TabContents* page_d_contents = CreateTabContents();
-  strip.AddTabContents(page_b_contents, -1, PageTransition::LINK, false);
-  strip.AddTabContents(page_c_contents, -1, PageTransition::LINK, false);
-  strip.AddTabContents(page_d_contents, -1, PageTransition::LINK, false);
+  strip.AddTabContents(page_b_contents, -1, false, PageTransition::LINK, false);
+  strip.AddTabContents(page_c_contents, -1, false, PageTransition::LINK, false);
+  strip.AddTabContents(page_d_contents, -1, false, PageTransition::LINK, false);
 
   strip.SelectTabContentsAt(2, true);
 
@@ -1172,7 +1180,8 @@ TEST_F(TabStripModelTest, NavigationForgettingDoesntAffectNewTab) {
 
   // Now simulate opening a new tab at the end of the TabStrip.
   TabContents* new_tab_contents1 = CreateTabContents();
-  strip.AddTabContents(new_tab_contents1, -1, PageTransition::TYPED, true);
+  strip.AddTabContents(new_tab_contents1, -1, false, PageTransition::TYPED,
+      true);
 
   // At this point, if we close this tab the last selected one should be
   // re-selected.
@@ -1185,7 +1194,8 @@ TEST_F(TabStripModelTest, NavigationForgettingDoesntAffectNewTab) {
 
   // Open a new tab again.
   TabContents* new_tab_contents2 = CreateTabContents();
-  strip.AddTabContents(new_tab_contents2, -1, PageTransition::TYPED, true);
+  strip.AddTabContents(new_tab_contents2, -1, false, PageTransition::TYPED,
+      true);
 
   // Now select the first tab.
   strip.SelectTabContentsAt(0, true);