Attempt 2 at Moving Browser::AddTypes to TabStripModel. The first

attempt caused some flakiness on the new tab warm startup test. The
only difference between this and the last is that I'm leaving some
suspcious code in TabStripModel::InsertTabContents and
Browser::AddTabWithURL. I'll try to isolate why those changes caused
flakiness after I land the rest.

Here's the description:

Moves Browser::AddTypes to TabStripModel. This patch is primarily
cleanup before I fix 29933, but has a couple of interesting bits
beyond the enum change:
. AddTabContents now supports adding pinned.
. Converted InsertTabContents call in extensionstabmodule to pass in
  nothing (Rafael said old code was wrong).

BUG=29933
TEST=none
TBR=ben@chromium.org

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

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

3 files changed, 175 insertions, 114 deletions

diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc
index 7db00b0..5304100 100644
--- a/chrome/browser/tabs/tab_strip_model.cc
+++ b/chrome/browser/tabs/tab_strip_model.cc
@@ -105,18 +105,18 @@ bool TabStripModel::ContainsIndex(int index) const {
 }
 
 void TabStripModel::AppendTabContents(TabContents* contents, bool foreground) {
-  // Tabs opened in the foreground using this method inherit the group of the
-  // previously selected tab.
   int index = order_controller_->DetermineInsertionIndexForAppending();
-  InsertTabContentsAt(index, contents, foreground, foreground);
+  InsertTabContentsAt(index, contents,
+                      foreground ? (ADD_INHERIT_GROUP | ADD_SELECTED) :
+                                   ADD_NONE);
 }
 
 void TabStripModel::InsertTabContentsAt(int index,
                                         TabContents* contents,
-                                        bool foreground,
-                                        bool inherit_group,
-                                        bool pinned) {
-  index = ConstrainInsertionIndex(index, contents->is_app() || pinned);
+                                        int add_types) {
+  bool foreground = add_types & ADD_SELECTED;
+  index = ConstrainInsertionIndex(index, contents->is_app() ||
+                                  add_types & ADD_PINNED);
 
   // In tab dragging situations, if the last tab in the window was detached
   // then the user aborted the drag, we will have the |closing_all_| member
@@ -129,8 +129,8 @@ void TabStripModel::InsertTabContentsAt(int index,
   // since the old contents and the new contents will be the same...
   TabContents* selected_contents = GetSelectedTabContents();
   TabContentsData* data = new TabContentsData(contents);
-  data->pinned = pinned;
-  if (inherit_group && selected_contents) {
+  data->pinned = (add_types & ADD_PINNED) == ADD_PINNED;
+  if ((add_types & ADD_INHERIT_GROUP) && selected_contents) {
     if (foreground) {
       // Forget any existing relationships, we don't want to make things too
       // confusing by having multiple groups active at the same time.
@@ -150,9 +150,8 @@ void TabStripModel::InsertTabContentsAt(int index,
   FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
       TabInsertedAt(contents, index, foreground));
 
-  if (foreground) {
+  if (foreground)
     ChangeSelectedContentsFrom(selected_contents, index, false);
-  }
 }
 
 void TabStripModel::ReplaceNavigationControllerAt(
@@ -160,7 +159,9 @@ void TabStripModel::ReplaceNavigationControllerAt(
   // This appears to be OK with no flicker since no redraw event
   // occurs between the call to add an aditional tab and one to close
   // the previous tab.
-  InsertTabContentsAt(index + 1, controller->tab_contents(), true, true);
+  InsertTabContentsAt(
+      index + 1, controller->tab_contents(),
+      ADD_SELECTED | ADD_INHERIT_GROUP);
   std::vector<int> closing_tabs;
   closing_tabs.push_back(index);
   InternalCloseTabs(closing_tabs, CLOSE_NONE);
@@ -479,24 +480,24 @@ int TabStripModel::ConstrainInsertionIndex(int index, bool mini_tab) {
 
 void TabStripModel::AddTabContents(TabContents* contents,
                                    int index,
-                                   bool force_index,
                                    PageTransition::Type transition,
-                                   bool foreground) {
+                                   int add_types) {
   // If the newly-opened tab is part of the same task as the parent tab, we want
   // to inherit the parent's "group" attribute, so that if this tab is then
   // closed we'll jump back to the parent tab.
   // TODO(jbs): Perhaps instead of trying to infer this we should expose
   // inherit_group directly to callers, who may have more context
-  bool inherit_group = false;
+  bool inherit_group = (add_types & ADD_INHERIT_GROUP) == ADD_INHERIT_GROUP;
 
-  if (transition == PageTransition::LINK && !force_index) {
+  if (transition == PageTransition::LINK &&
+      (add_types & ADD_FORCE_INDEX) == 0) {
     // We assume tabs opened via link clicks are part of the same task as their
     // parent.  Note that when |force_index| is true (e.g. when the user
     // drag-and-drops a link to the tab strip), callers aren't really handling
     // link clicks, they just want to score the navigation like a link click in
     // the history backend, so we don't inherit the group in this case.
     index = order_controller_->DetermineInsertionIndex(
-        contents, transition, foreground);
+        contents, transition, add_types & ADD_SELECTED);
     inherit_group = true;
   } else {
     // For all other types, respect what was passed to us, normalizing -1s and
@@ -514,12 +515,17 @@ void TabStripModel::AddTabContents(TabContents* contents,
     // is re-selected, not the next-adjacent.
     inherit_group = true;
   }
-  InsertTabContentsAt(index, contents, foreground, inherit_group);
+  InsertTabContentsAt(
+      index, contents,
+      add_types | (inherit_group ? ADD_INHERIT_GROUP : 0));
   // Reset the index, just in case insert ended up moving it on us.
   index = GetIndexOfTabContents(contents);
+
   if (inherit_group && transition == PageTransition::TYPED)
     contents_data_.at(index)->reset_group_on_select = true;
 
+  // TODO(sky): figure out why this is here and not in InsertTabContentsAt. When
+  // here we seem to get failures in startup perf tests.
   // Ensure that the new TabContentsView begins at the same size as the
   // previous TabContentsView if it existed.  Otherwise, the initial WebKit
   // layout will be performed based on a width of 0 pixels, causing a
@@ -529,7 +535,7 @@ void TabStripModel::AddTabContents(TabContents* contents,
   // layout is performed with sane view dimensions even when we're opening a
   // new background tab.
   if (TabContents* old_contents = GetSelectedTabContents()) {
-    if (!foreground) {
+    if ((add_types & ADD_SELECTED) == 0) {
       contents->view()->SizeContents(old_contents->view()->GetContainerSize());
       // We need to hide the contents or else we get and execute paints for
       // background tabs. With enough background tabs they will steal the
diff --git a/chrome/browser/tabs/tab_strip_model.h b/chrome/browser/tabs/tab_strip_model.h
index 38954f4..f9cb205 100644
--- a/chrome/browser/tabs/tab_strip_model.h
+++ b/chrome/browser/tabs/tab_strip_model.h
@@ -303,6 +303,29 @@ class TabStripModel : public NotificationObserver {
     CLOSE_CREATE_HISTORICAL_TAB    = 1 << 1,
   };
 
+  // Constants used when adding tabs.
+  enum AddTabTypes {
+    // Used to indicate nothing special should happen to the newly inserted
+    // tab.
+    ADD_NONE          = 0,
+
+    // The tab should be selected.
+    ADD_SELECTED      = 1 << 0,
+
+    // The tab should be pinned.
+    ADD_PINNED        = 1 << 1,
+
+    // If not set the insertion index of the TabContents is left up to the Order
+    // Controller associated, so the final insertion index may differ from the
+    // specified index. Otherwise the index supplied is used.
+    ADD_FORCE_INDEX   = 1 << 2,
+
+    // If set the newly inserted tab inherits the group of the currently
+    // selected tab. If not set the tab may still inherit the group under
+    // certain situations.
+    ADD_INHERIT_GROUP = 1 << 3,
+  };
+
   static const int kNoTab = -1;
 
   // Construct a TabStripModel with a delegate to help it do certain things
@@ -360,24 +383,19 @@ class TabStripModel : public NotificationObserver {
   // foreground inherit the group of the previously selected tab.
   void AppendTabContents(TabContents* contents, bool foreground);
 
-  // TODO(sky): convert callers over to new variant, and consider using a
-  // bitmask rather than bools.
-  void InsertTabContentsAt(int index,
-                           TabContents* contents,
-                           bool foreground,
-                           bool inherit_group) {
-    InsertTabContentsAt(index, contents, foreground, inherit_group, false);
-  }
-
-  // Adds the specified TabContents in the specified location. If
-  // |inherit_group| is true, the new contents is linked to the current tab's
-  // group. This adjusts the index such that all app tabs occur before non-app
-  // tabs.
+  // Adds the specified TabContents at the specified location. |add_types| is a
+  // bitmask of AddTypes; see it for details.
+  //
+  // All append/insert methods end up in this method.
+  //
+  // NOTE: adding a tab using this method does NOT query the order controller,
+  // as such the ADD_FORCE_INDEX AddType is meaningless here.  The only time the
+  // |index| is changed is if using the index would result in breaking the
+  // constraint that all mini-tabs occur before non-mini-tabs.
+  // See also AddTabContents.
   void InsertTabContentsAt(int index,
                            TabContents* contents,
-                           bool foreground,
-                           bool inherit_group,
-                           bool pinned);
+                           int add_types);
 
   // Closes the TabContents at the specified index. This causes the TabContents
   // to be destroyed, but it may not happen immediately (e.g. if it's a
@@ -546,15 +564,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. 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|.
+  // specified insertion index, transition, etc. |add_types| is a bitmask of
+  // AddTypes; see it for details. This method ends up calling into
+  // InsertTabContentsAt to do the actual inertion.
   void AddTabContents(TabContents* contents,
                       int index,
-                      bool force_index,
                       PageTransition::Type transition,
-                      bool foreground);
+                      int add_types);
 
   // Closes the selected TabContents.
   void CloseSelectedTab();
diff --git a/chrome/browser/tabs/tab_strip_model_unittest.cc b/chrome/browser/tabs/tab_strip_model_unittest.cc
index 11fb91d..7089226 100644
--- a/chrome/browser/tabs/tab_strip_model_unittest.cc
+++ b/chrome/browser/tabs/tab_strip_model_unittest.cc
@@ -352,7 +352,7 @@ TEST_F(TabStripModelTest, TestBasicAPI) {
   // Test InsertTabContentsAt, foreground tab.
   TabContents* contents2 = CreateTabContents();
   {
-    tabstrip.InsertTabContentsAt(1, contents2, true, false);
+    tabstrip.InsertTabContentsAt(1, contents2, TabStripModel::ADD_SELECTED);
 
     EXPECT_EQ(2, tabstrip.count());
     EXPECT_EQ(2, observer.GetStateCount());
@@ -368,7 +368,7 @@ TEST_F(TabStripModelTest, TestBasicAPI) {
   // Test InsertTabContentsAt, background tab.
   TabContents* contents3 = CreateTabContents();
   {
-    tabstrip.InsertTabContentsAt(2, contents3, false, false);
+    tabstrip.InsertTabContentsAt(2, contents3, TabStripModel::ADD_NONE);
 
     EXPECT_EQ(3, tabstrip.count());
     EXPECT_EQ(1, observer.GetStateCount());
@@ -533,11 +533,16 @@ TEST_F(TabStripModelTest, TestBasicOpenerAPI) {
 
   // We use |InsertTabContentsAt| here instead of AppendTabContents so that
   // openership relationships are preserved.
-  tabstrip.InsertTabContentsAt(tabstrip.count(), contents1, false, true);
-  tabstrip.InsertTabContentsAt(tabstrip.count(), contents2, false, true);
-  tabstrip.InsertTabContentsAt(tabstrip.count(), contents3, false, true);
-  tabstrip.InsertTabContentsAt(tabstrip.count(), contents4, false, true);
-  tabstrip.InsertTabContentsAt(tabstrip.count(), contents5, false, true);
+  tabstrip.InsertTabContentsAt(tabstrip.count(), contents1,
+                               TabStripModel::ADD_INHERIT_GROUP);
+  tabstrip.InsertTabContentsAt(tabstrip.count(), contents2,
+                               TabStripModel::ADD_INHERIT_GROUP);
+  tabstrip.InsertTabContentsAt(tabstrip.count(), contents3,
+                               TabStripModel::ADD_INHERIT_GROUP);
+  tabstrip.InsertTabContentsAt(tabstrip.count(), contents4,
+                               TabStripModel::ADD_INHERIT_GROUP);
+  tabstrip.InsertTabContentsAt(tabstrip.count(), contents5,
+                               TabStripModel::ADD_INHERIT_GROUP);
 
   // All the tabs should have the same opener.
   for (int i = 1; i < tabstrip.count(); ++i)
@@ -580,11 +585,11 @@ static void InsertTabContentses(TabStripModel* tabstrip,
                                 TabContents* contents2,
                                 TabContents* contents3) {
   tabstrip->InsertTabContentsAt(GetInsertionIndex(tabstrip, contents1),
-                                contents1, false, true);
+                                contents1, TabStripModel::ADD_INHERIT_GROUP);
   tabstrip->InsertTabContentsAt(GetInsertionIndex(tabstrip, contents2),
-                                contents2, false, true);
+                                contents2, TabStripModel::ADD_INHERIT_GROUP);
   tabstrip->InsertTabContentsAt(GetInsertionIndex(tabstrip, contents3),
-                                contents3, false, true);
+                                contents3, TabStripModel::ADD_INHERIT_GROUP);
 }
 
 // Tests opening background tabs.
@@ -699,7 +704,9 @@ TEST_F(TabStripModelTest, TestInsertionIndexDetermination) {
   int insert_index = tabstrip.order_controller()->DetermineInsertionIndex(
       fg_link_contents, PageTransition::LINK, true);
   EXPECT_EQ(1, insert_index);
-  tabstrip.InsertTabContentsAt(insert_index, fg_link_contents, true, true);
+  tabstrip.InsertTabContentsAt(insert_index, fg_link_contents,
+                               TabStripModel::ADD_SELECTED |
+                               TabStripModel::ADD_INHERIT_GROUP);
   EXPECT_EQ(1, tabstrip.selected_index());
   EXPECT_EQ(fg_link_contents, tabstrip.GetSelectedTabContents());
 
@@ -713,7 +720,8 @@ TEST_F(TabStripModelTest, TestInsertionIndexDetermination) {
       fg_nonlink_contents, PageTransition::AUTO_BOOKMARK, true);
   EXPECT_EQ(tabstrip.count(), insert_index);
   // We break the opener relationship...
-  tabstrip.InsertTabContentsAt(insert_index, fg_nonlink_contents, false, false);
+  tabstrip.InsertTabContentsAt(insert_index, fg_nonlink_contents,
+                               TabStripModel::ADD_NONE);
   // Now select it, so that user_gesture == true causes the opener relationship
   // to be forgotten...
   tabstrip.SelectTabContentsAt(tabstrip.count() - 1, true);
@@ -801,10 +809,12 @@ TEST_F(TabStripModelTest, TestSelectOnClose) {
   // Finally test that when a tab has no "siblings" that the opener is
   // selected.
   TabContents* other_contents = CreateTabContents();
-  tabstrip.InsertTabContentsAt(1, other_contents, false, false);
+  tabstrip.InsertTabContentsAt(1, other_contents, TabStripModel::ADD_NONE);
   EXPECT_EQ(2, tabstrip.count());
   TabContents* opened_contents = CreateTabContents();
-  tabstrip.InsertTabContentsAt(2, opened_contents, true, true);
+  tabstrip.InsertTabContentsAt(2, opened_contents,
+                               TabStripModel::ADD_SELECTED |
+                               TabStripModel::ADD_INHERIT_GROUP);
   EXPECT_EQ(2, tabstrip.selected_index());
   tabstrip.CloseTabContentsAt(2, TabStripModel::CLOSE_NONE);
   EXPECT_EQ(0, tabstrip.selected_index());
@@ -960,12 +970,14 @@ TEST_F(TabStripModelTest, AddTabContents_MiddleClickLinksAndClose) {
   // Open the Home Page
   TabContents* homepage_contents = CreateTabContents();
   tabstrip.AddTabContents(
-      homepage_contents, -1, false, PageTransition::AUTO_BOOKMARK, true);
+      homepage_contents, -1, PageTransition::AUTO_BOOKMARK,
+      TabStripModel::ADD_SELECTED);
 
   // Open some other tab, by user typing.
   TabContents* typed_page_contents = CreateTabContents();
   tabstrip.AddTabContents(
-      typed_page_contents, -1, false, PageTransition::TYPED, true);
+      typed_page_contents, -1, PageTransition::TYPED,
+      TabStripModel::ADD_SELECTED);
 
   EXPECT_EQ(2, tabstrip.count());
 
@@ -976,13 +988,16 @@ TEST_F(TabStripModelTest, AddTabContents_MiddleClickLinksAndClose) {
   // page.
   TabContents* middle_click_contents1 = CreateTabContents();
   tabstrip.AddTabContents(
-    middle_click_contents1, -1, false, PageTransition::LINK, false);
+      middle_click_contents1, -1, PageTransition::LINK,
+      TabStripModel::ADD_NONE);
   TabContents* middle_click_contents2 = CreateTabContents();
   tabstrip.AddTabContents(
-    middle_click_contents2, -1, false, PageTransition::LINK, false);
+      middle_click_contents2, -1, PageTransition::LINK,
+      TabStripModel::ADD_NONE);
   TabContents* middle_click_contents3 = CreateTabContents();
   tabstrip.AddTabContents(
-    middle_click_contents3, -1, false, PageTransition::LINK, false);
+      middle_click_contents3, -1, PageTransition::LINK,
+      TabStripModel::ADD_NONE);
 
   EXPECT_EQ(5, tabstrip.count());
 
@@ -1024,12 +1039,14 @@ TEST_F(TabStripModelTest, AddTabContents_LeftClickPopup) {
   // Open the Home Page
   TabContents* homepage_contents = CreateTabContents();
   tabstrip.AddTabContents(
-    homepage_contents, -1, false, PageTransition::AUTO_BOOKMARK, true);
+      homepage_contents, -1, PageTransition::AUTO_BOOKMARK,
+      TabStripModel::ADD_SELECTED);
 
   // Open some other tab, by user typing.
   TabContents* typed_page_contents = CreateTabContents();
   tabstrip.AddTabContents(
-    typed_page_contents, -1, false, PageTransition::TYPED, true);
+      typed_page_contents, -1, PageTransition::TYPED,
+      TabStripModel::ADD_SELECTED);
 
   EXPECT_EQ(2, tabstrip.count());
 
@@ -1038,8 +1055,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, false, PageTransition::LINK,
-      true);
+  tabstrip.AddTabContents(left_click_contents, -1, PageTransition::LINK,
+                          TabStripModel::ADD_SELECTED);
 
   // Verify the state meets our expectations.
   EXPECT_EQ(3, tabstrip.count());
@@ -1072,12 +1089,14 @@ TEST_F(TabStripModelTest, AddTabContents_CreateNewBlankTab) {
   // Open the Home Page
   TabContents* homepage_contents = CreateTabContents();
   tabstrip.AddTabContents(
-    homepage_contents, -1, false, PageTransition::AUTO_BOOKMARK, true);
+      homepage_contents, -1, PageTransition::AUTO_BOOKMARK,
+      TabStripModel::ADD_SELECTED);
 
   // Open some other tab, by user typing.
   TabContents* typed_page_contents = CreateTabContents();
   tabstrip.AddTabContents(
-    typed_page_contents, -1, false, PageTransition::TYPED, true);
+      typed_page_contents, -1, PageTransition::TYPED,
+      TabStripModel::ADD_SELECTED);
 
   EXPECT_EQ(2, tabstrip.count());
 
@@ -1086,8 +1105,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, false, PageTransition::TYPED,
-      true);
+  tabstrip.AddTabContents(new_blank_contents, -1, PageTransition::TYPED,
+                          TabStripModel::ADD_SELECTED);
 
   // Verify the state of the tabstrip.
   EXPECT_EQ(3, tabstrip.count());
@@ -1098,10 +1117,12 @@ 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, false, PageTransition::TYPED, false);
+      background_blank_contents1, -1, PageTransition::TYPED,
+      TabStripModel::ADD_NONE);
   TabContents* background_blank_contents2 = CreateTabContents();
   tabstrip.AddTabContents(
-      background_blank_contents2, -1, false, PageTransition::GENERATED, false);
+      background_blank_contents2, -1, PageTransition::GENERATED,
+      TabStripModel::ADD_NONE);
   EXPECT_EQ(5, tabstrip.count());
   EXPECT_EQ(homepage_contents, tabstrip.GetTabContentsAt(0));
   EXPECT_EQ(typed_page_contents, tabstrip.GetTabContentsAt(1));
@@ -1123,12 +1144,14 @@ TEST_F(TabStripModelTest, AddTabContents_ForgetOpeners) {
   // Open the Home Page
   TabContents* homepage_contents = CreateTabContents();
   tabstrip.AddTabContents(
-    homepage_contents, -1, false, PageTransition::AUTO_BOOKMARK, true);
+      homepage_contents, -1, PageTransition::AUTO_BOOKMARK,
+      TabStripModel::ADD_SELECTED);
 
   // Open some other tab, by user typing.
   TabContents* typed_page_contents = CreateTabContents();
   tabstrip.AddTabContents(
-    typed_page_contents, -1, false, PageTransition::TYPED, true);
+      typed_page_contents, -1, PageTransition::TYPED,
+      TabStripModel::ADD_SELECTED);
 
   EXPECT_EQ(2, tabstrip.count());
 
@@ -1139,13 +1162,16 @@ TEST_F(TabStripModelTest, AddTabContents_ForgetOpeners) {
   // page.
   TabContents* middle_click_contents1 = CreateTabContents();
   tabstrip.AddTabContents(
-    middle_click_contents1, -1, false, PageTransition::LINK, false);
+      middle_click_contents1, -1, PageTransition::LINK,
+      TabStripModel::ADD_NONE);
   TabContents* middle_click_contents2 = CreateTabContents();
   tabstrip.AddTabContents(
-    middle_click_contents2, -1, false, PageTransition::LINK, false);
+      middle_click_contents2, -1, PageTransition::LINK,
+      TabStripModel::ADD_NONE);
   TabContents* middle_click_contents3 = CreateTabContents();
   tabstrip.AddTabContents(
-    middle_click_contents3, -1, false, PageTransition::LINK, false);
+      middle_click_contents3, -1, PageTransition::LINK,
+      TabStripModel::ADD_NONE);
 
   // Break out of the context by selecting a tab in a different context.
   EXPECT_EQ(typed_page_contents, tabstrip.GetTabContentsAt(4));
@@ -1184,12 +1210,14 @@ TEST_F(TabStripModelTest, AppendContentsReselectionTest) {
   // Open the Home Page
   TabContents* homepage_contents = CreateTabContents();
   tabstrip.AddTabContents(
-    homepage_contents, -1, false, PageTransition::AUTO_BOOKMARK, true);
+      homepage_contents, -1, PageTransition::AUTO_BOOKMARK,
+      TabStripModel::ADD_SELECTED);
 
   // Open some other tab, by user typing.
   TabContents* typed_page_contents = CreateTabContents();
   tabstrip.AddTabContents(
-    typed_page_contents, -1, false, PageTransition::TYPED, false);
+      typed_page_contents, -1, PageTransition::TYPED,
+      TabStripModel::ADD_NONE);
 
   // The selected tab should still be the first.
   EXPECT_EQ(0, tabstrip.selected_index());
@@ -1214,15 +1242,16 @@ TEST_F(TabStripModelTest, ReselectionConsidersChildrenTest) {
   // Open page A
   TabContents* page_a_contents = CreateTabContents();
   strip.AddTabContents(
-      page_a_contents, -1, false, PageTransition::AUTO_BOOKMARK, true);
+      page_a_contents, -1, PageTransition::AUTO_BOOKMARK,
+      TabStripModel::ADD_SELECTED);
 
   // Simulate middle click to open page A.A and A.B
   TabContents* page_a_a_contents = CreateTabContents();
-  strip.AddTabContents(page_a_a_contents, -1, false, PageTransition::LINK,
-      false);
+  strip.AddTabContents(page_a_a_contents, -1, PageTransition::LINK,
+                       TabStripModel::ADD_NONE);
   TabContents* page_a_b_contents = CreateTabContents();
-  strip.AddTabContents(page_a_b_contents, -1, false, PageTransition::LINK,
-      false);
+  strip.AddTabContents(page_a_b_contents, -1, PageTransition::LINK,
+                       TabStripModel::ADD_NONE);
 
   // Select page A.A
   strip.SelectTabContentsAt(1, true);
@@ -1230,8 +1259,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, false, PageTransition::LINK,
-      false);
+  strip.AddTabContents(page_a_a_a_contents, -1, PageTransition::LINK,
+                       TabStripModel::ADD_NONE);
 
   EXPECT_EQ(page_a_a_a_contents, strip.GetTabContentsAt(2));
 
@@ -1263,24 +1292,27 @@ TEST_F(TabStripModelTest, AddTabContents_NewTabAtEndOfStripInheritsGroup) {
 
   // Open page A
   TabContents* page_a_contents = CreateTabContents();
-  strip.AddTabContents(page_a_contents, -1, false, PageTransition::START_PAGE,
-      true);
+  strip.AddTabContents(page_a_contents, -1, PageTransition::START_PAGE,
+                       TabStripModel::ADD_SELECTED);
 
   // 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, 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.AddTabContents(page_b_contents, -1, PageTransition::LINK,
+                       TabStripModel::ADD_NONE);
+  strip.AddTabContents(page_c_contents, -1, PageTransition::LINK,
+                       TabStripModel::ADD_NONE);
+  strip.AddTabContents(page_d_contents, -1, PageTransition::LINK,
+                       TabStripModel::ADD_NONE);
 
   // 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, false, PageTransition::TYPED,
-                       true);
+  strip.AddTabContents(new_tab_contents, -1, PageTransition::TYPED,
+                       TabStripModel::ADD_SELECTED);
 
   EXPECT_EQ(4, strip.GetIndexOfTabContents(new_tab_contents));
   EXPECT_EQ(4, strip.selected_index());
@@ -1295,7 +1327,8 @@ 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, false, PageTransition::TYPED, true);
+  strip.AddTabContents(page_e_contents, -1, PageTransition::TYPED,
+                       TabStripModel::ADD_SELECTED);
 
   EXPECT_EQ(4, strip.GetIndexOfTabContents(page_e_contents));
   EXPECT_EQ(4, strip.selected_index());
@@ -1310,8 +1343,8 @@ 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, false,
-                       PageTransition::AUTO_BOOKMARK, true);
+  strip.AddTabContents(page_f_contents, -1, PageTransition::AUTO_BOOKMARK,
+                       TabStripModel::ADD_SELECTED);
 
   EXPECT_EQ(4, strip.GetIndexOfTabContents(page_f_contents));
   EXPECT_EQ(4, strip.selected_index());
@@ -1335,21 +1368,24 @@ TEST_F(TabStripModelTest, NavigationForgetsOpeners) {
 
   // Open page A
   TabContents* page_a_contents = CreateTabContents();
-  strip.AddTabContents(page_a_contents, -1, false, PageTransition::START_PAGE,
-      true);
+  strip.AddTabContents(page_a_contents, -1, PageTransition::START_PAGE,
+                       TabStripModel::ADD_SELECTED);
 
   // 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, 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.AddTabContents(page_b_contents, -1, PageTransition::LINK,
+                       TabStripModel::ADD_NONE);
+  strip.AddTabContents(page_c_contents, -1, PageTransition::LINK,
+                       TabStripModel::ADD_NONE);
+  strip.AddTabContents(page_d_contents, -1, PageTransition::LINK,
+                       TabStripModel::ADD_NONE);
 
   // Open page E in a different opener group from page A.
   TabContents* page_e_contents = CreateTabContents();
-  strip.AddTabContents(page_e_contents, -1, false,
-                       PageTransition::START_PAGE, false);
+  strip.AddTabContents(page_e_contents, -1, PageTransition::START_PAGE,
+                       TabStripModel::ADD_NONE);
 
   // Tell the TabStripModel that we are navigating page D via a link click.
   strip.SelectTabContentsAt(3, true);
@@ -1381,15 +1417,18 @@ 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, false, PageTransition::START_PAGE,
-      true);
+  strip.AddTabContents(page_a_contents, -1, PageTransition::START_PAGE,
+                       TabStripModel::ADD_SELECTED);
 
   TabContents* page_b_contents = CreateTabContents();
   TabContents* page_c_contents = CreateTabContents();
   TabContents* page_d_contents = CreateTabContents();
-  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.AddTabContents(page_b_contents, -1, PageTransition::LINK,
+                       TabStripModel::ADD_NONE);
+  strip.AddTabContents(page_c_contents, -1, PageTransition::LINK,
+                       TabStripModel::ADD_NONE);
+  strip.AddTabContents(page_d_contents, -1, PageTransition::LINK,
+                       TabStripModel::ADD_NONE);
 
   strip.SelectTabContentsAt(2, true);
 
@@ -1399,8 +1438,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, false, PageTransition::TYPED,
-      true);
+  strip.AddTabContents(new_tab_contents1, -1, PageTransition::TYPED,
+                       TabStripModel::ADD_SELECTED);
 
   // At this point, if we close this tab the last selected one should be
   // re-selected.
@@ -1413,8 +1452,8 @@ TEST_F(TabStripModelTest, NavigationForgettingDoesntAffectNewTab) {
 
   // Open a new tab again.
   TabContents* new_tab_contents2 = CreateTabContents();
-  strip.AddTabContents(new_tab_contents2, -1, false, PageTransition::TYPED,
-      true);
+  strip.AddTabContents(new_tab_contents2, -1, PageTransition::TYPED,
+                       TabStripModel::ADD_SELECTED);
 
   // Now select the first tab.
   strip.SelectTabContentsAt(0, true);
@@ -1526,7 +1565,7 @@ TEST_F(TabStripModelTest, Apps) {
   // Attempt to insert tab1 (an app tab) at position 1. This isn't a legal
   // position and tab1 should end up at position 0.
   {
-    tabstrip.InsertTabContentsAt(1, contents1, false, false);
+    tabstrip.InsertTabContentsAt(1, contents1, TabStripModel::ADD_NONE);
 
     ASSERT_EQ(1, observer.GetStateCount());
     State state(contents1, 0, MockTabStripModelObserver::INSERT);
@@ -1540,7 +1579,7 @@ TEST_F(TabStripModelTest, Apps) {
 
   // Insert tab 2 at position 1.
   {
-    tabstrip.InsertTabContentsAt(1, contents2, false, false);
+    tabstrip.InsertTabContentsAt(1, contents2, TabStripModel::ADD_NONE);
 
     ASSERT_EQ(1, observer.GetStateCount());
     State state(contents2, 1, MockTabStripModelObserver::INSERT);
@@ -1596,7 +1635,7 @@ TEST_F(TabStripModelTest, Apps) {
     tabstrip.DetachTabContentsAt(2);
     observer.ClearStates();
 
-    tabstrip.InsertTabContentsAt(0, contents3, false, false);
+    tabstrip.InsertTabContentsAt(0, contents3, TabStripModel::ADD_NONE);
 
     ASSERT_EQ(1, observer.GetStateCount());
     State state(contents3, 2, MockTabStripModelObserver::INSERT);
@@ -1757,7 +1796,7 @@ TEST_F(TabStripModelTest, Pinning) {
   // Insert "4" between "1" and "3". As "1" and "4" are pinned, "4" should end
   // up after them.
   {
-    tabstrip.InsertTabContentsAt(1, contents4, false, false);
+    tabstrip.InsertTabContentsAt(1, contents4, TabStripModel::ADD_NONE);
 
     ASSERT_EQ(1, observer.GetStateCount());
     State state(contents4, 2, MockTabStripModelObserver::INSERT);