Adjusts tab strip model to deal with app tabs. There were a couple of

places where I left them using the variable with pinned when it should
be app because those places need to be radically whacked. I'll do that
next.

BUG=32845
TEST=none yet

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

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

4 files changed, 191 insertions, 300 deletions

diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc
index 409815a..4e3708c 100644
--- a/chrome/browser/tabs/tab_strip_model.cc
+++ b/chrome/browser/tabs/tab_strip_model.cc
@@ -102,6 +102,13 @@ void TabStripModel::InsertTabContentsAt(int index,
                                         TabContents* contents,
                                         bool foreground,
                                         bool inherit_group) {
+  // Make sure the index maintains that all app tab occurs before non-app tabs.
+  int first_non_app = IndexOfFirstNonAppTab();
+  if (contents->app())
+    index = std::min(first_non_app, index);
+  else
+    index = std::max(first_non_app, index);
+
   // 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
   // set (see DetachTabContentsAt) which will mess with our mojo here. We need
@@ -113,7 +120,6 @@ 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 = (index != count() && index < IndexOfFirstNonPinnedTab());
   if (inherit_group && selected_contents) {
     if (foreground) {
       // Forget any existing relationships, we don't want to make things too
@@ -190,7 +196,33 @@ void TabStripModel::SelectTabContentsAt(int index, bool user_gesture) {
 
 void TabStripModel::MoveTabContentsAt(int index, int to_position,
                                       bool select_after_move) {
-  MoveTabContentsAtImpl(index, to_position, select_after_move, true);
+  DCHECK(ContainsIndex(index));
+  if (index == to_position)
+    return;
+
+  int first_non_app_tab = IndexOfFirstNonAppTab();
+  if ((index < first_non_app_tab && to_position >= first_non_app_tab) ||
+      (to_position < first_non_app_tab && index >= first_non_app_tab)) {
+    // This would result in app tabs mixed with non-app tabs. We don't allow
+    // that.
+    return;
+  }
+
+  TabContentsData* moved_data = contents_data_.at(index);
+  contents_data_.erase(contents_data_.begin() + index);
+  contents_data_.insert(contents_data_.begin() + to_position, moved_data);
+
+  // if !select_after_move, keep the same tab selected as was selected before.
+  if (select_after_move || index == selected_index_) {
+    selected_index_ = to_position;
+  } else if (index < selected_index_ && to_position >= selected_index_) {
+    selected_index_--;
+  } else if (index > selected_index_ && to_position <= selected_index_) {
+    selected_index_++;
+  }
+
+  FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
+                    TabMoved(moved_data->contents, index, to_position));
 }
 
 TabContents* TabStripModel::GetSelectedTabContents() const {
@@ -366,29 +398,19 @@ void TabStripModel::SetTabBlocked(int index, bool blocked) {
 
 void TabStripModel::SetTabPinned(int index, bool pinned) {
   DCHECK(ContainsIndex(index));
+  int first_non_app_tab = IndexOfFirstNonAppTab();
+  if (index >= first_non_app_tab)
+    return;  // Only allow pinning of app tabs.
+
   if (contents_data_[index]->pinned == pinned)
     return;
 
-  int first_non_pinned_tab = IndexOfFirstNonPinnedTab();
-
   contents_data_[index]->pinned = pinned;
 
-  if (pinned && index > first_non_pinned_tab) {
-    // The tab is being pinned but beyond the pinned tabs. Move it to the end
-    // of the pinned tabs.
-    MoveTabContentsAtImpl(index, first_non_pinned_tab,
-                          selected_index() == index, false);
-  } else if (!pinned && index < first_non_pinned_tab - 1) {
-    // The tab is being unpinned, but is within the pinned tabs, move it to
-    // be after the set of pinned tabs.
-    MoveTabContentsAtImpl(index, first_non_pinned_tab - 1,
-                          selected_index() == index, false);
-  } else {
-    // Tab didn't move, but it's pinned state changed. Notify observers.
-    FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
+  // Notify observers of new state.
+  FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
                       TabPinnedStateChanged(contents_data_[index]->contents,
                                             index));
-  }
 }
 
 bool TabStripModel::IsTabPinned(int index) const {
@@ -396,12 +418,11 @@ bool TabStripModel::IsTabPinned(int index) const {
 }
 
 bool TabStripModel::IsAppTab(int index) const {
-  // TODO (sky): this is temporary and should be integrated with real apps.
-  return browser_defaults::kPinnedTabsActLikeApps && IsTabPinned(index);
+  return GetTabContentsAt(index)->app();
 }
 
 bool TabStripModel::IsPhantomTab(int index) const {
-  return IsTabPinned(index) && IsAppTab(index) &&
+  return IsTabPinned(index) &&
          GetTabContentsAt(index)->controller().needs_reload();
 }
 
@@ -409,12 +430,12 @@ bool TabStripModel::IsTabBlocked(int index) const {
   return contents_data_[index]->blocked;
 }
 
-int TabStripModel::IndexOfFirstNonPinnedTab() const {
+int TabStripModel::IndexOfFirstNonAppTab() const {
   for (size_t i = 0; i < contents_data_.size(); ++i) {
-    if (!contents_data_[i]->pinned)
+    if (!contents_data_[i]->contents->app())
       return static_cast<int>(i);
   }
-  // No pinned tabs.
+  // No app tabs.
   return count();
 }
 
@@ -519,7 +540,8 @@ bool TabStripModel::IsContextMenuCommandEnabled(
   switch (command_id) {
     case CommandNewTab:
     case CommandCloseTab:
-      return true;
+      // Phantom tabs can't be closed.
+      return !IsPhantomTab(context_index);
     case CommandReload:
       if (TabContents* contents = GetTabContentsAt(context_index)) {
         return contents->delegate()->CanReloadContents(contents);
@@ -527,20 +549,24 @@ bool TabStripModel::IsContextMenuCommandEnabled(
         return false;
       }
     case CommandCloseOtherTabs:
-      return count() > 1;
+      // Close other doesn't effect app tabs.
+      return count() > IndexOfFirstNonAppTab();
     case CommandCloseTabsToRight:
-      return context_index < (count() - 1);
+      // Close doesn't effect app tabs.
+      return count() != IndexOfFirstNonAppTab() &&
+          context_index < (count() - 1);
     case CommandCloseTabsOpenedBy: {
       int next_index = GetIndexOfNextTabContentsOpenedBy(
           &GetTabContentsAt(context_index)->controller(), context_index, true);
-      return next_index != kNoTab;
+      return next_index != kNoTab && !IsAppTab(next_index);
     }
     case CommandDuplicate:
       return delegate_->CanDuplicateContentsAt(context_index);
     case CommandRestoreTab:
       return delegate_->CanRestoreTab();
     case CommandTogglePinned:
-      return true;
+      // Only app tabs can be pinned.
+      return IsAppTab(context_index);
     case CommandBookmarkAllTabs: {
       return delegate_->CanBookmarkAllTabs();
     }
@@ -575,7 +601,7 @@ void TabStripModel::ExecuteContextMenuCommand(
       TabContents* contents = GetTabContentsAt(context_index);
       std::vector<int> closing_tabs;
       for (int i = count() - 1; i >= 0; --i) {
-        if (GetTabContentsAt(i) != contents && !IsTabPinned(i))
+        if (GetTabContentsAt(i) != contents && !IsAppTab(i))
           closing_tabs.push_back(i);
       }
       InternalCloseTabs(closing_tabs, true);
@@ -585,7 +611,7 @@ void TabStripModel::ExecuteContextMenuCommand(
       UserMetrics::RecordAction("TabContextMenu_CloseTabsToRight", profile_);
       std::vector<int> closing_tabs;
       for (int i = count() - 1; i > context_index; --i) {
-        if (!IsTabPinned(i))
+        if (!IsAppTab(i))
           closing_tabs.push_back(i);
       }
       InternalCloseTabs(closing_tabs, true);
@@ -596,7 +622,7 @@ void TabStripModel::ExecuteContextMenuCommand(
       std::vector<int> closing_tabs = GetIndexesOpenedBy(context_index);
       for (std::vector<int>::iterator i = closing_tabs.begin();
            i != closing_tabs.end();) {
-        if (IsTabPinned(*i))
+        if (IsAppTab(*i))
           i = closing_tabs.erase(i);
         else
           ++i;
@@ -736,43 +762,6 @@ bool TabStripModel::InternalCloseTabs(std::vector<int> indices,
   return retval;
 }
 
-void TabStripModel::MoveTabContentsAtImpl(int index, int to_position,
-                                          bool select_after_move,
-                                          bool update_pinned_state) {
-  DCHECK(ContainsIndex(index));
-  if (index == to_position)
-    return;
-
-  bool pinned_state_changed = !update_pinned_state;
-
-  if (update_pinned_state) {
-    bool is_pinned = IsTabPinned(index);
-    if (is_pinned && to_position >= IndexOfFirstNonPinnedTab()) {
-      contents_data_[index]->pinned = false;
-    } else if (!is_pinned && to_position < IndexOfFirstNonPinnedTab()) {
-      contents_data_[index]->pinned = true;
-    }
-    pinned_state_changed = is_pinned != contents_data_[index]->pinned;
-  }
-
-  TabContentsData* moved_data = contents_data_.at(index);
-  contents_data_.erase(contents_data_.begin() + index);
-  contents_data_.insert(contents_data_.begin() + to_position, moved_data);
-
-  // if !select_after_move, keep the same tab selected as was selected before.
-  if (select_after_move || index == selected_index_) {
-    selected_index_ = to_position;
-  } else if (index < selected_index_ && to_position >= selected_index_) {
-    selected_index_--;
-  } else if (index > selected_index_ && to_position <= selected_index_) {
-    selected_index_++;
-  }
-
-  FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
-                    TabMoved(moved_data->contents, index, to_position,
-                             pinned_state_changed));
-}
-
 TabContents* TabStripModel::GetContentsAt(int index) const {
   CHECK(ContainsIndex(index)) <<
       "Failed to find: " << index << " in: " << count() << " entries.";
@@ -815,8 +804,7 @@ void TabStripModel::SelectRelativeTab(bool next) {
   int delta = next ? 1 : -1;
   do {
     index = (index + count() + delta) % count();
-  } while (index != selected_index_ && IsTabPinned(index) && IsAppTab(index) &&
-           IsPhantomTab(index));
+  } while (index != selected_index_ && IsPhantomTab(index));
   SelectTabContentsAt(index, true);
 }
 
diff --git a/chrome/browser/tabs/tab_strip_model.h b/chrome/browser/tabs/tab_strip_model.h
index 788aaa3..24eb20e 100644
--- a/chrome/browser/tabs/tab_strip_model.h
+++ b/chrome/browser/tabs/tab_strip_model.h
@@ -59,21 +59,21 @@ class TabStripModelObserver {
   // (selected).
   virtual void TabInsertedAt(TabContents* contents,
                              int index,
-                             bool foreground) { }
+                             bool foreground) {}
 
   // The specified TabContents at |index| is being closed (and eventually
   // destroyed).
-  virtual void TabClosingAt(TabContents* contents, int index) { }
+  virtual void TabClosingAt(TabContents* contents, int index) {}
 
   // The specified TabContents at |index| is being detached, perhaps to be
   // inserted in another TabStripModel. The implementer should take whatever
   // action is necessary to deal with the TabContents no longer being present.
-  virtual void TabDetachedAt(TabContents* contents, int index) { }
+  virtual void TabDetachedAt(TabContents* contents, int index) {}
 
   // The selected TabContents is about to change from |old_contents| at |index|.
   // This gives observers a chance to prepare for an impending switch before it
   // happens.
-  virtual void TabDeselectedAt(TabContents* contents, int index) { }
+  virtual void TabDeselectedAt(TabContents* contents, int index) {}
 
   // The selected TabContents changed from |old_contents| to |new_contents| at
   // |index|. |user_gesture| specifies whether or not this was done by a user
@@ -82,14 +82,12 @@ class TabStripModelObserver {
   virtual void TabSelectedAt(TabContents* old_contents,
                              TabContents* new_contents,
                              int index,
-                             bool user_gesture) { }
+                             bool user_gesture) {}
 
-  // The specified TabContents at |from_index| was moved to |to_index|. If
-  // the pinned state of the tab is changing |pinned_state_changed| is true.
+  // The specified TabContents at |from_index| was moved to |to_index|.
   virtual void TabMoved(TabContents* contents,
                         int from_index,
-                        int to_index,
-                        bool pinned_state_changed) { }
+                        int to_index) {}
 
   // The specified TabContents at |index| changed in some way. |contents| may
   // be an entirely different object and the old value is no longer available
@@ -106,15 +104,12 @@ class TabStripModelObserver {
                              TabContents* new_contents, int index) {}
 
   // Invoked when the pinned state of a tab changes.
-  // NOTE: this is only invoked if the tab doesn't move as a result of its
-  // pinned state changing. If the tab moves as a result, the observer is
-  // notified by way of the TabMoved method with |pinned_state_changed| true.
-  virtual void TabPinnedStateChanged(TabContents* contents, int index) { }
+  virtual void TabPinnedStateChanged(TabContents* contents, int index) {}
 
   // Invoked when the blocked state of a tab changes.
   // NOTE: This is invoked when a tab becomes blocked/unblocked by a tab modal
   // window.
-  virtual void TabBlockedStateChanged(TabContents* contents, int index) { }
+  virtual void TabBlockedStateChanged(TabContents* contents, int index) {}
 
   // The TabStripModel now no longer has any phantom tabs. The implementer may
   // use this as a trigger to try and close the window containing the
@@ -234,22 +229,20 @@ class TabStripModelDelegate {
 //  tasks like adding new Tabs from just a URL, etc.
 //
 // Each tab may be any one of the following states:
-// . Pinned. The view typically renders pinned tabs differently. The model makes
-//   sure all pinned tabs are organized at the beginning of the tabstrip.
-//   Inserting a tab between pinned tabs implicitly makes the inserted tab
-//   pinned. Similarly moving a tab may pin or unpin the tab, again enforcing
-//   that all pinned tabs occur at the beginning of the tabstrip. Lastly,
-//   changing the pinned state of a tab moves the tab to be grouped with the
-//   pinned or unpinned tabs. For example, if the first two tabs are pinned, and
-//   the tenth tab is pinned, it is moved to become the third tab.
-// . App. An app tab corresponds to an app extension.
-// . Phantom. Only pinned app tabs may be made phantom (or if
-//   browser_defaults::kPinnedTabsActLikeApps is true then any pinned tab may be
-//   made phantom). When a tab that can be made phantom is closed the renderer
-//   is shutdown, a new TabContents/NavigationController is created that has
-//   not yet loaded the renderer and observers are notified via the
-//   TabReplacedAt method. When a phantom tab is selected the renderer is
-//   loaded and the tab is no longer phantom.
+// . App. Corresponds to an extension that wants an app tab. App tabs are
+//   rendered differently than non-app tabs. The model makes sure all app tabs
+//   are at the beginning of the tab strip. Requests to insert a non-app tab
+//   before app tabs or an app tab after the app tabs is ignored (the UI
+//   disallows this). Similarly requests to move an app tab to be with non-app
+//   tabs is ignored.
+// . Pinned. Only app tabs can be pinned. A pinned tab is made phantom when
+///  closed.
+// . Phantom. Only app pinned tabs may be made phantom. When a tab that can be
+//   made phantom is closed the renderer is shutdown, a new
+//   TabContents/NavigationController is created that has not yet loaded the
+//   renderer and observers are notified via the TabReplacedAt method. When a
+//   phantom tab is selected the renderer is loaded and the tab is no longer
+//   phantom.
 //   Phantom tabs do not prevent the tabstrip from closing, for example if the
 //   tabstrip has one phantom and one non-phantom tab and the non-phantom tab is
 //   closed, then the tabstrip/browser are closed.
@@ -326,8 +319,8 @@ class TabStripModel : public NotificationObserver {
 
   // Adds the specified TabContents in the specified location. If
   // |inherit_group| is true, the new contents is linked to the current tab's
-  // group. If there are pinned tabs at or before |index|, then the newly
-  // inserted tab is pinned.
+  // group. This adjusts the index such that all app tabs occur before non-app
+  // tabs.
   void InsertTabContentsAt(int index,
                            TabContents* contents,
                            bool foreground,
@@ -369,7 +362,8 @@ class TabStripModel : public NotificationObserver {
   // If |select_after_move| is false, whatever tab was selected before the move
   // will still be selected, but it's index may have incremented or decremented
   // one slot.
-  // See class description for how pinning is effected by this.
+  // NOTE: this does nothing if the move would result in app tabs and non-app
+  // tabs mixing.
   void MoveTabContentsAt(int index, int to_position, bool select_after_move);
 
   // Returns the currently selected TabContents, or NULL if there is none.
@@ -456,25 +450,24 @@ class TabStripModel : public NotificationObserver {
   void SetTabPinned(int index, bool pinned);
 
   // Returns true if the tab at |index| is pinned.
+  // See description above class for details on pinned tabs.
   bool IsTabPinned(int index) const;
 
   // Is the tab at |index| an app?
-  // See description above class for details on this.
-  // This is currently only true if browser_defaults::kPinnedTabsActLikeApps is
-  // true and the tab is pinned.
+  // See description above class for details on app tabs.
   bool IsAppTab(int index) const;
 
   // Returns true if the tab is a phantom tab. A phantom tab is one where the
   // renderer has not been loaded.
-  // See description above class for details on this.
+  // See description above class for details on phantom tabs.
   bool IsPhantomTab(int index) const;
 
   // Returns true if the tab at |index| is blocked by a tab modal dialog.
   bool IsTabBlocked(int index) const;
 
-  // Returns the index of the first tab that is not pinned. This returns
-  // |count()| if all of the tabs are pinned, and 0 if no tabs are pinned.
-  int IndexOfFirstNonPinnedTab() const;
+  // Returns the index of the first tab that is not an app. This returns
+  // |count()| if all of the tabs are apps, and 0 if none of the tabs are apps.
+  int IndexOfFirstNonAppTab() const;
 
   // Command level API /////////////////////////////////////////////////////////
 
@@ -575,10 +568,6 @@ class TabStripModel : public NotificationObserver {
   bool InternalCloseTabs(std::vector<int> indices,
                          bool create_historical_tabs);
 
-  void MoveTabContentsAtImpl(int index, int to_position,
-                             bool select_after_move,
-                             bool update_pinned_state);
-
   TabContents* GetContentsAt(int index) const;
 
   // The actual implementation of SelectTabContentsAt. Takes the previously
@@ -669,6 +658,7 @@ class TabStripModel : public NotificationObserver {
     bool reset_group_on_select;
 
     // Is the tab pinned?
+    // TODO(sky): decide if we really want this, or call it phantomable.
     bool pinned;
 
     // Is the tab interaction blocked by a modal dialog?
diff --git a/chrome/browser/tabs/tab_strip_model_order_controller.cc b/chrome/browser/tabs/tab_strip_model_order_controller.cc
index de0ab23..36db14a7 100644
--- a/chrome/browser/tabs/tab_strip_model_order_controller.cc
+++ b/chrome/browser/tabs/tab_strip_model_order_controller.cc
@@ -24,17 +24,17 @@ int TabStripModelOrderController::DetermineInsertionIndex(
     PageTransition::Type transition,
     bool foreground) {
   int tab_count = tabstrip_->count();
-  int first_non_pinned_tab = tabstrip_->IndexOfFirstNonPinnedTab();
   if (!tab_count)
     return 0;
 
+  int first_non_app_tab = tabstrip_->IndexOfFirstNonAppTab();
   if (transition == PageTransition::LINK && tabstrip_->selected_index() != -1) {
     if (foreground) {
       // If the page was opened in the foreground by a link click in another
       // tab, insert it adjacent to the tab that opened that link.
       // TODO(beng): (http://b/1085481) may want to open right of all locked
       //             tabs?
-      return std::max(first_non_pinned_tab,
+      return std::max(first_non_app_tab,
                       tabstrip_->selected_index() + 1);
     }
     NavigationController* opener =
@@ -46,7 +46,7 @@ int TabStripModelOrderController::DetermineInsertionIndex(
     if (index != TabStripModel::kNoTab)
       return index + 1;
     // Otherwise insert adjacent to opener...
-    return std::max(first_non_pinned_tab, tabstrip_->selected_index() + 1);
+    return std::max(first_non_app_tab, tabstrip_->selected_index() + 1);
   }
   // In other cases, such as Ctrl+T, open at the end of the strip.
   return tab_count;
diff --git a/chrome/browser/tabs/tab_strip_model_unittest.cc b/chrome/browser/tabs/tab_strip_model_unittest.cc
index 00afd03..7330a41 100644
--- a/chrome/browser/tabs/tab_strip_model_unittest.cc
+++ b/chrome/browser/tabs/tab_strip_model_unittest.cc
@@ -137,6 +137,9 @@ class TabStripModelTest : public RenderViewHostTestHarness {
 
       actual += IntToString(GetID(model.GetTabContentsAt(i)));
 
+      if (model.IsAppTab(i))
+        actual += "a";
+
       if (model.IsTabPinned(i))
         actual += "p";
     }
@@ -248,11 +251,9 @@ class MockTabStripModelObserver : public TabStripModelObserver {
     states_.push_back(s);
   }
   virtual void TabMoved(
-      TabContents* contents, int from_index, int to_index,
-      bool pinned_state_changed) {
+      TabContents* contents, int from_index, int to_index) {
     State* s = new State(contents, to_index, MOVE);
     s->src_index = from_index;
-    s->pinned_state_changed = pinned_state_changed;
     states_.push_back(s);
   }
 
@@ -1278,8 +1279,65 @@ TEST_F(TabStripModelTest, NavigationForgettingDoesntAffectNewTab) {
   strip.CloseAllTabs();
 }
 
-// Tests various permutations of pinning tabs.
-TEST_F(TabStripModelTest, Pinning) {
+// Tests that fast shutdown is attempted appropriately.
+TEST_F(TabStripModelTest, FastShutdown) {
+  TabStripDummyDelegate delegate(NULL);
+  TabStripModel tabstrip(&delegate, profile());
+  MockTabStripModelObserver observer;
+  tabstrip.AddObserver(&observer);
+
+  EXPECT_TRUE(tabstrip.empty());
+
+  // Make sure fast shutdown is attempted when tabs that share a RPH are shut
+  // down.
+  {
+    TabContents* contents1 = CreateTabContents();
+    TabContents* contents2 = CreateTabContentsWithSharedRPH(contents1);
+
+    SetID(contents1, 1);
+    SetID(contents2, 2);
+
+    tabstrip.AppendTabContents(contents1, true);
+    tabstrip.AppendTabContents(contents2, true);
+
+    // Turn on the fake unload listener so the tabs don't actually get shut
+    // down when we call CloseAllTabs()---we need to be able to check that
+    // fast shutdown was attempted.
+    delegate.set_run_unload_listener(true);
+    tabstrip.CloseAllTabs();
+    // On a mock RPH this checks whether we *attempted* fast shutdown.
+    // A real RPH would reject our attempt since there is an unload handler.
+    EXPECT_TRUE(contents1->process()->fast_shutdown_started());
+    EXPECT_EQ(2, tabstrip.count());
+
+    delegate.set_run_unload_listener(false);
+    tabstrip.CloseAllTabs();
+    EXPECT_TRUE(tabstrip.empty());
+  }
+
+  // Make sure fast shutdown is not attempted when only some tabs that share a
+  // RPH are shut down.
+  {
+    TabContents* contents1 = CreateTabContents();
+    TabContents* contents2 = CreateTabContentsWithSharedRPH(contents1);
+
+    SetID(contents1, 1);
+    SetID(contents2, 2);
+
+    tabstrip.AppendTabContents(contents1, true);
+    tabstrip.AppendTabContents(contents2, true);
+
+    tabstrip.CloseTabContentsAt(1);
+    EXPECT_FALSE(contents1->process()->fast_shutdown_started());
+    EXPECT_EQ(1, tabstrip.count());
+
+    tabstrip.CloseAllTabs();
+    EXPECT_TRUE(tabstrip.empty());
+  }
+}
+
+// Tests various permutations of apps.
+TEST_F(TabStripModelTest, Apps) {
   TabStripDummyDelegate delegate(NULL);
   TabStripModel tabstrip(&delegate, profile());
   MockTabStripModelObserver observer;
@@ -1290,7 +1348,9 @@ TEST_F(TabStripModelTest, Pinning) {
   typedef MockTabStripModelObserver::State State;
 
   TabContents* contents1 = CreateTabContents();
+  contents1->set_app(true);
   TabContents* contents2 = CreateTabContents();
+  contents2->set_app(true);
   TabContents* contents3 = CreateTabContents();
 
   SetID(contents1, 1);
@@ -1301,242 +1361,95 @@ TEST_F(TabStripModelTest, Pinning) {
   // builds on the state established in the previous. This is important if you
   // ever insert tests rather than append.
 
-  // Initial state, three tabs, first selected.
-  tabstrip.AppendTabContents(contents1, true);
-  tabstrip.AppendTabContents(contents2, false);
-  tabstrip.AppendTabContents(contents3, false);
+  // Initial state, tab3 only and selected.
+  tabstrip.AppendTabContents(contents3, true);
 
   observer.ClearStates();
 
-  // Pin the first tab, this shouldn't visually reorder anything.
-  {
-    tabstrip.SetTabPinned(0, true);
-
-    // As the order didn't change, we should get a pinned notification.
-    ASSERT_EQ(1, observer.GetStateCount());
-    State state(contents1, 0, MockTabStripModelObserver::PINNED);
-    EXPECT_TRUE(observer.StateEquals(0, state));
-
-    // And verify the state.
-    EXPECT_EQ("1p 2 3", GetPinnedState(tabstrip));
-
-    observer.ClearStates();
-  }
-
-  // Unpin the first tab.
-  {
-    tabstrip.SetTabPinned(0, false);
-
-    // As the order didn't change, we should get a pinned notification.
-    ASSERT_EQ(1, observer.GetStateCount());
-    State state(contents1, 0, MockTabStripModelObserver::PINNED);
-    EXPECT_TRUE(observer.StateEquals(0, state));
-
-    // And verify the state.
-    EXPECT_EQ("1 2 3", GetPinnedState(tabstrip));
-
-    observer.ClearStates();
-  }
-
-  // Pin the 3rd tab, which should move it to the front.
+  // 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.SetTabPinned(2, true);
+    tabstrip.InsertTabContentsAt(1, contents1, false, false);
 
-    // The pinning should have resulted in a move.
     ASSERT_EQ(1, observer.GetStateCount());
-    State state(contents3, 0, MockTabStripModelObserver::MOVE);
-    state.src_index = 2;
-    state.pinned_state_changed = true;
+    State state(contents1, 0, MockTabStripModelObserver::INSERT);
     EXPECT_TRUE(observer.StateEquals(0, state));
 
     // And verify the state.
-    EXPECT_EQ("3p 1 2", GetPinnedState(tabstrip));
+    EXPECT_EQ("1a 3", GetPinnedState(tabstrip));
 
     observer.ClearStates();
   }
 
-  // Pin the tab "1", which shouldn't move anything.
+  // Insert tab 2 at position 1.
   {
-    tabstrip.SetTabPinned(1, true);
+    tabstrip.InsertTabContentsAt(1, contents2, false, false);
 
-    // As the order didn't change, we should get a pinned notification.
     ASSERT_EQ(1, observer.GetStateCount());
-    State state(contents1, 1, MockTabStripModelObserver::PINNED);
+    State state(contents2, 1, MockTabStripModelObserver::INSERT);
     EXPECT_TRUE(observer.StateEquals(0, state));
 
     // And verify the state.
-    EXPECT_EQ("3p 1p 2", GetPinnedState(tabstrip));
+    EXPECT_EQ("1a 2a 3", GetPinnedState(tabstrip));
 
     observer.ClearStates();
   }
 
-  // Move tab "2" to the front, which should pin it.
+  // Try to move tab 3 to position 0. This isn't legal and should be ignored.
   {
     tabstrip.MoveTabContentsAt(2, 0, false);
 
-    // As the order didn't change, we should get a pinned notification.
-    ASSERT_EQ(1, observer.GetStateCount());
-    State state(contents2, 0, MockTabStripModelObserver::MOVE);
-    state.src_index = 2;
-    state.pinned_state_changed = true;
-    EXPECT_TRUE(observer.StateEquals(0, state));
+    ASSERT_EQ(0, observer.GetStateCount());
 
-    // And verify the state.
-    EXPECT_EQ("2p 3p 1p", GetPinnedState(tabstrip));
+    // And verify the state didn't change.
+    EXPECT_EQ("1a 2a 3", GetPinnedState(tabstrip));
 
     observer.ClearStates();
   }
 
-  // Unpin tab "2", which implicitly moves it to the end.
+  // Try to move tab 0 to position 3. This isn't legal and should be ignored.
   {
-    tabstrip.SetTabPinned(0, false);
+    tabstrip.MoveTabContentsAt(0, 2, false);
 
-    ASSERT_EQ(1, observer.GetStateCount());
-    State state(contents2, 2, MockTabStripModelObserver::MOVE);
-    state.src_index = 0;
-    state.pinned_state_changed = true;
-    EXPECT_TRUE(observer.StateEquals(0, state));
+    ASSERT_EQ(0, observer.GetStateCount());
 
-    // And verify the state.
-    EXPECT_EQ("3p 1p 2", GetPinnedState(tabstrip));
+    // And verify the state didn't change.
+    EXPECT_EQ("1a 2a 3", GetPinnedState(tabstrip));
 
     observer.ClearStates();
   }
 
-  // Drag tab "3" to after "1', which should not change the pinned state.
+  // Try to move tab 0 to position 1. This is a legal move.
   {
     tabstrip.MoveTabContentsAt(0, 1, false);
 
     ASSERT_EQ(1, observer.GetStateCount());
-    State state(contents3, 1, MockTabStripModelObserver::MOVE);
+    State state(contents1, 1, MockTabStripModelObserver::MOVE);
     state.src_index = 0;
     EXPECT_TRUE(observer.StateEquals(0, state));
 
-    // And verify the state.
-    EXPECT_EQ("1p 3p 2", GetPinnedState(tabstrip));
+    // And verify the state didn't change.
+    EXPECT_EQ("2a 1a 3", GetPinnedState(tabstrip));
 
     observer.ClearStates();
   }
 
-  // Unpin tab "1".
+  // Remove tab3 and insert at position 0. It should be forced to position 2.
   {
-    tabstrip.SetTabPinned(0, false);
-
-    ASSERT_EQ(1, observer.GetStateCount());
-    State state(contents1, 1, MockTabStripModelObserver::MOVE);
-    state.src_index = 0;
-    state.pinned_state_changed = true;
-    EXPECT_TRUE(observer.StateEquals(0, state));
-
-    // And verify the state.
-    EXPECT_EQ("3p 1 2", GetPinnedState(tabstrip));
-
+    tabstrip.DetachTabContentsAt(2);
     observer.ClearStates();
-  }
 
-  // Unpin tab "3".
-  {
-    tabstrip.SetTabPinned(0, false);
+    tabstrip.InsertTabContentsAt(0, contents3, false, false);
 
     ASSERT_EQ(1, observer.GetStateCount());
-    State state(contents3, 0, MockTabStripModelObserver::PINNED);
+    State state(contents3, 2, MockTabStripModelObserver::INSERT);
     EXPECT_TRUE(observer.StateEquals(0, state));
 
-    EXPECT_EQ("3 1 2", GetPinnedState(tabstrip));
-
-    observer.ClearStates();
-  }
-
-  // Unpin tab "3" again, as it's unpinned nothing should change.
-  {
-    tabstrip.SetTabPinned(0, false);
-
-    ASSERT_EQ(0, observer.GetStateCount());
-
-    EXPECT_EQ("3 1 2", GetPinnedState(tabstrip));
-  }
-
-  // Pin "3" and "1".
-  {
-    tabstrip.SetTabPinned(0, true);
-    tabstrip.SetTabPinned(1, true);
-
-    EXPECT_EQ("3p 1p 2", GetPinnedState(tabstrip));
+    // And verify the state didn't change.
+    EXPECT_EQ("2a 1a 3", GetPinnedState(tabstrip));
 
     observer.ClearStates();
   }
 
-  TabContents* contents4 = CreateTabContents();
-  SetID(contents4, 4);
-
-  // Insert "4" between "3" and "1". As "3" and "1" are pinned, "4" should
-  // be pinned too.
-  {
-    tabstrip.InsertTabContentsAt(1, contents4, false, false);
-
-    ASSERT_EQ(1, observer.GetStateCount());
-    State state(contents4, 1, MockTabStripModelObserver::INSERT);
-    EXPECT_TRUE(observer.StateEquals(0, state));
-
-    EXPECT_EQ("3p 4p 1p 2", GetPinnedState(tabstrip));
-  }
-
   tabstrip.CloseAllTabs();
 }
-
-// Tests that fast shutdown is attempted appropriately.
-TEST_F(TabStripModelTest, FastShutdown) {
-  TabStripDummyDelegate delegate(NULL);
-  TabStripModel tabstrip(&delegate, profile());
-  MockTabStripModelObserver observer;
-  tabstrip.AddObserver(&observer);
-
-  EXPECT_TRUE(tabstrip.empty());
-
-  // Make sure fast shutdown is attempted when tabs that share a RPH are shut
-  // down.
-  {
-    TabContents* contents1 = CreateTabContents();
-    TabContents* contents2 = CreateTabContentsWithSharedRPH(contents1);
-
-    SetID(contents1, 1);
-    SetID(contents2, 2);
-
-    tabstrip.AppendTabContents(contents1, true);
-    tabstrip.AppendTabContents(contents2, true);
-
-    // Turn on the fake unload listener so the tabs don't actually get shut
-    // down when we call CloseAllTabs()---we need to be able to check that
-    // fast shutdown was attempted.
-    delegate.set_run_unload_listener(true);
-    tabstrip.CloseAllTabs();
-    // On a mock RPH this checks whether we *attempted* fast shutdown.
-    // A real RPH would reject our attempt since there is an unload handler.
-    EXPECT_TRUE(contents1->process()->fast_shutdown_started());
-    EXPECT_EQ(2, tabstrip.count());
-
-    delegate.set_run_unload_listener(false);
-    tabstrip.CloseAllTabs();
-    EXPECT_TRUE(tabstrip.empty());
-  }
-
-  // Make sure fast shutdown is not attempted when only some tabs that share a
-  // RPH are shut down.
-  {
-    TabContents* contents1 = CreateTabContents();
-    TabContents* contents2 = CreateTabContentsWithSharedRPH(contents1);
-
-    SetID(contents1, 1);
-    SetID(contents2, 2);
-
-    tabstrip.AppendTabContents(contents1, true);
-    tabstrip.AppendTabContents(contents2, true);
-
-    tabstrip.CloseTabContentsAt(1);
-    EXPECT_FALSE(contents1->process()->fast_shutdown_started());
-    EXPECT_EQ(1, tabstrip.count());
-
-    tabstrip.CloseAllTabs();
-    EXPECT_TRUE(tabstrip.empty());
-  }
-}