The initial registration of the browser accelerators for toolkit_views was sharing the accelerator table withe the GTK implementation.

It was missing some accelerators that are not defined in the table but in standard_menus.cc.
Turns out it's probably easier for toolkit_views and Gtk not to share that accelerator map.
I reverted the Gtk browser view to use a non shared accelerator table, and moved the accelerator table file to be toolkit_views specific.

BUG=None
TEST=Run Chrome (toolkit_views). Make sure accelerators (such as Ctrl-T, Ctrl-N...) work
Review URL: http://codereview.chromium.org/160544

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

5 files changed, 132 insertions, 17 deletions

diff --git a/chrome/browser/gtk/browser_window_gtk.cc b/chrome/browser/gtk/browser_window_gtk.cc
index d6bfe1a..3c41e44 100644
--- a/chrome/browser/gtk/browser_window_gtk.cc
+++ b/chrome/browser/gtk/browser_window_gtk.cc
@@ -21,7 +21,6 @@
 #include "base/string_util.h"
 #include "base/time.h"
 #include "chrome/app/chrome_dll_resource.h"
-#include "chrome/browser/accelerator_table_linux.h"
 #include "chrome/browser/bookmarks/bookmark_utils.h"
 #include "chrome/browser/browser.h"
 #include "chrome/browser/browser_list.h"
@@ -152,16 +151,118 @@ gfx::Rect GetInitialWindowBounds(GtkWindow* window) {
   return gfx::Rect(x, y, width, height);
 }
 
+// Keep this in sync with various context menus which display the accelerators.
+const struct AcceleratorMapping {
+  guint keyval;
+  int command_id;
+  GdkModifierType modifier_type;
+} kAcceleratorMap[] = {
+  // Focus.
+  { GDK_k, IDC_FOCUS_SEARCH, GDK_CONTROL_MASK },
+  { GDK_e, IDC_FOCUS_SEARCH, GDK_CONTROL_MASK },
+  { XF86XK_Search, IDC_FOCUS_SEARCH, GdkModifierType(0) },
+  { GDK_l, IDC_FOCUS_LOCATION, GDK_CONTROL_MASK },
+  { GDK_d, IDC_FOCUS_LOCATION, GDK_MOD1_MASK },
+  { GDK_F6, IDC_FOCUS_LOCATION, GdkModifierType(0) },
+  { XF86XK_OpenURL, IDC_FOCUS_LOCATION, GdkModifierType(0) },
+  { XF86XK_Go, IDC_FOCUS_LOCATION, GdkModifierType(0) },
+
+  // Tab/window controls.
+  { GDK_Page_Down, IDC_SELECT_NEXT_TAB, GDK_CONTROL_MASK },
+  { GDK_Page_Up, IDC_SELECT_PREVIOUS_TAB, GDK_CONTROL_MASK },
+  { GDK_w, IDC_CLOSE_TAB, GDK_CONTROL_MASK },
+  { GDK_t, IDC_RESTORE_TAB,
+    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 },
+  { GDK_3, IDC_SELECT_TAB_2, GDK_CONTROL_MASK },
+  { GDK_4, IDC_SELECT_TAB_3, GDK_CONTROL_MASK },
+  { GDK_5, IDC_SELECT_TAB_4, GDK_CONTROL_MASK },
+  { GDK_6, IDC_SELECT_TAB_5, GDK_CONTROL_MASK },
+  { GDK_7, IDC_SELECT_TAB_6, GDK_CONTROL_MASK },
+  { GDK_8, IDC_SELECT_TAB_7, GDK_CONTROL_MASK },
+  { GDK_9, IDC_SELECT_LAST_TAB, GDK_CONTROL_MASK },
+
+  { GDK_1, IDC_SELECT_TAB_0, GDK_MOD1_MASK },
+  { GDK_2, IDC_SELECT_TAB_1, GDK_MOD1_MASK },
+  { GDK_3, IDC_SELECT_TAB_2, GDK_MOD1_MASK },
+  { GDK_4, IDC_SELECT_TAB_3, GDK_MOD1_MASK },
+  { GDK_5, IDC_SELECT_TAB_4, GDK_MOD1_MASK },
+  { GDK_6, IDC_SELECT_TAB_5, GDK_MOD1_MASK },
+  { GDK_7, IDC_SELECT_TAB_6, GDK_MOD1_MASK },
+  { GDK_8, IDC_SELECT_TAB_7, GDK_MOD1_MASK },
+  { GDK_9, IDC_SELECT_LAST_TAB, GDK_MOD1_MASK },
+
+  { GDK_F4, IDC_CLOSE_TAB, GDK_CONTROL_MASK },
+  { GDK_F4, IDC_CLOSE_WINDOW, GDK_MOD1_MASK },
+
+  // Zoom level.
+  { GDK_plus, IDC_ZOOM_PLUS,
+    GdkModifierType(GDK_CONTROL_MASK | GDK_SHIFT_MASK) },
+  { GDK_equal, IDC_ZOOM_PLUS, GDK_CONTROL_MASK },
+  { XF86XK_ZoomIn, IDC_ZOOM_PLUS, GdkModifierType(0) },
+  { GDK_0, IDC_ZOOM_NORMAL, GDK_CONTROL_MASK },
+  { GDK_minus, IDC_ZOOM_MINUS, GDK_CONTROL_MASK },
+  { GDK_underscore, IDC_ZOOM_MINUS,
+    GdkModifierType(GDK_CONTROL_MASK | GDK_SHIFT_MASK) },
+  { XF86XK_ZoomOut, IDC_ZOOM_MINUS, GdkModifierType(0) },
+
+  // Find in page.
+  { GDK_g, IDC_FIND_NEXT, GDK_CONTROL_MASK },
+  { GDK_F3, IDC_FIND_NEXT, GdkModifierType(0) },
+  { GDK_g, IDC_FIND_PREVIOUS,
+    GdkModifierType(GDK_CONTROL_MASK | GDK_SHIFT_MASK) },
+  { GDK_F3, IDC_FIND_PREVIOUS, GDK_SHIFT_MASK },
+
+  // Navigation / toolbar buttons.
+  { GDK_Home, IDC_HOME, GDK_MOD1_MASK },
+  { XF86XK_HomePage, IDC_HOME, GdkModifierType(0) },
+  { GDK_Escape, IDC_STOP, GdkModifierType(0) },
+  { XF86XK_Stop, IDC_STOP, GdkModifierType(0) },
+  { GDK_Left, IDC_BACK, GDK_MOD1_MASK },
+  { GDK_BackSpace, IDC_BACK, GdkModifierType(0) },
+  { XF86XK_Back, IDC_BACK, GdkModifierType(0) },
+  { GDK_Right, IDC_FORWARD, GDK_MOD1_MASK },
+  { GDK_BackSpace, IDC_FORWARD, GDK_SHIFT_MASK },
+  { XF86XK_Forward, IDC_FORWARD, GdkModifierType(0) },
+  { GDK_r, IDC_RELOAD, GDK_CONTROL_MASK },
+  { GDK_F5, IDC_RELOAD, GdkModifierType(0) },
+  { GDK_F5, IDC_RELOAD, GDK_CONTROL_MASK },
+  { GDK_F5, IDC_RELOAD, GDK_SHIFT_MASK },
+  { XF86XK_Reload, IDC_RELOAD, GdkModifierType(0) },
+  { XF86XK_Refresh, IDC_RELOAD, GdkModifierType(0) },
+
+  // Miscellany.
+  { GDK_d, IDC_STAR, GDK_CONTROL_MASK },
+  { XF86XK_AddFavorite, IDC_STAR, GdkModifierType(0) },
+  { 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_p, IDC_PRINT, GDK_CONTROL_MASK },
+  { GDK_Escape, IDC_TASK_MANAGER, GDK_SHIFT_MASK },
+
+#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) },
+#endif
+};
+
 int GetCommandId(guint accel_key, GdkModifierType modifier) {
   // Bug 9806: If capslock is on, we will get a capital letter as accel_key.
   accel_key = gdk_keyval_to_lower(accel_key);
   // Filter modifier to only include accelerator modifiers.
   modifier = static_cast<GdkModifierType>(
       modifier & gtk_accelerator_get_default_mod_mask());
-  for (size_t i = 0; i < browser::kAcceleratorMapLength; ++i) {
-    if (browser::kAcceleratorMap[i].keyval == accel_key &&
-        browser::kAcceleratorMap[i].modifier_type == modifier)
-      return browser::kAcceleratorMap[i].command_id;
+  for (size_t i = 0; i < arraysize(kAcceleratorMap); ++i) {
+    if (kAcceleratorMap[i].keyval == accel_key &&
+        kAcceleratorMap[i].modifier_type == modifier)
+      return kAcceleratorMap[i].command_id;
   }
   NOTREACHED();
   return 0;
@@ -1326,11 +1427,11 @@ void BrowserWindowGtk::ConnectAccelerators() {
   // Drop the initial ref on |accel_group| so |window_| will own it.
   g_object_unref(accel_group);
 
-  for (size_t i = 0; i < browser::kAcceleratorMapLength; ++i) {
+  for (size_t i = 0; i < arraysize(kAcceleratorMap); ++i) {
     gtk_accel_group_connect(
         accel_group,
-        browser::kAcceleratorMap[i].keyval,
-        browser::kAcceleratorMap[i].modifier_type,
+        kAcceleratorMap[i].keyval,
+        kAcceleratorMap[i].modifier_type,
         GtkAccelFlags(0),
         g_cclosure_new(G_CALLBACK(OnGtkAccelerator), this, NULL));
   }
diff --git a/chrome/browser/accelerator_table_linux.cc b/chrome/browser/views/accelerator_table_gtk.cc
index 179d2b6..7579769 100644
--- a/chrome/browser/accelerator_table_linux.cc
+++ b/chrome/browser/views/accelerator_table_gtk.cc
@@ -5,14 +5,13 @@
 #include <gdk/gdkkeysyms.h>
 #include <X11/XF86keysym.h>
 
-#include "chrome/browser/accelerator_table_linux.h"
+#include "chrome/browser/views/accelerator_table_gtk.h"
 
 #include "base/basictypes.h"
 #include "chrome/app/chrome_dll_resource.h"
 
 namespace browser {
 
-// Keep this in sync with various context menus which display the accelerators.
 const AcceleratorMapping kAcceleratorMap[] = {
   // Focus.
   { GDK_k, IDC_FOCUS_SEARCH, GDK_CONTROL_MASK },
@@ -25,6 +24,10 @@ const AcceleratorMapping kAcceleratorMap[] = {
   { XF86XK_Go, IDC_FOCUS_LOCATION, GdkModifierType(0) },
 
   // Tab/window controls.
+  { 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_Page_Down, IDC_SELECT_NEXT_TAB, GDK_CONTROL_MASK },
   { GDK_Page_Up, IDC_SELECT_PREVIOUS_TAB, GDK_CONTROL_MASK },
   { GDK_w, IDC_CLOSE_TAB, GDK_CONTROL_MASK },
@@ -114,20 +117,25 @@ const AcceleratorMapping kAcceleratorMap[] = {
   { GDK_d, IDC_STAR, GDK_CONTROL_MASK },
   { XF86XK_AddFavorite, IDC_STAR, GdkModifierType(0) },
   { XF86XK_Favorites, IDC_SHOW_BOOKMARK_BAR, GdkModifierType(0) },
+  { GDK_b, IDC_SHOW_BOOKMARK_BAR, GDK_CONTROL_MASK },
   { XF86XK_History, IDC_SHOW_HISTORY, GdkModifierType(0) },
+  { GDK_h, IDC_SHOW_HISTORY, GDK_CONTROL_MASK },
+  { GDK_j, IDC_SHOW_DOWNLOADS, GDK_CONTROL_MASK },
   { GDK_o, IDC_OPEN_FILE, GDK_CONTROL_MASK },
   { GDK_F11, IDC_FULLSCREEN, GdkModifierType(0) },
   { GDK_u, IDC_VIEW_SOURCE, GDK_CONTROL_MASK },
+  { GDK_j, IDC_DEV_TOOLS, GdkModifierType(GDK_CONTROL_MASK | GDK_MOD1_MASK) },
   { GDK_p, IDC_PRINT, GDK_CONTROL_MASK },
   { GDK_Escape, IDC_TASK_MANAGER, GDK_SHIFT_MASK },
-
-#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) },
-#endif
+  { GDK_b, IDC_SHOW_BOOKMARK_MANAGER,
+    GdkModifierType(GDK_CONTROL_MASK | GDK_SHIFT_MASK) },
+  { GDK_F1, IDC_HELP_PAGE,  GdkModifierType(0) },
+  { GDK_q, IDC_EXIT, GdkModifierType(GDK_CONTROL_MASK | GDK_SHIFT_MASK) },
 };
 
 const size_t kAcceleratorMapLength = arraysize(kAcceleratorMap);
diff --git a/chrome/browser/accelerator_table_linux.h b/chrome/browser/views/accelerator_table_gtk.h
index f419b2f..830fdb2 100644
--- a/chrome/browser/accelerator_table_linux.h
+++ b/chrome/browser/views/accelerator_table_gtk.h
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#ifndef CHROME_BROWSER_VIEWS_ACCELERATOR_TABLE_GTK_H_
+#define CHROME_BROWSER_VIEWS_ACCELERATOR_TABLE_GTK_H_
 
 #include <gtk/gtk.h>
 
@@ -21,3 +23,5 @@ namespace browser {
   // The numbers of elements in kAcceleratorMap.
   extern const size_t kAcceleratorMapLength;
 }
+
+#endif  // CHROME_BROWSER_VIEWS_ACCELERATOR_TABLE_GTK_H_
diff --git a/chrome/browser/views/frame/browser_view.cc b/chrome/browser/views/frame/browser_view.cc
index 120c2ef..b85fec0 100644
--- a/chrome/browser/views/frame/browser_view.cc
+++ b/chrome/browser/views/frame/browser_view.cc
@@ -69,7 +69,7 @@
 #include "chrome/browser/jumplist.h"
 #include "views/controls/scrollbar/native_scroll_bar.h"
 #elif defined(OS_LINUX)
-#include "chrome/browser/accelerator_table_linux.h"
+#include "chrome/browser/views/accelerator_table_gtk.h"
 #include <gtk/gtk.h>
 
 #include "views/window/hit_test.h"
@@ -601,7 +601,7 @@ void BrowserView::SetBounds(const gfx::Rect& bounds) {
 void BrowserView::Close() {
   // BrowserWindowClosing will usually cause the bubble to remove itself from
   // the set, so we need to iterate in a way that's safe against deletion.
-  for (BubbleSet::iterator i = browser_bubbles_.begin(); 
+  for (BubbleSet::iterator i = browser_bubbles_.begin();
        i != browser_bubbles_.end();) {
     BubbleSet::iterator bubble = i++;
     (*bubble)->BrowserWindowClosing();
diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp
index d8de523..f53b26b 100644
--- a/chrome/chrome.gyp
+++ b/chrome/chrome.gyp
@@ -591,8 +591,6 @@
       'sources': [
         # All .cc, .h, .m, and .mm files under browser except for tests and
         # mocks.
-	'browser/accelerator_table_linux.cc',
-	'browser/accelerator_table_linux.h',
         'browser/alternate_nav_url_fetcher.cc',
         'browser/alternate_nav_url_fetcher.h',
         'browser/app_controller_mac.h',
@@ -1667,6 +1665,8 @@
         'browser/views/about_ipc_dialog.h',
         'browser/views/about_network_dialog.cc',
         'browser/views/about_network_dialog.h',
+        'browser/views/accelerator_table_gtk.cc',
+        'browser/views/accelerator_table_gtk.h',
         'browser/views/autocomplete/autocomplete_popup_contents_view.cc',
         'browser/views/autocomplete/autocomplete_popup_contents_view.h',
         'browser/views/autocomplete/autocomplete_popup_win.cc',
@@ -2149,6 +2149,8 @@
                 ['include', '^browser/dock_info.cc'],
                 ['include', '^browser/dock_info.h'],
                 ['include', '^browser/extensions/'],
+                ['include', '^browser/views/accelerator_table_gtk.cc'],
+                ['include', '^browser/views/accelerator_table_gtk.h'],
                 ['include', '^browser/views/autocomplete/autocomplete_popup_contents_view.cc'],
                 ['include', '^browser/views/autocomplete/autocomplete_popup_contents_view.h'],
                 ['include', '^browser/views/autocomplete/autocomplete_popup_gtk.cc'],