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

6 files changed, 79 insertions, 49 deletions

diff --git a/app/menus/simple_menu_model.cc b/app/menus/simple_menu_model.cc
index 7f01b97..f6de9ad 100644
--- a/app/menus/simple_menu_model.cc
+++ b/app/menus/simple_menu_model.cc
@@ -1,6 +1,6 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved. Use of this
-// source code is governed by a BSD-style license that can be found in the
-// LICENSE file.
+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
 
 #include "app/menus/simple_menu_model.h"
 
@@ -18,7 +18,7 @@ SimpleMenuModel::~SimpleMenuModel() {
 }
 
 void SimpleMenuModel::AddItem(int command_id, const string16& label) {
-  Item item = { command_id, label, TYPE_COMMAND, -1, NULL };
+  Item item = { command_id, label, SkBitmap(), TYPE_COMMAND, -1, NULL };
   items_.push_back(item);
 }
 
@@ -27,12 +27,12 @@ void SimpleMenuModel::AddItemWithStringId(int command_id, int string_id) {
 }
 
 void SimpleMenuModel::AddSeparator() {
-  Item item = { -1, string16(), TYPE_SEPARATOR, -1, NULL };
+  Item item = { -1, string16(), SkBitmap(), TYPE_SEPARATOR, -1, NULL };
   items_.push_back(item);
 }
 
 void SimpleMenuModel::AddCheckItem(int command_id, const string16& label) {
-  Item item = { command_id, label, TYPE_CHECK, -1, NULL };
+  Item item = { command_id, label, SkBitmap(), TYPE_CHECK, -1, NULL };
   items_.push_back(item);
 }
 
@@ -42,7 +42,7 @@ void SimpleMenuModel::AddCheckItemWithStringId(int command_id, int string_id) {
 
 void SimpleMenuModel::AddRadioItem(int command_id, const string16& label,
                                    int group_id) {
-  Item item = { command_id, label, TYPE_RADIO, group_id, NULL };
+  Item item = { command_id, label, SkBitmap(), TYPE_RADIO, group_id, NULL };
   items_.push_back(item);
 }
 
@@ -52,7 +52,7 @@ void SimpleMenuModel::AddRadioItemWithStringId(int command_id, int string_id,
 }
 
 void SimpleMenuModel::AddSubMenu(const string16& label, MenuModel* model) {
-  Item item = { -1, label, TYPE_SUBMENU, -1, model };
+  Item item = { -1, label, SkBitmap(), TYPE_SUBMENU, -1, model };
   items_.push_back(item);
 }
 
@@ -62,7 +62,7 @@ void SimpleMenuModel::AddSubMenuWithStringId(int string_id, MenuModel* model) {
 
 void SimpleMenuModel::InsertItemAt(
     int index, int command_id, const string16& label) {
-  Item item = { command_id, label, TYPE_COMMAND, -1, NULL };
+  Item item = { command_id, label, SkBitmap(), TYPE_COMMAND, -1, NULL };
   items_.insert(items_.begin() + FlipIndex(index), item);
 }
 
@@ -72,13 +72,13 @@ void SimpleMenuModel::InsertItemWithStringIdAt(
 }
 
 void SimpleMenuModel::InsertSeparatorAt(int index) {
-  Item item = { -1, string16(), TYPE_SEPARATOR, -1, NULL };
+  Item item = { -1, string16(), SkBitmap(), TYPE_SEPARATOR, -1, NULL };
   items_.insert(items_.begin() + FlipIndex(index), item);
 }
 
 void SimpleMenuModel::InsertCheckItemAt(
     int index, int command_id, const string16& label) {
-  Item item = { command_id, label, TYPE_CHECK, -1, NULL };
+  Item item = { command_id, label, SkBitmap(), TYPE_CHECK, -1, NULL };
   items_.insert(items_.begin() + FlipIndex(index), item);
 }
 
@@ -90,7 +90,7 @@ void SimpleMenuModel::InsertCheckItemWithStringIdAt(
 
 void SimpleMenuModel::InsertRadioItemAt(
     int index, int command_id, const string16& label, int group_id) {
-  Item item = { command_id, label, TYPE_RADIO, group_id, NULL };
+  Item item = { command_id, label, SkBitmap(), TYPE_RADIO, group_id, NULL };
   items_.insert(items_.begin() + FlipIndex(index), item);
 }
 
@@ -102,7 +102,7 @@ void SimpleMenuModel::InsertRadioItemWithStringIdAt(
 
 void SimpleMenuModel::InsertSubMenuAt(
     int index, const string16& label, MenuModel* model) {
-  Item item = { -1, label, TYPE_SUBMENU, -1, model };
+  Item item = { -1, label, SkBitmap(), TYPE_SUBMENU, -1, model };
   items_.insert(items_.begin() + FlipIndex(index), item);
 }
 
@@ -111,6 +111,10 @@ void SimpleMenuModel::InsertSubMenuWithStringIdAt(
   InsertSubMenuAt(index, l10n_util::GetStringUTF16(string_id), model);
 }
 
+void SimpleMenuModel::SetIcon(int index, const SkBitmap& icon) {
+  items_[index].icon = icon;
+}
+
 int SimpleMenuModel::GetIndexOfCommandId(int command_id) {
   std::vector<Item>::iterator itr;
   for (itr = items_.begin(); itr != items_.end(); itr++) {
@@ -125,6 +129,12 @@ int SimpleMenuModel::GetIndexOfCommandId(int command_id) {
 // SimpleMenuModel, MenuModel implementation:
 
 bool SimpleMenuModel::HasIcons() const {
+  for (std::vector<Item>::const_iterator iter = items_.begin();
+       iter != items_.end(); ++iter) {
+    if (!iter->icon.isNull())
+      return true;
+  }
+
   return false;
 }
 
@@ -175,7 +185,11 @@ int SimpleMenuModel::GetGroupIdAt(int index) const {
 }
 
 bool SimpleMenuModel::GetIconAt(int index, SkBitmap* icon) const {
-  return false;
+  if (items_[index].icon.isNull())
+    return false;
+
+  *icon = items_[index].icon;
+  return true;
 }
 
 bool SimpleMenuModel::IsEnabledAt(int index) const {
diff --git a/app/menus/simple_menu_model.h b/app/menus/simple_menu_model.h
index 59e741e..5307f12 100644
--- a/app/menus/simple_menu_model.h
+++ b/app/menus/simple_menu_model.h
@@ -9,6 +9,7 @@
 
 #include "base/string16.h"
 #include "app/menus/menu_model.h"
+#include "third_party/skia/include/core/SkBitmap.h"
 
 namespace menus {
 
@@ -75,6 +76,9 @@ class SimpleMenuModel : public MenuModel {
   void InsertSubMenuAt(int index, const string16& label, MenuModel* model);
   void InsertSubMenuWithStringIdAt(int index, int string_id, MenuModel* model);
 
+  // Sets the icon for the item at |index|.
+  void SetIcon(int index, const SkBitmap& icon);
+
   // Clears all items. Note that it does not free MenuModel of submenu.
   void Clear() {
     items_.clear();
@@ -115,6 +119,7 @@ class SimpleMenuModel : public MenuModel {
   struct Item {
     int command_id;
     string16 label;
+    SkBitmap icon;
     ItemType type;
     int group_id;
     MenuModel* submenu;
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);