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

1 files changed, 38 insertions, 22 deletions

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();