GTK: Display context menus on right-clicking a bookmark menu item.

- Refactors events so as a side effect, shift or control clicking a bookmark
  menu item opens with the correct disposition.
- Moves away from use of MenuGtk, as this menu does just too much weird stuff
  with events to use the general class.

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

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

4 files changed, 162 insertions, 130 deletions

diff --git a/chrome/browser/bookmarks/bookmark_menu_controller_gtk.cc b/chrome/browser/bookmarks/bookmark_menu_controller_gtk.cc
index 006c26d..6787338 100644
--- a/chrome/browser/bookmarks/bookmark_menu_controller_gtk.cc
+++ b/chrome/browser/bookmarks/bookmark_menu_controller_gtk.cc
@@ -4,17 +4,35 @@
 
 #include "chrome/browser/bookmarks/bookmark_menu_controller_gtk.h"
 
+#include <gtk/gtk.h>
+
 #include "app/l10n_util.h"
 #include "app/resource_bundle.h"
 #include "base/string_util.h"
 #include "chrome/browser/bookmarks/bookmark_context_menu.h"
+#include "chrome/browser/gtk/menu_gtk.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/tab_contents/page_navigator.h"
+#include "chrome/common/gtk_util.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 #include "webkit/glue/window_open_disposition.h"
 
-const int kEmptyId = -1;
+namespace {
+
+void SetImageMenuItem(GtkWidget* menu_item, const SkBitmap& bitmap) {
+  GdkPixbuf* pixbuf = gfx::GdkPixbufFromSkBitmap(&bitmap);
+  gtk_image_menu_item_set_image(GTK_IMAGE_MENU_ITEM(menu_item),
+                                gtk_image_new_from_pixbuf(pixbuf));
+  g_object_unref(pixbuf);
+}
+
+BookmarkNode* GetNodeFromMenuItem(GtkWidget* menu_item) {
+  return static_cast<BookmarkNode*>(
+      g_object_get_data(G_OBJECT(menu_item), "bookmark-node"));
+}
+
+}  // namespace
 
 BookmarkMenuController::BookmarkMenuController(Browser* browser,
                                                Profile* profile,
@@ -28,83 +46,143 @@ BookmarkMenuController::BookmarkMenuController(Browser* browser,
       page_navigator_(navigator),
       parent_window_(window),
       node_(node) {
-  menu_.reset(new MenuGtk(this, false));
-  int next_menu_id = 1;
-  BuildMenu(node, start_child_index, menu_.get(), &next_menu_id);
+  menu_.Own(gtk_menu_new());
+  BuildMenu(node, start_child_index, menu_.get());
+  gtk_widget_show_all(menu_.get());
 }
 
 BookmarkMenuController::~BookmarkMenuController() {
   profile_->GetBookmarkModel()->RemoveObserver(this);
+  menu_.Destroy();
 }
 
 void BookmarkMenuController::Popup(GtkWidget* widget, gint button_type,
                                    guint32 timestamp) {
   profile_->GetBookmarkModel()->AddObserver(this);
-  menu_->Popup(widget, button_type, timestamp);
+
+  gtk_menu_popup(GTK_MENU(menu_.get()), NULL, NULL,
+                 &MenuGtk::MenuPositionFunc,
+                 widget, button_type, timestamp);
 }
 
 void BookmarkMenuController::BookmarkModelChanged() {
-  menu_->Cancel();
+  gtk_menu_popdown(GTK_MENU(menu_.get()));
 }
 
 void BookmarkMenuController::BookmarkNodeFavIconLoaded(BookmarkModel* model,
                                                        BookmarkNode* node) {
-  if (node_to_menu_id_map_.find(node) != node_to_menu_id_map_.end())
-    menu_->SetIcon(node->GetFavIcon(), node_to_menu_id_map_[node]);
-}
-
-bool BookmarkMenuController::IsCommandEnabled(int id) const {
-  return id != kEmptyId;
+  std::map<BookmarkNode*, GtkWidget*>::iterator it =
+      node_to_menu_widget_map_.find(node);
+  if (it != node_to_menu_widget_map_.end())
+    SetImageMenuItem(it->second, node->GetFavIcon());
 }
 
-void BookmarkMenuController::ExecuteCommand(int id) {
+void BookmarkMenuController::NavigateToMenuItem(
+    GtkWidget* menu_item,
+    WindowOpenDisposition disposition) {
+  BookmarkNode* node = GetNodeFromMenuItem(menu_item);
+  DCHECK(node);
   DCHECK(page_navigator_);
-  DCHECK(menu_id_to_node_map_.find(id) != menu_id_to_node_map_.end());
-  GURL url = menu_id_to_node_map_[id]->GetURL();
   page_navigator_->OpenURL(
-      url, GURL(),
-      // TODO(erg): Plumb mouse events here so things like shift-click or
-      // ctrl-click do the right things.
-      //
-      // event_utils::DispositionFromEventFlags(mouse_event_flags),
-      CURRENT_TAB,
-      PageTransition::AUTO_BOOKMARK);
+      node->GetURL(), GURL(), disposition, PageTransition::AUTO_BOOKMARK);
 }
 
 void BookmarkMenuController::BuildMenu(BookmarkNode* parent,
                                        int start_child_index,
-                                       MenuGtk* menu,
-                                       int* next_menu_id) {
+                                       GtkWidget* menu) {
   DCHECK(!parent->GetChildCount() ||
          start_child_index < parent->GetChildCount());
   for (int i = start_child_index; i < parent->GetChildCount(); ++i) {
     BookmarkNode* node = parent->GetChild(i);
-    int id = *next_menu_id;
 
-    (*next_menu_id)++;
+    GtkWidget* menu_item = gtk_image_menu_item_new_with_label(
+        WideToUTF8(node->GetTitle()).c_str());
+
     if (node->is_url()) {
       SkBitmap icon = node->GetFavIcon();
       if (icon.width() == 0) {
         icon = *ResourceBundle::GetSharedInstance().
             GetBitmapNamed(IDR_DEFAULT_FAVICON);
       }
-      menu->AppendMenuItemWithIcon(id, WideToUTF8(node->GetTitle()), icon);
-      node_to_menu_id_map_[node] = id;
+      SetImageMenuItem(menu_item, icon);
+      g_object_set_data(G_OBJECT(menu_item), "bookmark-node", node);
+      g_signal_connect(G_OBJECT(menu_item), "activate",
+                       G_CALLBACK(OnMenuItemActivated), this);
+      g_signal_connect(G_OBJECT(menu_item), "button-press-event",
+                       G_CALLBACK(OnButtonPressed), this);
+      g_signal_connect(G_OBJECT(menu_item), "button-release-event",
+                       G_CALLBACK(OnButtonReleased), this);
     } else if (node->is_folder()) {
       SkBitmap* folder_icon = ResourceBundle::GetSharedInstance().
           GetBitmapNamed(IDR_BOOKMARK_BAR_FOLDER);
-      MenuGtk* submenu =
-          menu->AppendSubMenuWithIcon(id, WideToUTF8(node->GetTitle()),
-                                      *folder_icon);
-      BuildMenu(node, 0, submenu, next_menu_id);
+      SetImageMenuItem(menu_item, *folder_icon);
+
+      GtkWidget* submenu = gtk_menu_new();
+      BuildMenu(node, 0, submenu);
+      gtk_menu_item_set_submenu(GTK_MENU_ITEM(menu_item), submenu);
     } else {
       NOTREACHED();
     }
-    menu_id_to_node_map_[id] = node;
+
+    gtk_menu_shell_append(GTK_MENU_SHELL(menu), menu_item);
+    node_to_menu_widget_map_[node] = menu_item;
   }
 
   if (parent->GetChildCount() == 0) {
-    menu->AppendMenuItemWithLabel(
-        kEmptyId, l10n_util::GetStringUTF8(IDS_MENU_EMPTY_SUBMENU));
+    GtkWidget* empty_menu = gtk_menu_item_new_with_label(
+        l10n_util::GetStringUTF8(IDS_MENU_EMPTY_SUBMENU).c_str());
+    gtk_widget_set_sensitive(empty_menu, FALSE);
+    gtk_menu_shell_append(GTK_MENU_SHELL(menu), empty_menu);
   }
 }
+
+gboolean BookmarkMenuController::OnButtonPressed(
+    GtkWidget* sender,
+    GdkEventButton* event,
+    BookmarkMenuController* controller) {
+  if (event->button == 3) {
+    // Show the right click menu and stop processing this button event.
+    BookmarkNode* node = GetNodeFromMenuItem(sender);
+    BookmarkNode* parent = node->GetParent();
+    std::vector<BookmarkNode*> nodes;
+    nodes.push_back(node);
+    controller->context_menu_.reset(
+        new BookmarkContextMenu(
+            GTK_WINDOW(gtk_widget_get_toplevel(sender)),
+            controller->profile_, controller->browser_,
+            controller->page_navigator_, parent, nodes,
+            BookmarkContextMenu::BOOKMARK_BAR));
+    controller->context_menu_->PopupAsContext(event->time);
+    return TRUE;
+  }
+
+  return FALSE;
+}
+
+gboolean BookmarkMenuController::OnButtonReleased(
+    GtkWidget* sender,
+    GdkEventButton* event,
+    BookmarkMenuController* controller) {
+  // TODO(erg): The OnButtonPressed and OnButtonReleased handlers should have
+  // the same guard code that prevents them from interfering with DnD as
+  // BookmarkBarGtk's versions.
+
+  // Releasing button 1 should trigger the bookmark menu.
+  if (event->button == 1) {
+    WindowOpenDisposition disposition =
+        event_utils::DispositionFromEventFlags(event->state);
+    controller->NavigateToMenuItem(sender, disposition);
+
+    // We need to manually dismiss the popup menu because we're overriding
+    // button-release-event.
+    gtk_menu_popdown(GTK_MENU(controller->menu_.get()));
+    return TRUE;
+  }
+
+  return FALSE;
+}
+
+void BookmarkMenuController::OnMenuItemActivated(
+    GtkMenuItem* menu_item, BookmarkMenuController* controller) {
+  controller->NavigateToMenuItem(GTK_WIDGET(menu_item), CURRENT_TAB);
+}
diff --git a/chrome/browser/bookmarks/bookmark_menu_controller_gtk.h b/chrome/browser/bookmarks/bookmark_menu_controller_gtk.h
index b7914c3..c260bcf 100644
--- a/chrome/browser/bookmarks/bookmark_menu_controller_gtk.h
+++ b/chrome/browser/bookmarks/bookmark_menu_controller_gtk.h
@@ -5,11 +5,14 @@
 #ifndef CHROME_BROWSER_BOOKMARKS_BOOKMARK_MENU_CONTROLLER_GTK_H_
 #define CHROME_BROWSER_BOOKMARKS_BOOKMARK_MENU_CONTROLLER_GTK_H_
 
+#include <gtk/gtk.h>
+
 #include <map>
 
 #include "base/scoped_ptr.h"
 #include "chrome/browser/bookmarks/base_bookmark_model_observer.h"
-#include "chrome/browser/gtk/menu_gtk.h"
+#include "chrome/common/owned_widget_gtk.h"
+#include "webkit/glue/window_open_disposition.h"
 
 class BookmarkContextMenu;
 class Browser;
@@ -18,10 +21,7 @@ class PageNavigator;
 class BookmarkModel;
 class BookmarkNode;
 
-typedef struct _GtkWidget GtkWidget;
-
-class BookmarkMenuController : public BaseBookmarkModelObserver,
-                               public MenuGtk::Delegate {
+class BookmarkMenuController : public BaseBookmarkModelObserver {
  public:
   // Creates a BookmarkMenuController showing the children of |node| starting
   // at index |start_child_index|.
@@ -34,6 +34,7 @@ class BookmarkMenuController : public BaseBookmarkModelObserver,
                          bool show_other_folder);
   virtual ~BookmarkMenuController();
 
+  // Pops up the menu.
   void Popup(GtkWidget* widget, gint button_type, guint32 timestamp);
 
   // Overridden from BaseBookmarkModelObserver:
@@ -41,15 +42,32 @@ class BookmarkMenuController : public BaseBookmarkModelObserver,
   virtual void BookmarkNodeFavIconLoaded(BookmarkModel* model,
                                          BookmarkNode* node);
 
-  // Overridden from MenuGtk::Delegate:
-  virtual bool IsCommandEnabled(int id) const;
-  virtual void ExecuteCommand(int id);
-
  private:
+  // Recursively change the bookmark hierarchy rooted in |parent| into a set of
+  // gtk menus rooted in |menu|.
   void BuildMenu(BookmarkNode* parent,
                  int start_child_index,
-                 MenuGtk* menu,
-                 int* next_menu_id);
+                 GtkWidget* menu);
+
+  // Calls the page navigator to navigate to the node represented by
+  // |menu_item|.
+  void NavigateToMenuItem(GtkWidget* menu_item,
+                          WindowOpenDisposition disposition);
+
+  // Button press and release events for a GtkMenuItem. We have to override
+  // these separate from OnMenuItemActivated because we need to handle right
+  // clicks and opening bookmarks with different dispositions.
+  static gboolean OnButtonPressed(GtkWidget* sender,
+                                  GdkEventButton* event,
+                                  BookmarkMenuController* controller);
+  static gboolean OnButtonReleased(GtkWidget* sender,
+                                   GdkEventButton* event,
+                                   BookmarkMenuController* controller);
+
+  // We respond to the activate signal because things other than mouse button
+  // events can trigger it.
+  static void OnMenuItemActivated(GtkMenuItem* menuitem,
+                                  BookmarkMenuController* controller);
 
   Browser* browser_;
   Profile* profile_;
@@ -61,16 +79,17 @@ class BookmarkMenuController : public BaseBookmarkModelObserver,
   // The node we're showing the contents of.
   BookmarkNode* node_;
 
-  scoped_ptr<MenuGtk> menu_;
-
-  // Maps from menu id to BookmarkNode.
-  std::map<int, BookmarkNode*> menu_id_to_node_map_;
+  // Our bookmark menus. We don't use the MenuGtk class because we have to do
+  // all sorts of weird non-standard things with this menu, like:
+  // - The menu is a drag target
+  // - The menu items have context menus.
+  OwnedWidgetGtk menu_;
 
-  // Mapping from node to menu id. This only contains entries for nodes of type
-  // URL.
-  std::map<BookmarkNode*, int> node_to_menu_id_map_;
+  // Mapping from node to GtkMenuItem menu id. This only contains entries for
+  // nodes of type URL.
+  std::map<BookmarkNode*, GtkWidget*> node_to_menu_widget_map_;
 
-  // Used when a context menu is shown.
+  // Owns our right click context menu.
   scoped_ptr<BookmarkContextMenu> context_menu_;
 
   DISALLOW_COPY_AND_ASSIGN(BookmarkMenuController);
diff --git a/chrome/browser/gtk/menu_gtk.cc b/chrome/browser/gtk/menu_gtk.cc
index b6df673..ca3da0e 100644
--- a/chrome/browser/gtk/menu_gtk.cc
+++ b/chrome/browser/gtk/menu_gtk.cc
@@ -11,31 +11,6 @@
 #include "chrome/common/gtk_util.h"
 #include "skia/include/SkBitmap.h"
 
-namespace {
-
-struct SetIconState {
-  bool found;
-  const SkBitmap* icon;
-  int id;
-};
-
-void SetIconImpl(GtkWidget* widget, void* raw) {
-  SetIconState* data = reinterpret_cast<SetIconState*>(raw);
-  int this_id =
-      reinterpret_cast<int>(g_object_get_data(G_OBJECT(widget), "menu-id"));
-
-  if (this_id == data->id) {
-    GdkPixbuf* pixbuf = gfx::GdkPixbufFromSkBitmap(data->icon);
-    gtk_image_menu_item_set_image(GTK_IMAGE_MENU_ITEM(widget),
-                                  gtk_image_new_from_pixbuf(pixbuf));
-    g_object_unref(pixbuf);
-
-    data->found = true;
-  }
-}
-
-}  // namespace
-
 MenuGtk::MenuGtk(MenuGtk::Delegate* delegate,
                  const MenuCreateMaterial* menu_data,
                  GtkAccelGroup* accel_group)
@@ -54,7 +29,6 @@ MenuGtk::MenuGtk(MenuGtk::Delegate* delegate, bool load)
 }
 
 MenuGtk::~MenuGtk() {
-  STLDeleteElements(&children_);
   menu_.Destroy();
   if (dummy_accel_group_)
     g_object_unref(dummy_accel_group_);
@@ -73,20 +47,6 @@ void MenuGtk::AppendMenuItemWithIcon(int command_id,
   AddMenuItemWithId(menu_item, command_id);
 }
 
-MenuGtk* MenuGtk::AppendSubMenuWithIcon(int command_id,
-                                        const std::string& label,
-                                        const SkBitmap& icon) {
-  GtkWidget* menu_item = BuildMenuItemWithImage(label, icon);
-
-  MenuGtk* submenu = new MenuGtk(delegate_, false);
-  gtk_menu_item_set_submenu(GTK_MENU_ITEM(menu_item), submenu->menu_.get());
-  children_.push_back(submenu);
-
-  AddMenuItemWithId(menu_item, command_id);
-
-  return submenu;
-}
-
 void MenuGtk::AppendSeparator() {
   GtkWidget* menu_item = gtk_separator_menu_item_new();
   gtk_widget_show(menu_item);
@@ -122,25 +82,6 @@ void MenuGtk::Cancel() {
   gtk_menu_popdown(GTK_MENU(menu_.get()));
 }
 
-bool MenuGtk::SetIcon(const SkBitmap& icon, int item_id) {
-  // First search items in this menu.
-  SetIconState state;
-  state.found = false;
-  state.icon = &icon;
-  state.id = item_id;
-  gtk_container_foreach(GTK_CONTAINER(menu_.get()), SetIconImpl, &state);
-  if (state.found)
-    return true;
-
-  for (std::vector<MenuGtk*>::iterator it = children_.begin();
-       it != children_.end(); ++it) {
-    if ((*it)->SetIcon(icon, item_id))
-      return true;
-  }
-
-  return false;
-}
-
 // static
 std::string MenuGtk::ConvertAcceleratorsFromWindowsStyle(
     const std::string& label) {
diff --git a/chrome/browser/gtk/menu_gtk.h b/chrome/browser/gtk/menu_gtk.h
index b7afd67..24eef9a 100644
--- a/chrome/browser/gtk/menu_gtk.h
+++ b/chrome/browser/gtk/menu_gtk.h
@@ -51,8 +51,6 @@ class MenuGtk {
   void AppendMenuItemWithLabel(int command_id, const std::string& label);
   void AppendMenuItemWithIcon(int command_id, const std::string& label,
                               const SkBitmap& icon);
-  MenuGtk* AppendSubMenuWithIcon(int command_id, const std::string& label,
-                                 const SkBitmap& icon);
   void AppendSeparator();
 
   // Displays the menu. |timestamp| is the time of activation. The popup is
@@ -82,6 +80,17 @@ class MenuGtk {
   static std::string ConvertAcceleratorsFromWindowsStyle(
       const std::string& label);
 
+  // Repositions the menu to be right under the button.  Alignment is set as
+  // object data on |void_widget| with the tag "left_align".  If "left_align"
+  // is true, it aligns the left side of the menu with the left side of the
+  // button. Otherwise it aligns the right side of the menu with the right side
+  // of the button. Public since some menus have odd requirements that don't
+  // belong in a public class.
+  static void MenuPositionFunc(GtkMenu* menu,
+                               int* x,
+                               int* y,
+                               gboolean* push_in,
+                               void* void_widget);
  private:
   // A recursive function that transforms a MenuCreateMaterial tree into a set
   // of GtkMenuItems.
@@ -109,17 +118,6 @@ class MenuGtk {
   // via |delegate_|.
   static void OnMenuItemActivatedById(GtkMenuItem* menuitem, MenuGtk* menu);
 
-  // Repositions the menu to be right under the button.
-  // Alignment is set as object data on |void_widget| with the tag "left_align".
-  // If "left_align" is true, it aligns the left side of the menu with the left
-  // side of the button. Otherwise it aligns the right side of the menu with the
-  // right side of the button.
-  static void MenuPositionFunc(GtkMenu* menu,
-                               int* x,
-                               int* y,
-                               gboolean* push_in,
-                               void* void_widget);
-
   // Sets the check mark and enabled/disabled state on our menu items.
   static void SetMenuItemInfo(GtkWidget* widget, void* raw_menu);
 
@@ -134,10 +132,6 @@ class MenuGtk {
   // gtk_menu_popup() does not appear to take ownership of popup menus, so
   // MenuGtk explicitly manages the lifetime of the menu.
   OwnedWidgetGtk menu_;
-
-  // MenuGtk instances of submenus; we keep references to these objects just to
-  // clean up during our destructor.
-  std::vector<MenuGtk*> children_;
 };
 
 #endif  // CHROME_BROWSER_GTK_MENU_GTK_H_