linux: Update how menu icons are assigned.

Slightly change the architecture of how icons are assigned to menu items. Also,
add stock icons for some additional menu items. And always show icons on
overflow menus.

BUG=59101
TEST=The major change here is architectural change, so there is little visible
change in functionality. But if you right click on a link, it should now show
an icon for the 'Open in new tab' menu.

patch by sadrul@chromium.org
original review: http://codereview.chromium.org/3714003/show

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

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

12 files changed, 59 insertions, 24 deletions

diff --git a/chrome/browser/gtk/back_forward_button_gtk.cc b/chrome/browser/gtk/back_forward_button_gtk.cc
index d5a7f55..b55b587 100644
--- a/chrome/browser/gtk/back_forward_button_gtk.cc
+++ b/chrome/browser/gtk/back_forward_button_gtk.cc
@@ -74,7 +74,7 @@ void BackForwardButtonGtk::StoppedShowing() {
   button_->UnsetPaintOverride();
 }
 
-bool BackForwardButtonGtk::AlwaysShowImages() const {
+bool BackForwardButtonGtk::AlwaysShowIconForCmd(int command_id) const {
   return true;
 }
 
diff --git a/chrome/browser/gtk/back_forward_button_gtk.h b/chrome/browser/gtk/back_forward_button_gtk.h
index f32b5bb..c136627 100644
--- a/chrome/browser/gtk/back_forward_button_gtk.h
+++ b/chrome/browser/gtk/back_forward_button_gtk.h
@@ -26,7 +26,7 @@ class BackForwardButtonGtk : MenuGtk::Delegate {
 
   // MenuGtk::Delegate implementation.
   virtual void StoppedShowing();
-  bool AlwaysShowImages() const;
+  virtual bool AlwaysShowIconForCmd(int command_id) const;
 
   GtkWidget* widget() { return button_->widget(); }
 
diff --git a/chrome/browser/gtk/browser_actions_toolbar_gtk.cc b/chrome/browser/gtk/browser_actions_toolbar_gtk.cc
index a825d94..53776cf 100644
--- a/chrome/browser/gtk/browser_actions_toolbar_gtk.cc
+++ b/chrome/browser/gtk/browser_actions_toolbar_gtk.cc
@@ -672,6 +672,10 @@ void BrowserActionsToolbarGtk::StoppedShowing() {
   overflow_button_->UnsetPaintOverride();
 }
 
+bool BrowserActionsToolbarGtk::AlwaysShowIconForCmd(int command_id) const {
+  return true;
+}
+
 void BrowserActionsToolbarGtk::DragStarted(BrowserActionButton* button,
                                            GdkDragContext* drag_context) {
   // No representation of the widget following the cursor.
diff --git a/chrome/browser/gtk/browser_actions_toolbar_gtk.h b/chrome/browser/gtk/browser_actions_toolbar_gtk.h
index a0190b3..455efee 100644
--- a/chrome/browser/gtk/browser_actions_toolbar_gtk.h
+++ b/chrome/browser/gtk/browser_actions_toolbar_gtk.h
@@ -127,6 +127,7 @@ class BrowserActionsToolbarGtk : public ExtensionToolbarModel::Observer,
 
   // MenuGtk::Delegate implementation.
   virtual void StoppedShowing();
+  virtual bool AlwaysShowIconForCmd(int command_id) const;
 
   // Called by the BrowserActionButton in response to drag-begin.
   void DragStarted(BrowserActionButton* button, GdkDragContext* drag_context);
diff --git a/chrome/browser/gtk/browser_titlebar.cc b/chrome/browser/gtk/browser_titlebar.cc
index 5074e5d..9556ae6 100644
--- a/chrome/browser/gtk/browser_titlebar.cc
+++ b/chrome/browser/gtk/browser_titlebar.cc
@@ -656,7 +656,7 @@ void BrowserTitlebar::ShowFaviconMenu(GdkEventButton* event) {
     favicon_menu_model_.reset(
         new PopupPageMenuModel(this, browser_window_->browser()));
 
-    favicon_menu_.reset(new MenuGtk(this, favicon_menu_model_.get()));
+    favicon_menu_.reset(new MenuGtk(NULL, favicon_menu_model_.get()));
   }
 
   favicon_menu_->Popup(app_mode_favicon_, reinterpret_cast<GdkEvent*>(event));
@@ -749,7 +749,7 @@ gboolean BrowserTitlebar::OnButtonPressed(GtkWidget* widget,
 void BrowserTitlebar::ShowContextMenu() {
   if (!context_menu_.get()) {
     context_menu_model_.reset(new ContextMenuModel(this));
-    context_menu_.reset(new MenuGtk(this, context_menu_model_.get()));
+    context_menu_.reset(new MenuGtk(NULL, context_menu_model_.get()));
   }
 
   context_menu_->PopupAsContext(gtk_get_current_event_time());
diff --git a/chrome/browser/gtk/browser_titlebar.h b/chrome/browser/gtk/browser_titlebar.h
index 35491fa..431eddf4 100644
--- a/chrome/browser/gtk/browser_titlebar.h
+++ b/chrome/browser/gtk/browser_titlebar.h
@@ -17,7 +17,6 @@
 #include "app/menus/simple_menu_model.h"
 #include "app/gtk_signal.h"
 #include "base/scoped_ptr.h"
-#include "chrome/browser/gtk/menu_gtk.h"
 #include "chrome/common/notification_observer.h"
 #include "chrome/common/notification_registrar.h"
 
@@ -30,8 +29,7 @@ class TabContents;
 
 class BrowserTitlebar : public NotificationObserver,
                         public ActiveWindowWatcherX::Observer,
-                        public menus::SimpleMenuModel::Delegate,
-                        public MenuGtk::Delegate {
+                        public menus::SimpleMenuModel::Delegate {
  public:
   BrowserTitlebar(BrowserWindowGtk* browser_window, GtkWindow* window);
   virtual ~BrowserTitlebar();
diff --git a/chrome/browser/gtk/browser_toolbar_gtk.cc b/chrome/browser/gtk/browser_toolbar_gtk.cc
index e49e4f9..a710bdc 100644
--- a/chrome/browser/gtk/browser_toolbar_gtk.cc
+++ b/chrome/browser/gtk/browser_toolbar_gtk.cc
@@ -339,7 +339,7 @@ GtkIconSet* BrowserToolbarGtk::GetIconSetForId(int idr) {
 // Always show images because we desire that the upgrade icon always show when
 // an upgrade is available regardless of the system setting.
 bool BrowserToolbarGtk::AlwaysShowIconForCmd(int command_id) const {
-  return (command_id == IDC_UPGRADE_DIALOG);
+  return command_id == IDC_UPGRADE_DIALOG;
 }
 
 // menus::AcceleratorProvider
diff --git a/chrome/browser/gtk/download_item_gtk.cc b/chrome/browser/gtk/download_item_gtk.cc
index 536d2b4..4f44eb1 100644
--- a/chrome/browser/gtk/download_item_gtk.cc
+++ b/chrome/browser/gtk/download_item_gtk.cc
@@ -113,6 +113,26 @@ class DownloadShelfContextMenuGtk : public DownloadShelfContextMenu,
     gtk_widget_queue_draw(download_item_->menu_button_);
   }
 
+  virtual GtkWidget* GetImageForCommandId(int command_id) const {
+    const char* stock;
+    switch (command_id) {
+      case SHOW_IN_FOLDER:
+      case OPEN_WHEN_COMPLETE:
+        stock = GTK_STOCK_OPEN;
+        break;
+
+      case CANCEL:
+        stock = GTK_STOCK_CANCEL;
+        break;
+
+      case ALWAYS_OPEN_TYPE:
+      case TOGGLE_PAUSE:
+        stock = NULL;
+    }
+
+    return stock ? gtk_image_new_from_stock(stock, GTK_ICON_SIZE_MENU) : NULL;
+  }
+
  private:
   // The menu we show on Popup(). We keep a pointer to it for a couple reasons:
   //  * we don't want to have to recreate the menu every time it's popped up.
diff --git a/chrome/browser/gtk/menu_gtk.cc b/chrome/browser/gtk/menu_gtk.cc
index 104fca3..5d56621 100644
--- a/chrome/browser/gtk/menu_gtk.cc
+++ b/chrome/browser/gtk/menu_gtk.cc
@@ -117,10 +117,14 @@ int CalculateMenuYPosition(const GdkRectangle* screen_rect,
 
 }  // namespace
 
-GtkWidget* MenuGtk::Delegate::GetImageForCommandId(int command_id) const {
-  const char *stock;
+GtkWidget* MenuGtk::Delegate::GetDefaultImageForCommandId(int command_id) {
+  const char* stock;
   switch (command_id) {
     case IDC_NEW_TAB:
+    case IDC_CONTENT_CONTEXT_OPENIMAGENEWTAB:
+    case IDC_CONTENT_CONTEXT_OPENLINKNEWTAB:
+    case IDC_CONTENT_CONTEXT_OPENAVNEWTAB:
+    case IDC_CONTENT_CONTEXT_OPENFRAMENEWTAB:
       stock = GTK_STOCK_NEW;
       break;
 
@@ -249,6 +253,10 @@ GtkWidget* MenuGtk::Delegate::GetImageForCommandId(int command_id) const {
   return stock ? gtk_image_new_from_stock(stock, GTK_ICON_SIZE_MENU) : NULL;
 }
 
+GtkWidget* MenuGtk::Delegate::GetImageForCommandId(int command_id) const {
+  return GetDefaultImageForCommandId(command_id);
+}
+
 MenuGtk::MenuGtk(MenuGtk::Delegate* delegate,
                  menus::MenuModel* model)
     : delegate_(delegate),
@@ -310,7 +318,8 @@ GtkWidget* MenuGtk::AppendSeparator() {
 }
 
 GtkWidget* MenuGtk::AppendMenuItem(int command_id, GtkWidget* menu_item) {
-  if (delegate_ && delegate_->AlwaysShowIconForCmd(command_id))
+  if (delegate_ && delegate_->AlwaysShowIconForCmd(command_id) &&
+      GTK_IS_IMAGE_MENU_ITEM(menu_item))
     gtk_util::SetAlwaysShowImage(menu_item);
 
   return AppendMenuItemToMenu(command_id, NULL, menu_item, menu_, true);
@@ -388,7 +397,7 @@ void MenuGtk::UpdateMenu() {
 }
 
 GtkWidget* MenuGtk::BuildMenuItemWithImage(const std::string& label,
-                                  GtkWidget *image) {
+                                  GtkWidget* image) {
   GtkWidget* menu_item =
       gtk_image_menu_item_new_with_mnemonic(label.c_str());
   gtk_image_menu_item_set_image(GTK_IMAGE_MENU_ITEM(menu_item), image);
@@ -398,18 +407,19 @@ GtkWidget* MenuGtk::BuildMenuItemWithImage(const std::string& label,
 GtkWidget* MenuGtk::BuildMenuItemWithImage(const std::string& label,
                                            const SkBitmap& icon) {
   GdkPixbuf* pixbuf = gfx::GdkPixbufFromSkBitmap(&icon);
-  GtkWidget *menu_item = BuildMenuItemWithImage(label,
-                                gtk_image_new_from_pixbuf(pixbuf));
+  GtkWidget* menu_item = BuildMenuItemWithImage(label,
+      gtk_image_new_from_pixbuf(pixbuf));
   g_object_unref(pixbuf);
   return menu_item;
 }
 
 GtkWidget* MenuGtk::BuildMenuItemWithLabel(const std::string& label,
                                            int command_id) {
-  GtkWidget *img =
-    delegate_ ? delegate_->GetImageForCommandId(command_id) : NULL;
+  GtkWidget* img =
+      delegate_ ? delegate_->GetImageForCommandId(command_id) :
+                  MenuGtk::Delegate::GetDefaultImageForCommandId(command_id);
   return img ? BuildMenuItemWithImage(label, img) :
-    gtk_menu_item_new_with_mnemonic(label.c_str());
+               gtk_menu_item_new_with_mnemonic(label.c_str());
 }
 
 void MenuGtk::BuildMenuFromModel() {
@@ -462,8 +472,9 @@ void MenuGtk::BuildSubmenuFromModel(menus::MenuModel* model, GtkWidget* menu) {
           menu_item = BuildMenuItemWithImage(label, icon);
         else
           menu_item = BuildMenuItemWithLabel(label, command_id);
-        if (delegate_ && delegate_->AlwaysShowIconForCmd(command_id))
-            gtk_util::SetAlwaysShowImage(menu_item);
+        if (delegate_ && delegate_->AlwaysShowIconForCmd(command_id) &&
+            GTK_IS_IMAGE_MENU_ITEM(menu_item))
+          gtk_util::SetAlwaysShowImage(menu_item);
         break;
       }
 
diff --git a/chrome/browser/gtk/menu_gtk.h b/chrome/browser/gtk/menu_gtk.h
index 2f05813..7e4185b 100644
--- a/chrome/browser/gtk/menu_gtk.h
+++ b/chrome/browser/gtk/menu_gtk.h
@@ -50,6 +50,8 @@ class MenuGtk {
 
     // Returns an icon for the menu item, if available.
     virtual GtkWidget* GetImageForCommandId(int command_id) const;
+
+    static GtkWidget* GetDefaultImageForCommandId(int command_id);
   };
 
   MenuGtk(MenuGtk::Delegate* delegate, menus::MenuModel* model);
@@ -129,7 +131,7 @@ class MenuGtk {
                                     const SkBitmap& icon);
 
   GtkWidget* BuildMenuItemWithImage(const std::string& label,
-                                    GtkWidget *image);
+                                    GtkWidget* image);
 
   GtkWidget* BuildMenuItemWithLabel(const std::string& label,
                                     int command_id);
diff --git a/chrome/browser/gtk/tabs/tab_gtk.cc b/chrome/browser/gtk/tabs/tab_gtk.cc
index 36b86a3..04bc592 100644
--- a/chrome/browser/gtk/tabs/tab_gtk.cc
+++ b/chrome/browser/gtk/tabs/tab_gtk.cc
@@ -125,11 +125,10 @@ class TabGtk::ContextMenuController : public menus::SimpleMenuModel::Delegate,
       case TabStripModel::CommandTogglePinned:
       case TabStripModel::CommandBookmarkAllTabs:
       case TabStripModel::CommandUseVerticalTabs:
-      default:
         return NULL;
     }
 
-    return MenuGtk::Delegate::GetImageForCommandId(browser_cmd_id);
+    return MenuGtk::Delegate::GetDefaultImageForCommandId(browser_cmd_id);
   }
 
   // The context menu.
diff --git a/chrome/browser/tab_contents/render_view_context_menu_gtk.cc b/chrome/browser/tab_contents/render_view_context_menu_gtk.cc
index 4a12380..28d2fd8 100644
--- a/chrome/browser/tab_contents/render_view_context_menu_gtk.cc
+++ b/chrome/browser/tab_contents/render_view_context_menu_gtk.cc
@@ -48,6 +48,6 @@ void RenderViewContextMenuGtk::StoppedShowing() {
 }
 
 bool RenderViewContextMenuGtk::AlwaysShowIconForCmd(int command_id) const {
-  return (command_id >= IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST &&
-      command_id <= IDC_EXTENSIONS_CONTEXT_CUSTOM_LAST);
+  return command_id >= IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST &&
+      command_id <= IDC_EXTENSIONS_CONTEXT_CUSTOM_LAST;
 }