Suppress KeyUp events after PreHandleKeyboardEvent consumes RawKeyDown

content/browser/renderer_host/render_widget_host_impl.cc

Index: content/browser/renderer_host/render_widget_host_impl.cc
diff --git a/content/browser/renderer_host/render_widget_host_impl.cc b/content/browser/renderer_host/render_widget_host_impl.cc
index 925ca787cc2d5b7a76e8d779c0a422158e38991f..d5f59195273eba63fd32208550b3e7ca1f030630 100644
--- a/content/browser/renderer_host/render_widget_host_impl.cc
+++ b/content/browser/renderer_host/render_widget_host_impl.cc
@@ -205,7 +205,7 @@ RenderWidgetHostImpl::RenderWidgetHostImpl(RenderWidgetHostDelegate* delegate,
       text_direction_updated_(false),
       text_direction_(blink::WebTextDirectionLeftToRight),
       text_direction_canceled_(false),
-      suppress_next_char_events_(false),
+      suppress_events_until_keydown_(false),
       pending_mouse_lock_request_(false),
       allow_privileged_mouse_lock_(false),
       has_touch_handler_(false),
@@ -387,8 +387,8 @@ void RenderWidgetHostImpl::SendScreenRects() {
   waiting_for_screen_rects_ack_ = true;
 }
 
-void RenderWidgetHostImpl::SuppressNextCharEvents() {
-  suppress_next_char_events_ = true;
+void RenderWidgetHostImpl::SuppressEventsUntilKeyDown() {
+  suppress_events_until_keydown_ = true;
 }
 
 void RenderWidgetHostImpl::FlushInput() {
@@ -1118,10 +1118,10 @@ void RenderWidgetHostImpl::ForwardKeyboardEvent(
   // First, let keypress listeners take a shot at handling the event.  If a
   // listener handles the event, it should not be propagated to the renderer.
   if (KeyPressListenersHandleEvent(key_event)) {
-    // Some keypresses that are accepted by the listener might have follow up
-    // char events, which should be ignored.
+    // Some keypresses that are accepted by the listener may be followed by Char

[dtapuska, 2016/11/08 15:18:03]

I guess the odd thing is that on platforms that generate KeyDown (aka Android
and not RawKeyDown). They will get the KeyUp message.

[foolip, 2016/11/08 15:23:15]

I didn't try to understand the split between RawKeyDown and KeyDown. Does
Android generate only KeyDown, or both RawKeyDown and KeyDown?

+    // and KeyUp events, which should be ignored.
     if (key_event.type == WebKeyboardEvent::RawKeyDown)
-      suppress_next_char_events_ = true;
+      suppress_events_until_keydown_ = true;
     return;
   }
 
@@ -1130,27 +1130,28 @@ void RenderWidgetHostImpl::ForwardKeyboardEvent(
   if (!WebInputEvent::isKeyboardEventType(key_event.type))
     return;
 
-  if (suppress_next_char_events_) {
-    // If preceding RawKeyDown event was handled by the browser, then we need
-    // suppress all Char events generated by it. Please note that, one
-    // RawKeyDown event may generate multiple Char events, so we can't reset
-    // |suppress_next_char_events_| until we get a KeyUp or a RawKeyDown.
-    if (key_event.type == WebKeyboardEvent::Char)
+  if (suppress_events_until_keydown_) {
+    // If the preceding RawKeyDown event was handled by the browser, then we
+    // need to suppress all events generated by it until the next RawKeyDown or
+    // KeyDown event.
+    if (key_event.type == WebKeyboardEvent::KeyUp ||
+        key_event.type == WebKeyboardEvent::Char)
       return;
-    // We get a KeyUp or a RawKeyDown event.
-    suppress_next_char_events_ = false;
+    DCHECK(key_event.type == WebKeyboardEvent::RawKeyDown ||
+           key_event.type == WebKeyboardEvent::KeyDown);
+    suppress_events_until_keydown_ = false;
   }
 
   bool is_shortcut = false;
 
   // Only pre-handle the key event if it's not handled by the input method.
   if (delegate_ && !key_event.skip_in_browser) {
-    // We need to set |suppress_next_char_events_| to true if
+    // We need to set |suppress_events_until_keydown_| to true if
     // PreHandleKeyboardEvent() returns true, but |this| may already be
-    // destroyed at that time. So set |suppress_next_char_events_| true here,
-    // then revert it afterwards when necessary.
+    // destroyed at that time. So set |suppress_events_until_keydown_| true
+    // here, then revert it afterwards when necessary.
     if (key_event.type == WebKeyboardEvent::RawKeyDown)
-      suppress_next_char_events_ = true;
+      suppress_events_until_keydown_ = true;
 
     // Tab switching/closing accelerators aren't sent to the renderer to avoid
     // a hung/malicious renderer from interfering.
@@ -1158,7 +1159,7 @@ void RenderWidgetHostImpl::ForwardKeyboardEvent(
       return;
 
     if (key_event.type == WebKeyboardEvent::RawKeyDown)
-      suppress_next_char_events_ = false;
+      suppress_events_until_keydown_ = false;
   }
 
   if (touch_emulator_ && touch_emulator_->HandleKeyboardEvent(key_event))
@@ -1350,7 +1351,7 @@ void RenderWidgetHostImpl::RendererExited(base::TerminationStatus status,
   waiting_for_screen_rects_ack_ = false;
 
   // Must reset these to ensure that keyboard events work with a new renderer.
-  suppress_next_char_events_ = false;
+  suppress_events_until_keydown_ = false;
 
   // Reset some fields in preparation for recovering from a crash.
   ResetSizeAndRepaintPendingFlags();
@@ -2003,7 +2004,7 @@ void RenderWidgetHostImpl::OnUnexpectedEventAck(UnexpectedEventAckType type) {
   if (type == BAD_ACK_MESSAGE) {
     bad_message::ReceivedBadMessage(process_, bad_message::RWH_BAD_ACK_MESSAGE);
   } else if (type == UNEXPECTED_EVENT_TYPE) {
-    suppress_next_char_events_ = false;
+    suppress_events_until_keydown_ = false;
   }
 }