Add icons to SimpleMenuModel.

Also get rid of one of the MenuGtk constructors (now down to 2).
Also, stamp out one more use of MenuGtk::Delegate as a controller class.

BUG=31365
TEST=manual (in debug mode) + trybots

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

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

4 files changed, 46 insertions, 35 deletions

diff --git a/chrome/browser/gtk/browser_actions_toolbar_gtk.cc b/chrome/browser/gtk/browser_actions_toolbar_gtk.cc
index 65fe134..994d81f 100644
--- a/chrome/browser/gtk/browser_actions_toolbar_gtk.cc
+++ b/chrome/browser/gtk/browser_actions_toolbar_gtk.cc
@@ -43,14 +43,12 @@ const int kButtonSize = 29;
 // use their maximum allowed size.
 const int kButtonPadding = 3;
 
-// The padding to the left, top, and bottom of the browser actions toolbar
+// The padding to the left, top and bottom of the browser actions toolbar
 // separator.
 const int kSeparatorPadding = 2;
 
 const char* kDragTarget = "application/x-chrome-browseraction";
 
-const char* kMenuItemKey = "__CHROME_OVERFLOW_MENU_ITEM_KEY__";
-
 GtkTargetEntry GetDragTargetEntry() {
   static std::string drag_target_string(kDragTarget);
   GtkTargetEntry drag_target;
@@ -69,6 +67,8 @@ gint WidthForIconCount(gint icon_count) {
 
 }  // namespace
 
+using menus::SimpleMenuModel;
+
 class BrowserActionButton : public NotificationObserver,
                             public ImageLoadingTracker::Observer,
                             public ExtensionContextMenuModel::PopupDelegate,
@@ -589,7 +589,7 @@ void BrowserActionsToolbarGtk::AnimationEnded(const Animation* animation) {
   UpdateChevronVisibility();
 }
 
-void BrowserActionsToolbarGtk::ExecuteCommandById(int command_id) {
+void BrowserActionsToolbarGtk::ExecuteCommand(int command_id) {
   Extension* extension = model_->GetExtensionByIndex(command_id);
   ExtensionAction* browser_action = extension->browser_action();
 
@@ -805,7 +805,7 @@ gboolean BrowserActionsToolbarGtk::OnGripperButtonPress(
 
 gboolean BrowserActionsToolbarGtk::OnOverflowButtonPress(
     GtkWidget* overflow, GdkEventButton* event) {
-  overflow_menu_.reset(new MenuGtk(this));
+  overflow_menu_model_.reset(new SimpleMenuModel(this));
 
   int visible_icon_count =
       gtk_chrome_shrinkable_hbox_get_visible_child_count(
@@ -818,16 +818,14 @@ gboolean BrowserActionsToolbarGtk::OnOverflowButtonPress(
     Extension* extension = model_->GetExtensionByIndex(model_index);
     BrowserActionButton* button = extension_button_map_[extension->id()].get();
 
-    GtkWidget* menu_item = overflow_menu_->AppendMenuItemWithIcon(
-        model_index,
-        extension->name(),
-        button->GetIcon());
-
-    g_object_set_data(G_OBJECT(menu_item), kMenuItemKey, button);
+    overflow_menu_model_->AddItem(model_index, UTF8ToUTF16(extension->name()));
+    overflow_menu_model_->SetIcon(overflow_menu_model_->GetItemCount() - 1,
+                                  button->GetIcon());
 
     // TODO(estade): set the menu item's tooltip.
   }
 
+  overflow_menu_.reset(new MenuGtk(this, overflow_menu_model_.get()));
   signals_.Connect(overflow_menu_->widget(), "button-press-event",
                    G_CALLBACK(OnOverflowMenuButtonPressThunk), this);
 
@@ -847,10 +845,26 @@ gboolean BrowserActionsToolbarGtk::OnOverflowMenuButtonPress(
   if (!menu_item)
     return FALSE;
 
-  void* data = g_object_get_data(G_OBJECT(menu_item), kMenuItemKey);
-  DCHECK(data);
-  BrowserActionButton* button = static_cast<BrowserActionButton*>(data);
-  button->GetContextMenu()->PopupAsContext(event->time);
+  int item_index = g_list_index(GTK_MENU_SHELL(overflow)->children, menu_item);
+  if (item_index == -1) {
+    NOTREACHED();
+    return FALSE;
+  }
+
+  item_index += gtk_chrome_shrinkable_hbox_get_visible_child_count(
+      GTK_CHROME_SHRINKABLE_HBOX(button_hbox_.get()));
+  if (profile_->IsOffTheRecord())
+    item_index = model_->IncognitoIndexToOriginal(item_index);
+
+  Extension* extension = model_->GetExtensionByIndex(item_index);
+  ExtensionButtonMap::iterator it = extension_button_map_.find(
+      extension->id());
+  if (it == extension_button_map_.end()) {
+    NOTREACHED();
+    return FALSE;
+  }
+
+  it->second.get()->GetContextMenu()->PopupAsContext(event->time);
   return TRUE;
 }
 
diff --git a/chrome/browser/gtk/browser_actions_toolbar_gtk.h b/chrome/browser/gtk/browser_actions_toolbar_gtk.h
index 5de4ffb..467d9ea 100644
--- a/chrome/browser/gtk/browser_actions_toolbar_gtk.h
+++ b/chrome/browser/gtk/browser_actions_toolbar_gtk.h
@@ -11,6 +11,7 @@
 #include <string>
 
 #include "app/gtk_signal.h"
+#include "app/menus/simple_menu_model.h"
 #include "app/slide_animation.h"
 #include "base/linked_ptr.h"
 #include "base/task.h"
@@ -31,7 +32,8 @@ typedef struct _GtkWidget GtkWidget;
 
 class BrowserActionsToolbarGtk : public ExtensionToolbarModel::Observer,
                                  public AnimationDelegate,
-                                 public MenuGtk::Delegate {
+                                 public MenuGtk::Delegate,
+                                 public menus::SimpleMenuModel::Delegate {
  public:
   explicit BrowserActionsToolbarGtk(Browser* browser);
   virtual ~BrowserActionsToolbarGtk();
@@ -99,10 +101,18 @@ class BrowserActionsToolbarGtk : public ExtensionToolbarModel::Observer,
   virtual void AnimationProgressed(const Animation* animation);
   virtual void AnimationEnded(const Animation* animation);
 
-  // MenuGtk::Delegate implementation.
+  // SimpleMenuModel::Delegate implementation.
   // In our case, |command_id| is be the index into the model's extension list.
-  virtual bool IsCommandEnabled(int command_id) const { return true; }
-  virtual void ExecuteCommandById(int command_id);
+  virtual bool IsCommandIdChecked(int command_id) const { return false; }
+  virtual bool IsCommandIdEnabled(int command_id) const { return true; }
+  virtual bool GetAcceleratorForCommandId(
+      int command_id,
+      menus::Accelerator* accelerator) {
+    return false;
+  }
+  virtual void ExecuteCommand(int command_id);
+
+  // MenuGtk::Delegate implementation.
   virtual void StoppedShowing();
   virtual bool AlwaysShowImages() const { return true; }
 
@@ -160,6 +170,7 @@ class BrowserActionsToolbarGtk : public ExtensionToolbarModel::Observer,
 
   OverflowButton overflow_button_;
   scoped_ptr<MenuGtk> overflow_menu_;
+  scoped_ptr<menus::SimpleMenuModel> overflow_menu_model_;
 
   // The vertical separator between the overflow button and the page/app menus.
   GtkWidget* separator_;
diff --git a/chrome/browser/gtk/menu_gtk.cc b/chrome/browser/gtk/menu_gtk.cc
index 7670b6c..1e9a8e7 100644
--- a/chrome/browser/gtk/menu_gtk.cc
+++ b/chrome/browser/gtk/menu_gtk.cc
@@ -117,16 +117,6 @@ MenuGtk::MenuGtk(MenuGtk::Delegate* delegate,
   BuildMenuIn(menu_, menu_data);
 }
 
-MenuGtk::MenuGtk(MenuGtk::Delegate* delegate)
-    : delegate_(delegate),
-      model_(NULL),
-      dummy_accel_group_(gtk_accel_group_new()),
-      menu_(gtk_menu_new()),
-      factory_(this) {
-  g_object_ref_sink(menu_);
-  ConnectSignalHandlers();
-}
-
 MenuGtk::MenuGtk(MenuGtk::Delegate* delegate,
                  menus::MenuModel* model)
     : delegate_(delegate),
@@ -134,11 +124,10 @@ MenuGtk::MenuGtk(MenuGtk::Delegate* delegate,
       dummy_accel_group_(gtk_accel_group_new()),
       menu_(gtk_menu_new()),
       factory_(this) {
-  g_object_ref_sink(menu_);
   DCHECK(model);
+  g_object_ref_sink(menu_);
   ConnectSignalHandlers();
-  if (model)
-    BuildMenuFromModel();
+  BuildMenuFromModel();
 }
 
 MenuGtk::~MenuGtk() {
diff --git a/chrome/browser/gtk/menu_gtk.h b/chrome/browser/gtk/menu_gtk.h
index 5f33b6b..ed5342e 100644
--- a/chrome/browser/gtk/menu_gtk.h
+++ b/chrome/browser/gtk/menu_gtk.h
@@ -60,9 +60,6 @@ class MenuGtk {
   // Builds a MenuGtk that uses |delegate| to perform actions and |menu_data|
   // to create the menu.
   MenuGtk(MenuGtk::Delegate* delegate, const MenuCreateMaterial* menu_data);
-  // Builds a MenuGtk that uses |delegate| to perform actions and build the
-  // menu.
-  explicit MenuGtk(MenuGtk::Delegate* delegate);
   // Creates a MenuGtk that uses |delegate| to perform actions.  Builds the
   // menu using |model_|.
   MenuGtk(MenuGtk::Delegate* delegate, menus::MenuModel* model);