GTK: Use page and app menu models.

- implement more MenuModel support for MenuGtk
- copy some accelerators around

TODO: centralize accelerator list (currently they are all listed twice)
TODO: get rid of standard_menus.cc (it is still used by the popup favicon menu)

BUG=NONE
TEST=page and app menus still work, still display accelerators, accelerators still work

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

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

18 files changed, 282 insertions, 113 deletions

diff --git a/chrome/browser/gtk/blocked_popup_container_view_gtk.cc b/chrome/browser/gtk/blocked_popup_container_view_gtk.cc
index bc4bb1c..82aa848 100644
--- a/chrome/browser/gtk/blocked_popup_container_view_gtk.cc
+++ b/chrome/browser/gtk/blocked_popup_container_view_gtk.cc
@@ -149,7 +149,7 @@ bool BlockedPopupContainerViewGtk::IsItemChecked(int id) const {
   return false;
 }
 
-void BlockedPopupContainerViewGtk::ExecuteCommand(int id) {
+void BlockedPopupContainerViewGtk::ExecuteCommandById(int id) {
   DCHECK_GT(id, 0);
   size_t id_size_t = static_cast<size_t>(id);
 
@@ -220,7 +220,7 @@ void BlockedPopupContainerViewGtk::Init() {
 
 void BlockedPopupContainerViewGtk::OnMenuButtonClicked(
     GtkButton *button, BlockedPopupContainerViewGtk* container) {
-  container->launch_menu_.reset(new MenuGtk(container, false));
+  container->launch_menu_.reset(new MenuGtk(container));
 
   // Set items 1 .. popup_count as individual popups.
   size_t popup_count = container->model_->GetBlockedPopupCount();
diff --git a/chrome/browser/gtk/blocked_popup_container_view_gtk.h b/chrome/browser/gtk/blocked_popup_container_view_gtk.h
index 50a256b3..a0bfa6c 100644
--- a/chrome/browser/gtk/blocked_popup_container_view_gtk.h
+++ b/chrome/browser/gtk/blocked_popup_container_view_gtk.h
@@ -63,7 +63,7 @@ class BlockedPopupContainerViewGtk : public BlockedPopupContainerView,
   // Overridden from MenuGtk::Delegate:
   virtual bool IsCommandEnabled(int command_id) const;
   virtual bool IsItemChecked(int command_id) const;
-  virtual void ExecuteCommand(int command_id);
+  virtual void ExecuteCommandById(int command_id);
 
  private:
   // For the static constructor BlockedPopupContainerView::Create().
diff --git a/chrome/browser/gtk/bookmark_context_menu_gtk.cc b/chrome/browser/gtk/bookmark_context_menu_gtk.cc
index 8c94609..5bf57ac 100644
--- a/chrome/browser/gtk/bookmark_context_menu_gtk.cc
+++ b/chrome/browser/gtk/bookmark_context_menu_gtk.cc
@@ -293,7 +293,7 @@ void BookmarkContextMenuGtk::DelegateDestroyed() {
   delegate_ = NULL;
 }
 
-void BookmarkContextMenuGtk::ExecuteCommand(int id) {
+void BookmarkContextMenuGtk::ExecuteCommandById(int id) {
   if (delegate_)
     delegate_->WillExecuteCommand();
 
@@ -526,7 +526,7 @@ void BookmarkContextMenuGtk::ModelChanged() {
 }
 
 void BookmarkContextMenuGtk::CreateMenuObject() {
-  menu_.reset(new MenuGtk(this, false));
+  menu_.reset(new MenuGtk(this));
 }
 
 void BookmarkContextMenuGtk::AppendItem(int id) {
diff --git a/chrome/browser/gtk/bookmark_context_menu_gtk.h b/chrome/browser/gtk/bookmark_context_menu_gtk.h
index faa4bed..a266840 100644
--- a/chrome/browser/gtk/bookmark_context_menu_gtk.h
+++ b/chrome/browser/gtk/bookmark_context_menu_gtk.h
@@ -70,7 +70,7 @@ class BookmarkContextMenuGtk : public BookmarkModelObserver,
   void DelegateDestroyed();
 
   // Menu::Delegate / MenuGtk::Delegate methods.
-  virtual void ExecuteCommand(int id);
+  virtual void ExecuteCommandById(int id);
   virtual bool IsItemChecked(int id) const;
   virtual bool IsCommandEnabled(int id) const;
 
diff --git a/chrome/browser/gtk/bookmark_manager_gtk.cc b/chrome/browser/gtk/bookmark_manager_gtk.cc
index cc3b9f3..a7e5ee7 100644
--- a/chrome/browser/gtk/bookmark_manager_gtk.cc
+++ b/chrome/browser/gtk/bookmark_manager_gtk.cc
@@ -1335,7 +1335,7 @@ gboolean BookmarkManagerGtk::OnTreeViewKeyPress(
 
   if (bm->organize_menu_.get() &&
       bm->organize_menu_->IsCommandEnabled(command)) {
-    bm->organize_menu_->ExecuteCommand(command);
+    bm->organize_menu_->ExecuteCommandById(command);
     return TRUE;
   }
 
diff --git a/chrome/browser/gtk/browser_titlebar.cc b/chrome/browser/gtk/browser_titlebar.cc
index 55c192e..bfd4b42 100644
--- a/chrome/browser/gtk/browser_titlebar.cc
+++ b/chrome/browser/gtk/browser_titlebar.cc
@@ -125,14 +125,14 @@ GdkColor PickLuminosityContrastingColor(const GdkColor* base,
 
 MenuCreateMaterial g_favicon_menu[] = {
   { MENU_NORMAL, IDC_BACK, IDS_CONTENT_CONTEXT_BACK, 0, NULL,
-    GDK_Left, GDK_MOD1_MASK, true },
+    GDK_Left, GDK_MOD1_MASK },
   { MENU_NORMAL, IDC_FORWARD, IDS_CONTENT_CONTEXT_FORWARD, 0, NULL,
-    GDK_Right, GDK_MOD1_MASK, true },
+    GDK_Right, GDK_MOD1_MASK },
   { MENU_NORMAL, IDC_RELOAD, IDS_APP_MENU_RELOAD, 0, NULL,
-    GDK_R, GDK_CONTROL_MASK, true },
+    GDK_R, GDK_CONTROL_MASK },
   { MENU_SEPARATOR },
   { MENU_NORMAL, IDC_RESTORE_TAB, IDS_RESTORE_TAB, 0, NULL,
-    GDK_T, GDK_CONTROL_MASK | GDK_SHIFT_MASK, true },
+    GDK_T, GDK_CONTROL_MASK | GDK_SHIFT_MASK },
   { MENU_NORMAL, IDC_DUPLICATE_TAB, IDS_APP_MENU_DUPLICATE_APP_WINDOW },
   { MENU_NORMAL, IDC_COPY_URL, IDS_APP_MENU_COPY_URL },
   { MENU_NORMAL, IDC_SHOW_AS_TAB, IDS_SHOW_AS_TAB },
@@ -501,7 +501,7 @@ void BrowserTitlebar::UpdateTextColor() {
 void BrowserTitlebar::ShowFaviconMenu(GdkEventButton* event) {
   if (!favicon_menu_.get()) {
     favicon_menu_.reset(new MenuGtk(this,
-        GetFaviconMenu(browser_window_->browser()->profile(), this), NULL));
+        GetFaviconMenu(browser_window_->browser()->profile(), this)));
   }
 
   favicon_menu_->Popup(app_mode_favicon_, reinterpret_cast<GdkEvent*>(event));
@@ -596,12 +596,12 @@ void BrowserTitlebar::ShowContextMenu() {
   if (!context_menu_.get()) {
     static const MenuCreateMaterial context_menu_blueprint[] = {
         { MENU_NORMAL, IDC_NEW_TAB, IDS_TAB_CXMENU_NEWTAB, 0, NULL,
-            GDK_t, GDK_CONTROL_MASK, true },
+            GDK_t, GDK_CONTROL_MASK },
         { MENU_NORMAL, IDC_RESTORE_TAB, IDS_RESTORE_TAB, 0, NULL,
-            GDK_t, GDK_CONTROL_MASK | GDK_SHIFT_MASK, true },
+            GDK_t, GDK_CONTROL_MASK | GDK_SHIFT_MASK },
         { MENU_SEPARATOR },
         { MENU_NORMAL, IDC_TASK_MANAGER, IDS_TASK_MANAGER, 0, NULL,
-            GDK_Escape, GDK_SHIFT_MASK, true },
+            GDK_Escape, GDK_SHIFT_MASK },
 #if !defined(OS_CHROMEOS)
         { MENU_SEPARATOR },
         { MENU_CHECKBOX, kShowWindowDecorationsCommand,
@@ -610,7 +610,7 @@ void BrowserTitlebar::ShowContextMenu() {
         { MENU_END },
     };
 
-    context_menu_.reset(new MenuGtk(this, context_menu_blueprint, NULL));
+    context_menu_.reset(new MenuGtk(this, context_menu_blueprint));
   }
 
   context_menu_->PopupAsContext(gtk_get_current_event_time());
@@ -646,7 +646,7 @@ bool BrowserTitlebar::IsItemChecked(int command_id) const {
   return false;
 }
 
-void BrowserTitlebar::ExecuteCommand(int command_id) {
+void BrowserTitlebar::ExecuteCommandById(int command_id) {
   if (command_id == kShowWindowDecorationsCommand) {
     PrefService* prefs = browser_window_->browser()->profile()->GetPrefs();
     prefs->SetBoolean(prefs::kUseCustomChromeFrame,
diff --git a/chrome/browser/gtk/browser_titlebar.h b/chrome/browser/gtk/browser_titlebar.h
index d9d75e1..a604be2 100644
--- a/chrome/browser/gtk/browser_titlebar.h
+++ b/chrome/browser/gtk/browser_titlebar.h
@@ -124,7 +124,7 @@ class BrowserTitlebar : public MenuGtk::Delegate,
   // MenuGtk::Delegate implementation:
   virtual bool IsCommandEnabled(int command_id) const;
   virtual bool IsItemChecked(int command_id) const;
-  virtual void ExecuteCommand(int command_id);
+  virtual void ExecuteCommandById(int command_id);
 
   // Overridden from NotificationObserver:
   virtual void Observe(NotificationType type,
diff --git a/chrome/browser/gtk/browser_toolbar_gtk.cc b/chrome/browser/gtk/browser_toolbar_gtk.cc
index 13a0508..ff224e4 100644
--- a/chrome/browser/gtk/browser_toolbar_gtk.cc
+++ b/chrome/browser/gtk/browser_toolbar_gtk.cc
@@ -11,8 +11,10 @@
 #include "app/gfx/gtk_util.h"
 #include "app/gtk_dnd_util.h"
 #include "app/l10n_util.h"
+#include "app/menus/accelerator_gtk.h"
 #include "app/resource_bundle.h"
 #include "base/base_paths.h"
+#include "base/keyboard_codes_posix.h"
 #include "base/logging.h"
 #include "base/path_service.h"
 #include "chrome/app/chrome_dll_resource.h"
@@ -79,6 +81,8 @@ BrowserToolbarGtk::BrowserToolbarGtk(Browser* browser, BrowserWindowGtk* window)
                                            this,
                                            browser)),
       model_(browser->toolbar_model()),
+      page_menu_model_(this, browser),
+      app_menu_model_(this, browser),
       browser_(browser),
       window_(window),
       profile_(NULL),
@@ -107,13 +111,10 @@ BrowserToolbarGtk::~BrowserToolbarGtk() {
 
   offscreen_entry_.Destroy();
 
-  // When we created our MenuGtk objects, we pass them a pointer to our accel
-  // group. Make sure to tear them down before |accel_group_|.
   page_menu_.reset();
   app_menu_.reset();
   page_menu_button_.Destroy();
   app_menu_button_.Destroy();
-  g_object_unref(accel_group_);
 }
 
 void BrowserToolbarGtk::Init(Profile* profile,
@@ -145,14 +146,6 @@ void BrowserToolbarGtk::Init(Profile* profile,
   gtk_widget_set_size_request(toolbar_, -1, ShouldOnlyShowLocation() ?
       kToolbarHeightLocationBarOnly : kToolbarHeight);
 
-  // A GtkAccelGroup is not InitiallyUnowned, meaning we get a real reference
-  // count starting at one.  We don't want the lifetime to be managed by the
-  // top level window, since the lifetime should be tied to the C++ object.
-  // When we add the accelerator group, the window will take a reference, but
-  // we still hold on to the original, and thus own a reference to the group.
-  accel_group_ = gtk_accel_group_new();
-  gtk_window_add_accel_group(top_level_window, accel_group_);
-
   // Group back and forward into an hbox so there's no spacing between them.
   GtkWidget* back_forward_hbox_ = gtk_hbox_new(FALSE, 0);
 
@@ -214,8 +207,7 @@ void BrowserToolbarGtk::Init(Profile* profile,
       theme_provider_->GetRTLEnabledPixbufNamed(IDR_MENU_PAGE));
   gtk_container_add(GTK_CONTAINER(page_menu), page_menu_image_);
 
-  page_menu_.reset(new MenuGtk(this, GetStandardPageMenu(profile_, this),
-                               accel_group_));
+  page_menu_.reset(new MenuGtk(this, &page_menu_model_));
   gtk_box_pack_start(GTK_BOX(menus_hbox_), page_menu, FALSE, FALSE, 0);
 
   GtkWidget* chrome_menu = BuildToolbarMenuButton(
@@ -226,7 +218,8 @@ void BrowserToolbarGtk::Init(Profile* profile,
   app_menu_image_ = gtk_image_new_from_pixbuf(
       theme_provider_->GetRTLEnabledPixbufNamed(IDR_MENU_CHROME));
   gtk_container_add(GTK_CONTAINER(chrome_menu), app_menu_image_);
-  app_menu_.reset(new MenuGtk(this, GetStandardAppMenu(), accel_group_));
+
+  app_menu_.reset(new MenuGtk(this, &app_menu_model_));
   gtk_box_pack_start(GTK_BOX(menus_hbox_), chrome_menu, FALSE, FALSE, 0);
 
   gtk_box_pack_start(GTK_BOX(toolbar_), menus_hbox_, FALSE, FALSE, 0);
@@ -336,11 +329,20 @@ void BrowserToolbarGtk::EnabledStateChangedForCommand(int id, bool enabled) {
 
 // MenuGtk::Delegate -----------------------------------------------------------
 
-bool BrowserToolbarGtk::IsCommandEnabled(int command_id) const {
-  return browser_->command_updater()->IsCommandEnabled(command_id);
+void BrowserToolbarGtk::StoppedShowing() {
+  gtk_chrome_button_unset_paint_state(
+      GTK_CHROME_BUTTON(page_menu_button_.get()));
+  gtk_chrome_button_unset_paint_state(
+      GTK_CHROME_BUTTON(app_menu_button_.get()));
+}
+
+// menus::SimpleMenuModel::Delegate
+
+bool BrowserToolbarGtk::IsCommandIdEnabled(int id) const {
+  return browser_->command_updater()->IsCommandEnabled(id);
 }
 
-bool BrowserToolbarGtk::IsItemChecked(int id) const {
+bool BrowserToolbarGtk::IsCommandIdChecked(int id) const {
   if (!profile_)
     return false;
 
@@ -362,11 +364,110 @@ void BrowserToolbarGtk::ExecuteCommand(int id) {
   browser_->ExecuteCommand(id);
 }
 
-void BrowserToolbarGtk::StoppedShowing() {
-  gtk_chrome_button_unset_paint_state(
-      GTK_CHROME_BUTTON(page_menu_button_.get()));
-  gtk_chrome_button_unset_paint_state(
-      GTK_CHROME_BUTTON(app_menu_button_.get()));
+bool BrowserToolbarGtk::GetAcceleratorForCommandId(
+    int id,
+    menus::Accelerator* accelerator) {
+  switch (id) {
+    case IDC_ZOOM_PLUS:
+      *accelerator = menus::AcceleratorGtk(base::VKEY_ADD, false, true, false);
+      break;
+
+    case IDC_ZOOM_NORMAL:
+      *accelerator = menus::AcceleratorGtk(base::VKEY_NUMPAD0,
+                                           false, true, false);
+      break;
+
+    case IDC_ZOOM_MINUS:
+      *accelerator = menus::AcceleratorGtk(base::VKEY_SUBTRACT,
+                                           false, true, false);
+      break;
+
+    case IDC_VIEW_SOURCE:
+      *accelerator = menus::AcceleratorGtk(base::VKEY_U, false, true, false);
+      break;
+
+    case IDC_DEV_TOOLS:
+      *accelerator = menus::AcceleratorGtk(base::VKEY_I, true, true, false);
+      break;
+
+    case IDC_DEV_TOOLS_CONSOLE:
+      *accelerator = menus::AcceleratorGtk(base::VKEY_J, true, true, false);
+      break;
+
+    case IDC_TASK_MANAGER:
+      *accelerator = menus::AcceleratorGtk(base::VKEY_ESCAPE,
+                                           true, false, false);
+      break;
+
+    case IDC_CUT:
+      *accelerator = menus::AcceleratorGtk(base::VKEY_X, false, true, false);
+      break;
+
+    case IDC_COPY:
+      *accelerator = menus::AcceleratorGtk(base::VKEY_C, false, true, false);
+      break;
+
+    case IDC_PASTE:
+      *accelerator = menus::AcceleratorGtk(base::VKEY_V, false, true, false);
+      break;
+
+    case IDC_FIND:
+      *accelerator = menus::AcceleratorGtk(base::VKEY_F, false, true, false);
+      break;
+
+    case IDC_SAVE_PAGE:
+      *accelerator = menus::AcceleratorGtk(base::VKEY_S, false, true, false);
+      break;
+
+    case IDC_PRINT:
+      *accelerator = menus::AcceleratorGtk(base::VKEY_P, false, true, false);
+      break;
+
+    case IDC_NEW_TAB:
+      *accelerator = menus::AcceleratorGtk(base::VKEY_T, false, true, false);
+      break;
+
+    case IDC_NEW_WINDOW:
+      *accelerator = menus::AcceleratorGtk(base::VKEY_N, false, true, false);
+      break;
+
+    case IDC_NEW_INCOGNITO_WINDOW:
+      *accelerator = menus::AcceleratorGtk(base::VKEY_N, true, true, false);
+      break;
+
+    case IDC_SHOW_BOOKMARK_BAR:
+      *accelerator = menus::AcceleratorGtk(base::VKEY_B, false, true, false);
+      break;
+
+    case IDC_FULLSCREEN:
+      *accelerator = menus::AcceleratorGtk(base::VKEY_F11, false, false, false);
+      break;
+
+    case IDC_SHOW_HISTORY:
+      *accelerator = menus::AcceleratorGtk(base::VKEY_H, false, true, false);
+      break;
+
+    case IDC_SHOW_BOOKMARK_MANAGER:
+      *accelerator = menus::AcceleratorGtk(base::VKEY_B, true, true, false);
+      break;
+
+    case IDC_SHOW_DOWNLOADS:
+      *accelerator = menus::AcceleratorGtk(base::VKEY_J, false, true, false);
+      break;
+
+    case IDC_HELP_PAGE:
+      *accelerator = menus::AcceleratorGtk(base::VKEY_F1, false, false, false);
+      break;
+
+    case IDC_EXIT:
+      *accelerator = menus::AcceleratorGtk(base::VKEY_Q, true, true, false);
+      break;
+
+    default:
+      return false;
+  }
+
+  return true;
 }
 
 // NotificationObserver --------------------------------------------------------
diff --git a/chrome/browser/gtk/browser_toolbar_gtk.h b/chrome/browser/gtk/browser_toolbar_gtk.h
index 5a769fc..73ffd74 100644
--- a/chrome/browser/gtk/browser_toolbar_gtk.h
+++ b/chrome/browser/gtk/browser_toolbar_gtk.h
@@ -8,11 +8,14 @@
 #include <gtk/gtk.h>
 #include <string>
 
+#include "app/menus/simple_menu_model.h"
 #include "base/scoped_ptr.h"
+#include "chrome/browser/app_menu_model.h"
 #include "chrome/browser/bubble_positioner.h"
 #include "chrome/browser/command_updater.h"
 #include "chrome/browser/gtk/menu_bar_helper.h"
 #include "chrome/browser/gtk/menu_gtk.h"
+#include "chrome/browser/page_menu_model.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/common/notification_observer.h"
 #include "chrome/common/notification_registrar.h"
@@ -37,6 +40,7 @@ class ToolbarStarToggleGtk;
 // events back to the Browser.
 class BrowserToolbarGtk : public CommandUpdater::CommandObserver,
                           public ProfileSyncServiceObserver,
+                          public menus::SimpleMenuModel::Delegate,
                           public MenuGtk::Delegate,
                           public NotificationObserver,
                           public BubblePositioner,
@@ -77,11 +81,15 @@ class BrowserToolbarGtk : public CommandUpdater::CommandObserver,
   virtual void EnabledStateChangedForCommand(int id, bool enabled);
 
   // Overridden from MenuGtk::Delegate:
-  virtual bool IsCommandEnabled(int command_id) const;
-  virtual bool IsItemChecked(int id) const;
-  virtual void ExecuteCommand(int command_id);
   virtual void StoppedShowing();
 
+  // Overridden from menus::SimpleMenuModel::Delegate:
+  virtual bool IsCommandIdEnabled(int id) const;
+  virtual bool IsCommandIdChecked(int id) const;
+  virtual void ExecuteCommand(int id);
+  virtual bool GetAcceleratorForCommandId(int id,
+                                          menus::Accelerator* accelerator);
+
   // NotificationObserver implementation.
   void Observe(NotificationType type,
                const NotificationSource& source,
@@ -183,9 +191,6 @@ class BrowserToolbarGtk : public CommandUpdater::CommandObserver,
   // The location bar view.
   scoped_ptr<LocationBarViewGtk> location_bar_;
 
-  // A pointer to our window's accelerator list.
-  GtkAccelGroup* accel_group_;
-
   // All the buttons in the toolbar.
   scoped_ptr<BackForwardButtonGtk> back_, forward_;
   scoped_ptr<CustomDrawButton> reload_;
@@ -208,6 +213,9 @@ class BrowserToolbarGtk : public CommandUpdater::CommandObserver,
   scoped_ptr<MenuGtk> page_menu_;
   scoped_ptr<MenuGtk> app_menu_;
 
+  PageMenuModel page_menu_model_;
+  AppMenuModel app_menu_model_;
+
   Browser* browser_;
   BrowserWindowGtk* window_;
   Profile* profile_;
diff --git a/chrome/browser/gtk/browser_window_gtk.cc b/chrome/browser/gtk/browser_window_gtk.cc
index 3e8e9b7..bee1948 100644
--- a/chrome/browser/gtk/browser_window_gtk.cc
+++ b/chrome/browser/gtk/browser_window_gtk.cc
@@ -208,6 +208,10 @@ const struct AcceleratorMapping {
   { GDK_w, IDC_CLOSE_TAB, GDK_CONTROL_MASK },
   { GDK_t, IDC_RESTORE_TAB,
     GdkModifierType(GDK_CONTROL_MASK | GDK_SHIFT_MASK) },
+  { GDK_t, IDC_NEW_TAB, GDK_CONTROL_MASK },
+  { GDK_n, IDC_NEW_WINDOW, GDK_CONTROL_MASK },
+  { GDK_n, IDC_NEW_INCOGNITO_WINDOW,
+    GdkModifierType(GDK_CONTROL_MASK | GDK_SHIFT_MASK) },
 
   { GDK_1, IDC_SELECT_TAB_0, GDK_CONTROL_MASK },
   { GDK_2, IDC_SELECT_TAB_1, GDK_CONTROL_MASK },
@@ -288,6 +292,13 @@ const struct AcceleratorMapping {
   { XF86XK_Reload, IDC_RELOAD, GdkModifierType(0) },
   { XF86XK_Refresh, IDC_RELOAD, GdkModifierType(0) },
 
+  // Dev tools.
+  { GDK_u, IDC_VIEW_SOURCE, GDK_CONTROL_MASK },
+  { GDK_i, IDC_DEV_TOOLS, GDK_CONTROL_MASK },
+  { GDK_j, IDC_DEV_TOOLS_CONSOLE,
+    GdkModifierType(GDK_CONTROL_MASK | GDK_SHIFT_MASK) },
+  { GDK_Escape, IDC_TASK_MANAGER, GDK_SHIFT_MASK },
+
   // Miscellany.
   { GDK_d, IDC_BOOKMARK_ALL_TABS,
     GdkModifierType(GDK_CONTROL_MASK | GDK_SHIFT_MASK) },
@@ -296,19 +307,26 @@ const struct AcceleratorMapping {
   { XF86XK_Favorites, IDC_SHOW_BOOKMARK_BAR, GdkModifierType(0) },
   { XF86XK_History, IDC_SHOW_HISTORY, GdkModifierType(0) },
   { GDK_o, IDC_OPEN_FILE, GDK_CONTROL_MASK },
-  { GDK_F11, IDC_FULLSCREEN, GdkModifierType(0) },
-  { GDK_u, IDC_VIEW_SOURCE, GDK_CONTROL_MASK },
+  { GDK_f, IDC_FIND, GDK_CONTROL_MASK },
   { GDK_p, IDC_PRINT, GDK_CONTROL_MASK },
-  { GDK_Escape, IDC_TASK_MANAGER, GDK_SHIFT_MASK },
+  { GDK_b, IDC_SHOW_BOOKMARK_BAR, GDK_CONTROL_MASK },
+  { GDK_b, IDC_SHOW_BOOKMARK_MANAGER,
+    GdkModifierType(GDK_CONTROL_MASK | GDK_SHIFT_MASK) },
+  { GDK_F11, IDC_FULLSCREEN, GdkModifierType(0) },
   { GDK_Delete, IDC_CLEAR_BROWSING_DATA,
     GdkModifierType(GDK_CONTROL_MASK | GDK_SHIFT_MASK) },
+  { GDK_h, IDC_SHOW_HISTORY, GDK_CONTROL_MASK },
+  { GDK_j, IDC_SHOW_DOWNLOADS, GDK_CONTROL_MASK },
+  { GDK_F1, IDC_HELP_PAGE, GdkModifierType(0) },
 
 #if defined(OS_CHROMEOS)
   { GDK_f, IDC_FULLSCREEN,
     GdkModifierType(GDK_CONTROL_MASK | GDK_MOD1_MASK) },
   { GDK_Delete, IDC_TASK_MANAGER,
     GdkModifierType(GDK_CONTROL_MASK | GDK_MOD1_MASK) },
-  { GDK_comma, IDC_CONTROL_PANEL, GdkModifierType(GDK_CONTROL_MASK) },
+  { GDK_comma, IDC_CONTROL_PANEL, GDK_CONTROL_MASK },
+#else
+  { GDK_q, IDC_EXIT, GdkModifierType(GDK_CONTROL_MASK | GDK_SHIFT_MASK) },
 #endif
 };
 
diff --git a/chrome/browser/gtk/download_item_gtk.cc b/chrome/browser/gtk/download_item_gtk.cc
index 0512239..7cb1112 100644
--- a/chrome/browser/gtk/download_item_gtk.cc
+++ b/chrome/browser/gtk/download_item_gtk.cc
@@ -93,7 +93,7 @@ class DownloadShelfContextMenuGtk : public DownloadShelfContextMenu,
     if (menu_.get() == NULL ||
         (download_is_complete && !menu_is_for_complete_download_)) {
       menu_.reset(new MenuGtk(this, download_is_complete ?
-          finished_download_menu : in_progress_download_menu, NULL));
+          finished_download_menu : in_progress_download_menu));
       menu_is_for_complete_download_ = download_is_complete;
     }
     menu_->Popup(widget, event);
@@ -108,7 +108,7 @@ class DownloadShelfContextMenuGtk : public DownloadShelfContextMenu,
     return ItemIsChecked(id);
   }
 
-  virtual void ExecuteCommand(int id) {
+  virtual void ExecuteCommandById(int id) {
     ExecuteItemCommand(id);
   }
 
diff --git a/chrome/browser/gtk/menu_gtk.cc b/chrome/browser/gtk/menu_gtk.cc
index e5b421f..2b2b354 100644
--- a/chrome/browser/gtk/menu_gtk.cc
+++ b/chrome/browser/gtk/menu_gtk.cc
@@ -21,15 +21,24 @@ using gtk_util::ConvertAcceleratorsFromWindowsStyle;
 bool MenuGtk::block_activation_ = false;
 
 MenuGtk::MenuGtk(MenuGtk::Delegate* delegate,
-                 const MenuCreateMaterial* menu_data,
-                 GtkAccelGroup* accel_group)
+                 const MenuCreateMaterial* menu_data)
+    : delegate_(delegate),
+      model_(NULL),
+      dummy_accel_group_(gtk_accel_group_new()),
+      menu_(gtk_menu_new()),
+      factory_(this) {
+  DCHECK(menu_data);
+  ConnectSignalHandlers();
+  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) {
   ConnectSignalHandlers();
-  BuildMenuIn(menu_, menu_data, accel_group);
 }
 
 MenuGtk::MenuGtk(MenuGtk::Delegate* delegate,
@@ -39,7 +48,7 @@ MenuGtk::MenuGtk(MenuGtk::Delegate* delegate,
       dummy_accel_group_(gtk_accel_group_new()),
       menu_(gtk_menu_new()),
       factory_(this) {
-  DCHECK(delegate || model);
+  DCHECK(model);
   ConnectSignalHandlers();
   if (model)
     BuildMenuFromModel();
@@ -87,6 +96,12 @@ void MenuGtk::AppendSeparator() {
 }
 
 void MenuGtk::AppendMenuItem(int command_id, GtkWidget* menu_item) {
+  AppendMenuItemToMenu(command_id, menu_item, menu_);
+}
+
+void MenuGtk::AppendMenuItemToMenu(int command_id,
+                                   GtkWidget* menu_item,
+                                   GtkWidget* menu) {
   g_object_set_data(G_OBJECT(menu_item), "menu-id",
                     reinterpret_cast<void*>(command_id));
 
@@ -94,7 +109,7 @@ void MenuGtk::AppendMenuItem(int command_id, GtkWidget* menu_item) {
                    G_CALLBACK(OnMenuItemActivated), this);
 
   gtk_widget_show(menu_item);
-  gtk_menu_shell_append(GTK_MENU_SHELL(menu_), menu_item);
+  gtk_menu_shell_append(GTK_MENU_SHELL(menu), menu_item);
 }
 
 void MenuGtk::Popup(GtkWidget* widget, GdkEvent* event) {
@@ -132,8 +147,7 @@ void MenuGtk::Cancel() {
 }
 
 void MenuGtk::BuildMenuIn(GtkWidget* menu,
-                          const MenuCreateMaterial* menu_data,
-                          GtkAccelGroup* accel_group) {
+                          const MenuCreateMaterial* menu_data) {
   // We keep track of the last menu item in order to group radio items.
   GtkWidget* last_menu_item = NULL;
   for (; menu_data->type != MENU_END; ++menu_data) {
@@ -177,7 +191,7 @@ void MenuGtk::BuildMenuIn(GtkWidget* menu,
 
     if (menu_data->submenu) {
       GtkWidget* submenu = gtk_menu_new();
-      BuildMenuIn(submenu, menu_data->submenu, accel_group);
+      BuildMenuIn(submenu, menu_data->submenu);
       gtk_menu_item_set_submenu(GTK_MENU_ITEM(menu_item), submenu);
     } else if (menu_data->custom_submenu) {
       gtk_menu_item_set_submenu(GTK_MENU_ITEM(menu_item),
@@ -191,8 +205,7 @@ void MenuGtk::BuildMenuIn(GtkWidget* menu,
       // keys.
       gtk_widget_add_accelerator(menu_item,
                                  "activate",
-                                 menu_data->only_show || !accel_group ?
-                                     dummy_accel_group_ : accel_group,
+                                 dummy_accel_group_,
                                  menu_data->accel_key,
                                  GdkModifierType(menu_data->accel_modifiers),
                                  GTK_ACCEL_VISIBLE);
@@ -227,18 +240,18 @@ GtkWidget* MenuGtk::BuildMenuItemWithImage(const std::string& label,
 }
 
 void MenuGtk::BuildMenuFromModel() {
-  for (int i = 0; i < model_->GetItemCount(); ++i) {
-    GtkWidget* menu_item = NULL;
-
-    // TODO(estade): support these commands.
-    DCHECK_NE(model_->GetTypeAt(i), menus::MenuModel::TYPE_RADIO);
-    DCHECK_NE(model_->GetTypeAt(i), menus::MenuModel::TYPE_SUBMENU);
+  BuildSubmenuFromModel(model_, menu_);
+}
 
+void MenuGtk::BuildSubmenuFromModel(menus::MenuModel* model, GtkWidget* menu) {
+  GtkWidget* last_menu_item = NULL;
+  GtkWidget* menu_item = NULL;
+  for (int i = 0; i < model->GetItemCount(); ++i) {
     SkBitmap icon;
     std::string label =
-        ConvertAcceleratorsFromWindowsStyle(UTF16ToUTF8(model_->GetLabelAt(i)));
+        ConvertAcceleratorsFromWindowsStyle(UTF16ToUTF8(model->GetLabelAt(i)));
 
-    switch (model_->GetTypeAt(i)) {
+    switch (model->GetTypeAt(i)) {
       case menus::MenuModel::TYPE_SEPARATOR:
         menu_item = gtk_separator_menu_item_new();
         break;
@@ -247,8 +260,19 @@ void MenuGtk::BuildMenuFromModel() {
         menu_item = gtk_check_menu_item_new_with_mnemonic(label.c_str());
         break;
 
+      case menus::MenuModel::TYPE_RADIO:
+        if (last_menu_item && GTK_IS_RADIO_MENU_ITEM(last_menu_item)) {
+          menu_item = gtk_radio_menu_item_new_with_mnemonic_from_widget(
+              GTK_RADIO_MENU_ITEM(last_menu_item), label.c_str());
+        } else {
+          menu_item = gtk_radio_menu_item_new_with_mnemonic(
+              NULL, label.c_str());
+        }
+        break;
+
+      case menus::MenuModel::TYPE_SUBMENU:
       case menus::MenuModel::TYPE_COMMAND:
-        if (model_->GetIconAt(i, &icon))
+        if (model->GetIconAt(i, &icon))
           menu_item = BuildMenuItemWithImage(label, icon);
         else
           menu_item = gtk_menu_item_new_with_mnemonic(label.c_str());
@@ -258,8 +282,14 @@ void MenuGtk::BuildMenuFromModel() {
         NOTREACHED();
     }
 
+    if (model->GetTypeAt(i) == menus::MenuModel::TYPE_SUBMENU) {
+      GtkWidget* submenu = gtk_menu_new();
+      BuildSubmenuFromModel(model->GetSubmenuModelAt(i), submenu);
+      gtk_menu_item_set_submenu(GTK_MENU_ITEM(menu_item), submenu);
+    }
+
     menus::AcceleratorGtk accelerator;
-    if (model_->GetAcceleratorAt(i, &accelerator)) {
+    if (model->GetAcceleratorAt(i, &accelerator)) {
       gtk_widget_add_accelerator(menu_item,
                                  "activate",
                                  dummy_accel_group_,
@@ -268,7 +298,12 @@ void MenuGtk::BuildMenuFromModel() {
                                  GTK_ACCEL_VISIBLE);
     }
 
-    AppendMenuItem(i, menu_item);
+    g_object_set_data(G_OBJECT(menu_item), "model",
+                      reinterpret_cast<void*>(model));
+    AppendMenuItemToMenu(i, menu_item, menu);
+
+    last_menu_item = menu_item;
+    menu_item = NULL;
   }
 }
 
@@ -301,9 +336,13 @@ void MenuGtk::OnMenuItemActivated(GtkMenuItem* menuitem, MenuGtk* menu) {
                                                       "menu-id"));
   }
 
+  menus::MenuModel* model =
+      reinterpret_cast<menus::MenuModel*>(
+          g_object_get_data(G_OBJECT(menuitem), "model"));
+
   // The menu item can still be activated by hotkeys even if it is disabled.
-  if (menu->IsCommandEnabled(id))
-    menu->ExecuteCommand(id);
+  if (menu->IsCommandEnabled(model, id))
+    menu->ExecuteCommand(model, id);
 }
 
 // static
@@ -375,23 +414,23 @@ void MenuGtk::UpdateMenu() {
 }
 
 // http://crbug.com/31365
-bool MenuGtk::IsCommandEnabled(int id) {
-  return model_ ? model_->IsEnabledAt(id) :
-                  delegate_->IsCommandEnabled(id);
+bool MenuGtk::IsCommandEnabled(menus::MenuModel* model, int id) {
+  return model ? model->IsEnabledAt(id) :
+                 delegate_->IsCommandEnabled(id);
 }
 
 // http://crbug.com/31365
-void MenuGtk::ExecuteCommand(int id) {
-  if (model_)
-    model_->ActivatedAt(id);
+void MenuGtk::ExecuteCommand(menus::MenuModel* model, int id) {
+  if (model)
+    model->ActivatedAt(id);
   else
-    delegate_->ExecuteCommand(id);
+    delegate_->ExecuteCommandById(id);
 }
 
 // http://crbug.com/31365
-bool MenuGtk::IsItemChecked(int id) {
-  return model_ ? model_->IsItemCheckedAt(id) :
-                  delegate_->IsItemChecked(id);
+bool MenuGtk::IsItemChecked(menus::MenuModel* model, int id) {
+  return model ? model->IsItemCheckedAt(id) :
+                 delegate_->IsItemChecked(id);
 }
 
 // static
@@ -426,6 +465,10 @@ void MenuGtk::SetMenuItemInfo(GtkWidget* widget, gpointer userdata) {
                                     "menu-id"));
   }
 
+  menus::MenuModel* model =
+      reinterpret_cast<menus::MenuModel*>(
+          g_object_get_data(G_OBJECT(widget), "model"));
+
   if (GTK_IS_CHECK_MENU_ITEM(widget)) {
     GtkCheckMenuItem* item = GTK_CHECK_MENU_ITEM(widget);
 
@@ -440,12 +483,12 @@ void MenuGtk::SetMenuItemInfo(GtkWidget* widget, gpointer userdata) {
     // root of the MenuGtk and we want to disable *all* MenuGtks, including
     // submenus.
     block_activation_ = true;
-    gtk_check_menu_item_set_active(item, menu->IsItemChecked(id));
+    gtk_check_menu_item_set_active(item, menu->IsItemChecked(model, id));
     block_activation_ = false;
   }
 
   if (GTK_IS_MENU_ITEM(widget)) {
-    gtk_widget_set_sensitive(widget, menu->IsCommandEnabled(id));
+    gtk_widget_set_sensitive(widget, menu->IsCommandEnabled(model, id));
 
     GtkWidget* submenu = gtk_menu_item_get_submenu(GTK_MENU_ITEM(widget));
     if (submenu) {
diff --git a/chrome/browser/gtk/menu_gtk.h b/chrome/browser/gtk/menu_gtk.h
index 427abbb..84ad7fc0 100644
--- a/chrome/browser/gtk/menu_gtk.h
+++ b/chrome/browser/gtk/menu_gtk.h
@@ -38,7 +38,7 @@ class MenuGtk {
     virtual std::string GetLabel(int command_id) const { return std::string(); }
 
     // Executes the command.
-    virtual void ExecuteCommand(int command_id) {}
+    virtual void ExecuteCommandById(int command_id) {}
 
     // Called when the menu stops showing. This will be called along with
     // ExecuteCommand if the user clicks an item, but will also be called when
@@ -52,12 +52,12 @@ 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,
-          GtkAccelGroup* accel_group);
+  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_| if non-NULL.
-  // TODO(estade): MenuModel support is only partial. Only TYPE_SEPARATOR and
-  // TYPE_COMMAND are currently implemented.
+  // menu using |model_|.
   MenuGtk(MenuGtk::Delegate* delegate, menus::MenuModel* model);
   ~MenuGtk();
 
@@ -71,6 +71,9 @@ class MenuGtk {
   void AppendCheckMenuItemWithLabel(int command_id, const std::string& label);
   void AppendSeparator();
   void AppendMenuItem(int command_id, GtkWidget* menu_item);
+  void AppendMenuItemToMenu(int command_id,
+                            GtkWidget* menu_item,
+                            GtkWidget* menu);
 
   // Displays the menu. |timestamp| is the time of activation. The popup is
   // statically positioned at |widget|.
@@ -119,8 +122,7 @@ class MenuGtk {
   // A recursive function that transforms a MenuCreateMaterial tree into a set
   // of GtkMenuItems.
   void BuildMenuIn(GtkWidget* menu,
-                   const MenuCreateMaterial* menu_data,
-                   GtkAccelGroup* accel_group);
+                   const MenuCreateMaterial* menu_data);
 
   // Builds a GtkImageMenuItem.
   GtkWidget* BuildMenuItemWithImage(const std::string& label,
@@ -128,18 +130,20 @@ class MenuGtk {
 
   // A function that creates a GtkMenu from |model_|.
   void BuildMenuFromModel();
+  // Implementation of the above; called recursively.
+  void BuildSubmenuFromModel(menus::MenuModel* model, GtkWidget* menu);
 
   // Contains implementation for OnMenuShow.
   void UpdateMenu();
 
-  // Dispatches to either |model_| (if it is non-null) or |delegate_|. The
+  // Dispatches to either |model| (if it is non-null) or |delegate_|. The
   // reason for this awkwardness is that we are in a transitional period where
   // we support both MenuModel and Delegate as a menu controller.
   // TODO(estade): remove controller functions from Delegate.
   // http://crbug.com/31365
-  bool IsCommandEnabled(int id);
-  void ExecuteCommand(int id);
-  bool IsItemChecked(int id);
+  bool IsCommandEnabled(menus::MenuModel* model, int id);
+  void ExecuteCommand(menus::MenuModel* model, int id);
+  bool IsItemChecked(menus::MenuModel* model, int id);
 
   // Callback for when a menu item is clicked.
   static void OnMenuItemActivated(GtkMenuItem* menuitem, MenuGtk* menu);
diff --git a/chrome/browser/gtk/standard_menus.cc b/chrome/browser/gtk/standard_menus.cc
index a855dd7..573251a 100644
--- a/chrome/browser/gtk/standard_menus.cc
+++ b/chrome/browser/gtk/standard_menus.cc
@@ -51,9 +51,9 @@ struct MenuCreateMaterial developer_menu_materials_no_inspector[] = {
 struct MenuCreateMaterial standard_page_menu_materials[] = {
   { MENU_NORMAL, IDC_CREATE_SHORTCUTS, IDS_CREATE_SHORTCUTS },
   { MENU_SEPARATOR },
-  { MENU_NORMAL, IDC_CUT, IDS_CUT, 0, NULL, GDK_x, GDK_CONTROL_MASK, true },
-  { MENU_NORMAL, IDC_COPY, IDS_COPY, 0, NULL, GDK_c, GDK_CONTROL_MASK, true },
-  { MENU_NORMAL, IDC_PASTE, IDS_PASTE, 0, NULL, GDK_v, GDK_CONTROL_MASK, true },
+  { MENU_NORMAL, IDC_CUT, IDS_CUT, 0, NULL, GDK_x, GDK_CONTROL_MASK },
+  { MENU_NORMAL, IDC_COPY, IDS_COPY, 0, NULL, GDK_c, GDK_CONTROL_MASK },
+  { MENU_NORMAL, IDC_PASTE, IDS_PASTE, 0, NULL, GDK_v, GDK_CONTROL_MASK },
   { MENU_SEPARATOR },
   { MENU_NORMAL, IDC_FIND, IDS_FIND, 0, NULL, GDK_f, GDK_CONTROL_MASK },
   { MENU_NORMAL, IDC_SAVE_PAGE, IDS_SAVE_PAGE, 0, NULL, GDK_s,
@@ -118,7 +118,7 @@ const MenuCreateMaterial* GetStandardPageMenu(Profile* profile,
   EncodingMenuController::EncodingMenuItemList items;
   controller.GetEncodingMenuItems(profile, &items);
 
-  MenuGtk* encodings_menu = new MenuGtk(delegate, false);
+  MenuGtk* encodings_menu = new MenuGtk(delegate);
   GSList* radio_group = NULL;
   for (EncodingMenuController::EncodingMenuItemList::const_iterator i =
            items.begin();
diff --git a/chrome/browser/gtk/standard_menus.h b/chrome/browser/gtk/standard_menus.h
index 3a8d806..28fcefb 100644
--- a/chrome/browser/gtk/standard_menus.h
+++ b/chrome/browser/gtk/standard_menus.h
@@ -44,11 +44,6 @@ struct MenuCreateMaterial {
   // GDK modifiers for the menu items (i.e., shift, ctrl, etc).
   unsigned int accel_modifiers;
 
-  // If true, the accelerator is only for show (does not do anything), although
-  // the same key combination may be handled by GTK. Windows handles this in
-  // toolbar_view.cc::GetAcceleratorInfo().
-  bool only_show;
-
   // If non-NULL, specifies a custom submenu to be used. MenuGtk will take
   // ownership of this submenu.
   MenuGtk* custom_submenu;
diff --git a/chrome/browser/gtk/task_manager_gtk.cc b/chrome/browser/gtk/task_manager_gtk.cc
index 7fbb81a..9f679fe 100644
--- a/chrome/browser/gtk/task_manager_gtk.cc
+++ b/chrome/browser/gtk/task_manager_gtk.cc
@@ -190,7 +190,7 @@ class TaskManagerGtk::ContextMenuController : public MenuGtk::Delegate {
  public:
   explicit ContextMenuController(TaskManagerGtk* task_manager)
       : task_manager_(task_manager) {
-    menu_.reset(new MenuGtk(this, false));
+    menu_.reset(new MenuGtk(this));
     for (int i = kTaskManagerPage; i < kTaskManagerColumnCount; i++) {
       menu_->AppendCheckMenuItemWithLabel(
           i, l10n_util::GetStringUTF8(TaskManagerColumnIDToResourceID(i)));
@@ -225,7 +225,7 @@ class TaskManagerGtk::ContextMenuController : public MenuGtk::Delegate {
     return TreeViewColumnIsVisible(task_manager_->treeview_, colid);
   }
 
-  virtual void ExecuteCommand(int command_id) {
+  virtual void ExecuteCommandById(int command_id) {
     if (!task_manager_)
       return;
 
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 e94575d..f8c4617 100644
--- a/chrome/browser/tab_contents/render_view_context_menu_gtk.cc
+++ b/chrome/browser/tab_contents/render_view_context_menu_gtk.cc
@@ -25,7 +25,7 @@ RenderViewContextMenuGtk::~RenderViewContextMenuGtk() {
 
 void RenderViewContextMenuGtk::DoInit() {
   DoneMakingMenu(&menu_);
-  gtk_menu_.reset(new MenuGtk(this, menu_.data(), NULL));
+  gtk_menu_.reset(new MenuGtk(this, menu_.data()));
 }
 
 void RenderViewContextMenuGtk::Popup(const gfx::Point& point) {
@@ -42,7 +42,7 @@ bool RenderViewContextMenuGtk::IsItemChecked(int id) const {
   return ItemIsChecked(id);
 }
 
-void RenderViewContextMenuGtk::ExecuteCommand(int id) {
+void RenderViewContextMenuGtk::ExecuteCommandById(int id) {
   ExecuteItemCommand(id);
 }
 
diff --git a/chrome/browser/tab_contents/render_view_context_menu_gtk.h b/chrome/browser/tab_contents/render_view_context_menu_gtk.h
index c972855..a929d8a 100644
--- a/chrome/browser/tab_contents/render_view_context_menu_gtk.h
+++ b/chrome/browser/tab_contents/render_view_context_menu_gtk.h
@@ -34,7 +34,7 @@ class RenderViewContextMenuGtk : public RenderViewContextMenu,
   // Menu::Delegate implementation ---------------------------------------------
   virtual bool IsCommandEnabled(int id) const;
   virtual bool IsItemChecked(int id) const;
-  virtual void ExecuteCommand(int id);
+  virtual void ExecuteCommandById(int id);
   virtual std::string GetLabel(int id) const;
   virtual void StoppedShowing();