[Windows] Fix issue 30123: REGRESSION: Alt-D causes system beep

BUG=30123
TEST=Press Alt+D should not beep.

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

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

2 files changed, 39 insertions, 1 deletions

diff --git a/chrome/browser/views/frame/browser_view.cc b/chrome/browser/views/frame/browser_view.cc
index c422d76..8d03e14 100644
--- a/chrome/browser/views/frame/browser_view.cc
+++ b/chrome/browser/views/frame/browser_view.cc
@@ -421,6 +421,7 @@ BrowserView::BrowserView(Browser* browser)
 #if defined(OS_WIN)
       hung_window_detector_(&hung_plugin_action_),
       ticker_(0),
+      ignore_next_char_event_(false),
 #endif
       extension_shelf_(NULL),
       last_focused_view_storage_id_(
@@ -1292,6 +1293,19 @@ bool BrowserView::PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event,
 }
 
 void BrowserView::HandleKeyboardEvent(const NativeWebKeyboardEvent& event) {
+#if defined(OS_WIN)
+  // Previous calls to TranslateMessage can generate Char events as well as
+  // RawKeyDown events, even if the latter triggered an accelerator.  In these
+  // cases, we discard the Char events.
+  if (event.type == WebKit::WebInputEvent::Char && ignore_next_char_event_) {
+    ignore_next_char_event_ = false;
+    return;
+  }
+  // It's necessary to reset this flag, because a RawKeyDown event may not
+  // always generate a Char event.
+  ignore_next_char_event_ = false;
+#endif
+
   if (event.type == WebKit::WebInputEvent::RawKeyDown) {
     views::FocusManager* focus_manager = GetFocusManager();
     DCHECK(focus_manager);
@@ -1305,8 +1319,24 @@ void BrowserView::HandleKeyboardEvent(const NativeWebKeyboardEvent& event) {
         (event.modifiers & NativeWebKeyboardEvent::AltKey) ==
             NativeWebKeyboardEvent::AltKey);
 
-    if (focus_manager->ProcessAccelerator(accelerator))
+#if defined(OS_WIN)
+    // This is tricky: we want to set ignore_next_char_event_ if
+    // ProcessAccelerator returns true. But ProcessAccelerator might delete
+    // |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;
+#endif
+
+    if (focus_manager->ProcessAccelerator(accelerator)) {
+      // DANGER: |this| could be deleted now!
       return;
+    }
+
+#if defined(OS_WIN)
+    // ProcessAccelerator didn't handle the accelerator, so we know both
+    // that |this| is still valid, and that we didn't want to set the flag.
+    ignore_next_char_event_ = false;
+#endif
   }
 
 #if defined(OS_WIN)
diff --git a/chrome/browser/views/frame/browser_view.h b/chrome/browser/views/frame/browser_view.h
index f070cdf..57ffddd 100644
--- a/chrome/browser/views/frame/browser_view.h
+++ b/chrome/browser/views/frame/browser_view.h
@@ -526,6 +526,14 @@ class BrowserView : public BrowserWindow,
 
   // The custom JumpList for Windows 7.
   scoped_ptr<JumpList> jumplist_;
+
+  // Whether to ignore the next Char keyboard event.
+  // If a RawKeyDown event was handled as a shortcut key, then we're done
+  // handling it and should eat any Char event that the translate phase may
+  // have produced from it. (Handling this event may cause undesirable effects,
+  // such as a beep if DefWindowProc() has no default handling for the given
+  // Char.)
+  bool ignore_next_char_event_;
 #endif
 
   // The timer used to update frames for the Loading Animation.