Simplify TabGtk::ContextMenuController by adding methods to dynamically build a MenuGtk and use them.

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

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

4 files changed, 63 insertions, 67 deletions

diff --git a/chrome/browser/gtk/browser_toolbar_gtk.cc b/chrome/browser/gtk/browser_toolbar_gtk.cc
index eee898d..2083b58 100644
--- a/chrome/browser/gtk/browser_toolbar_gtk.cc
+++ b/chrome/browser/gtk/browser_toolbar_gtk.cc
@@ -379,9 +379,9 @@ gboolean BrowserToolbarGtk::OnBackForwardPressEvent(GtkWidget* widget,
 void BrowserToolbarGtk::ShowBackForwardMenu(GtkWidget* widget,
                                             gint button_type) {
   if (widget == back_->widget()) {
-    back_forward_menu_.reset(new MenuGtk(back_menu_model_.get()));
+    back_forward_menu_.reset(new MenuGtk(back_menu_model_.get(), true));
   } else {
-    back_forward_menu_.reset(new MenuGtk(forward_menu_model_.get()));
+    back_forward_menu_.reset(new MenuGtk(forward_menu_model_.get(), true));
   }
 
   back_forward_menu_->Popup(widget, button_type, gtk_get_current_event_time());
diff --git a/chrome/browser/gtk/menu_gtk.cc b/chrome/browser/gtk/menu_gtk.cc
index 1a852d5..059755a 100644
--- a/chrome/browser/gtk/menu_gtk.cc
+++ b/chrome/browser/gtk/menu_gtk.cc
@@ -43,11 +43,12 @@ MenuGtk::MenuGtk(MenuGtk::Delegate* delegate,
   BuildMenuIn(menu_.get(), menu_data, accel_group);
 }
 
-MenuGtk::MenuGtk(MenuGtk::Delegate* delegate)
+MenuGtk::MenuGtk(MenuGtk::Delegate* delegate, bool load)
     : delegate_(delegate),
       dummy_accel_group_(NULL),
       menu_(gtk_menu_new()) {
-  BuildMenuFromDelegate();
+  if (load)
+    BuildMenuFromDelegate();
 }
 
 MenuGtk::~MenuGtk() {
@@ -56,6 +57,26 @@ MenuGtk::~MenuGtk() {
     g_object_unref(dummy_accel_group_);
 }
 
+void MenuGtk::AppendMenuItemWithLabel(int command_id,
+                                      const std::string& label) {
+  GtkWidget* menu_item = gtk_menu_item_new_with_label(label.c_str());
+
+  g_object_set_data(G_OBJECT(menu_item), "menu-id",
+                    reinterpret_cast<void*>(command_id));
+
+  g_signal_connect(G_OBJECT(menu_item), "activate",
+                   G_CALLBACK(OnMenuItemActivatedById), this);
+
+  gtk_widget_show(menu_item);
+  gtk_menu_shell_append(GTK_MENU_SHELL(menu_.get()), menu_item);
+}
+
+void MenuGtk::AppendSeparator() {
+  GtkWidget* menu_item = gtk_separator_menu_item_new();
+  gtk_widget_show(menu_item);
+  gtk_menu_shell_append(GTK_MENU_SHELL(menu_.get()), menu_item);
+}
+
 void MenuGtk::Popup(GtkWidget* widget, GdkEvent* event) {
   DCHECK(event->type == GDK_BUTTON_PRESS)
       << "Non-button press event sent to RunMenuAt";
diff --git a/chrome/browser/gtk/menu_gtk.h b/chrome/browser/gtk/menu_gtk.h
index d3bd65d..9b9a724 100644
--- a/chrome/browser/gtk/menu_gtk.h
+++ b/chrome/browser/gtk/menu_gtk.h
@@ -41,11 +41,15 @@ class MenuGtk {
   // to create the menu.
   MenuGtk(MenuGtk::Delegate* delegate, const MenuCreateMaterial* menu_data,
           GtkAccelGroup* accel_group);
-  // Builds a MenuGtk that uses |delegate| to create the menu as well as perform
-  // actions.
-  explicit MenuGtk(MenuGtk::Delegate* delegate);
+  // Creates a MenuGtk that uses |delegate| to perform actions.  Builds the
+  // menu using |delegate| if |load| is true.
+  MenuGtk(MenuGtk::Delegate* delegate, bool load);
   ~MenuGtk();
 
+  // These methods are used to build the menu dynamically.
+  void AppendMenuItemWithLabel(int command_id, const std::string& label);
+  void AppendSeparator();
+
   // Displays the menu. |timestamp| is the time of activation. The popup is
   // statically positioned at |widget|.
   void Popup(GtkWidget* widget, gint button_type, guint32 timestamp);
diff --git a/chrome/browser/gtk/tabs/tab_gtk.cc b/chrome/browser/gtk/tabs/tab_gtk.cc
index 02100e1..3a72bf3 100644
--- a/chrome/browser/gtk/tabs/tab_gtk.cc
+++ b/chrome/browser/gtk/tabs/tab_gtk.cc
@@ -18,7 +18,33 @@ class TabGtk::ContextMenuController : public MenuGtk::Delegate {
  public:
   explicit ContextMenuController(TabGtk* tab)
       : tab_(tab) {
-    menu_.reset(new MenuGtk(this));
+    menu_.reset(new MenuGtk(this, false));
+    menu_->AppendMenuItemWithLabel(
+        TabStripModel::CommandNewTab,
+        l10n_util::GetStringUTF8(IDS_TAB_CXMENU_NEWTAB));
+    menu_->AppendSeparator();
+    menu_->AppendMenuItemWithLabel(
+        TabStripModel::CommandReload,
+        l10n_util::GetStringUTF8(IDS_TAB_CXMENU_RELOAD));
+    menu_->AppendMenuItemWithLabel(
+        TabStripModel::CommandDuplicate,
+        l10n_util::GetStringUTF8(IDS_TAB_CXMENU_DUPLICATE));
+    menu_->AppendSeparator();
+    menu_->AppendMenuItemWithLabel(
+        TabStripModel::CommandCloseTab,
+        l10n_util::GetStringUTF8(IDS_TAB_CXMENU_CLOSETAB));
+    menu_->AppendMenuItemWithLabel(
+        TabStripModel::CommandCloseOtherTabs,
+        l10n_util::GetStringUTF8(IDS_TAB_CXMENU_CLOSEOTHERTABS));
+    menu_->AppendMenuItemWithLabel(
+        TabStripModel::CommandCloseTabsToRight,
+        l10n_util::GetStringUTF8(IDS_TAB_CXMENU_CLOSETABSTORIGHT));
+    menu_->AppendMenuItemWithLabel(
+        TabStripModel::CommandCloseTabsOpenedBy,
+        l10n_util::GetStringUTF8(IDS_TAB_CXMENU_CLOSETABSOPENEDBY));
+    menu_->AppendMenuItemWithLabel(
+        TabStripModel::CommandRestoreTab,
+        l10n_util::GetStringUTF8(IDS_RESTORE_TAB));
   }
 
   virtual ~ContextMenuController() {}
@@ -34,76 +60,21 @@ class TabGtk::ContextMenuController : public MenuGtk::Delegate {
 
  private:
 
-  // Converts the gtk command id to the tab command id.  Gtk includes the
-  // menu separators, but TabStripModel::ContextMenuCommand does not, so
-  // readjust the id to take into account separators.
-  // TODO(jhawkins): Add AppendMenuItemWithLabel and AppendSeparator methods
-  // to MenuGtk so we can avoid this hacky conversion.
-  int gtk_command_to_tab_command(int command_id) const {
-    if (command_id == 1)
-      return command_id;
-    else if (command_id < 5)
-      return command_id - 1;
-    else
-      return command_id - 2;
-  }
-
   // MenuGtk::Delegate implementation:
   virtual bool IsCommandEnabled(int command_id) const {
     if (!tab_)
       return false;
 
-    TabStripModel::ContextMenuCommand id;
-    id = static_cast<TabStripModel::ContextMenuCommand>(
-        gtk_command_to_tab_command(command_id));
-
-    return tab_->delegate()->IsCommandEnabledForTab(id, tab_);
+    return tab_->delegate()->IsCommandEnabledForTab(
+        static_cast<TabStripModel::ContextMenuCommand>(command_id), tab_);
   }
 
   virtual void ExecuteCommand(int command_id) {
     if (!tab_)
       return;
 
-    TabStripModel::ContextMenuCommand id;
-    id = static_cast<TabStripModel::ContextMenuCommand>(
-        gtk_command_to_tab_command(command_id));
-
-    tab_->delegate()->ExecuteCommandForTab(id, tab_);
-  }
-
-  virtual int GetItemCount() const {
-    return TabStripModel::CommandLast;
-  }
-
-  virtual bool IsItemSeparator(int command_id) const {
-    if (command_id == 2 || command_id == 5)
-      return true;
-
-    return false;
-  }
-
-  virtual std::string GetLabel(int command_id) const {
-    switch (command_id) {
-      case 1:
-        return WideToUTF8(l10n_util::GetString(IDS_TAB_CXMENU_NEWTAB));
-      case 3:
-        return WideToUTF8(l10n_util::GetString(IDS_TAB_CXMENU_RELOAD));
-      case 4:
-        return WideToUTF8(l10n_util::GetString(IDS_TAB_CXMENU_DUPLICATE));
-      case 6:
-        return WideToUTF8(l10n_util::GetString(IDS_TAB_CXMENU_CLOSETAB));
-      case 7:
-        return WideToUTF8(l10n_util::GetString(IDS_TAB_CXMENU_CLOSEOTHERTABS));
-      case 8:
-        return WideToUTF8(l10n_util::GetString(
-            IDS_TAB_CXMENU_CLOSETABSTORIGHT));
-      case 9:
-        return WideToUTF8(l10n_util::GetString(
-            IDS_TAB_CXMENU_CLOSETABSOPENEDBY));
-      default:
-        NOTREACHED();
-        return std::string();
-    }
+    tab_->delegate()->ExecuteCommandForTab(
+        static_cast<TabStripModel::ContextMenuCommand>(command_id), tab_);
   }
 
   // The context menu.