Combine the WM_CHAR with WM_KEY* for key event flow.

ui/base/ime/input_method_win.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/base/ime/input_method_win.h"
 
 #include "base/auto_reset.h"
 #include "base/basictypes.h"
+#include "base/command_line.h"
 #include "base/profiler/scoped_tracker.h"
 #include "ui/base/ime/text_input_client.h"
 #include "ui/base/ime/win/tsf_input_scope.h"
+#include "ui/base/ui_base_switches.h"
 #include "ui/events/event.h"
 #include "ui/events/event_constants.h"
 #include "ui/events/event_utils.h"
 #include "ui/events/keycodes/keyboard_codes.h"
 #include "ui/gfx/win/dpi.h"
 #include "ui/gfx/win/hwnd_util.h"
 
 namespace ui {
 namespace {
 
 // Extra number of chars before and after selection (or composition) range which
 // is returned to IME for improving conversion accuracy.
 static const size_t kExtraNumberOfChars = 20;
 
 }  // namespace
 
 InputMethodWin::InputMethodWin(internal::InputMethodDelegate* delegate,
                                HWND toplevel_window_handle)
     : toplevel_window_handle_(toplevel_window_handle),
       pending_requested_direction_(base::i18n::UNKNOWN_DIRECTION),
       accept_carriage_return_(false),
       enabled_(false),
       is_candidate_popup_open_(false),
-      composing_window_handle_(NULL),
-      suppress_next_char_(false) {
+      composing_window_handle_(NULL) {
   SetDelegate(delegate);
 }
 
 void InputMethodWin::OnFocus() {
   InputMethodBase::OnFocus();
   if (GetTextInputClient())
     UpdateIMEState();
 }
 
 void InputMethodWin::OnBlur() {
@@ skipping 50 matching lines @@
     *result = original_result;
   return !!handled;
 }
 
 void InputMethodWin::DispatchKeyEvent(ui::KeyEvent* event) {
   if (!event->HasNativeEvent()) {
     DispatchFabricatedKeyEvent(event);
     return;
   }
 
+  std::vector<MSG> char_msgs;
   const base::NativeEvent& native_key_event = event->native_event();
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableMergeKeyCharEvents)) {
+    // Combines the WM_KEY* and WM_CHAR messages in the event processing flow
+    // which is necessary to let Chrome IME extension to process the key event
+    // and perform corresponding IME actions.
+    // Chrome IME extension may wants to consume certain key events based on
+    // the character information of WM_CHAR messages. Holding WM_KEY* messages
+    // until WM_CHAR is processed by the IME extension is not feasible because
+    // there is no way to know wether there will or not be a WM_CHAR following
+    // the WM_KEY*.
+    // Chrome never handles dead chars so it is safe to remove/ignore
+    // WM_*DEADCHAR messages.
+    MSG msg;
+    while (::PeekMessage(&msg, native_key_event.hwnd, WM_CHAR, WM_DEADCHAR,

[ananta, 2015/09/01 21:53:45]

Peeking out WM_CHAR's here will solve the issue you guys are trying to address.
However that effectively means that we lose the flexibility provided by the ui
base event handling model, where in we can have handlers for messages at various
stages.

[Shu Chen, 2015/09/02 02:51:07]

I don't see losing flexibility by this cl.
Chrome will still support standalone WM_CHAR without physical key events.
Also, Chrome can still handle the key events by its character information at any
stages through KeyEvent::GetCharacter(). I will upload a new patch to use
KeyEvent::set_character() to fill in the character information.
However, in some edge cases (e.g. double dead keys), multiple WM_CHAR will be
generated for single keydown.
Therefore, there is no way to use KeyEvent::set_character() to fill in the
character information.
IMO, that is rare and it seems a reasonable cost to support Chrome IMEs.

What's more, I don't think it is good to handle is_char()==true events at
multiple stages, especially in Chrome common code, because is_char() is really
specific for Windows platform.
So, ideally, the use of is_char() and specially code for WM_CHAR should always
be in the platform-dependent layer at lower level of Chrome.

+                         PM_REMOVE)) {
+      if (msg.message == WM_CHAR)
+        char_msgs.push_back(msg);
+    }
+    while (::PeekMessage(&msg, native_key_event.hwnd, WM_SYSCHAR,
+                         WM_SYSDEADCHAR, PM_REMOVE)) {
+      if (msg.message == WM_SYSCHAR)
+        char_msgs.push_back(msg);
+    }
+  }
+
+  BOOL handled = FALSE;
   if (native_key_event.message == WM_CHAR) {

[ananta, 2015/09/01 21:53:45]

Will this code execute if the kEnableMergeKeyCharEvents command line is enabled?

[Shu Chen, 2015/09/02 02:51:07]

Yes. This remains the original behavior to support WM_CHAR messages without a
physical key event.
For example, some tests may generate WM_CHAR directly (or some 3rd party apps).

-    BOOL handled;
     OnChar(native_key_event.hwnd, native_key_event.message,
            native_key_event.wParam, native_key_event.lParam, &handled);
     if (handled)
       event->StopPropagation();
     return;
   }
   // Handles ctrl-shift key to change text direction and layout alignment.
   if (ui::IMM32Manager::IsRTLKeyboardLayoutInstalled() &&
       !IsTextInputTypeNone()) {
     // TODO: shouldn't need to generate a KeyEvent here.
@@ skipping 10 matching lines @@
     } else if (key.type() == ui::ET_KEY_RELEASED &&
                (code == ui::VKEY_SHIFT || code == ui::VKEY_CONTROL) &&
                pending_requested_direction_ != base::i18n::UNKNOWN_DIRECTION) {
       GetTextInputClient()->ChangeTextDirectionAndLayoutAlignment(
           pending_requested_direction_);
       pending_requested_direction_ = base::i18n::UNKNOWN_DIRECTION;
     }
   }
 
   ui::EventDispatchDetails details = DispatchKeyEventPostIME(event);
-  if (!details.dispatcher_destroyed)
-    suppress_next_char_ = event->stopped_propagation();
+  if (details.dispatcher_destroyed || details.target_destroyed ||
+      event->stopped_propagation()) {
+    return;
+  }
+
+  for (size_t i = 0; i < char_msgs.size(); ++i) {
+    msg = char_msgs[i];
+    OnChar(msg.hwnd, msg.message, msg.wParam, msg.lParam, &handled);
+  }
 }
 
 void InputMethodWin::OnTextInputTypeChanged(const TextInputClient* client) {
   if (!IsTextInputClientFocused(client) || !IsWindowFocused(client))
     return;
   imm32_manager_.CancelIME(toplevel_window_handle_);
   UpdateIMEState();
 }
 
 void InputMethodWin::OnCaretBoundsChanged(const TextInputClient* client) {
@@ skipping 65 matching lines @@
                                UINT message,
                                WPARAM wparam,
                                LPARAM lparam,
                                BOOL* handled) {
   // TODO(vadimt): Remove ScopedTracker below once crbug.com/440919 is fixed.
   tracked_objects::ScopedTracker tracking_profile(
       FROM_HERE_WITH_EXPLICIT_FUNCTION("440919 InputMethodWin::OnChar"));
 
   *handled = TRUE;
 
-  if (suppress_next_char_) {
-    suppress_next_char_ = false;
-    return 0;
-  }
-
   // We need to send character events to the focused text input client event if
   // its text input type is ui::TEXT_INPUT_TYPE_NONE.
   if (GetTextInputClient()) {
     const base::char16 kCarriageReturn = L'\r';
     const base::char16 ch = static_cast<base::char16>(wparam);
     // A mask to determine the previous key state from |lparam|. The value is 1
     // if the key is down before the message is sent, or it is 0 if the key is
     // up.
     const uint32 kPrevKeyDownBit = 0x40000000;
     if (ch == kCarriageReturn && !(lparam & kPrevKeyDownBit))
@@ skipping 338 matching lines @@
   // window. So we can safely assume that |attached_window_handle| is ready for
   // receiving keyboard input as long as it is an active window. This works well
   // even when the |attached_window_handle| becomes active but has not received
   // WM_FOCUS yet.
   return toplevel_window_handle_ &&
       GetActiveWindow() == toplevel_window_handle_;
 }
 
 void InputMethodWin::DispatchFabricatedKeyEvent(ui::KeyEvent* event) {
   if (event->is_char()) {
-    if (suppress_next_char_) {
-      suppress_next_char_ = false;
-      return;
-    }
     if (GetTextInputClient()) {
       GetTextInputClient()->InsertChar(
           static_cast<base::char16>(event->key_code()),
           ui::GetModifiersFromKeyState());
       return;
     }
   }
   ignore_result(DispatchKeyEventPostIME(event));
 }
 
@@ skipping 27 matching lines @@
       enabled_ = true;
       break;
   }
 
   imm32_manager_.SetTextInputMode(window_handle, text_input_mode);
   tsf_inputscope::SetInputScopeForTsfUnawareWindow(
       window_handle, text_input_type, text_input_mode);
 }
 
 }  // namespace ui