Patch for accelerator clean-up from Hamaji.

See http://codereview.chromium.org/99161

TBR=hamami

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

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

7 files changed, 60 insertions, 57 deletions

diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_win.cc b/chrome/browser/autocomplete/autocomplete_edit_view_win.cc
index a51dbaa..4543c30 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_win.cc
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_win.cc
@@ -1642,6 +1642,8 @@ bool AutocompleteEditViewWin::OnKeyDownOnlyWritable(TCHAR key,
   // WM_SYSKEYDOWN, so we need to check (flags & KF_ALTDOWN) in various places
   // in this function even with a WM_SYSKEYDOWN handler.
 
+  // Update LocationBarView::ShouldLookupAccelerators() as well when you add
+  // some key combinations here.
   int count = repeat_count;
   switch (key) {
     case VK_RETURN:
diff --git a/chrome/browser/tab_contents/tab_contents_view_win.cc b/chrome/browser/tab_contents/tab_contents_view_win.cc
index e0ee88e..5f47164 100644
--- a/chrome/browser/tab_contents/tab_contents_view_win.cc
+++ b/chrome/browser/tab_contents/tab_contents_view_win.cc
@@ -358,7 +358,7 @@ void TabContentsViewWin::HandleKeyboardEvent(
       // |this| if the accelerator is a "close tab" one. So we speculatively
       // set the flag and fix it if no event was handled.
       ignore_next_char_event_ = true;
-      if (focus_manager->ProcessAccelerator(accelerator, false)) {
+      if (focus_manager->ProcessAccelerator(accelerator)) {
         // DANGER: |this| could be deleted now!
         return;
       } else {
diff --git a/chrome/browser/task_manager.cc b/chrome/browser/task_manager.cc
index e411c75..7e882d0 100644
--- a/chrome/browser/task_manager.cc
+++ b/chrome/browser/task_manager.cc
@@ -784,9 +784,7 @@ void TaskManagerContents::Init(TaskManagerTableModel* table_model) {
   SetContextMenuController(this);
   kill_button_.reset(new views::NativeButton(
       this, l10n_util::GetString(IDS_TASK_MANAGER_KILL)));
-  // TODO(hamaji): Use accelerator once the bug in FocusManager fixed.
-  //               http://crbug.com/11073
-  // kill_button_->AddAccelerator(views::Accelerator('E', false, false, false));
+  kill_button_->AddAccelerator(views::Accelerator('E', false, false, false));
   kill_button_->SetAccessibleKeyboardShortcut(L"E");
   about_memory_link_.reset(new views::Link(
       l10n_util::GetString(IDS_TASK_MANAGER_ABOUT_MEMORY_LINK)));
@@ -920,8 +918,6 @@ void TaskManagerContents::OnDoubleClick() {
 void TaskManagerContents::OnKeyDown(unsigned short virtual_keycode) {
   if (virtual_keycode == VK_RETURN)
     task_manager_->ActivateFocusedTab();
-  else if (virtual_keycode == 'E')
-    task_manager_->KillSelectedProcesses();
 }
 
 // views::LinkController implementation
diff --git a/chrome/browser/views/location_bar_view.cc b/chrome/browser/views/location_bar_view.cc
index 3499c6d..3311a52 100644
--- a/chrome/browser/views/location_bar_view.cc
+++ b/chrome/browser/views/location_bar_view.cc
@@ -788,13 +788,43 @@ void LocationBarView::KeywordHintView::Layout() {
   }
 }
 
-// We don't translate accelerators for ALT + numpad digit, they are used for
-// entering special characters.
 bool LocationBarView::ShouldLookupAccelerators(const views::KeyEvent& e) {
-  if (!e.IsAltDown())
-    return true;
+  int c = e.GetCharacter();
+  // We don't translate accelerators for ALT + numpad digit, they are used for
+  // entering special characters.
+  if (e.IsAltDown() && win_util::IsNumPadDigit(c, e.IsExtendedKey()))
+    return false;
+
+  // Skip accelerators for key combinations omnibox wants to crack. This list
+  // should be synced with AutocompleteEditViewWin::OnKeyDownOnlyWritable().
+  //
+  // We cannot return false for all keys because we still need to handle some
+  // accelerators (e.g., F5 for reload the page should work even when the
+  // Omnibox gets focused).
+  switch (c) {
+  case VK_RETURN:
+    return false;
+
+  case VK_UP:
+  case VK_DOWN:
+    return e.IsAltDown();
+
+  case VK_DELETE:
+  case VK_INSERT:
+    return e.IsAltDown() || !e.IsShiftDown();
 
-  return !win_util::IsNumPadDigit(e.GetCharacter(), e.IsExtendedKey());
+  case 'X':
+  case 'V':
+    return e.IsAltDown() || !e.IsControlDown();
+
+  case VK_BACK:
+  case VK_TAB:
+  case 0xbb:
+    return false;
+
+  default:
+    return true;
+  }
 }
 
 // ShowInfoBubbleTask-----------------------------------------------------------
diff --git a/chrome/views/controls/tree/tree_view.cc b/chrome/views/controls/tree/tree_view.cc
index 8c130bd..7677828 100644
--- a/chrome/views/controls/tree/tree_view.cc
+++ b/chrome/views/controls/tree/tree_view.cc
@@ -475,7 +475,7 @@ bool TreeView::OnKeyDown(int virtual_key_code) {
                                         win_util::IsShiftPressed(),
                                         win_util::IsCtrlPressed(),
                                         win_util::IsAltPressed()));
-    fm->ProcessAccelerator(accelerator, true);
+    fm->ProcessAccelerator(accelerator);
     return true;
   }
   return false;
diff --git a/chrome/views/focus/focus_manager.cc b/chrome/views/focus/focus_manager.cc
index 8d22506..4b4120f 100644
--- a/chrome/views/focus/focus_manager.cc
+++ b/chrome/views/focus/focus_manager.cc
@@ -307,18 +307,11 @@ bool FocusManager::OnKeyDown(HWND window, UINT message, WPARAM wparam,
   // really processed a key event. If the key combination matches an
   // accelerator, the accelerator is triggered, otherwise we forward the
   // event to the HWND.
-  int modifiers = 0;
-  if (win_util::IsShiftPressed())
-    modifiers |= Event::EF_SHIFT_DOWN;
-  if (win_util::IsCtrlPressed())
-    modifiers |= Event::EF_CONTROL_DOWN;
-  if (win_util::IsAltPressed())
-    modifiers |= Event::EF_ALT_DOWN;
   Accelerator accelerator(Accelerator(static_cast<int>(virtual_key_code),
                                       win_util::IsShiftPressed(),
                                       win_util::IsCtrlPressed(),
                                       win_util::IsAltPressed()));
-  if (ProcessAccelerator(accelerator, true)) {
+  if (ProcessAccelerator(accelerator)) {
     // If a shortcut was activated for this keydown message, do not
     // propagate the message further.
     return false;
@@ -649,36 +642,26 @@ void FocusManager::UnregisterAccelerators(AcceleratorTarget* target) {
   }
 }
 
-bool FocusManager::ProcessAccelerator(const Accelerator& accelerator,
-                                      bool prioritary_accelerators_only) {
-  if (!prioritary_accelerators_only ||
-      accelerator.IsCtrlDown() || accelerator.IsAltDown() ||
-      accelerator.GetKeyCode() == VK_ESCAPE ||
-      accelerator.GetKeyCode() == VK_RETURN ||
-      (accelerator.GetKeyCode() >= VK_F1 &&
-          accelerator.GetKeyCode() <= VK_F24) ||
-      (accelerator.GetKeyCode() >= VK_BROWSER_BACK &&
-          accelerator.GetKeyCode() <= VK_BROWSER_HOME)) {
-    FocusManager* focus_manager = this;
-    do {
-      AcceleratorTarget* target =
-          focus_manager->GetTargetForAccelerator(accelerator);
-      if (target) {
-        // If there is focused view, we give it a chance to process that
-        // accelerator.
-        if (!focused_view_ ||
-            !focused_view_->OverrideAccelerator(accelerator)) {
-          if (target->AcceleratorPressed(accelerator))
-            return true;
-        }
+bool FocusManager::ProcessAccelerator(const Accelerator& accelerator) {
+  FocusManager* focus_manager = this;
+  do {
+    AcceleratorTarget* target =
+        focus_manager->GetTargetForAccelerator(accelerator);
+    if (target) {
+      // If there is focused view, we give it a chance to process that
+      // accelerator.
+      if (!focused_view_ ||
+          !focused_view_->OverrideAccelerator(accelerator)) {
+        if (target->AcceleratorPressed(accelerator))
+          return true;
       }
+    }
 
-      // When dealing with child windows that have their own FocusManager (such
-      // as ConstrainedWindow), we still want the parent FocusManager to process
-      // the accelerator if the child window did not process it.
-      focus_manager = focus_manager->GetParentFocusManager();
-    } while (focus_manager);
-  }
+    // When dealing with child windows that have their own FocusManager (such
+    // as ConstrainedWindow), we still want the parent FocusManager to process
+    // the accelerator if the child window did not process it.
+    focus_manager = focus_manager->GetParentFocusManager();
+  } while (focus_manager);
   return false;
 }
 
diff --git a/chrome/views/focus/focus_manager.h b/chrome/views/focus/focus_manager.h
index 491cbbb..f9e8d2d 100644
--- a/chrome/views/focus/focus_manager.h
+++ b/chrome/views/focus/focus_manager.h
@@ -257,16 +257,8 @@ class FocusManager : public NotificationObserver {
   void UnregisterAccelerators(AcceleratorTarget* target);
 
   // Activate the target associated with the specified accelerator if any.
-  // If |prioritary_accelerators_only| is true, only the following accelerators
-  // are allowed:
-  // - a key combination including Ctrl or Alt
-  // - the escape key
-  // - the enter key
-  // - any F key (F1, F2, F3 ...)
-  // - any browser specific keys (as available on special keyboards)
   // Returns true if an accelerator was activated.
-  bool ProcessAccelerator(const Accelerator& accelerator,
-                          bool prioritary_accelerators_only);
+  bool ProcessAccelerator(const Accelerator& accelerator);
 
   // NotificationObserver method.
   void Observe(NotificationType type,