Attempt 2 at relanding your patch. I believe the problem was the removal of the observer in ~TabStrip. By the time TabStrip's destructor was run the model's destructor had already run. I've changed BrowserView to explicitly delete the TabStrip before the browser. See the change between v1 and v2 of this patch in BrowserView for the relevant part.

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

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

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

3 files changed, 38 insertions, 2 deletions

diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc
index b5095a2..6d05ebf 100644
--- a/chrome/browser/tabs/tab_strip_model.cc
+++ b/chrome/browser/tabs/tab_strip_model.cc
@@ -574,15 +574,29 @@ bool TabStripModel::IsContextMenuCommandEnabled(
       return delegate_->CanRestoreTab();
     case CommandTogglePinned:
       return true;
-    case CommandBookmarkAllTabs: {
+    case CommandBookmarkAllTabs:
       return delegate_->CanBookmarkAllTabs();
-    }
+    case CommandUseVerticalTabs:
+      return true;
     default:
       NOTREACHED();
   }
   return false;
 }
 
+bool TabStripModel::IsContextMenuCommandChecked(
+    int context_index,
+    ContextMenuCommand command_id) const {
+  switch (command_id) {
+    case CommandUseVerticalTabs:
+      return delegate()->UseVerticalTabs();
+    default:
+      NOTREACHED();
+      break;
+  }
+  return false;
+}
+
 void TabStripModel::ExecuteContextMenuCommand(
     int context_index, ContextMenuCommand command_id) {
   DCHECK(command_id > CommandFirst && command_id < CommandLast);
@@ -655,6 +669,15 @@ void TabStripModel::ExecuteContextMenuCommand(
       delegate_->BookmarkAllTabs();
       break;
     }
+
+    case CommandUseVerticalTabs: {
+      UserMetrics::RecordAction(
+          UserMetricsAction("TabContextMenu_UseVerticalTabs"),
+          profile_);
+
+      delegate()->ToggleUseVerticalTabs();
+      break;
+    }
     default:
       NOTREACHED();
   }
diff --git a/chrome/browser/tabs/tab_strip_model.h b/chrome/browser/tabs/tab_strip_model.h
index 427fb6e..90b4419 100644
--- a/chrome/browser/tabs/tab_strip_model.h
+++ b/chrome/browser/tabs/tab_strip_model.h
@@ -223,6 +223,12 @@ class TabStripModelDelegate {
 
   // Creates a bookmark folder containing a bookmark for all open tabs.
   virtual void BookmarkAllTabs() = 0;
+
+  // Returns true if the vertical tabstrip presentation should be used.
+  virtual bool UseVerticalTabs() const = 0;
+
+  // Toggles the use of the vertical tabstrip.
+  virtual void ToggleUseVerticalTabs() = 0;
 };
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -547,6 +553,7 @@ class TabStripModel : public NotificationObserver {
     CommandRestoreTab,
     CommandTogglePinned,
     CommandBookmarkAllTabs,
+    CommandUseVerticalTabs,
     CommandLast
   };
 
@@ -554,6 +561,10 @@ class TabStripModel : public NotificationObserver {
   bool IsContextMenuCommandEnabled(int context_index,
                                    ContextMenuCommand command_id) const;
 
+  // Returns true if the specified command is checked.
+  bool IsContextMenuCommandChecked(int context_index,
+                                   ContextMenuCommand command_id) const;
+
   // Performs the action associated with the specified command for the given
   // TabStripModel index |context_index|.
   void ExecuteContextMenuCommand(int context_index,
diff --git a/chrome/browser/tabs/tab_strip_model_unittest.cc b/chrome/browser/tabs/tab_strip_model_unittest.cc
index 451cc81..44c56e6 100644
--- a/chrome/browser/tabs/tab_strip_model_unittest.cc
+++ b/chrome/browser/tabs/tab_strip_model_unittest.cc
@@ -71,6 +71,8 @@ class TabStripDummyDelegate : public TabStripModelDelegate {
   virtual bool CanCloseContentsAt(int index) { return can_close_ ; }
   virtual bool CanBookmarkAllTabs() const { return false; }
   virtual void BookmarkAllTabs() {}
+  virtual bool UseVerticalTabs() const { return false; }
+  virtual void ToggleUseVerticalTabs() {}
 
  private:
   // A dummy TabContents we give to callers that expect us to actually build a